diff options
| -rw-r--r-- | compiler/rustc_resolve/src/late/diagnostics.rs | 343 | ||||
| -rw-r--r-- | tests/ui/privacy/suggest-box-new.rs | 2 | ||||
| -rw-r--r-- | tests/ui/privacy/suggest-box-new.stderr | 38 |
3 files changed, 211 insertions, 172 deletions
diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 4d0f965a4da..28f77b31124 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -7,6 +7,7 @@ use crate::{Module, ModuleKind, ModuleOrUniformRoot}; use crate::{PathResult, PathSource, Segment}; use rustc_hir::def::Namespace::{self, *}; +use rustc_ast::ptr::P; use rustc_ast::visit::{FnCtxt, FnKind, LifetimeCtxt}; use rustc_ast::{ self as ast, AssocItemKind, Expr, ExprKind, GenericParam, GenericParamKind, Item, ItemKind, @@ -1334,7 +1335,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { let ns = source.namespace(); let is_expected = &|res| source.is_expected(res); - let path_sep = |err: &mut Diagnostic, expr: &Expr, kind: DefKind| { + let path_sep = |this: &mut Self, err: &mut Diagnostic, expr: &Expr, kind: DefKind| { const MESSAGE: &str = "use the path separator to refer to an item"; let (lhs_span, rhs_span) = match &expr.kind { @@ -1355,7 +1356,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { true } else if kind == DefKind::Struct && let Some(lhs_source_span) = lhs_span.find_ancestor_inside(expr.span) - && let Ok(snippet) = self.r.tcx.sess.source_map().span_to_snippet(lhs_source_span) + && let Ok(snippet) = this.r.tcx.sess.source_map().span_to_snippet(lhs_source_span) { // The LHS is a type that originates from a macro call. // We have to add angle brackets around it. @@ -1390,13 +1391,13 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { } }; - let mut bad_struct_syntax_suggestion = |def_id: DefId| { - let (followed_by_brace, closing_brace) = self.followed_by_brace(span); + let mut bad_struct_syntax_suggestion = |this: &mut Self, def_id: DefId| { + let (followed_by_brace, closing_brace) = this.followed_by_brace(span); match source { PathSource::Expr(Some( parent @ Expr { kind: ExprKind::Field(..) | ExprKind::MethodCall(..), .. }, - )) if path_sep(err, &parent, DefKind::Struct) => {} + )) if path_sep(this, err, &parent, DefKind::Struct) => {} PathSource::Expr( None | Some(Expr { @@ -1433,7 +1434,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { } PathSource::Expr(_) | PathSource::TupleStruct(..) | PathSource::Pat => { let span = find_span(&source, err); - err.span_label(self.r.def_span(def_id), format!("`{path_str}` defined here")); + err.span_label(this.r.def_span(def_id), format!("`{path_str}` defined here")); let (tail, descr, applicability, old_fields) = match source { PathSource::Pat => ("", "pattern", Applicability::MachineApplicable, None), @@ -1443,44 +1444,20 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { Applicability::MachineApplicable, Some( args.iter() - .map(|a| self.r.tcx.sess.source_map().span_to_snippet(*a).ok()) + .map(|a| this.r.tcx.sess.source_map().span_to_snippet(*a).ok()) .collect::<Vec<Option<String>>>(), ), ), _ => (": val", "literal", Applicability::HasPlaceholders, None), }; - let fields = match def_id.as_local() { - Some(def_id) => { - self.r.struct_constructors.get(&def_id).cloned().map(|(_, _, f)| f) - } - None => Some( - self.r - .tcx - .associated_item_def_ids(def_id) - .iter() - .map(|field_id| self.r.tcx.visibility(field_id)) - .collect(), - ), - }; - - let hidden_fields = fields.map_or(false, |fields| { - fields - .iter() - .filter(|vis| { - !self.r.is_accessible_from(**vis, self.parent_scope.module) - }) - .next() - .is_some() - }); - - if !hidden_fields { + if !this.has_private_fields(def_id) { // If the fields of the type are private, we shouldn't be suggesting using // the struct literal syntax at all, as that will cause a subsequent error. - let field_ids = self.r.field_def_ids(def_id); + let field_ids = this.r.field_def_ids(def_id); let (fields, applicability) = match field_ids { Some(field_ids) => { - let fields = field_ids.iter().map(|&id| self.r.tcx.item_name(id)); + let fields = field_ids.iter().map(|&id| this.r.tcx.item_name(id)); let fields = if let Some(old_fields) = old_fields { fields @@ -1516,6 +1493,20 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { applicability, ); } + if let PathSource::Expr(Some(Expr { + kind: ExprKind::Call(path, ref args), + span: call_span, + .. + })) = source + { + this.suggest_alternative_construction_methods( + def_id, + err, + path.span, + *call_span, + &args[..], + ); + } } _ => { err.span_label(span, fallback_label.to_string()); @@ -1565,7 +1556,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { Res::Def(kind @ (DefKind::Mod | DefKind::Trait), _), PathSource::Expr(Some(parent)), ) => { - if !path_sep(err, &parent, kind) { + if !path_sep(self, err, &parent, kind) { return false; } } @@ -1599,13 +1590,13 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { let (ctor_def, ctor_vis, fields) = if let Some(struct_ctor) = struct_ctor { if let PathSource::Expr(Some(parent)) = source { if let ExprKind::Field(..) | ExprKind::MethodCall(..) = parent.kind { - bad_struct_syntax_suggestion(def_id); + bad_struct_syntax_suggestion(self, def_id); return true; } } struct_ctor } else { - bad_struct_syntax_suggestion(def_id); + bad_struct_syntax_suggestion(self, def_id); return true; }; @@ -1633,133 +1624,13 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { err.set_primary_message( "cannot initialize a tuple struct which contains private fields", ); - if !def_id.is_local() { - // Look at all the associated functions without receivers in the type's - // inherent impls to look for builders that return `Self` - let mut items = self - .r - .tcx - .inherent_impls(def_id) - .iter() - .flat_map(|i| self.r.tcx.associated_items(i).in_definition_order()) - // Only assoc fn with no receivers. - .filter(|item| { - matches!(item.kind, ty::AssocKind::Fn) - && !item.fn_has_self_parameter - }) - .filter_map(|item| { - // Only assoc fns that return `Self` - let fn_sig = self.r.tcx.fn_sig(item.def_id).skip_binder(); - let ret_ty = fn_sig.output(); - let ret_ty = self.r.tcx.erase_late_bound_regions(ret_ty); - let ty::Adt(def, _args) = ret_ty.kind() else { - return None; - }; - // Check for `-> Self` - if def.did() == def_id { - let order = if item.name.as_str().starts_with("new") - && fn_sig.inputs().skip_binder().len() == args.len() - { - 0 - } else if item.name.as_str().starts_with("new") - || item.name.as_str().starts_with("default") - { - // Give higher precedence to functions with a name that - // imply construction. - 1 - } else if fn_sig.inputs().skip_binder().len() == args.len() - { - 2 - } else { - 3 - }; - Some((order, item.name)) - } else { - None - } - }) - .collect::<Vec<_>>(); - items.sort_by_key(|(order, _)| *order); - match &items[..] { - [] => {} - [(_, name)] => { - err.span_suggestion_verbose( - span.shrink_to_hi(), - format!( - "you might have meant to use the `{name}` associated \ - function", - ), - format!("::{name}"), - Applicability::MaybeIncorrect, - ); - } - _ => { - // We use this instead of `span_suggestions` to retain output - // sort order. - err.span_suggestions_with_style( - span.shrink_to_hi(), - "you might have meant to use an associated function to \ - build this type", - items - .iter() - .map(|(_, name)| format!("::{name}")) - .collect::<Vec<String>>(), - Applicability::MaybeIncorrect, - SuggestionStyle::ShowAlways, - ); - } - } - // We'd ideally use `type_implements_trait` but don't have access to - // the trait solver here. We can't use `get_diagnostic_item` or - // `all_traits` in resolve either. So instead we abuse the import - // suggestion machinery to get `std::default::Default` and perform some - // checks to confirm that we got *only* that trait. We then see if the - // Adt we have has a direct implementation of `Default`. If so, we - // provide a structured suggestion. - let default_trait = self - .r - .lookup_import_candidates( - Ident::with_dummy_span(sym::Default), - Namespace::TypeNS, - &self.parent_scope, - &|res: Res| matches!(res, Res::Def(DefKind::Trait, _)), - ) - .iter() - .filter_map(|candidate| candidate.did) - .filter(|did| { - self.r - .tcx - .get_attrs(*did, sym::rustc_diagnostic_item) - .any(|attr| attr.value_str() == Some(sym::Default)) - }) - .next(); - if let Some(default_trait) = default_trait - && self.r.extern_crate_map.iter().flat_map(|(_, crate_)| { - self - .r - .tcx - .implementations_of_trait((*crate_, default_trait)) - }) - .filter_map(|(_, simplified_self_ty)| *simplified_self_ty) - .filter_map(|simplified_self_ty| match simplified_self_ty { - SimplifiedType::Adt(did) => Some(did), - _ => None - }) - .any(|did| did == def_id) - { - err.multipart_suggestion( - "consider using the `Default` trait", - vec![ - (path.span.shrink_to_lo(), "<".to_string()), - ( - path.span.shrink_to_hi().with_hi(call_span.hi()), - " as std::default::Default>::default()".to_string(), - ), - ], - Applicability::MaybeIncorrect, - ); - } - } + self.suggest_alternative_construction_methods( + def_id, + err, + path.span, + *call_span, + &args[..], + ); // Use spans of the tuple struct definition. self.r.field_def_ids(def_id).map(|field_ids| { field_ids @@ -1806,7 +1677,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { err.span_label(span, "constructor is not visible here due to private fields"); } (Res::Def(DefKind::Union | DefKind::Variant, def_id), _) if ns == ValueNS => { - bad_struct_syntax_suggestion(def_id); + bad_struct_syntax_suggestion(self, def_id); } (Res::Def(DefKind::Ctor(_, CtorKind::Const), def_id), _) if ns == ValueNS => { match source { @@ -1852,6 +1723,148 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { true } + fn suggest_alternative_construction_methods( + &mut self, + def_id: DefId, + err: &mut Diagnostic, + path_span: Span, + call_span: Span, + args: &[P<Expr>], + ) { + if def_id.is_local() { + return; + } + // Look at all the associated functions without receivers in the type's + // inherent impls to look for builders that return `Self` + let mut items = self + .r + .tcx + .inherent_impls(def_id) + .iter() + .flat_map(|i| self.r.tcx.associated_items(i).in_definition_order()) + // Only assoc fn with no receivers. + .filter(|item| matches!(item.kind, ty::AssocKind::Fn) && !item.fn_has_self_parameter) + .filter_map(|item| { + // Only assoc fns that return `Self` + let fn_sig = self.r.tcx.fn_sig(item.def_id).skip_binder(); + let ret_ty = fn_sig.output(); + let ret_ty = self.r.tcx.erase_late_bound_regions(ret_ty); + let ty::Adt(def, _args) = ret_ty.kind() else { + return None; + }; + // Check for `-> Self` + let input_len = fn_sig.inputs().skip_binder().len(); + if def.did() == def_id { + let order = if item.name.as_str().starts_with("new") { + 0 + } else if input_len == args.len() { + 2 + } else { + 3 + }; + Some((order, item.name)) + } else { + None + } + }) + .collect::<Vec<_>>(); + items.sort_by_key(|(order, _)| *order); + match &items[..] { + [] => {} + [(_, name)] => { + err.span_suggestion_verbose( + path_span.shrink_to_hi(), + format!("you might have meant to use the `{name}` associated function",), + format!("::{name}"), + Applicability::MaybeIncorrect, + ); + } + _ => { + err.span_suggestions_with_style( + path_span.shrink_to_hi(), + "you might have meant to use an associated function to build this type", + items.iter().map(|(_, name)| format!("::{name}")).collect::<Vec<String>>(), + Applicability::MaybeIncorrect, + SuggestionStyle::ShowAlways, + ); + } + } + // We'd ideally use `type_implements_trait` but don't have access to + // the trait solver here. We can't use `get_diagnostic_item` or + // `all_traits` in resolve either. So instead we abuse the import + // suggestion machinery to get `std::default::Default` and perform some + // checks to confirm that we got *only* that trait. We then see if the + // Adt we have has a direct implementation of `Default`. If so, we + // provide a structured suggestion. + let default_trait = self + .r + .lookup_import_candidates( + Ident::with_dummy_span(sym::Default), + Namespace::TypeNS, + &self.parent_scope, + &|res: Res| matches!(res, Res::Def(DefKind::Trait, _)), + ) + .iter() + .filter_map(|candidate| candidate.did) + .filter(|did| { + self.r + .tcx + .get_attrs(*did, sym::rustc_diagnostic_item) + .any(|attr| attr.value_str() == Some(sym::Default)) + }) + .next(); + let Some(default_trait) = default_trait else { + return; + }; + if self + .r + .extern_crate_map + .iter() + // FIXME: This doesn't include impls like `impl Default for String`. + .flat_map(|(_, crate_)| self.r.tcx.implementations_of_trait((*crate_, default_trait))) + .filter_map(|(_, simplified_self_ty)| *simplified_self_ty) + .filter_map(|simplified_self_ty| match simplified_self_ty { + SimplifiedType::Adt(did) => Some(did), + _ => None, + }) + .any(|did| did == def_id) + { + err.multipart_suggestion( + "consider using the `Default` trait", + vec![ + (path_span.shrink_to_lo(), "<".to_string()), + ( + path_span.shrink_to_hi().with_hi(call_span.hi()), + " as std::default::Default>::default()".to_string(), + ), + ], + Applicability::MaybeIncorrect, + ); + } + } + + fn has_private_fields(&self, def_id: DefId) -> bool { + let fields = match def_id.as_local() { + Some(def_id) => self.r.struct_constructors.get(&def_id).cloned().map(|(_, _, f)| f), + None => Some( + self.r + .tcx + .associated_item_def_ids(def_id) + .iter() + .map(|field_id| self.r.tcx.visibility(field_id)) + .collect(), + ), + }; + + fields.map_or(false, |fields| { + fields + .iter() + .filter(|vis| !self.r.is_accessible_from(**vis, self.parent_scope.module)) + .next() + .is_some() + }) + } + /// Given the target `ident` and `kind`, search for the similarly named associated item /// in `self.current_trait_ref`. pub(crate) fn find_similarly_named_assoc_item( diff --git a/tests/ui/privacy/suggest-box-new.rs b/tests/ui/privacy/suggest-box-new.rs index e483d3f3af0..657dd37dc68 100644 --- a/tests/ui/privacy/suggest-box-new.rs +++ b/tests/ui/privacy/suggest-box-new.rs @@ -11,4 +11,6 @@ fn main() { })), x: () }; + let _ = std::collections::HashMap(); + //~^ ERROR expected function, tuple struct or tuple variant, found struct `std::collections::HashMap` } diff --git a/tests/ui/privacy/suggest-box-new.stderr b/tests/ui/privacy/suggest-box-new.stderr index 46fb860eb70..f58f661a468 100644 --- a/tests/ui/privacy/suggest-box-new.stderr +++ b/tests/ui/privacy/suggest-box-new.stderr @@ -1,3 +1,27 @@ +error[E0423]: expected function, tuple struct or tuple variant, found struct `std::collections::HashMap` + --> $DIR/suggest-box-new.rs:14:13 + | +LL | let _ = std::collections::HashMap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + --> $SRC_DIR/std/src/collections/hash/map.rs:LL:COL + | + = note: `std::collections::HashMap` defined here + | +help: you might have meant to use an associated function to build this type + | +LL | let _ = std::collections::HashMap::new(); + | +++++ +LL | let _ = std::collections::HashMap::with_capacity(); + | +++++++++++++++ +LL | let _ = std::collections::HashMap::with_hasher(); + | +++++++++++++ +LL | let _ = std::collections::HashMap::with_capacity_and_hasher(); + | ++++++++++++++++++++++++++ +help: consider using the `Default` trait + | +LL | let _ = <std::collections::HashMap as std::default::Default>::default(); + | + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + error[E0423]: cannot initialize a tuple struct which contains private fields --> $DIR/suggest-box-new.rs:8:19 | @@ -14,18 +38,18 @@ help: you might have meant to use an associated function to build this type | LL | wtf: Some(Box::new(U { | +++++ -LL | wtf: Some(Box::new_uninit_in(U { - | +++++++++++++++ -LL | wtf: Some(Box::new_zeroed_in(U { - | +++++++++++++++ -LL | wtf: Some(Box::new_uninit_slice(U { - | ++++++++++++++++++ +LL | wtf: Some(Box::new_uninit(U { + | ++++++++++++ +LL | wtf: Some(Box::new_zeroed(U { + | ++++++++++++ +LL | wtf: Some(Box::new_in(U { + | ++++++++ and 10 other candidates help: consider using the `Default` trait | LL | wtf: Some(<Box as std::default::Default>::default()), | + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -error: aborting due to previous error +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0423`. |