Implement RFC3373 non local definitions lint

Author: Urgau

compiler/rustc_lint/messages.ftl

@@ -406,6 +406,10 @@ 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} outermost bodies
+    .deprecation = this may become deny-by-default in the Edition 2024 and higher
+
 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,141 @@
+use rustc_hir::{Body, GenericArg, Item, ItemKind, Path, QPath, TyKind};
+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 self_ty_has_local_parent = if let TyKind::Path(QPath::Resolved(
+                        _,
+                        self_ty_path,
+                    )) = impl_.self_ty.kind
+                    {
+                        has_local_parent(self_ty_path, cx, parent_impl)
+                    } else {
+                        false
+                    };
+
+                    let of_trait_has_local_parent = if let Some(of_trait) = impl_.of_trait {
+                        has_local_parent(of_trait.path, cx, parent_impl)
+                    } else {
+                        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 !(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 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
+                    && let TyKind::Path(QPath::Resolved(_, ty_path)) = ty.kind
+                    && has_local_parent(ty_path, cx, parent_impl)
+                {
+                    return true;
+                }
+            }
+        }
+    }
+
+    false
+}