diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index d9d0dd93010c7..82443c30eff99 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -1659,10 +1659,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.report_unknown_field( adt_ty, variant, - expr, field, ast_fields, adt.variant_descr(), + expr.span, ) }; @@ -2046,16 +2046,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &self, ty: Ty<'tcx>, variant: &'tcx ty::VariantDef, - expr: &hir::Expr<'_>, field: &hir::ExprField<'_>, skip_fields: &[hir::ExprField<'_>], kind_name: &str, + expr_span: Span, ) -> ErrorGuaranteed { if variant.is_recovered() { let guar = self .tcx .sess - .delay_span_bug(expr.span, "parser recovered but no error was emitted"); + .delay_span_bug(expr_span, "parser recovered but no error was emitted"); self.set_tainted_by_errors(guar); return guar; } @@ -2099,7 +2099,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); err.span_label(field.ident.span, "field does not exist"); err.span_suggestion_verbose( - expr.span, + expr_span, format!( "`{adt}::{variant}` is a tuple {kind_name}, use the appropriate syntax", adt = ty, @@ -2117,7 +2117,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { err.span_label(variant_ident_span, format!("`{ty}` defined here")); err.span_label(field.ident.span, "field does not exist"); err.span_suggestion_verbose( - expr.span, + expr_span, format!("`{ty}` is a tuple {kind_name}, use the appropriate syntax",), format!("{ty}(/* fields */)"), Applicability::HasPlaceholders, @@ -2125,8 +2125,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } }, _ => { - // prevent all specified fields from being suggested - let available_field_names = self.available_field_names(variant, expr, skip_fields); + // Prevent all specified fields from being suggested with `skip_fields`. + let available_field_names = + self.available_field_names(variant, field.ident.span, skip_fields); if let Some(field_name) = find_best_match_for_name(&available_field_names, field.ident.name, None) { @@ -2170,15 +2171,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { fn available_field_names( &self, variant: &'tcx ty::VariantDef, - expr: &hir::Expr<'_>, + span: Span, skip_fields: &[hir::ExprField<'_>], ) -> Vec { + let block = self.tcx.hir().local_def_id_to_hir_id(self.body_id); + let (span, def_scope) = self.tcx.adjust_span_and_get_scope(span, variant.def_id, block); + variant .fields .iter() .filter(|field| { skip_fields.iter().all(|&skip| skip.ident.name != field.name) - && self.is_field_suggestable(field, expr.hir_id, expr.span) + && self.is_field_suggestable(field, def_scope, span) }) .map(|field| field.name) .collect() @@ -2409,7 +2413,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, expr, def, ident); + self.suggest_fields_on_recordish(&mut err, def, ident); } ty::Param(param_ty) => { self.point_at_param_definition(&mut err, param_ty); @@ -2571,11 +2575,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { fn suggest_fields_on_recordish( &self, err: &mut Diagnostic, - expr: &hir::Expr<'_>, def: ty::AdtDef<'tcx>, field: Ident, ) { - let available_field_names = self.available_field_names(def.non_enum_variant(), expr, &[]); + let available_field_names = + self.available_field_names(def.non_enum_variant(), field.span, &[]); if let Some(suggested_field_name) = find_best_match_for_name(&available_field_names, field.name, None) { diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs index 9999fa2e59ccf..f2db299631b5b 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs @@ -7,6 +7,7 @@ use rustc_ast::util::parser::{ExprPrecedence, PREC_POSTFIX}; use rustc_errors::{Applicability, Diagnostic, MultiSpan}; use rustc_hir as hir; use rustc_hir::def::{CtorKind, CtorOf, DefKind}; +use rustc_hir::def_id::DefId; use rustc_hir::lang_items::LangItem; use rustc_hir::{ AsyncGeneratorKind, Expr, ExprKind, GeneratorKind, GenericBound, HirId, Node, Path, QPath, @@ -1686,14 +1687,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } + /// Returns `true` if the given `field` can be suggested in diagnostics. + /// + /// The `span` represents the precise expected usage site, normalized to + /// macros 2.0 and adjusted to the expansion that defined the scope. pub(crate) fn is_field_suggestable( &self, field: &ty::FieldDef, - hir_id: HirId, + def_scope: DefId, span: Span, ) -> 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 be visible in the containing scope. + field.vis.is_accessible_from(def_scope, self.tcx) // The field must not be unstable. && !matches!( self.tcx.eval_stability(field.did, None, rustc_span::DUMMY_SP, None), diff --git a/compiler/rustc_hir_typeck/src/pat.rs b/compiler/rustc_hir_typeck/src/pat.rs index 3f9c9b3381baa..6fc956b4c1c98 100644 --- a/compiler/rustc_hir_typeck/src/pat.rs +++ b/compiler/rustc_hir_typeck/src/pat.rs @@ -1407,7 +1407,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { adt.variant_descr(), &inexistent_fields, &mut unmentioned_fields, - pat, variant, args, )) @@ -1431,10 +1430,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { tcx.sess.emit_err(errors::UnionPatDotDot { span: pat.span }); } } else if !unmentioned_fields.is_empty() { + let block = self.tcx.hir().local_def_id_to_hir_id(self.body_id); + // It's hard to pinpoint who is to blame for the unmentioned fields and for whom those + // fields are actually inaccessible since the fields that *are* mentioned may come from + // different expansions. Instead of employing a heuristic that tries to find the most + // relevant expansion, just blame the struct pattern `pat` which isn't great but + // at least it's technically correct. + let (span, def_scope) = + self.tcx.adjust_span_and_get_scope(pat.span, variant.def_id, block); + let accessible_unmentioned_fields: Vec<_> = unmentioned_fields .iter() .copied() - .filter(|(field, _)| self.is_field_suggestable(field, pat.hir_id, pat.span)) + .filter(|(field, _)| self.is_field_suggestable(field, def_scope, span)) .collect(); if !has_rest_pat { @@ -1570,7 +1578,6 @@ 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>, ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { @@ -1614,7 +1621,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); if let [(field_def, field)] = unmentioned_fields.as_slice() - && self.is_field_suggestable(field_def, pat.hir_id, pat.span) + && let block = self.tcx.hir().local_def_id_to_hir_id(self.body_id) + && let (span, def_scope) = + self.tcx.adjust_span_and_get_scope(pat_field.ident.span, variant.def_id, block) + && self.is_field_suggestable(field_def, def_scope, span) { let suggested_name = find_best_match_for_name(&[field.name], pat_field.ident.name, None); @@ -1881,17 +1891,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let len = unmentioned_fields.len(); let (prefix, postfix, sp) = match fields { [] => match &pat.kind { - PatKind::Struct(path, [], false) => { + PatKind::Struct(path, [], false) if path.span().eq_ctxt(pat.span) => { (" { ", " }", path.span().shrink_to_hi().until(pat.span.shrink_to_hi())) } _ => return err, }, [.., field] => { - // Account for last field having a trailing comma or parse recovery at the tail of - // the pattern to avoid invalid suggestion (#78511). - let tail = field.span.shrink_to_hi().with_hi(pat.span.hi()); match &pat.kind { - PatKind::Struct(..) => (", ", " }", tail), + // Account for last field having a trailing comma or parse recovery at the tail of + // the pattern to avoid invalid suggestion (#78511). + PatKind::Struct(..) if field.span.eq_ctxt(pat.span) => { + (", ", " }", field.span.shrink_to_hi().with_hi(pat.span.hi())) + } _ => return err, } } diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index b7d2e3d9493f9..0e7d5adc62441 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -2510,16 +2510,26 @@ impl<'tcx> TyCtxt<'tcx> { // FIXME(vincenzopalazzo): move the HirId to a LocalDefId pub fn adjust_ident_and_get_scope( self, - mut ident: Ident, + ident: Ident, scope: DefId, block: hir::HirId, ) -> (Ident, DefId) { - let scope = ident - .span + let (span, scope) = self.adjust_span_and_get_scope(ident.span, scope, block); + (Ident { span, ..ident }, scope) + } + + // FIXME(vincenzopalazzo): move the HirId to a LocalDefId + pub fn adjust_span_and_get_scope( + self, + mut span: Span, + scope: DefId, + block: hir::HirId, + ) -> (Span, DefId) { + let scope = span .normalize_to_macros_2_0_and_adjust(self.expn_that_defined(scope)) .and_then(|actual_expansion| actual_expansion.expn_data().parent_module) .unwrap_or_else(|| self.parent_module(block).to_def_id()); - (ident, scope) + (span, scope) } /// Returns `true` if the debuginfo for `span` should be collapsed to the outermost expansion diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index d271519a8a385..90492a8a547a4 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -1102,39 +1102,45 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { }); return if all_ns_failed { - let resolutions = match module { - ModuleOrUniformRoot::Module(module) => Some(self.resolutions(module).borrow()), - _ => None, - }; - let resolutions = resolutions.as_ref().into_iter().flat_map(|r| r.iter()); - let names = resolutions - .filter_map(|(BindingKey { ident: i, .. }, resolution)| { - if i.name == ident.name { - return None; - } // Never suggest the same name - match *resolution.borrow() { - NameResolution { binding: Some(name_binding), .. } => { - match name_binding.kind { - NameBindingKind::Import { binding, .. } => { - match binding.kind { - // Never suggest the name that has binding error - // i.e., the name that cannot be previously resolved - NameBindingKind::Res(Res::Err) => None, - _ => Some(i.name), - } + let names = if let ModuleOrUniformRoot::Module(module) = module { + let mut adjusted_sp = ident.span; + adjusted_sp.normalize_to_macros_2_0_and_adjust(module.expansion); + + self.resolutions(module) + .borrow() + .iter() + .filter_map(|(BindingKey { ident: i, .. }, resolution)| { + // Never suggest the same name + if i.name == ident.name { + return None; + } + // Don't suggest hygienic names from different syntax contexts. + if !i.span.normalize_to_macros_2_0().eq_ctxt(adjusted_sp) { + return None; + } + match *resolution.borrow() { + NameResolution { binding: Some(name_binding), .. } => { + // Never suggest the name that has binding error + // i.e., the name that cannot be previously resolved + if let NameBindingKind::Import { binding, .. } = name_binding.kind + && let NameBindingKind::Res(Res::Err) = binding.kind + { + return None; } - _ => Some(i.name), } + NameResolution { ref single_imports, .. } + if single_imports.is_empty() => + { + return None; + } + _ => {} } - NameResolution { ref single_imports, .. } - if single_imports.is_empty() => - { - None - } - _ => Some(i.name), - } - }) - .collect::>(); + Some(i.name) + }) + .collect() + } else { + Vec::new() + }; let lev_suggestion = find_best_match_for_name(&names, ident.name, None).map(|suggestion| { diff --git a/tests/ui/did_you_mean/dont-suggest-hygienic-fields.rs b/tests/ui/did_you_mean/dont-suggest-hygienic-fields.rs index fb7040b2df095..e17bcd817f078 100644 --- a/tests/ui/did_you_mean/dont-suggest-hygienic-fields.rs +++ b/tests/ui/did_you_mean/dont-suggest-hygienic-fields.rs @@ -4,6 +4,11 @@ #![feature(decl_macro)] +// +// Test cases where we should *not* suggest hygienic fields since +// they are inaccessible in the respective syntax context: +// + macro compound($Ty:ident) { #[derive(Default)] struct $Ty { @@ -18,6 +23,24 @@ macro component($Ty:ident) { compound! { Compound } component! { Component } +macro inject_into_expr_bad($field:ident) {{ + struct Casket { + field: (), + } + + const CASKET: Casket = Casket { field: () }; + + CASKET.$field +}} + +macro inject_into_pat_bad($( $any:tt )*) {{ + struct Casket { field: () } + + const CASKET: Casket = Casket { field: () }; + + let Casket { $( $any )* } = CASKET; //~ ERROR pattern does not mention field `field` +}} + fn main() { let ty = Compound::default(); @@ -32,10 +55,21 @@ fn main() { let ty = Component(90); let _ = ty.0; //~ ERROR no field `0` on type `Component` + + let _ = inject_into_expr_bad!(field); //~ ERROR no field `field` on type `main::Casket` + + inject_into_pat_bad!(field: _); //~ ERROR struct `main::Casket` does not have a field named `field` } +// +// Test cases where we *should* suggest hygienic fields since +// they're accessible in the respective syntax context: +// + environment!(); +struct Case { field: () } + macro environment() { struct Crate { field: () } @@ -44,4 +78,28 @@ macro environment() { const CRATE: Crate = Crate { fiel: () }; //~^ ERROR struct `Crate` has no field named `fiel` //~| HELP a field with a similar name exists + + // `field` isn't in the same syntax context but in a parent one + // which means it's accessible and should be suggested. + const CASE: Case = Case { fiel: () }; + //~^ ERROR struct `Case` has no field named `fiel` + //~| HELP a field with a similar name exists +} + +macro inject_into_expr_good($field_def_site:ident, $field_use_site:ident) {{ + struct Carton { + $field_def_site: (), + } + + const CARTON: Carton = Carton { $field_def_site: () }; + + CARTON.$field_use_site +}} + +fn scope() { + // It's immaterial that `CARTON` / the field expression as a whole isn't accessible in this + // syntax context, the identifier substituted for `$field_use_site` *is*. + let _ = inject_into_expr_good!(field, fiel); + //~^ ERROR no field `fiel` on type `Carton` + //~| HELP a field with a similar name exists } diff --git a/tests/ui/did_you_mean/dont-suggest-hygienic-fields.stderr b/tests/ui/did_you_mean/dont-suggest-hygienic-fields.stderr index 7066d29760e7c..2ed5bd21311c5 100644 --- a/tests/ui/did_you_mean/dont-suggest-hygienic-fields.stderr +++ b/tests/ui/did_you_mean/dont-suggest-hygienic-fields.stderr @@ -1,5 +1,5 @@ error[E0560]: struct `Crate` has no field named `fiel` - --> $DIR/dont-suggest-hygienic-fields.rs:44:34 + --> $DIR/dont-suggest-hygienic-fields.rs:78:34 | LL | environment!(); | -------------- in this macro invocation @@ -9,26 +9,37 @@ LL | const CRATE: Crate = Crate { fiel: () }; | = note: this error originates in the macro `environment` (in Nightly builds, run with -Z macro-backtrace for more info) +error[E0560]: struct `Case` has no field named `fiel` + --> $DIR/dont-suggest-hygienic-fields.rs:84:31 + | +LL | environment!(); + | -------------- in this macro invocation +... +LL | const CASE: Case = Case { fiel: () }; + | ^^^^ help: a field with a similar name exists: `field` + | + = note: this error originates in the macro `environment` (in Nightly builds, run with -Z macro-backtrace for more info) + error[E0609]: no field `field` on type `Compound` - --> $DIR/dont-suggest-hygienic-fields.rs:24:16 + --> $DIR/dont-suggest-hygienic-fields.rs:47:16 | LL | let _ = ty.field; | ^^^^^ unknown field error[E0609]: no field `fieeld` on type `Compound` - --> $DIR/dont-suggest-hygienic-fields.rs:25:16 + --> $DIR/dont-suggest-hygienic-fields.rs:48:16 | LL | let _ = ty.fieeld; | ^^^^^^ unknown field error[E0026]: struct `Compound` does not have a field named `field` - --> $DIR/dont-suggest-hygienic-fields.rs:27:20 + --> $DIR/dont-suggest-hygienic-fields.rs:50:20 | LL | let Compound { field } = ty; | ^^^^^ struct `Compound` does not have this field error: pattern requires `..` due to inaccessible fields - --> $DIR/dont-suggest-hygienic-fields.rs:27:9 + --> $DIR/dont-suggest-hygienic-fields.rs:50:9 | LL | let Compound { field } = ty; | ^^^^^^^^^^^^^^^^^^ @@ -39,12 +50,41 @@ LL | let Compound { field, .. } = ty; | ++++ error[E0609]: no field `0` on type `Component` - --> $DIR/dont-suggest-hygienic-fields.rs:34:16 + --> $DIR/dont-suggest-hygienic-fields.rs:57:16 | LL | let _ = ty.0; | ^ unknown field -error: aborting due to 6 previous errors +error[E0609]: no field `field` on type `main::Casket` + --> $DIR/dont-suggest-hygienic-fields.rs:59:35 + | +LL | let _ = inject_into_expr_bad!(field); + | ^^^^^ unknown field + +error[E0026]: struct `main::Casket` does not have a field named `field` + --> $DIR/dont-suggest-hygienic-fields.rs:61:26 + | +LL | inject_into_pat_bad!(field: _); + | ^^^^^ struct `main::Casket` does not have this field + +error[E0027]: pattern does not mention field `field` + --> $DIR/dont-suggest-hygienic-fields.rs:41:9 + | +LL | let Casket { $( $any )* } = CASKET; + | ^^^^^^^^^^^^^^^^^^^^^ missing field `field` +... +LL | inject_into_pat_bad!(field: _); + | ------------------------------ in this macro invocation + | + = note: this error originates in the macro `inject_into_pat_bad` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0609]: no field `fiel` on type `Carton` + --> $DIR/dont-suggest-hygienic-fields.rs:102:43 + | +LL | let _ = inject_into_expr_good!(field, fiel); + | ^^^^ help: a field with a similar name exists: `field` + +error: aborting due to 11 previous errors -Some errors have detailed explanations: E0026, E0560, E0609. +Some errors have detailed explanations: E0026, E0027, E0560, E0609. For more information about an error, try `rustc --explain E0026`. diff --git a/tests/ui/did_you_mean/unresolved-imports-dont-suggest-hygienic-names.rs b/tests/ui/did_you_mean/unresolved-imports-dont-suggest-hygienic-names.rs new file mode 100644 index 0000000000000..19c67addb9451 --- /dev/null +++ b/tests/ui/did_you_mean/unresolved-imports-dont-suggest-hygienic-names.rs @@ -0,0 +1,27 @@ +// On unresolved imports, do not suggest hygienic names from different syntax contexts. +#![feature(decl_macro)] + +mod module { + macro make() { + pub struct Struct; + } + + make!(); +} + +// Do not suggest `Struct` since isn't accessible in this syntax context. +use module::Strukt; //~ ERROR unresolved import `module::Strukt` + +pub struct Type; + +environment!(); + +macro environment() { + // Just making sure that the implementation has correctly adjusted the + // spans to the right expansion and keeps suggesting `Type` here. + use crate::Typ; + //~^ ERROR unresolved import `crate::Typ` + //~| HELP a similar name exists in the module +} + +fn main() {} diff --git a/tests/ui/did_you_mean/unresolved-imports-dont-suggest-hygienic-names.stderr b/tests/ui/did_you_mean/unresolved-imports-dont-suggest-hygienic-names.stderr new file mode 100644 index 0000000000000..084ffcc8641f3 --- /dev/null +++ b/tests/ui/did_you_mean/unresolved-imports-dont-suggest-hygienic-names.stderr @@ -0,0 +1,23 @@ +error[E0432]: unresolved import `module::Strukt` + --> $DIR/unresolved-imports-dont-suggest-hygienic-names.rs:13:5 + | +LL | use module::Strukt; + | ^^^^^^^^^^^^^^ no `Strukt` in `module` + +error[E0432]: unresolved import `crate::Typ` + --> $DIR/unresolved-imports-dont-suggest-hygienic-names.rs:22:9 + | +LL | environment!(); + | -------------- in this macro invocation +... +LL | use crate::Typ; + | ^^^^^^^--- + | | | + | | help: a similar name exists in the module: `Type` + | no `Typ` in the root + | + = note: this error originates in the macro `environment` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0432`. diff --git a/tests/ui/did_you_mean/issue-38054-do-not-show-unresolved-names.rs b/tests/ui/did_you_mean/unresolved-imports-dont-suggest-unresolved-names.rs similarity index 66% rename from tests/ui/did_you_mean/issue-38054-do-not-show-unresolved-names.rs rename to tests/ui/did_you_mean/unresolved-imports-dont-suggest-unresolved-names.rs index a1ef68ecfe213..c1eded8c6c2da 100644 --- a/tests/ui/did_you_mean/issue-38054-do-not-show-unresolved-names.rs +++ b/tests/ui/did_you_mean/unresolved-imports-dont-suggest-unresolved-names.rs @@ -1,3 +1,5 @@ +// Regression test for issue #38054. + use Foo; //~ ERROR unresolved use Foo1; //~ ERROR unresolved diff --git a/tests/ui/did_you_mean/issue-38054-do-not-show-unresolved-names.stderr b/tests/ui/did_you_mean/unresolved-imports-dont-suggest-unresolved-names.stderr similarity index 68% rename from tests/ui/did_you_mean/issue-38054-do-not-show-unresolved-names.stderr rename to tests/ui/did_you_mean/unresolved-imports-dont-suggest-unresolved-names.stderr index 852abaed724dc..b48c506e0cfa1 100644 --- a/tests/ui/did_you_mean/issue-38054-do-not-show-unresolved-names.stderr +++ b/tests/ui/did_you_mean/unresolved-imports-dont-suggest-unresolved-names.stderr @@ -1,11 +1,11 @@ error[E0432]: unresolved import `Foo` - --> $DIR/issue-38054-do-not-show-unresolved-names.rs:1:5 + --> $DIR/unresolved-imports-dont-suggest-unresolved-names.rs:3:5 | LL | use Foo; | ^^^ no `Foo` in the root error[E0432]: unresolved import `Foo1` - --> $DIR/issue-38054-do-not-show-unresolved-names.rs:3:5 + --> $DIR/unresolved-imports-dont-suggest-unresolved-names.rs:5:5 | LL | use Foo1; | ^^^^ no `Foo1` in the root