Auto merge of rust-lang#13867 - Veykril:meth-res-fallback, r=Veykril

Fallback to invisible associated functions and constants if no visible resolutions are found

Still lacking tests, will add those later
Fixes rust-lang/rust-analyzer#13126

Author: bors

crates/hir-ty/src/infer/path.rs

@@ -234,15 +234,16 @@ impl<'a> InferenceContext<'a> {
         let canonical_ty = self.canonicalize(ty.clone());
         let traits_in_scope = self.resolver.traits_in_scope(self.db.upcast());
 
-        method_resolution::iterate_method_candidates(
+        let mut not_visible = None;
+        let res = method_resolution::iterate_method_candidates(
             &canonical_ty.value,
             self.db,
             self.table.trait_env.clone(),
             &traits_in_scope,
             VisibleFromModule::Filter(self.resolver.module()),
             Some(name),
             method_resolution::LookupMode::Path,
-            move |_ty, item| {
+            |_ty, item, visible| {
                 let (def, container) = match item {
                     AssocItemId::FunctionId(f) => {
                         (ValueNs::FunctionId(f), f.lookup(self.db.upcast()).container)
@@ -277,10 +278,21 @@ impl<'a> InferenceContext<'a> {
                     }
                 };
 
-                self.write_assoc_resolution(id, item, substs.clone());
-                Some((def, Some(substs)))
+                if visible {
+                    Some((def, item, Some(substs)))
+                } else {
+                    if not_visible.is_none() {
+                        not_visible = Some((def, item, Some(substs)));
+                    }
+                    None
+                }
             },
-        )
+        );
+        let res = res.or(not_visible);
+        if let Some((_, item, Some(ref substs))) = res {
+            self.write_assoc_resolution(id, item, substs.clone());
+        }
+        res.map(|(def, _, substs)| (def, substs))
     }
 
     fn resolve_enum_variant_on_ty(

crates/hir-ty/src/method_resolution.rs

@@ -495,19 +495,25 @@ pub(crate) fn lookup_method(
     visible_from_module: VisibleFromModule,
     name: &Name,
 ) -> Option<(ReceiverAdjustments, FunctionId)> {
-    iterate_method_candidates(
+    let mut not_visible = None;
+    let res = iterate_method_candidates(
         ty,
         db,
         env,
         traits_in_scope,
         visible_from_module,
         Some(name),
         LookupMode::MethodCall,
-        |adjustments, f| match f {
-            AssocItemId::FunctionId(f) => Some((adjustments, f)),
+        |adjustments, f, visible| match f {
+            AssocItemId::FunctionId(f) if visible => Some((adjustments, f)),
+            AssocItemId::FunctionId(f) if not_visible.is_none() => {
+                not_visible = Some((adjustments, f));
+                None
+            }
             _ => None,
         },
-    )
+    );
+    res.or(not_visible)
 }
 
 /// Whether we're looking up a dotted method call (like `v.len()`) or a path
@@ -619,7 +625,7 @@ pub(crate) fn iterate_method_candidates<T>(
     visible_from_module: VisibleFromModule,
     name: Option<&Name>,
     mode: LookupMode,
-    mut callback: impl FnMut(ReceiverAdjustments, AssocItemId) -> Option<T>,
+    mut callback: impl FnMut(ReceiverAdjustments, AssocItemId, bool) -> Option<T>,
 ) -> Option<T> {
     let mut slot = None;
     iterate_method_candidates_dyn(
@@ -630,9 +636,9 @@ pub(crate) fn iterate_method_candidates<T>(
         visible_from_module,
         name,
         mode,
-        &mut |adj, item| {
+        &mut |adj, item, visible| {
             assert!(slot.is_none());
-            if let Some(it) = callback(adj, item) {
+            if let Some(it) = callback(adj, item, visible) {
                 slot = Some(it);
                 return ControlFlow::Break(());
             }
@@ -771,7 +777,7 @@ pub fn iterate_path_candidates(
         name,
         LookupMode::Path,
         // the adjustments are not relevant for path lookup
-        &mut |_, id| callback(id),
+        &mut |_, id, _| callback(id),
     )
 }
 
@@ -783,7 +789,7 @@ pub fn iterate_method_candidates_dyn(
     visible_from_module: VisibleFromModule,
     name: Option<&Name>,
     mode: LookupMode,
-    callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId) -> ControlFlow<()>,
+    callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId, bool) -> ControlFlow<()>,
 ) -> ControlFlow<()> {
     match mode {
         LookupMode::MethodCall => {
@@ -847,7 +853,7 @@ fn iterate_method_candidates_with_autoref(
     traits_in_scope: &FxHashSet<TraitId>,
     visible_from_module: VisibleFromModule,
     name: Option<&Name>,
-    mut callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId) -> ControlFlow<()>,
+    mut callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId, bool) -> ControlFlow<()>,
 ) -> ControlFlow<()> {
     if receiver_ty.value.is_general_var(Interner, &receiver_ty.binders) {
         // don't try to resolve methods on unknown types
@@ -908,7 +914,7 @@ fn iterate_method_candidates_by_receiver(
     traits_in_scope: &FxHashSet<TraitId>,
     visible_from_module: VisibleFromModule,
     name: Option<&Name>,
-    mut callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId) -> ControlFlow<()>,
+    mut callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId, bool) -> ControlFlow<()>,
 ) -> ControlFlow<()> {
     let mut table = InferenceTable::new(db, env);
     let receiver_ty = table.instantiate_canonical(receiver_ty.clone());
@@ -954,7 +960,7 @@ fn iterate_method_candidates_for_self_ty(
     traits_in_scope: &FxHashSet<TraitId>,
     visible_from_module: VisibleFromModule,
     name: Option<&Name>,
-    mut callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId) -> ControlFlow<()>,
+    mut callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId, bool) -> ControlFlow<()>,
 ) -> ControlFlow<()> {
     let mut table = InferenceTable::new(db, env);
     let self_ty = table.instantiate_canonical(self_ty.clone());
@@ -985,7 +991,7 @@ fn iterate_trait_method_candidates(
     name: Option<&Name>,
     receiver_ty: Option<&Ty>,
     receiver_adjustments: Option<ReceiverAdjustments>,
-    callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId) -> ControlFlow<()>,
+    callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId, bool) -> ControlFlow<()>,
 ) -> ControlFlow<()> {
     let db = table.db;
     let env = table.trait_env.clone();
@@ -1016,17 +1022,19 @@ fn iterate_trait_method_candidates(
         for &(_, item) in data.items.iter() {
             // Don't pass a `visible_from_module` down to `is_valid_candidate`,
             // since only inherent methods should be included into visibility checking.
-            if !is_valid_candidate(table, name, receiver_ty, item, self_ty, None) {
-                continue;
-            }
+            let visible = match is_valid_candidate(table, name, receiver_ty, item, self_ty, None) {
+                IsValidCandidate::Yes => true,
+                IsValidCandidate::NotVisible => false,
+                IsValidCandidate::No => continue,
+            };
             if !known_implemented {
                 let goal = generic_implements_goal(db, env.clone(), t, &canonical_self_ty);
                 if db.trait_solve(env.krate, goal.cast(Interner)).is_none() {
                     continue 'traits;
                 }
             }
             known_implemented = true;
-            callback(receiver_adjustments.clone().unwrap_or_default(), item)?;
+            callback(receiver_adjustments.clone().unwrap_or_default(), item, visible)?;
         }
     }
     ControlFlow::Continue(())
@@ -1039,7 +1047,7 @@ fn iterate_inherent_methods(
     receiver_ty: Option<&Ty>,
     receiver_adjustments: Option<ReceiverAdjustments>,
     visible_from_module: VisibleFromModule,
-    callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId) -> ControlFlow<()>,
+    callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId, bool) -> ControlFlow<()>,
 ) -> ControlFlow<()> {
     let db = table.db;
     let env = table.trait_env.clone();
@@ -1128,17 +1136,21 @@ fn iterate_inherent_methods(
         name: Option<&Name>,
         receiver_ty: Option<&Ty>,
         receiver_adjustments: Option<ReceiverAdjustments>,
-        callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId) -> ControlFlow<()>,
+        callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId, bool) -> ControlFlow<()>,
         traits: impl Iterator<Item = TraitId>,
     ) -> ControlFlow<()> {
         let db = table.db;
         for t in traits {
             let data = db.trait_data(t);
             for &(_, item) in data.items.iter() {
                 // We don't pass `visible_from_module` as all trait items should be visible.
-                if is_valid_candidate(table, name, receiver_ty, item, self_ty, None) {
-                    callback(receiver_adjustments.clone().unwrap_or_default(), item)?;
-                }
+                let visible =
+                    match is_valid_candidate(table, name, receiver_ty, item, self_ty, None) {
+                        IsValidCandidate::Yes => true,
+                        IsValidCandidate::NotVisible => false,
+                        IsValidCandidate::No => continue,
+                    };
+                callback(receiver_adjustments.clone().unwrap_or_default(), item, visible)?;
             }
         }
         ControlFlow::Continue(())
@@ -1152,17 +1164,25 @@ fn iterate_inherent_methods(
         receiver_ty: Option<&Ty>,
         receiver_adjustments: Option<ReceiverAdjustments>,
         visible_from_module: Option<ModuleId>,
-        callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId) -> ControlFlow<()>,
+        callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId, bool) -> ControlFlow<()>,
     ) -> ControlFlow<()> {
         let db = table.db;
         let impls_for_self_ty = impls.for_self_ty(self_ty);
         for &impl_def in impls_for_self_ty {
             for &item in &db.impl_data(impl_def).items {
-                if !is_valid_candidate(table, name, receiver_ty, item, self_ty, visible_from_module)
-                {
-                    continue;
-                }
-                callback(receiver_adjustments.clone().unwrap_or_default(), item)?;
+                let visible = match is_valid_candidate(
+                    table,
+                    name,
+                    receiver_ty,
+                    item,
+                    self_ty,
+                    visible_from_module,
+                ) {
+                    IsValidCandidate::Yes => true,
+                    IsValidCandidate::NotVisible => false,
+                    IsValidCandidate::No => continue,
+                };
+                callback(receiver_adjustments.clone().unwrap_or_default(), item, visible)?;
             }
         }
         ControlFlow::Continue(())
@@ -1191,7 +1211,7 @@ pub fn resolve_indexing_op(
 macro_rules! check_that {
     ($cond:expr) => {
         if !$cond {
-            return false;
+            return IsValidCandidate::No;
         }
     };
 }
@@ -1203,7 +1223,7 @@ fn is_valid_candidate(
     item: AssocItemId,
     self_ty: &Ty,
     visible_from_module: Option<ModuleId>,
-) -> bool {
+) -> IsValidCandidate {
     let db = table.db;
     match item {
         AssocItemId::FunctionId(m) => {
@@ -1214,13 +1234,13 @@ fn is_valid_candidate(
             check_that!(receiver_ty.is_none());
 
             check_that!(name.map_or(true, |n| data.name.as_ref() == Some(n)));
-            check_that!(visible_from_module.map_or(true, |from_module| {
-                let v = db.const_visibility(c).is_visible_from(db.upcast(), from_module);
-                if !v {
+
+            if let Some(from_module) = visible_from_module {
+                if !db.const_visibility(c).is_visible_from(db.upcast(), from_module) {
                     cov_mark::hit!(const_candidate_not_visible);
+                    return IsValidCandidate::NotVisible;
                 }
-                v
-            }));
+            }
             if let ItemContainerId::ImplId(impl_id) = c.lookup(db.upcast()).container {
                 let self_ty_matches = table.run_in_snapshot(|table| {
                     let expected_self_ty = TyBuilder::impl_self_ty(db, impl_id)
@@ -1230,35 +1250,39 @@ fn is_valid_candidate(
                 });
                 if !self_ty_matches {
                     cov_mark::hit!(const_candidate_self_type_mismatch);
-                    return false;
+                    return IsValidCandidate::No;
                 }
             }
-            true
+            IsValidCandidate::Yes
         }
-        _ => false,
+        _ => IsValidCandidate::No,
     }
 }
 
+enum IsValidCandidate {
+    Yes,
+    No,
+    NotVisible,
+}
+
 fn is_valid_fn_candidate(
     table: &mut InferenceTable<'_>,
     fn_id: FunctionId,
     name: Option<&Name>,
     receiver_ty: Option<&Ty>,
     self_ty: &Ty,
     visible_from_module: Option<ModuleId>,
-) -> bool {
+) -> IsValidCandidate {
     let db = table.db;
     let data = db.function_data(fn_id);
 
     check_that!(name.map_or(true, |n| n == &data.name));
-    check_that!(visible_from_module.map_or(true, |from_module| {
-        let v = db.function_visibility(fn_id).is_visible_from(db.upcast(), from_module);
-        if !v {
+    if let Some(from_module) = visible_from_module {
+        if !db.function_visibility(fn_id).is_visible_from(db.upcast(), from_module) {
             cov_mark::hit!(autoderef_candidate_not_visible);
+            return IsValidCandidate::NotVisible;
         }
-        v
-    }));
-
+    }
     table.run_in_snapshot(|table| {
         let container = fn_id.lookup(db.upcast()).container;
         let (impl_subst, expect_self_ty) = match container {
@@ -1297,7 +1321,7 @@ fn is_valid_fn_candidate(
             // We need to consider the bounds on the impl to distinguish functions of the same name
             // for a type.
             let predicates = db.generic_predicates(impl_id.into());
-            predicates
+            let valid = predicates
                 .iter()
                 .map(|predicate| {
                     let (p, b) = predicate
@@ -1312,12 +1336,16 @@ fn is_valid_fn_candidate(
                 // It's ok to get ambiguity here, as we may not have enough information to prove
                 // obligations. We'll check if the user is calling the selected method properly
                 // later anyway.
-                .all(|p| table.try_obligation(p.cast(Interner)).is_some())
+                .all(|p| table.try_obligation(p.cast(Interner)).is_some());
+            match valid {
+                true => IsValidCandidate::Yes,
+                false => IsValidCandidate::No,
+            }
         } else {
             // For `ItemContainerId::TraitId`, we check if `self_ty` implements the trait in
             // `iterate_trait_method_candidates()`.
             // For others, this function shouldn't be called.
-            true
+            IsValidCandidate::Yes
         }
     })
 }

crates/hir-ty/src/tests/method_resolution.rs

@@ -1896,3 +1896,24 @@ impl dyn Error + Send {
 "#,
     );
 }
+
+#[test]
+fn fallback_private_methods() {
+    check(
+        r#"
+mod module {
+    pub struct Struct;
+
+    impl Struct {
+        fn func(&self) {}
+    }
+}
+
+fn foo() {
+    let s = module::Struct;
+    s.func();
+  //^^^^^^^^ type: ()
+}
+"#,
+    );
+}

crates/hir/src/lib.rs

@@ -3274,7 +3274,7 @@ impl Type {
             with_local_impls.and_then(|b| b.id.containing_block()).into(),
             name,
             method_resolution::LookupMode::MethodCall,
-            &mut |_adj, id| callback(id),
+            &mut |_adj, id, _| callback(id),
         );
     }