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,201 @@
+use rustc_hir::{
+    def::DefKind, Body, FnRetTy, GenericArg, GenericParam, GenericParamKind, Item, ItemKind, Path,
+    QPath, TyKind,
+};
+use rustc_span::{def_id::DefId, sym, symbol::kw, 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 {
+            let parent_impl = cx.tcx.parent(item.owner_id.def_id.into());
+
+            // Per RFC we (currently) ignore anon-const (`const _: Ty = ...`).
+            if cx.tcx.def_kind(parent_impl) == DefKind::Const
+                && cx.tcx.opt_item_name(parent_impl) == Some(kw::Underscore)
+            {
+                return;
+            }
+
+            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 generics_has_local_parent =
+                        generics_params_has_local_parent(&impl_.generics.params, 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 checks if the if parent resolution
+/// def id correspond to the def id of the parent impl definition.
+///
+/// Given this path, we will look at the path (and ignore any generic args):
+///
+/// ```text
+///    std::convert::PartialEq<Foo<Bar>>
+///    ^^^^^^^^^^^^^^^^^^^^^^^
+/// ```
+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;
+    }
+
+    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))
+}