Implement RFC3373 non local definitions lint

Author: Urgau

compiler/rustc_lint/messages.ftl

@@ -406,6 +406,14 @@ lint_non_fmt_panic_unused =
     }
     .add_fmt_suggestion = or add a "{"{"}{"}"}" format string to use the message literally
 
+lint_non_local_definitions = non local definition should be avoided as they go against expectation
+    .help =
+        move this declaration outside the of the {$depth ->
+            [one] current body
+           *[other] {$depth} outermost bodies
+        }
+    .deprecation = this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
+
 lint_non_snake_case = {$sort} `{$name}` should have a snake case name
     .rename_or_convert_suggestion = rename the identifier or convert it to a snake case raw identifier
     .cannot_convert_note = `{$sc}` cannot be used as a raw identifier

compiler/rustc_lint/src/lib.rs

@@ -72,6 +72,7 @@ mod methods;
 mod multiple_supertrait_upcastable;
 mod non_ascii_idents;
 mod non_fmt_panic;
+mod non_local_def;
 mod nonstandard_style;
 mod noop_method_call;
 mod opaque_hidden_inferred_bound;
@@ -107,6 +108,7 @@ use methods::*;
 use multiple_supertrait_upcastable::*;
 use non_ascii_idents::*;
 use non_fmt_panic::NonPanicFmt;
+use non_local_def::*;
 use nonstandard_style::*;
 use noop_method_call::*;
 use opaque_hidden_inferred_bound::*;
@@ -233,6 +235,7 @@ late_lint_methods!(
             MissingDebugImplementations: MissingDebugImplementations,
             MissingDoc: MissingDoc,
             AsyncFnInTrait: AsyncFnInTrait,
+            NonLocalDefinitions: NonLocalDefinitions::default(),
         ]
     ]
 );

compiler/rustc_lint/src/lints.rs

@@ -1305,6 +1305,15 @@ pub struct SuspiciousDoubleRefCloneDiag<'a> {
     pub ty: Ty<'a>,
 }
 
+// non_local_defs.rs
+#[derive(LintDiagnostic)]
+#[diag(lint_non_local_definitions)]
+#[help(lint_help)]
+#[note(lint_deprecation)]
+pub struct NonLocalDefinitionsDiag {
+    pub depth: u32,
+}
+
 // pass_by_value.rs
 #[derive(LintDiagnostic)]
 #[diag(lint_pass_by_value)]

compiler/rustc_lint/src/non_local_def.rs

@@ -0,0 +1,243 @@
+use std::convert::identity;
+
+use rustc_hir::{
+    Body, FnRetTy, GenericArg, GenericBound, GenericParam, GenericParamKind, Item, ItemKind, Path,
+    QPath, TyKind, WherePredicate,
+};
+use rustc_span::{def_id::DefId, sym, MacroKind};
+
+use crate::{lints::NonLocalDefinitionsDiag, LateContext, LateLintPass, LintContext};
+
+declare_lint! {
+    /// The `non_local_definitions` lint checks for `impl` blocks and `#[macro_export]`
+    /// macro inside bodies (functions, enum discriminant, ...).
+    ///
+    /// ### Example
+    ///
+    /// ```rust
+    /// trait MyTrait {}
+    /// struct MyStruct;
+    ///
+    /// fn foo() {
+    ///     impl MyTrait for MyStruct {}
+    /// }
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// Creating non local definitions go against expectation and can create decrepencies
+    /// in tooling. In should be avoided.
+    pub NON_LOCAL_DEFINITIONS,
+    Warn,
+    "checks for non local definitions"
+}
+
+#[derive(Default)]
+pub struct NonLocalDefinitions {
+    is_in_body: u32,
+}
+
+impl_lint_pass!(NonLocalDefinitions => [NON_LOCAL_DEFINITIONS]);
+
+impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
+    fn check_body(&mut self, _cx: &LateContext<'tcx>, _body: &'tcx Body<'tcx>) {
+        self.is_in_body += 1;
+    }
+
+    fn check_body_post(&mut self, _cx: &LateContext<'tcx>, _body: &'tcx Body<'tcx>) {
+        self.is_in_body -= 1;
+    }
+
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
+        if self.is_in_body > 0 {
+            match item.kind {
+                ItemKind::Impl(impl_) => {
+                    // The RFC states:
+                    //
+                    // > An item nested inside an expression-containing item (through any
+                    // > level of nesting) may not define an impl Trait for Type unless
+                    // > either the **Trait** or the **Type** is also nested inside the
+                    // > same expression-containing item.
+                    //
+                    // To achieve this we get try to get the paths of the _Trait_ and
+                    // _Type_, and we look inside thoses paths to try a find in one
+                    // of them a type whose parent is the same as the impl definition.
+                    //
+                    // If that's the case this means that this impl block decleration
+                    // is using local items and so we don't lint on it.
+
+                    let parent_impl = cx.tcx.parent(item.owner_id.def_id.into());
+
+                    let generics_has_local_parent =
+                        generics_params_has_local_parent(&impl_.generics.params, cx, parent_impl)
+                            || generics_predicates_has_local_parent(
+                                &impl_.generics.predicates,
+                                cx,
+                                parent_impl,
+                            );
+
+                    let self_ty_has_local_parent =
+                        ty_kind_has_local_parent(&impl_.self_ty.kind, cx, parent_impl);
+
+                    let of_trait_has_local_parent = impl_
+                        .of_trait
+                        .map(|of_trait| path_has_local_parent(of_trait.path, cx, parent_impl))
+                        .unwrap_or(false);
+
+                    // If none of them have a local parent (LOGICAL NOR) this means that
+                    // this impl definition is a non-local definition and so we lint on it.
+                    if !(generics_has_local_parent
+                        || self_ty_has_local_parent
+                        || of_trait_has_local_parent)
+                    {
+                        cx.emit_span_lint(
+                            NON_LOCAL_DEFINITIONS,
+                            item.span,
+                            NonLocalDefinitionsDiag { depth: self.is_in_body },
+                        );
+                    }
+                }
+                ItemKind::Macro(_macro, MacroKind::Bang) => {
+                    if cx.tcx.has_attr(item.owner_id.def_id, sym::macro_export) {
+                        cx.emit_span_lint(
+                            NON_LOCAL_DEFINITIONS,
+                            item.span,
+                            NonLocalDefinitionsDiag { depth: self.is_in_body },
+                        );
+                    }
+                }
+                _ => {}
+            }
+        }
+    }
+}
+
+/// Given a path and a parent impl def id, this function tries to find if any of
+/// the segments, generic type argument have a parent def id that correspond to
+/// the def id of the parent impl definition.
+///
+/// Given this path, we will look at every segments and generic args:
+///
+///    std::convert::PartialEq<Foo<Bar>>
+///    ^^^  ^^^^^^^  ^^^^^^^^^ ^^^ ^^^
+///
+/// Note that we we are only interested in `TyKind::Path` as it's the only `TyKind`
+/// that can point to an item outside the "parent impl definition".
+fn path_has_local_parent(path: &Path<'_>, cx: &LateContext<'_>, parent_impl: DefId) -> bool {
+    if let Some(did) = path.res.opt_def_id()
+        && cx.tcx.parent(did) == parent_impl
+    {
+        return true;
+    }
+
+    for seg in path.segments {
+        if let Some(gargs) = seg.args {
+            for garg in gargs.args {
+                if let GenericArg::Type(ty) = garg
+                    && ty_kind_has_local_parent(&ty.kind, cx, parent_impl)
+                {
+                    return true;
+                }
+            }
+        }
+    }
+
+    false
+}
+
+/// Given a hir::TyKind we recurse into all it's variants to find all the hir::Path
+/// and execute [`path_has_local_parent`].
+fn ty_kind_has_local_parent(
+    ty_kind: &TyKind<'_>,
+    cx: &LateContext<'_>,
+    parent_impl: DefId,
+) -> bool {
+    match ty_kind {
+        TyKind::InferDelegation(_, _) => false,
+        TyKind::Slice(ty) | TyKind::Array(ty, _) => {
+            ty_kind_has_local_parent(&ty.kind, cx, parent_impl)
+        }
+        TyKind::Ptr(mut_ty) => ty_kind_has_local_parent(&mut_ty.ty.kind, cx, parent_impl),
+        TyKind::Ref(_, mut_ty) => ty_kind_has_local_parent(&mut_ty.ty.kind, cx, parent_impl),
+        TyKind::BareFn(bare_fn_ty) => {
+            bare_fn_ty
+                .decl
+                .inputs
+                .iter()
+                .any(|ty| ty_kind_has_local_parent(&ty.kind, cx, parent_impl))
+                || match bare_fn_ty.decl.output {
+                    FnRetTy::DefaultReturn(_) => false,
+                    FnRetTy::Return(ty) => ty_kind_has_local_parent(&ty.kind, cx, parent_impl),
+                }
+                || generics_params_has_local_parent(&bare_fn_ty.generic_params, cx, parent_impl)
+        }
+        TyKind::Never => false,
+        TyKind::Tup(tys) => {
+            tys.iter().any(|ty| ty_kind_has_local_parent(&ty.kind, cx, parent_impl))
+        }
+        TyKind::Path(QPath::Resolved(_, ty_path)) => {
+            path_has_local_parent(ty_path, cx, parent_impl)
+        }
+        TyKind::Path(_) => false,
+        TyKind::OpaqueDef(_, args, _) => args
+            .iter()
+            .filter_map(|garg| match garg {
+                GenericArg::Lifetime(_) => None,
+                GenericArg::Type(ty) => Some(ty),
+                GenericArg::Const(_) => None,
+                GenericArg::Infer(_) => None,
+            })
+            .any(|ty| ty_kind_has_local_parent(&ty.kind, cx, parent_impl)),
+        TyKind::TraitObject(bounds, _, _) => bounds.iter().any(|poly_trait_ref| {
+            path_has_local_parent(poly_trait_ref.trait_ref.path, cx, parent_impl)
+        }),
+        TyKind::Typeof(_) => false,
+        TyKind::Infer => false,
+        TyKind::Err(_) => false,
+    }
+}
+
+fn generics_params_has_local_parent(
+    generic_params: &[GenericParam<'_>],
+    cx: &LateContext<'_>,
+    parent_impl: DefId,
+) -> bool {
+    generic_params
+        .iter()
+        .filter_map(|gparam| match gparam.kind {
+            GenericParamKind::Lifetime { kind: _ } => None,
+            GenericParamKind::Type { default, synthetic: _ } => default,
+            GenericParamKind::Const { ty, default: _, is_host_effect: _ } => Some(ty),
+        })
+        .any(|ty| ty_kind_has_local_parent(&ty.kind, cx, parent_impl))
+}
+
+fn generics_predicates_has_local_parent(
+    predicates: &[WherePredicate<'_>],
+    cx: &LateContext<'_>,
+    parent_impl: DefId,
+) -> bool {
+    predicates
+        .iter()
+        .filter_map(|predicate| match predicate {
+            WherePredicate::BoundPredicate(bound_predicate) => Some(
+                ty_kind_has_local_parent(&bound_predicate.bounded_ty.kind, cx, parent_impl)
+                    || bound_predicate.bounds.iter().any(|gbound| match gbound {
+                        GenericBound::Trait(poly_trait_ref, _) => {
+                            path_has_local_parent(&poly_trait_ref.trait_ref.path, cx, parent_impl)
+                        }
+                        GenericBound::Outlives(_) => false,
+                    })
+                    || generics_params_has_local_parent(
+                        bound_predicate.bound_generic_params,
+                        cx,
+                        parent_impl,
+                    ),
+            ),
+            WherePredicate::RegionPredicate(_) => None,
+            WherePredicate::EqPredicate(_) => None,
+        })
+        .any(identity)
+}