Consider privacy more carefully when suggesting accessing fields

Author: compiler-errors

compiler/rustc_typeck/src/check/expr.rs

@@ -2535,15 +2535,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         );
 
         // try to add a suggestion in case the field is a nested field of a field of the Adt
-        if let Some((fields, substs)) = self.get_field_candidates(span, expr_t) {
-            for candidate_field in fields.iter() {
+        let mod_id = self.tcx.parent_module(id).to_def_id();
+        if let Some((fields, substs)) =
+            self.get_field_candidates_considering_privacy(span, expr_t, mod_id)
+        {
+            for candidate_field in fields {
                 if let Some(mut field_path) = self.check_for_nested_field_satisfying(
                     span,
                     &|candidate_field, _| candidate_field.ident(self.tcx()) == field,
                     candidate_field,
                     substs,
                     vec![],
-                    self.tcx.parent_module(id).to_def_id(),
+                    mod_id,
                 ) {
                     // field_path includes `field` that we're looking for, so pop it.
                     field_path.pop();
@@ -2567,22 +2570,28 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         err
     }
 
-    pub(crate) fn get_field_candidates(
+    pub(crate) fn get_field_candidates_considering_privacy(
         &self,
         span: Span,
-        base_t: Ty<'tcx>,
-    ) -> Option<(&[ty::FieldDef], SubstsRef<'tcx>)> {
-        debug!("get_field_candidates(span: {:?}, base_t: {:?}", span, base_t);
+        base_ty: Ty<'tcx>,
+        mod_id: DefId,
+    ) -> Option<(impl Iterator<Item = &'tcx ty::FieldDef> + 'tcx, SubstsRef<'tcx>)> {
+        debug!("get_field_candidates(span: {:?}, base_t: {:?}", span, base_ty);
 
-        for (base_t, _) in self.autoderef(span, base_t) {
+        for (base_t, _) in self.autoderef(span, base_ty) {
             match base_t.kind() {
                 ty::Adt(base_def, substs) if !base_def.is_enum() => {
-                    let fields = &base_def.non_enum_variant().fields;
-                    // For compile-time reasons put a limit on number of fields we search
-                    if fields.len() > 100 {
-                        return None;
-                    }
-                    return Some((fields, substs));
+                    let tcx = self.tcx;
+                    return Some((
+                        base_def
+                            .non_enum_variant()
+                            .fields
+                            .iter()
+                            .filter(move |field| field.vis.is_accessible_from(mod_id, tcx))
+                            // For compile-time reasons put a limit on number of fields we search
+                            .take(100),
+                        substs,
+                    ));
                 }
                 _ => {}
             }
@@ -2599,7 +2608,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         candidate_field: &ty::FieldDef,
         subst: SubstsRef<'tcx>,
         mut field_path: Vec<Ident>,
-        id: DefId,
+        mod_id: DefId,
     ) -> Option<Vec<Ident>> {
         debug!(
             "check_for_nested_field_satisfying(span: {:?}, candidate_field: {:?}, field_path: {:?}",
@@ -2615,20 +2624,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             let field_ty = candidate_field.ty(self.tcx, subst);
             if matches(candidate_field, field_ty) {
                 return Some(field_path);
-            } else if let Some((nested_fields, subst)) = self.get_field_candidates(span, field_ty) {
+            } else if let Some((nested_fields, subst)) =
+                self.get_field_candidates_considering_privacy(span, field_ty, mod_id)
+            {
                 // recursively search fields of `candidate_field` if it's a ty::Adt
                 for field in nested_fields {
-                    if field.vis.is_accessible_from(id, self.tcx) {
-                        if let Some(field_path) = self.check_for_nested_field_satisfying(
-                            span,
-                            matches,
-                            field,
-                            subst,
-                            field_path.clone(),
-                            id,
-                        ) {
-                            return Some(field_path);
-                        }
+                    if let Some(field_path) = self.check_for_nested_field_satisfying(
+                        span,
+                        matches,
+                        field,
+                        subst,
+                        field_path.clone(),
+                        mod_id,
+                    ) {
+                        return Some(field_path);
                     }
                 }
             }

compiler/rustc_typeck/src/check/method/suggest.rs

@@ -1334,10 +1334,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         item_name: Ident,
     ) {
         if let SelfSource::MethodCall(expr) = source
-            && let Some((fields, substs)) = self.get_field_candidates(span, actual)
+            && let mod_id = self.tcx.parent_module(expr.hir_id).to_def_id()
+            && let Some((fields, substs)) = self.get_field_candidates_considering_privacy(span, actual, mod_id)
         {
             let call_expr = self.tcx.hir().expect_expr(self.tcx.hir().get_parent_node(expr.hir_id));
-            for candidate_field in fields.iter() {
+            for candidate_field in fields {
                 if let Some(field_path) = self.check_for_nested_field_satisfying(
                     span,
                     &|_, field_ty| {
@@ -1353,7 +1354,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     candidate_field,
                     substs,
                     vec![],
-                    self.tcx.parent_module(expr.hir_id).to_def_id(),
+                    mod_id,
                 ) {
                     let field_path_str = field_path
                         .iter()

src/test/ui/issues/issue-31173.stderr

@@ -33,10 +33,6 @@ LL | pub struct TakeWhile<I, P> {
            which is required by `Cloned<TakeWhile<&mut std::vec::IntoIter<u8>, [closure@$DIR/issue-31173.rs:6:39: 6:43]>>: Iterator`
            `Cloned<TakeWhile<&mut std::vec::IntoIter<u8>, [closure@$DIR/issue-31173.rs:6:39: 6:43]>>: Iterator`
            which is required by `&mut Cloned<TakeWhile<&mut std::vec::IntoIter<u8>, [closure@$DIR/issue-31173.rs:6:39: 6:43]>>: Iterator`
-help: one of the expressions' fields has a method of the same name
-   |
-LL |         .it.collect();
-   |          +++
 
 error: aborting due to 2 previous errors
 

src/test/ui/issues/issue-39175.stderr

@@ -5,10 +5,6 @@ LL |     Command::new("echo").arg("hello").exec();
    |                                       ^^^^ method not found in `&mut Command`
    |
    = help: items from traits can only be used if the trait is in scope
-help: one of the expressions' fields has a method of the same name
-   |
-LL |     Command::new("echo").arg("hello").inner.exec();
-   |                                       ++++++
 help: the following trait is implemented but not in scope; perhaps add a `use` for it:
    |
 LL | use std::os::unix::process::CommandExt;

src/test/ui/mismatched_types/issue-36053-2.stderr

@@ -35,10 +35,6 @@ LL | pub struct Filter<I, P> {
            which is required by `Filter<Fuse<std::iter::Once<&str>>, [closure@$DIR/issue-36053-2.rs:7:39: 7:48]>: Iterator`
            `Filter<Fuse<std::iter::Once<&str>>, [closure@$DIR/issue-36053-2.rs:7:39: 7:48]>: Iterator`
            which is required by `&mut Filter<Fuse<std::iter::Once<&str>>, [closure@$DIR/issue-36053-2.rs:7:39: 7:48]>: Iterator`
-help: one of the expressions' fields has a method of the same name
-   |
-LL |     once::<&str>("str").fuse().filter(|a: &str| true).iter.count();
-   |                                                       +++++
 
 error: aborting due to 2 previous errors
 

src/test/ui/suggestions/import-trait-for-method-call.stderr

@@ -10,10 +10,6 @@ LL |     fn finish(&self) -> u64;
    |        ------ the method is available for `DefaultHasher` here
    |
    = help: items from traits can only be used if the trait is in scope
-help: one of the expressions' fields has a method of the same name
-   |
-LL |     h.0.finish()
-   |       ++
 help: the following trait is implemented but not in scope; perhaps add a `use` for it:
    |
 LL | use std::hash::Hasher;

src/test/ui/suggestions/mut-borrow-needed-by-trait.stderr

@@ -41,16 +41,6 @@ LL | pub struct BufWriter<W: Write> {
            `&dyn std::io::Write: std::io::Write`
            which is required by `BufWriter<&dyn std::io::Write>: std::io::Write`
    = note: this error originates in the macro `writeln` (in Nightly builds, run with -Z macro-backtrace for more info)
-help: one of the expressions' fields has a method of the same name
-  --> $SRC_DIR/core/src/macros/mod.rs:LL:COL
-   |
-LL |         $dst.inner.write_fmt($crate::format_args_nl!($($arg)*))
-   |              ++++++
-help: one of the expressions' fields has a method of the same name
-  --> $SRC_DIR/core/src/macros/mod.rs:LL:COL
-   |
-LL |         $dst.buf.write_fmt($crate::format_args_nl!($($arg)*))
-   |              ++++
 
 error: aborting due to 3 previous errors
 

src/test/ui/suggestions/suggest-using-chars.stderr

@@ -25,10 +25,6 @@ help: because of the in-memory representation of `&str`, to obtain an `Iterator`
    |
 LL |     let _ = String::from("bar").chars();
    |                                 ~~~~~
-help: one of the expressions' fields has a method of the same name
-   |
-LL |     let _ = String::from("bar").vec.iter();
-   |                                 ++++
 
 error[E0599]: no method named `iter` found for reference `&String` in the current scope
   --> $DIR/suggest-using-chars.rs:5:36
@@ -40,10 +36,6 @@ help: because of the in-memory representation of `&str`, to obtain an `Iterator`
    |
 LL |     let _ = (&String::from("bar")).chars();
    |                                    ~~~~~
-help: one of the expressions' fields has a method of the same name
-   |
-LL |     let _ = (&String::from("bar")).vec.iter();
-   |                                    ++++
 
 error[E0599]: no method named `iter` found for type `{integer}` in the current scope
   --> $DIR/suggest-using-chars.rs:6:15

src/test/ui/unique-object-noncopyable.stderr

@@ -23,14 +23,6 @@ LL | | >(Unique<T>, A);
            which is required by `Box<dyn Foo>: Clone`
            `dyn Foo: Clone`
            which is required by `Box<dyn Foo>: Clone`
-help: one of the expressions' fields has a method of the same name
-   |
-LL |     let _z = y.0.clone();
-   |                ++
-help: one of the expressions' fields has a method of the same name
-   |
-LL |     let _z = y.1.clone();
-   |                ++
 
 error: aborting due to previous error
 

src/test/ui/unique-pinned-nocopy.stderr

@@ -25,14 +25,6 @@ help: consider annotating `R` with `#[derive(Clone)]`
    |
 LL | #[derive(Clone)]
    |
-help: one of the expressions' fields has a method of the same name
-   |
-LL |     let _j = i.0.clone();
-   |                ++
-help: one of the expressions' fields has a method of the same name
-   |
-LL |     let _j = i.1.clone();
-   |                ++
 
 error: aborting due to previous error