clean up struct field suggestions

Author: fmease

compiler/rustc_hir_typeck/src/expr.rs

@@ -41,7 +41,6 @@ use rustc_infer::infer::DefineOpaqueTypes;
 use rustc_infer::infer::InferOk;
 use rustc_infer::traits::query::NoSolution;
 use rustc_infer::traits::ObligationCause;
-use rustc_middle::middle::stability;
 use rustc_middle::ty::adjustment::{Adjust, Adjustment, AllowTwoPhase};
 use rustc_middle::ty::error::{
     ExpectedFound,
@@ -1662,6 +1661,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     self.report_unknown_field(
                         adt_ty,
                         variant,
+                        expr_id,
                         field,
                         ast_fields,
                         adt.variant_descr(),
@@ -2049,6 +2049,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         &self,
         ty: Ty<'tcx>,
         variant: &'tcx ty::VariantDef,
+        expr_id: HirId,
         field: &hir::ExprField<'_>,
         skip_fields: &[hir::ExprField<'_>],
         kind_name: &str,
@@ -2129,9 +2130,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             },
             _ => {
                 // prevent all specified fields from being suggested
-                let skip_fields: Vec<_> = skip_fields.iter().map(|x| x.ident.name).collect();
+                let available_field_names =
+                    self.available_field_names(variant, expr_id, skip_fields);
                 if let Some(field_name) =
-                    self.suggest_field_name(variant, field.ident.name, &skip_fields, expr_span)
+                    find_best_match_for_name(&available_field_names, field.ident.name, None)
                 {
                     err.span_suggestion(
                         field.ident.span,
@@ -2153,10 +2155,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                                     format!("`{ty}` does not have this field"),
                                 );
                             }
-                            let mut available_field_names =
-                                self.available_field_names(variant, expr_span);
-                            available_field_names
-                                .retain(|name| skip_fields.iter().all(|skip| name != skip));
                             if available_field_names.is_empty() {
                                 err.note("all struct fields are already assigned");
                             } else {
@@ -2174,63 +2172,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         err.emit()
     }
 
-    // Return a hint about the closest match in field names
-    fn suggest_field_name(
-        &self,
-        variant: &'tcx ty::VariantDef,
-        field: Symbol,
-        skip: &[Symbol],
-        // The span where stability will be checked
-        span: Span,
-    ) -> Option<Symbol> {
-        let names = variant
-            .fields
-            .iter()
-            .filter_map(|field| {
-                // ignore already set fields and private fields from non-local crates
-                // and unstable fields.
-                if skip.iter().any(|&x| x == field.name)
-                    || (!variant.def_id.is_local() && !field.vis.is_public())
-                    || matches!(
-                        self.tcx.eval_stability(field.did, None, span, None),
-                        stability::EvalResult::Deny { .. }
-                    )
-                {
-                    None
-                } else {
-                    Some(field.name)
-                }
-            })
-            .collect::<Vec<Symbol>>();
-
-        find_best_match_for_name(&names, field, None)
-    }
-
     fn available_field_names(
         &self,
         variant: &'tcx ty::VariantDef,
-        access_span: Span,
+        expr_id: HirId,
+        skip_fields: &[hir::ExprField<'_>],
     ) -> Vec<Symbol> {
-        let body_owner_hir_id = self.tcx.hir().local_def_id_to_hir_id(self.body_id);
         variant
             .fields
             .iter()
             .filter(|field| {
-                let def_scope = self
-                    .tcx
-                    .adjust_ident_and_get_scope(
-                        field.ident(self.tcx),
-                        variant.def_id,
-                        body_owner_hir_id,
-                    )
-                    .1;
-                field.vis.is_accessible_from(def_scope, self.tcx)
-                    && !matches!(
-                        self.tcx.eval_stability(field.did, None, access_span, None),
-                        stability::EvalResult::Deny { .. }
-                    )
+                skip_fields.iter().all(|&skip| skip.ident.name != field.name)
+                    && self.is_field_suggestable(field, expr_id)
             })
-            .filter(|field| !self.tcx.is_doc_hidden(field.did))
             .map(|field| field.name)
             .collect()
     }
@@ -2460,7 +2414,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 self.suggest_first_deref_field(&mut err, expr, base, ident);
             }
             ty::Adt(def, _) if !def.is_enum() => {
-                self.suggest_fields_on_recordish(&mut err, def, ident, expr.span);
+                self.suggest_fields_on_recordish(&mut err, expr.hir_id, def, ident);
             }
             ty::Param(param_ty) => {
                 self.point_at_param_definition(&mut err, param_ty);
@@ -2622,12 +2576,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     fn suggest_fields_on_recordish(
         &self,
         err: &mut Diagnostic,
+        expr_id: HirId,
         def: ty::AdtDef<'tcx>,
         field: Ident,
-        access_span: Span,
     ) {
+        let available_field_names =
+            self.available_field_names(def.non_enum_variant(), expr_id, &[]);
         if let Some(suggested_field_name) =
-            self.suggest_field_name(def.non_enum_variant(), field.name, &[], access_span)
+            find_best_match_for_name(&available_field_names, field.name, None)
         {
             err.span_suggestion(
                 field.span,
@@ -2637,12 +2593,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             );
         } else {
             err.span_label(field.span, "unknown field");
-            let struct_variant_def = def.non_enum_variant();
-            let field_names = self.available_field_names(struct_variant_def, access_span);
-            if !field_names.is_empty() {
+            if !available_field_names.is_empty() {
                 err.note(format!(
                     "available fields are: {}",
-                    self.name_series_display(field_names),
+                    self.name_series_display(available_field_names),
                 ));
             }
         }

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

@@ -1687,4 +1687,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             false
         }
     }
+
+    pub(crate) fn is_field_suggestable(&self, field: &ty::FieldDef, hir_id: HirId) -> bool {
+        // The field must be visible in the containing module.
+        field.vis.is_accessible_from(self.tcx.parent_module(hir_id), self.tcx)
+            // The field must not be unstable.
+            && !matches!(
+                self.tcx.eval_stability(field.did, None, rustc_span::DUMMY_SP, None),
+                rustc_middle::middle::stability::EvalResult::Deny { .. }
+            )
+            // If the field is from an external crate it must not be `doc(hidden)`.
+            && (field.did.is_local() || !self.tcx.is_doc_hidden(field.did))
+            // If the field is hygienic it must not originate from a macro expansion.
+            && !self.tcx.def_ident_span(field.did).unwrap().normalize_to_macros_2_0().from_expansion()
+    }
 }

compiler/rustc_hir_typeck/src/pat.rs

@@ -12,7 +12,6 @@ use rustc_hir::pat_util::EnumerateAndAdjustIterator;
 use rustc_hir::{HirId, Pat, PatKind};
 use rustc_infer::infer;
 use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
-use rustc_middle::middle::stability::EvalResult;
 use rustc_middle::ty::{self, Adt, BindingMode, Ty, TypeVisitableExt};
 use rustc_session::lint::builtin::NON_EXHAUSTIVE_OMITTED_PATTERNS;
 use rustc_span::edit_distance::find_best_match_for_name;
@@ -1408,6 +1407,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 adt.variant_descr(),
                 &inexistent_fields,
                 &mut unmentioned_fields,
+                pat,
                 variant,
                 args,
             ))
@@ -1434,15 +1434,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             let accessible_unmentioned_fields: Vec<_> = unmentioned_fields
                 .iter()
                 .copied()
-                .filter(|(field, _)| {
-                    field.vis.is_accessible_from(tcx.parent_module(pat.hir_id), tcx)
-                        && !matches!(
-                            tcx.eval_stability(field.did, None, DUMMY_SP, None),
-                            EvalResult::Deny { .. }
-                        )
-                        // We only want to report the error if it is hidden and not local
-                        && !(tcx.is_doc_hidden(field.did) && !field.did.is_local())
-                })
+                .filter(|(field, _)| self.is_field_suggestable(field, pat.hir_id))
                 .collect();
 
             if !has_rest_pat {
@@ -1578,12 +1570,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         kind_name: &str,
         inexistent_fields: &[&hir::PatField<'tcx>],
         unmentioned_fields: &mut Vec<(&'tcx ty::FieldDef, Ident)>,
+        pat: &'tcx Pat<'tcx>,
         variant: &ty::VariantDef,
         args: &'tcx ty::List<ty::GenericArg<'tcx>>,
     ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
         let tcx = self.tcx;
-        let (field_names, t, plural) = if inexistent_fields.len() == 1 {
-            (format!("a field named `{}`", inexistent_fields[0].ident), "this", "")
+        let (field_names, t, plural) = if let [field] = inexistent_fields {
+            (format!("a field named `{}`", field.ident), "this", "")
         } else {
             (
                 format!(
@@ -1620,10 +1613,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 ),
             );
 
-            if unmentioned_fields.len() == 1 {
-                let input =
-                    unmentioned_fields.iter().map(|(_, field)| field.name).collect::<Vec<_>>();
-                let suggested_name = find_best_match_for_name(&input, pat_field.ident.name, None);
+            if let [(field_def, field)] = unmentioned_fields.as_slice()
+                && self.is_field_suggestable(field_def, pat.hir_id)
+            {
+                let suggested_name =
+                    find_best_match_for_name(&[field.name], pat_field.ident.name, None);
                 if let Some(suggested_name) = suggested_name {
                     err.span_suggestion(
                         pat_field.ident.span,
@@ -1646,22 +1640,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         PatKind::Lit(expr)
                             if !self.can_coerce(
                                 self.typeck_results.borrow().expr_ty(expr),
-                                self.field_ty(
-                                    unmentioned_fields[0].1.span,
-                                    unmentioned_fields[0].0,
-                                    args,
-                                ),
+                                self.field_ty(field.span, field_def, args),
                             ) => {}
                         _ => {
-                            let unmentioned_field = unmentioned_fields[0].1.name;
                             err.span_suggestion_short(
                                 pat_field.ident.span,
                                 format!(
                                     "`{}` has a field named `{}`",
                                     tcx.def_path_str(variant.def_id),
-                                    unmentioned_field
+                                    field.name,
                                 ),
-                                unmentioned_field.to_string(),
+                                field.name,
                                 Applicability::MaybeIncorrect,
                             );
                         }
@@ -1871,8 +1860,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         fields: &'tcx [hir::PatField<'tcx>],
     ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
         let inaccessible = if have_inaccessible_fields { " and inaccessible fields" } else { "" };
-        let field_names = if unmentioned_fields.len() == 1 {
-            format!("field `{}`{}", unmentioned_fields[0].1, inaccessible)
+        let field_names = if let [(_, field)] = unmentioned_fields {
+            format!("field `{field}`{inaccessible}")
         } else {
             let fields = unmentioned_fields
                 .iter()

tests/ui/did_you_mean/auxiliary/doc-hidden-fields.rs

@@ -0,0 +1,6 @@
+#[derive(Default)]
+pub struct B {
+    #[doc(hidden)]
+    pub hello: i32,
+    pub bye: i32,
+}

tests/ui/did_you_mean/dont-suggest-doc-hidden-fields.rs

@@ -0,0 +1,38 @@
+// Regression test for issue #93210.
+
+// aux-crate:doc_hidden_fields=doc-hidden-fields.rs
+// edition: 2021
+
+#[derive(Default)]
+pub struct A {
+    #[doc(hidden)]
+    pub hello: i32,
+    pub bye: i32,
+}
+
+#[derive(Default)]
+pub struct C {
+    pub hello: i32,
+    pub bye: i32,
+}
+
+fn main() {
+    // We want to list the field `hello` despite being marked
+    // `doc(hidden)` because it's defined in this crate.
+    A::default().hey;
+    //~^ ERROR no field `hey` on type `A`
+    //~| NOTE unknown field
+    //~| NOTE available fields are: `hello`, `bye`
+
+    // Here we want to hide the field `hello` since it's marked
+    // `doc(hidden)` and comes from an external crate.
+    doc_hidden_fields::B::default().hey;
+    //~^ ERROR no field `hey` on type `B`
+    //~| NOTE unknown field
+    //~| NOTE available fields are: `bye`
+
+    C::default().hey;
+    //~^ ERROR no field `hey` on type `C`
+    //~| NOTE unknown field
+    //~| NOTE available fields are: `hello`, `bye`
+}

tests/ui/did_you_mean/dont-suggest-doc-hidden-fields.stderr

@@ -0,0 +1,27 @@
+error[E0609]: no field `hey` on type `A`
+  --> $DIR/dont-suggest-doc-hidden-fields.rs:22:18
+   |
+LL |     A::default().hey;
+   |                  ^^^ unknown field
+   |
+   = note: available fields are: `hello`, `bye`
+
+error[E0609]: no field `hey` on type `B`
+  --> $DIR/dont-suggest-doc-hidden-fields.rs:29:37
+   |
+LL |     doc_hidden_fields::B::default().hey;
+   |                                     ^^^ unknown field
+   |
+   = note: available fields are: `bye`
+
+error[E0609]: no field `hey` on type `C`
+  --> $DIR/dont-suggest-doc-hidden-fields.rs:34:18
+   |
+LL |     C::default().hey;
+   |                  ^^^ unknown field
+   |
+   = note: available fields are: `hello`, `bye`
+
+error: aborting due to 3 previous errors
+
+For more information about this error, try `rustc --explain E0609`.

tests/ui/did_you_mean/dont-suggest-hygienic-fields.rs

@@ -0,0 +1,33 @@
+// Regression test for issue #116334.
+// Don't include hygienic fields in the list of available or similarly named fields.
+
+#![feature(decl_macro)]
+
+macro compound($Ty:ident) {
+    #[derive(Default)]
+    struct $Ty {
+        field: u32, // field `field` is hygienic
+    }
+}
+
+macro component($Ty:ident) {
+    struct $Ty(u64); // field `0` is hygienic (but still accessible via the constructor)
+}
+
+compound! { Compound }
+component! { Component }
+
+fn main() {
+    let ty = Compound::default();
+
+    let _ = ty.field; //~ ERROR no field `field` on type `Compound`
+    let _ = ty.fieeld; //~ ERROR no field `fieeld` on type `Compound`
+
+    let Compound { field } = ty;
+    //~^ ERROR struct `Compound` does not have a field named `field`
+    //~| ERROR pattern requires `..` due to inaccessible fields
+
+    let ty = Component(90);
+
+    let _ = ty.0; //~ ERROR no field `0` on type `Component`
+}