Eliminate false-positives with the type-system

Author: Urgau

compiler/rustc_lint/src/non_local_def.rs

@@ -1,6 +1,18 @@
-use rustc_hir::{def::DefKind, Body, Item, ItemKind, Node, Path, QPath, TyKind};
+use rustc_hir::{def::DefKind, Body, Item, ItemKind, Node, TyKind};
+use rustc_hir::{Path, QPath};
+use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
+use rustc_infer::infer::InferCtxt;
+use rustc_infer::traits::{Obligation, ObligationCause};
+use rustc_middle::query::Key;
+use rustc_middle::ty::TypeSuperFoldable;
+use rustc_middle::ty::{self, Binder, Ty, TyCtxt, TypeFoldable, TypeFolder};
 use rustc_span::def_id::{DefId, LOCAL_CRATE};
+use rustc_span::Span;
 use rustc_span::{sym, symbol::kw, ExpnKind, MacroKind};
+use rustc_trait_selection::infer::TyCtxtInferExt;
+use rustc_trait_selection::traits::error_reporting::ambiguity::{
+    compute_applicable_impls_for_diagnostics, Ambiguity,
+};
 
 use crate::lints::{NonLocalDefinitionsCargoUpdateNote, NonLocalDefinitionsDiag};
 use crate::{LateContext, LateLintPass, LintContext};
@@ -66,7 +78,9 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
             return;
         }
 
-        let parent = cx.tcx.parent(item.owner_id.def_id.into());
+        let def_id = item.owner_id.def_id.into();
+
+        let parent = cx.tcx.parent(def_id);
         let parent_def_kind = cx.tcx.def_kind(parent);
         let parent_opt_item_name = cx.tcx.opt_item_name(parent);
 
@@ -155,9 +169,54 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
                     .map(|of_trait| path_has_local_parent(of_trait.path, cx, parent, parent_parent))
                     .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 !(self_ty_has_local_parent || of_trait_has_local_parent) {
+                // Detecting if the impl definition is leaking outside of it's defining scope.
+                //
+                // Rule: for each impl, instantiate all local types with inference vars and
+                // then assemble candidates for that goal, if there are more than 1 (non-private
+                // impls), it does not leak.
+                //
+                // https://github.com/rust-lang/rust/issues/121621#issuecomment-1976826895
+                let impl_is_not_leaky = cx
+                    .tcx
+                    .impl_trait_ref(def_id)
+                    .map(|binder| {
+                        let infcx = cx.tcx.infer_ctxt().build();
+                        let trait_ref = binder
+                            .instantiate(cx.tcx, infcx.fresh_args_for_item(item.span, def_id));
+
+                        let trait_ref = trait_ref.fold_with(&mut LocalTypeInferenceFolder {
+                            infcx: &infcx,
+                            to_ignore: 1,
+                            impl_parent: parent,
+                            impl_parent_parent: parent_parent,
+                            span: item.span,
+                        });
+
+                        let poly_trait_obligation = Obligation::new(
+                            cx.tcx,
+                            ObligationCause::dummy(),
+                            ty::ParamEnv::empty(),
+                            Binder::dummy(trait_ref),
+                        );
+
+                        let ambiguities = compute_applicable_impls_for_diagnostics(
+                            &infcx,
+                            &poly_trait_obligation,
+                        );
+
+                        let mut it = ambiguities.iter().filter(|ambi| match ambi {
+                            Ambiguity::DefId(did) => {
+                                !did_has_local_parent(*did, cx.tcx, parent, parent_parent)
+                            }
+                            Ambiguity::ParamEnv(_) => false,
+                        });
+
+                        let _ = it.next();
+                        it.next().is_some()
+                    })
+                    .unwrap_or(false);
+
+                if !(self_ty_has_local_parent || of_trait_has_local_parent || impl_is_not_leaky) {
                     let const_anon = if self.body_depth == 1
                         && parent_def_kind == DefKind::Const
                         && parent_opt_item_name != Some(kw::Underscore)
@@ -207,6 +266,47 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
     }
 }
 
+/// Replace every local type by inference variable.
+///
+/// ```text
+/// <Global<Local> as std::cmp::PartialEq<Global<Local>>>
+/// to
+/// <Global<_> as std::cmp::PartialEq<Global<_>>>
+/// ```
+struct LocalTypeInferenceFolder<'a, 'tcx> {
+    infcx: &'a InferCtxt<'tcx>,
+    to_ignore: u32,
+    impl_parent: DefId,
+    impl_parent_parent: Option<DefId>,
+    span: Span,
+}
+
+impl<'a, 'tcx> TypeFolder<TyCtxt<'tcx>> for LocalTypeInferenceFolder<'a, 'tcx> {
+    fn interner(&self) -> TyCtxt<'tcx> {
+        self.infcx.tcx
+    }
+
+    fn fold_ty(&mut self, t: Ty<'tcx>) -> Ty<'tcx> {
+        if let Some(ty_did) = t.ty_def_id()
+            && did_has_local_parent(
+                ty_did,
+                self.infcx.tcx,
+                self.impl_parent,
+                self.impl_parent_parent,
+            )
+            && self.to_ignore == 0
+        {
+            self.infcx.next_ty_var(TypeVariableOrigin {
+                kind: TypeVariableOriginKind::TypeInference,
+                span: self.span,
+            })
+        } else {
+            self.to_ignore = self.to_ignore.saturating_sub(1);
+            t.super_fold_with(self)
+        }
+    }
+}
+
 /// 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.
 ///
@@ -216,16 +316,29 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
 ///    std::convert::PartialEq<Foo<Bar>>
 ///    ^^^^^^^^^^^^^^^^^^^^^^^
 /// ```
+#[inline]
 fn path_has_local_parent(
     path: &Path<'_>,
     cx: &LateContext<'_>,
     impl_parent: DefId,
     impl_parent_parent: Option<DefId>,
 ) -> bool {
-    path.res.opt_def_id().is_some_and(|did| {
-        did.is_local() && {
-            let res_parent = cx.tcx.parent(did);
-            res_parent == impl_parent || Some(res_parent) == impl_parent_parent
-        }
-    })
+    path.res
+        .opt_def_id()
+        .is_some_and(|did| did_has_local_parent(did, cx.tcx, impl_parent, impl_parent_parent))
+}
+
+/// Given a def id and a parent impl def id, this checks if the parent
+/// def id correspond to the def id of the parent impl definition.
+#[inline]
+fn did_has_local_parent(
+    did: DefId,
+    tcx: TyCtxt<'_>,
+    impl_parent: DefId,
+    impl_parent_parent: Option<DefId>,
+) -> bool {
+    did.is_local() && {
+        let res_parent = tcx.parent(did);
+        res_parent == impl_parent || Some(res_parent) == impl_parent_parent
+    }
 }

tests/ui/lint/non_local_definitions.rs

@@ -282,7 +282,6 @@ struct Cat;
 
 fn meow() {
     impl From<Cat> for () {
-    //~^ WARN non-local `impl` definition
         fn from(_: Cat) -> () {
             todo!()
         }
@@ -374,6 +373,72 @@ fn rawr() {
             todo!()
         }
     }
+
+    #[derive(Debug)]
+    struct Elephant;
+
+    impl From<Wrap<Wrap<Elephant>>> for () {
+    //~^ WARN non-local `impl` definition
+        fn from(_: Wrap<Wrap<Elephant>>) -> Self {
+            todo!()
+        }
+    }
+}
+
+pub trait StillNonLocal {}
+
+impl StillNonLocal for &str {}
+
+fn only_global() {
+    struct Foo;
+    impl StillNonLocal for &Foo {}
+    //~^ WARN non-local `impl` definition
+}
+
+struct GlobalSameFunction;
+
+fn same_function() {
+    struct Local1(GlobalSameFunction);
+    impl From<Local1> for GlobalSameFunction {
+    //~^ WARN non-local `impl` definition
+        fn from(x: Local1) -> GlobalSameFunction {
+            x.0
+        }
+    }
+
+    struct Local2(GlobalSameFunction);
+    impl From<Local2> for GlobalSameFunction {
+    //~^ WARN non-local `impl` definition
+        fn from(x: Local2) -> GlobalSameFunction {
+            x.0
+        }
+    }
+}
+
+struct GlobalDifferentFunction;
+
+fn diff_foo() {
+    struct Local(GlobalDifferentFunction);
+
+    impl From<Local> for GlobalDifferentFunction {
+    // FIXME(Urgau): Should warn but doesn't since we currently consider
+    // the other impl to be "global", but that's not the case for the type-system
+        fn from(x: Local) -> GlobalDifferentFunction {
+            x.0
+        }
+    }
+}
+
+fn diff_bar() {
+    struct Local(GlobalDifferentFunction);
+
+    impl From<Local> for GlobalDifferentFunction {
+    // FIXME(Urgau): Should warn but doesn't since we currently consider
+    // the other impl to be "global", but that's not the case for the type-system
+        fn from(x: Local) -> GlobalDifferentFunction {
+            x.0
+        }
+    }
 }
 
 macro_rules! m {
@@ -404,3 +469,24 @@ fn bitflags() {
         impl Flags {}
     };
 }
+
+// https://github.com/rust-lang/rust/issues/121621#issuecomment-1976826895
+fn commonly_reported() {
+    struct Local(u8);
+    impl From<Local> for u8 {
+        fn from(x: Local) -> u8 {
+            x.0
+        }
+    }
+}
+
+// https://github.com/rust-lang/rust/issues/121621#issue-2153187542
+pub trait Serde {}
+
+impl Serde for &[u8] {}
+impl Serde for &str {}
+
+fn serde() {
+    struct Thing;
+    impl Serde for &Thing {}
+}