From 42aa1273b0dd317c4aafdcf649f40cbad4499b60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 7 Nov 2023 19:32:48 +0000 Subject: [PATCH 01/14] When encountering struct fn call literal with private fields, suggest all builders When encountering code like `Box(42)`, suggest `Box::new(42)` and *all* other associated functions that return `-> Box`. --- compiler/rustc_errors/src/diagnostic.rs | 11 ++- .../src/diagnostics/diagnostic_builder.rs | 4 +- compiler/rustc_resolve/src/diagnostics.rs | 1 + .../rustc_resolve/src/late/diagnostics.rs | 90 +++++++++++++++---- ...xtern-prelude-from-opaque-fail-2018.stderr | 2 +- tests/ui/privacy/suggest-box-new.fixed | 15 ---- tests/ui/privacy/suggest-box-new.rs | 1 - tests/ui/privacy/suggest-box-new.stderr | 11 ++- .../suggest-tryinto-edition-change.stderr | 6 +- 9 files changed, 98 insertions(+), 43 deletions(-) delete mode 100644 tests/ui/privacy/suggest-box-new.fixed diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 470f318eb33e3..b379d17dc2297 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -759,6 +759,9 @@ impl Diagnostic { suggestions: impl IntoIterator, applicability: Applicability, ) -> &mut Self { + let mut suggestions: Vec<_> = suggestions.into_iter().collect(); + suggestions.sort(); + self.span_suggestions_with_style( sp, msg, @@ -768,7 +771,9 @@ impl Diagnostic { ) } - /// [`Diagnostic::span_suggestions()`] but you can set the [`SuggestionStyle`]. + /// [`Diagnostic::span_suggestions()`] but you can set the [`SuggestionStyle`]. This version + /// *doesn't* sort the suggestions, so the caller has control of the order in which they are + /// presented. pub fn span_suggestions_with_style( &mut self, sp: Span, @@ -777,9 +782,7 @@ impl Diagnostic { applicability: Applicability, style: SuggestionStyle, ) -> &mut Self { - let mut suggestions: Vec<_> = suggestions.into_iter().collect(); - suggestions.sort(); - + let suggestions: Vec<_> = suggestions.into_iter().collect(); debug_assert!( !(sp.is_empty() && suggestions.iter().any(|suggestion| suggestion.is_empty())), "Span must not be empty and have no suggestion" diff --git a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs index 2755a161d91e7..a078a9e00ae26 100644 --- a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs +++ b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs @@ -442,10 +442,12 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { self.formatting_init.extend(code_init); Ok(quote! { + let mut code: Vec<_> = #code_field.into_iter().collect(); + code.sort(); #diag.span_suggestions_with_style( #span_field, crate::fluent_generated::#slug, - #code_field, + code, #applicability, #style ); diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 28e6fe9b4b739..84225bbe39b2d 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -2608,6 +2608,7 @@ fn show_candidates( path_strings.extend(core_path_strings); path_strings.dedup_by(|a, b| a.0 == b.0); } + accessible_path_strings.sort(); if !accessible_path_strings.is_empty() { let (determiner, kind, name, through) = diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 6aecd3610c6e7..3b59af89a985f 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -15,7 +15,7 @@ use rustc_ast_pretty::pprust::where_bound_predicate_to_string; use rustc_data_structures::fx::FxHashSet; use rustc_errors::{ pluralize, struct_span_err, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, - MultiSpan, + MultiSpan, SuggestionStyle, }; use rustc_hir as hir; use rustc_hir::def::{self, CtorKind, CtorOf, DefKind}; @@ -29,6 +29,8 @@ use rustc_span::hygiene::MacroKind; use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::Span; +use rustc_middle::ty; + use std::borrow::Cow; use std::iter; use std::ops::Deref; @@ -1593,29 +1595,85 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { Some(Vec::from(pattern_spans)) } // e.g. `let _ = Enum::TupleVariant(field1, field2);` - _ if source.is_call() => { + PathSource::Expr(Some(Expr { kind: ExprKind::Call(_, ref args), .. })) => { err.set_primary_message( "cannot initialize a tuple struct which contains private fields", ); - if !def_id.is_local() - && self + 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(|impl_def_id| { - self.r.tcx.provided_trait_methods(*impl_def_id) + .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 }) - .any(|assoc| !assoc.fn_has_self_parameter && assoc.name == sym::new) - { - // FIXME: look for associated functions with Self return type, - // instead of relying only on the name and lack of self receiver. - err.span_suggestion_verbose( - span.shrink_to_hi(), - "you might have meant to use the `new` associated function", - "::new".to_string(), - Applicability::MaybeIncorrect, - ); + .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 + }; + return Some((order, item.name)); + } + None + }) + .collect::>(); + 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::>(), + Applicability::MaybeIncorrect, + SuggestionStyle::ShowAlways, + ); + } + } } // Use spans of the tuple struct definition. self.r.field_def_ids(def_id).map(|field_ids| { diff --git a/tests/ui/hygiene/extern-prelude-from-opaque-fail-2018.stderr b/tests/ui/hygiene/extern-prelude-from-opaque-fail-2018.stderr index 7ed15e89caafa..78e6376bca2e8 100644 --- a/tests/ui/hygiene/extern-prelude-from-opaque-fail-2018.stderr +++ b/tests/ui/hygiene/extern-prelude-from-opaque-fail-2018.stderr @@ -25,8 +25,8 @@ LL | a!(); | ---- in this macro invocation | = help: consider importing one of these items: - std::mem core::mem + std::mem = note: this error originates in the macro `a` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0433]: failed to resolve: use of undeclared crate or module `my_core` diff --git a/tests/ui/privacy/suggest-box-new.fixed b/tests/ui/privacy/suggest-box-new.fixed deleted file mode 100644 index f5ae5c2abfd99..0000000000000 --- a/tests/ui/privacy/suggest-box-new.fixed +++ /dev/null @@ -1,15 +0,0 @@ -// run-rustfix -#![allow(dead_code)] -struct U { - wtf: Option>>, - x: T, -} -fn main() { - U { - wtf: Some(Box::new(U { //~ ERROR cannot initialize a tuple struct which contains private fields - wtf: None, - x: (), - })), - x: () - }; -} diff --git a/tests/ui/privacy/suggest-box-new.rs b/tests/ui/privacy/suggest-box-new.rs index 2e18dba8b9fb2..e483d3f3af030 100644 --- a/tests/ui/privacy/suggest-box-new.rs +++ b/tests/ui/privacy/suggest-box-new.rs @@ -1,4 +1,3 @@ -// run-rustfix #![allow(dead_code)] struct U { wtf: Option>>, diff --git a/tests/ui/privacy/suggest-box-new.stderr b/tests/ui/privacy/suggest-box-new.stderr index ed7fa036408a4..dbed66d0ae4f0 100644 --- a/tests/ui/privacy/suggest-box-new.stderr +++ b/tests/ui/privacy/suggest-box-new.stderr @@ -1,5 +1,5 @@ error[E0423]: cannot initialize a tuple struct which contains private fields - --> $DIR/suggest-box-new.rs:9:19 + --> $DIR/suggest-box-new.rs:8:19 | LL | wtf: Some(Box(U { | ^^^ @@ -10,10 +10,17 @@ note: constructor is not visible here due to private fields = note: private field | = note: private field -help: you might have meant to use the `new` associated function +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 { + | ++++++++++++++++++ + and 10 other candidates error: aborting due to previous error diff --git a/tests/ui/suggestions/suggest-tryinto-edition-change.stderr b/tests/ui/suggestions/suggest-tryinto-edition-change.stderr index 671f5efddd979..057e37dbe109f 100644 --- a/tests/ui/suggestions/suggest-tryinto-edition-change.stderr +++ b/tests/ui/suggestions/suggest-tryinto-edition-change.stderr @@ -4,8 +4,8 @@ error[E0433]: failed to resolve: use of undeclared type `TryFrom` LL | let _i: i16 = TryFrom::try_from(0_i32).unwrap(); | ^^^^^^^ use of undeclared type `TryFrom` | - = note: 'std::convert::TryFrom' is included in the prelude starting in Edition 2021 = note: 'core::convert::TryFrom' is included in the prelude starting in Edition 2021 + = note: 'std::convert::TryFrom' is included in the prelude starting in Edition 2021 help: consider importing one of these items | LL + use core::convert::TryFrom; @@ -19,8 +19,8 @@ error[E0433]: failed to resolve: use of undeclared type `TryInto` LL | let _i: i16 = TryInto::try_into(0_i32).unwrap(); | ^^^^^^^ use of undeclared type `TryInto` | - = note: 'std::convert::TryInto' is included in the prelude starting in Edition 2021 = note: 'core::convert::TryInto' is included in the prelude starting in Edition 2021 + = note: 'std::convert::TryInto' is included in the prelude starting in Edition 2021 help: consider importing one of these items | LL + use core::convert::TryInto; @@ -34,8 +34,8 @@ error[E0433]: failed to resolve: use of undeclared type `FromIterator` LL | let _v: Vec<_> = FromIterator::from_iter(&[1]); | ^^^^^^^^^^^^ use of undeclared type `FromIterator` | - = note: 'std::iter::FromIterator' is included in the prelude starting in Edition 2021 = note: 'core::iter::FromIterator' is included in the prelude starting in Edition 2021 + = note: 'std::iter::FromIterator' is included in the prelude starting in Edition 2021 help: a trait with a similar name exists | LL | let _v: Vec<_> = IntoIterator::from_iter(&[1]); From f1ae02f4bd971acc384ed3422e235fb1eb07dfa7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 8 Nov 2023 18:24:49 +0000 Subject: [PATCH 02/14] Don't sort `span_suggestions`, leave that to caller --- compiler/rustc_errors/src/diagnostic.rs | 7 +------ compiler/rustc_hir_analysis/src/astconv/mod.rs | 6 +++++- compiler/rustc_hir_typeck/src/expr.rs | 3 ++- compiler/rustc_hir_typeck/src/method/suggest.rs | 14 ++++++-------- .../src/diagnostics/diagnostic_builder.rs | 4 +--- compiler/rustc_parse/src/validate_attr.rs | 1 + compiler/rustc_resolve/src/late/diagnostics.rs | 14 +++++++++----- src/tools/clippy/clippy_lints/src/booleans.rs | 3 ++- tests/ui/parser/bad-pointer-type.stderr | 4 ++-- tests/ui/parser/double-pointer.stderr | 4 ++-- 10 files changed, 31 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index b379d17dc2297..1c1da89c557ad 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -759,9 +759,6 @@ impl Diagnostic { suggestions: impl IntoIterator, applicability: Applicability, ) -> &mut Self { - let mut suggestions: Vec<_> = suggestions.into_iter().collect(); - suggestions.sort(); - self.span_suggestions_with_style( sp, msg, @@ -771,9 +768,7 @@ impl Diagnostic { ) } - /// [`Diagnostic::span_suggestions()`] but you can set the [`SuggestionStyle`]. This version - /// *doesn't* sort the suggestions, so the caller has control of the order in which they are - /// presented. + /// [`Diagnostic::span_suggestions()`] but you can set the [`SuggestionStyle`]. pub fn span_suggestions_with_style( &mut self, sp: Span, diff --git a/compiler/rustc_hir_analysis/src/astconv/mod.rs b/compiler/rustc_hir_analysis/src/astconv/mod.rs index 8126b45118138..3542fc7cd3325 100644 --- a/compiler/rustc_hir_analysis/src/astconv/mod.rs +++ b/compiler/rustc_hir_analysis/src/astconv/mod.rs @@ -945,7 +945,11 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { Applicability::MachineApplicable, ); } else { - match (types, traits) { + let mut types = types.to_vec(); + types.sort(); + let mut traits = traits.to_vec(); + traits.sort(); + match (&types[..], &traits[..]) { ([], []) => { err.span_suggestion_verbose( span, diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 6c99044d0aa4e..9180d4aad32f7 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -2703,7 +2703,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.get_field_candidates_considering_privacy(span, ty, mod_id, id) { let field_names = found_fields.iter().map(|field| field.name).collect::>(); - let candidate_fields: Vec<_> = found_fields + let mut candidate_fields: Vec<_> = found_fields .into_iter() .filter_map(|candidate_field| { self.check_for_nested_field_satisfying( @@ -2724,6 +2724,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .collect::() }) .collect::>(); + candidate_fields.sort(); let len = candidate_fields.len(); if len > 0 { diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs index 6956778f974c1..cdb08cfd7d966 100644 --- a/compiler/rustc_hir_typeck/src/method/suggest.rs +++ b/compiler/rustc_hir_typeck/src/method/suggest.rs @@ -1426,6 +1426,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if !suggs.is_empty() && let Some(span) = sugg_span { + suggs.sort(); err.span_suggestions( span.with_hi(item_name.span.lo()), "use fully-qualified syntax to disambiguate", @@ -2000,8 +2001,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.tcx.get_diagnostic_item(sym::Borrow), self.tcx.get_diagnostic_item(sym::BorrowMut), ]; - let candidate_fields: Vec<_> = fields - .iter() + let mut candidate_fields: Vec<_> = fields .filter_map(|candidate_field| { self.check_for_nested_field_satisfying( span, @@ -2035,6 +2035,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .join(".") }) .collect(); + candidate_fields.sort(); let len = candidate_fields.len(); if len > 0 { @@ -2567,13 +2568,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.tcx.item_name(*trait_did), ) }); + let mut sugg: Vec<_> = path_strings.chain(glob_path_strings).collect(); + sugg.sort(); - err.span_suggestions( - span, - msg, - path_strings.chain(glob_path_strings), - Applicability::MaybeIncorrect, - ); + err.span_suggestions(span, msg, sugg, Applicability::MaybeIncorrect); } fn suggest_valid_traits( diff --git a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs index a078a9e00ae26..2755a161d91e7 100644 --- a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs +++ b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs @@ -442,12 +442,10 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { self.formatting_init.extend(code_init); Ok(quote! { - let mut code: Vec<_> = #code_field.into_iter().collect(); - code.sort(); #diag.span_suggestions_with_style( #span_field, crate::fluent_generated::#slug, - code, + #code_field, #applicability, #style ); diff --git a/compiler/rustc_parse/src/validate_attr.rs b/compiler/rustc_parse/src/validate_attr.rs index f739659822047..e6c46fc252839 100644 --- a/compiler/rustc_parse/src/validate_attr.rs +++ b/compiler/rustc_parse/src/validate_attr.rs @@ -186,6 +186,7 @@ fn emit_malformed_attribute( msg.push_str(&format!("`{code}`")); suggestions.push(code); } + suggestions.sort(); if should_warn(name) { sess.buffer_lint(&ILL_FORMED_ATTRIBUTE_INPUT, span, ast::CRATE_NODE_ID, msg); } else { diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 3b59af89a985f..ce4e0eecf3905 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1639,9 +1639,10 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { } else { 3 }; - return Some((order, item.name)); + Some((order, item.name)) + } else { + None } - None }) .collect::>(); items.sort_by_key(|(order, _)| *order); @@ -2147,11 +2148,12 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { if suggest_only_tuple_variants { // Suggest only tuple variants regardless of whether they have fields and do not // suggest path with added parentheses. - let suggestable_variants = variants + let mut suggestable_variants = variants .iter() .filter(|(.., kind)| *kind == CtorKind::Fn) .map(|(variant, ..)| path_names_to_string(variant)) .collect::>(); + suggestable_variants.sort(); let non_suggestable_variant_count = variants.len() - suggestable_variants.len(); @@ -2202,7 +2204,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { } }; - let suggestable_variants = variants + let mut suggestable_variants = variants .iter() .filter(|(_, def_id, kind)| !needs_placeholder(*def_id, *kind)) .map(|(variant, _, kind)| (path_names_to_string(variant), kind)) @@ -2211,6 +2213,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { CtorKind::Fn => format!("({variant}())"), }) .collect::>(); + suggestable_variants.sort(); let no_suggestable_variant = suggestable_variants.is_empty(); if !no_suggestable_variant { @@ -2228,7 +2231,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { ); } - let suggestable_variants_with_placeholders = variants + let mut suggestable_variants_with_placeholders = variants .iter() .filter(|(_, def_id, kind)| needs_placeholder(*def_id, *kind)) .map(|(variant, _, kind)| (path_names_to_string(variant), kind)) @@ -2237,6 +2240,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { _ => None, }) .collect::>(); + suggestable_variants_with_placeholders.sort(); if !suggestable_variants_with_placeholders.is_empty() { let msg = diff --git a/src/tools/clippy/clippy_lints/src/booleans.rs b/src/tools/clippy/clippy_lints/src/booleans.rs index d4f2e316890ed..2cb599964d2b4 100644 --- a/src/tools/clippy/clippy_lints/src/booleans.rs +++ b/src/tools/clippy/clippy_lints/src/booleans.rs @@ -424,8 +424,9 @@ impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> { improvements.push(suggestion); } } - let nonminimal_bool_lint = |suggestions: Vec<_>| { + let nonminimal_bool_lint = |mut suggestions: Vec<_>| { if self.cx.tcx.lint_level_at_node(NONMINIMAL_BOOL, e.hir_id).0 != Level::Allow { + suggestions.sort(); span_lint_hir_and_then( self.cx, NONMINIMAL_BOOL, diff --git a/tests/ui/parser/bad-pointer-type.stderr b/tests/ui/parser/bad-pointer-type.stderr index b7225ca887dd4..e843c49886bc6 100644 --- a/tests/ui/parser/bad-pointer-type.stderr +++ b/tests/ui/parser/bad-pointer-type.stderr @@ -6,10 +6,10 @@ LL | fn foo(_: *()) { | help: add `mut` or `const` here | -LL | fn foo(_: *const ()) { - | +++++ LL | fn foo(_: *mut ()) { | +++ +LL | fn foo(_: *const ()) { + | +++++ error: aborting due to previous error diff --git a/tests/ui/parser/double-pointer.stderr b/tests/ui/parser/double-pointer.stderr index 28037f9326552..10aedbb92a1f0 100644 --- a/tests/ui/parser/double-pointer.stderr +++ b/tests/ui/parser/double-pointer.stderr @@ -6,10 +6,10 @@ LL | let dptr: **const i32 = &ptr; | help: add `mut` or `const` here | -LL | let dptr: *const *const i32 = &ptr; - | +++++ LL | let dptr: *mut *const i32 = &ptr; | +++ +LL | let dptr: *const *const i32 = &ptr; + | +++++ error: aborting due to previous error From a4f47de7ff7857938f2c2c694eea4255eaacfdcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 8 Nov 2023 21:58:44 +0000 Subject: [PATCH 03/14] On private tuple struct, suggest `Default::default` when possible --- .../rustc_resolve/src/late/diagnostics.rs | 57 ++++++++++++++++++- tests/ui/privacy/suggest-box-new.stderr | 4 ++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index ce4e0eecf3905..71732261efcf5 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1,6 +1,7 @@ use crate::diagnostics::{ImportSuggestion, LabelSuggestion, TypoSuggestion}; use crate::late::{AliasPossibility, LateResolutionVisitor, RibKind}; use crate::late::{LifetimeBinderKind, LifetimeRes, LifetimeRibKind, LifetimeUseSet}; +use crate::ty::fast_reject::SimplifiedType; use crate::{errors, path_names_to_string}; use crate::{Module, ModuleKind, ModuleOrUniformRoot}; use crate::{PathResult, PathSource, Segment}; @@ -1595,7 +1596,11 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { Some(Vec::from(pattern_spans)) } // e.g. `let _ = Enum::TupleVariant(field1, field2);` - PathSource::Expr(Some(Expr { kind: ExprKind::Call(_, ref args), .. })) => { + PathSource::Expr(Some(Expr { + kind: ExprKind::Call(path, ref args), + span: call_span, + .. + })) => { err.set_primary_message( "cannot initialize a tuple struct which contains private fields", ); @@ -1675,6 +1680,56 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { ); } } + // 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, + ); + } } // Use spans of the tuple struct definition. self.r.field_def_ids(def_id).map(|field_ids| { diff --git a/tests/ui/privacy/suggest-box-new.stderr b/tests/ui/privacy/suggest-box-new.stderr index dbed66d0ae4f0..46fb860eb70a2 100644 --- a/tests/ui/privacy/suggest-box-new.stderr +++ b/tests/ui/privacy/suggest-box-new.stderr @@ -21,6 +21,10 @@ LL | wtf: Some(Box::new_zeroed_in(U { LL | wtf: Some(Box::new_uninit_slice(U { | ++++++++++++++++++ and 10 other candidates +help: consider using the `Default` trait + | +LL | wtf: Some(::default()), + | + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: aborting due to previous error From e0e379b6fde67302b7922e90df3cdb5695cfd011 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 8 Nov 2023 21:59:10 +0000 Subject: [PATCH 04/14] Add test for public struct with private fields --- tests/ui/xcrate/auxiliary/xcrate_unit_struct.rs | 5 +++++ tests/ui/xcrate/xcrate-unit-struct.rs | 2 ++ tests/ui/xcrate/xcrate-unit-struct.stderr | 13 ++++++++++++- 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/ui/xcrate/auxiliary/xcrate_unit_struct.rs b/tests/ui/xcrate/auxiliary/xcrate_unit_struct.rs index 69ed498e7e1cd..4f8b3508398f2 100644 --- a/tests/ui/xcrate/auxiliary/xcrate_unit_struct.rs +++ b/tests/ui/xcrate/auxiliary/xcrate_unit_struct.rs @@ -18,6 +18,11 @@ pub struct TupleStruct(pub usize, pub &'static str); #[derive(Copy, Clone)] pub struct StructWithFields { + pub foo: isize, +} + +#[derive(Copy, Clone)] +pub struct StructWithPrivFields { foo: isize, } diff --git a/tests/ui/xcrate/xcrate-unit-struct.rs b/tests/ui/xcrate/xcrate-unit-struct.rs index c99cf77ce7a4a..bc14cd8d4c01c 100644 --- a/tests/ui/xcrate/xcrate-unit-struct.rs +++ b/tests/ui/xcrate/xcrate-unit-struct.rs @@ -8,5 +8,7 @@ extern crate xcrate_unit_struct; fn main() { let _ = xcrate_unit_struct::StructWithFields; //~^ ERROR expected value, found struct `xcrate_unit_struct::StructWithFields` + let _ = xcrate_unit_struct::StructWithPrivFields; + //~^ ERROR expected value, found struct `xcrate_unit_struct::StructWithPrivFields` let _ = xcrate_unit_struct::Struct; } diff --git a/tests/ui/xcrate/xcrate-unit-struct.stderr b/tests/ui/xcrate/xcrate-unit-struct.stderr index cee314568887c..c6402e929be77 100644 --- a/tests/ui/xcrate/xcrate-unit-struct.stderr +++ b/tests/ui/xcrate/xcrate-unit-struct.stderr @@ -9,6 +9,17 @@ LL | let _ = xcrate_unit_struct::StructWithFields; LL | pub struct StructWithFields { | --------------------------- `xcrate_unit_struct::StructWithFields` defined here -error: aborting due to previous error +error[E0423]: expected value, found struct `xcrate_unit_struct::StructWithPrivFields` + --> $DIR/xcrate-unit-struct.rs:11:13 + | +LL | let _ = xcrate_unit_struct::StructWithPrivFields; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use struct literal syntax instead: `xcrate_unit_struct::StructWithPrivFields { foo: val }` + | + ::: $DIR/auxiliary/xcrate_unit_struct.rs:25:1 + | +LL | pub struct StructWithPrivFields { + | ------------------------------- `xcrate_unit_struct::StructWithPrivFields` defined here + +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0423`. From 12a8bb8d9b06b42706e5ad2c5a93023066a7e6ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 8 Nov 2023 22:02:51 +0000 Subject: [PATCH 05/14] Do not suggest struct literal when fields are private --- .../rustc_resolve/src/late/diagnostics.rs | 99 ++++++++++++------- tests/ui/xcrate/xcrate-unit-struct.stderr | 2 +- 2 files changed, 65 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 71732261efcf5..4d0f965a4da4c 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1449,44 +1449,73 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { ), _ => (": val", "literal", Applicability::HasPlaceholders, None), }; - let field_ids = self.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 = if let Some(old_fields) = old_fields { - fields - .enumerate() - .map(|(idx, new)| (new, old_fields.get(idx))) - .map(|(new, old)| { - let new = new.to_ident_string(); - if let Some(Some(old)) = old - && new != *old - { - format!("{new}: {old}") - } else { - new - } - }) - .collect::>() - } else { - fields.map(|f| format!("{f}{tail}")).collect::>() - }; - (fields.join(", "), applicability) + let fields = match def_id.as_local() { + Some(def_id) => { + self.r.struct_constructors.get(&def_id).cloned().map(|(_, _, f)| f) } - None => ("/* fields */".to_string(), Applicability::HasPlaceholders), - }; - let pad = match field_ids { - Some(field_ids) if field_ids.is_empty() => "", - _ => " ", + None => Some( + self.r + .tcx + .associated_item_def_ids(def_id) + .iter() + .map(|field_id| self.r.tcx.visibility(field_id)) + .collect(), + ), }; - err.span_suggestion( - span, - format!("use struct {descr} syntax instead"), - format!("{path_str} {{{pad}{fields}{pad}}}"), - applicability, - ); + + 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 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 (fields, applicability) = match field_ids { + Some(field_ids) => { + let fields = field_ids.iter().map(|&id| self.r.tcx.item_name(id)); + + let fields = if let Some(old_fields) = old_fields { + fields + .enumerate() + .map(|(idx, new)| (new, old_fields.get(idx))) + .map(|(new, old)| { + let new = new.to_ident_string(); + if let Some(Some(old)) = old + && new != *old + { + format!("{new}: {old}") + } else { + new + } + }) + .collect::>() + } else { + fields.map(|f| format!("{f}{tail}")).collect::>() + }; + + (fields.join(", "), applicability) + } + None => ("/* fields */".to_string(), Applicability::HasPlaceholders), + }; + let pad = match field_ids { + Some(field_ids) if field_ids.is_empty() => "", + _ => " ", + }; + err.span_suggestion( + span, + format!("use struct {descr} syntax instead"), + format!("{path_str} {{{pad}{fields}{pad}}}"), + applicability, + ); + } } _ => { err.span_label(span, fallback_label.to_string()); diff --git a/tests/ui/xcrate/xcrate-unit-struct.stderr b/tests/ui/xcrate/xcrate-unit-struct.stderr index c6402e929be77..7365170b69ea2 100644 --- a/tests/ui/xcrate/xcrate-unit-struct.stderr +++ b/tests/ui/xcrate/xcrate-unit-struct.stderr @@ -13,7 +13,7 @@ error[E0423]: expected value, found struct `xcrate_unit_struct::StructWithPrivFi --> $DIR/xcrate-unit-struct.rs:11:13 | LL | let _ = xcrate_unit_struct::StructWithPrivFields; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use struct literal syntax instead: `xcrate_unit_struct::StructWithPrivFields { foo: val }` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ::: $DIR/auxiliary/xcrate_unit_struct.rs:25:1 | From 987155f35d6a534f58208a0f59a0df22cbaf38cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 8 Nov 2023 23:18:00 +0000 Subject: [PATCH 06/14] Suggest using builder on curly brace struct called as fn --- .../rustc_resolve/src/late/diagnostics.rs | 343 +++++++++--------- tests/ui/privacy/suggest-box-new.rs | 2 + 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 4d0f965a4da4c..28f77b3112435 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::>>(), ), ), _ => (": 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::>(); - 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::>(), - 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], + ) { + 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::>(); + 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::>(), + 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 e483d3f3af030..657dd37dc68ab 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 46fb860eb70a2..f58f661a468d3 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 _ = ::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(::default()), | + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -error: aborting due to previous error +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0423`. From 1dfec45dc99d7d699bdf470d042300102e1589bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 8 Nov 2023 23:23:26 +0000 Subject: [PATCH 07/14] Remove unnecessary .collect() --- compiler/rustc_errors/src/diagnostic.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 1c1da89c557ad..0aaa8ae69e16f 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -777,15 +777,15 @@ impl Diagnostic { applicability: Applicability, style: SuggestionStyle, ) -> &mut Self { - let suggestions: Vec<_> = suggestions.into_iter().collect(); - debug_assert!( - !(sp.is_empty() && suggestions.iter().any(|suggestion| suggestion.is_empty())), - "Span must not be empty and have no suggestion" - ); - let substitutions = suggestions .into_iter() - .map(|snippet| Substitution { parts: vec![SubstitutionPart { snippet, span: sp }] }) + .map(|snippet| { + debug_assert!( + !(sp.is_empty() && snippet.is_empty()), + "Span must not be empty and have no suggestion" + ); + Substitution { parts: vec![SubstitutionPart { snippet, span: sp }] } + }) .collect(); self.push_suggestion(CodeSuggestion { substitutions, From be0958f5ab7b7d24cd24e2796d21572419d2cd53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 8 Nov 2023 23:45:09 +0000 Subject: [PATCH 08/14] Suggest builder functions on struct literal with private fields --- compiler/rustc_hir_typeck/src/expr.rs | 69 ++++++++++++++++++++++++- tests/ui/privacy/suggest-box-new.rs | 3 ++ tests/ui/privacy/suggest-box-new.stderr | 39 +++++++++++++- 3 files changed, 109 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 9180d4aad32f7..9f27a6e5d84a4 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -1897,7 +1897,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .collect(); if !private_fields.is_empty() { - self.report_private_fields(adt_ty, span, private_fields, ast_fields); + self.report_private_fields(adt_ty, span, expr.span, private_fields, ast_fields); } else { self.report_missing_fields( adt_ty, @@ -2056,6 +2056,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &self, adt_ty: Ty<'tcx>, span: Span, + expr_span: Span, private_fields: Vec<&ty::FieldDef>, used_fields: &'tcx [hir::ExprField<'tcx>], ) { @@ -2100,6 +2101,72 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { were = pluralize!("was", remaining_private_fields_len), )); } + + if let ty::Adt(def, _) = adt_ty.kind() { + let def_id = def.did(); + let mut items = self + .tcx + .inherent_impls(def_id) + .iter() + .flat_map(|i| self.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.tcx.fn_sig(item.def_id).skip_binder(); + let ret_ty = fn_sig.output(); + let ret_ty = self.tcx.erase_late_bound_regions(ret_ty); + if !self.can_eq(self.param_env, ret_ty, adt_ty) { + 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 { 1 }; + Some((order, item.name, input_len)) + } else { + None + } + }) + .collect::>(); + items.sort_by_key(|(order, _, _)| *order); + match &items[..] { + [] => {} + [(_, name, args)] => { + err.span_suggestion_verbose( + span.shrink_to_hi().with_hi(expr_span.hi()), + format!("you might have meant to use the `{name}` associated function",), + format!( + "::{name}({})", + std::iter::repeat("_").take(*args).collect::>().join(", ") + ), + Applicability::MaybeIncorrect, + ); + } + _ => { + err.span_suggestions( + span.shrink_to_hi().with_hi(expr_span.hi()), + "you might have meant to use an associated function to build this type", + items + .iter() + .map(|(_, name, args)| { + format!( + "::{name}({})", + std::iter::repeat("_") + .take(*args) + .collect::>() + .join(", ") + ) + }) + .collect::>(), + Applicability::MaybeIncorrect, + ); + } + } + } + err.emit(); } diff --git a/tests/ui/privacy/suggest-box-new.rs b/tests/ui/privacy/suggest-box-new.rs index 657dd37dc68ab..7125285fc7764 100644 --- a/tests/ui/privacy/suggest-box-new.rs +++ b/tests/ui/privacy/suggest-box-new.rs @@ -13,4 +13,7 @@ fn main() { }; let _ = std::collections::HashMap(); //~^ ERROR expected function, tuple struct or tuple variant, found struct `std::collections::HashMap` + let _ = std::collections::HashMap {}; + //~^ ERROR cannot construct `HashMap<_, _, _>` with struct literal syntax due to private fields + let _ = Box {}; //~ ERROR cannot construct `Box<_, _>` with struct literal syntax due to private fields } diff --git a/tests/ui/privacy/suggest-box-new.stderr b/tests/ui/privacy/suggest-box-new.stderr index f58f661a468d3..79629dab6bea8 100644 --- a/tests/ui/privacy/suggest-box-new.stderr +++ b/tests/ui/privacy/suggest-box-new.stderr @@ -50,6 +50,43 @@ help: consider using the `Default` trait LL | wtf: Some(::default()), | + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -error: aborting due to 2 previous errors +error: cannot construct `HashMap<_, _, _>` with struct literal syntax due to private fields + --> $DIR/suggest-box-new.rs:16:13 + | +LL | let _ = std::collections::HashMap {}; + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: ... and other private field `base` that was not provided +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(_, _); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: cannot construct `Box<_, _>` with struct literal syntax due to private fields + --> $DIR/suggest-box-new.rs:18:13 + | +LL | let _ = Box {}; + | ^^^ + | + = note: ... and other private fields `0` and `1` that were not provided +help: you might have meant to use an associated function to build this type + | +LL | let _ = Box::new(_); + | ~~~~~~~~ +LL | let _ = Box::new_uninit(); + | ~~~~~~~~~~~~~~ +LL | let _ = Box::new_zeroed(); + | ~~~~~~~~~~~~~~ +LL | let _ = Box::new_in(_, _); + | ~~~~~~~~~~~~~~ + and 10 other candidates + +error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0423`. From 69edf8e78499214e5072d6908c77749359929d66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 8 Nov 2023 23:56:16 +0000 Subject: [PATCH 09/14] Suggest Default::default() for struct literals with private fields --- compiler/rustc_hir_typeck/src/expr.rs | 17 +++++++++++++++++ tests/ui/privacy/suggest-box-new.stderr | 8 ++++++++ 2 files changed, 25 insertions(+) diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 9f27a6e5d84a4..4c177dbab79ee 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -2165,6 +2165,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); } } + if let Some(default_trait) = self.tcx.get_diagnostic_item(sym::Default) + && self.infcx + .type_implements_trait(default_trait, [adt_ty], self.param_env) + .may_apply() + { + err.multipart_suggestion( + "consider using the `Default` trait", + vec![ + (span.shrink_to_lo(), "<".to_string()), + ( + span.shrink_to_hi().with_hi(expr_span.hi()), + " as std::default::Default>::default()".to_string(), + ), + ], + Applicability::MaybeIncorrect, + ); + } } err.emit(); diff --git a/tests/ui/privacy/suggest-box-new.stderr b/tests/ui/privacy/suggest-box-new.stderr index 79629dab6bea8..5fec35dc5c638 100644 --- a/tests/ui/privacy/suggest-box-new.stderr +++ b/tests/ui/privacy/suggest-box-new.stderr @@ -67,6 +67,10 @@ LL | let _ = std::collections::HashMap::with_hasher(_); | ~~~~~~~~~~~~~~~~ LL | let _ = std::collections::HashMap::with_capacity_and_hasher(_, _); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +help: consider using the `Default` trait + | +LL | let _ = ::default(); + | + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: cannot construct `Box<_, _>` with struct literal syntax due to private fields --> $DIR/suggest-box-new.rs:18:13 @@ -86,6 +90,10 @@ LL | let _ = Box::new_zeroed(); LL | let _ = Box::new_in(_, _); | ~~~~~~~~~~~~~~ and 10 other candidates +help: consider using the `Default` trait + | +LL | let _ = ::default(); + | + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: aborting due to 4 previous errors From 00265f0cc0e963f83d76b3c43c9d41e839522778 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 9 Nov 2023 00:58:10 +0000 Subject: [PATCH 10/14] fix tidy --- compiler/rustc_resolve/src/late/diagnostics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 28f77b3112435..a0b0abd655563 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1790,7 +1790,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { } } // 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 + // 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 From 4d16171f563761e19798c220ee415f84d62df26c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 9 Nov 2023 01:23:10 +0000 Subject: [PATCH 11/14] Account for number of arguments in suggestion --- .../rustc_resolve/src/late/diagnostics.rs | 29 ++++++++++++++---- tests/ui/privacy/suggest-box-new.stderr | 30 +++++++++---------- 2 files changed, 39 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index a0b0abd655563..b0bb9a383db92 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1762,16 +1762,16 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { } else { 3 }; - Some((order, item.name)) + Some((order, item.name, input_len)) } else { None } }) .collect::>(); - items.sort_by_key(|(order, _)| *order); + items.sort_by_key(|(order, _, _)| *order); match &items[..] { [] => {} - [(_, name)] => { + [(_, name, len)] if *len == args.len() => { err.span_suggestion_verbose( path_span.shrink_to_hi(), format!("you might have meant to use the `{name}` associated function",), @@ -1779,11 +1779,30 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { Applicability::MaybeIncorrect, ); } + [(_, name, len)] => { + err.span_suggestion_verbose( + path_span.shrink_to_hi().with_hi(call_span.hi()), + format!("you might have meant to use the `{name}` associated function",), + format!( + "::{name}({})", + std::iter::repeat("_").take(*len).collect::>().join(", ") + ), + Applicability::MaybeIncorrect, + ); + } _ => { err.span_suggestions_with_style( - path_span.shrink_to_hi(), + path_span.shrink_to_hi().with_hi(call_span.hi()), "you might have meant to use an associated function to build this type", - items.iter().map(|(_, name)| format!("::{name}")).collect::>(), + items + .iter() + .map(|(_, name, len)| { + format!( + "::{name}({})", + std::iter::repeat("_").take(*len).collect::>().join(", ") + ) + }) + .collect::>(), Applicability::MaybeIncorrect, SuggestionStyle::ShowAlways, ); diff --git a/tests/ui/privacy/suggest-box-new.stderr b/tests/ui/privacy/suggest-box-new.stderr index 5fec35dc5c638..2224f1c60d63c 100644 --- a/tests/ui/privacy/suggest-box-new.stderr +++ b/tests/ui/privacy/suggest-box-new.stderr @@ -10,13 +10,13 @@ LL | let _ = std::collections::HashMap(); 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(); - | ++++++++++++++++++++++++++ + | ~~~~~~~ +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 _ = ::default(); @@ -36,14 +36,14 @@ note: constructor is not visible here due to private fields = note: private field 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(U { - | ++++++++++++ -LL | wtf: Some(Box::new_zeroed(U { - | ++++++++++++ -LL | wtf: Some(Box::new_in(U { - | ++++++++ +LL | wtf: Some(Box::new(_)), + | ~~~~~~~~ +LL | wtf: Some(Box::new_uninit()), + | ~~~~~~~~~~~~~~ +LL | wtf: Some(Box::new_zeroed()), + | ~~~~~~~~~~~~~~ +LL | wtf: Some(Box::new_in(_, _)), + | ~~~~~~~~~~~~~~ and 10 other candidates help: consider using the `Default` trait | From a519c9b6b7ab7a1245cae8a9053b666e379f9d53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 14 Nov 2023 01:31:03 +0000 Subject: [PATCH 12/14] review comments --- compiler/rustc_hir_typeck/src/expr.rs | 32 ++++++----------- .../rustc_resolve/src/late/diagnostics.rs | 35 +++++++------------ 2 files changed, 24 insertions(+), 43 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 4c177dbab79ee..ab567edd0626d 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -2121,27 +2121,25 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if !self.can_eq(self.param_env, ret_ty, adt_ty) { 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 { 1 }; - Some((order, item.name, input_len)) - } else { - None - } + let order = !item.name.as_str().starts_with("new"); + Some((order, item.name, input_len)) }) .collect::>(); items.sort_by_key(|(order, _, _)| *order); + let suggestion = |name, args| { + format!( + "::{name}({})", + std::iter::repeat("_").take(args).collect::>().join(", ") + ) + }; match &items[..] { [] => {} [(_, name, args)] => { err.span_suggestion_verbose( span.shrink_to_hi().with_hi(expr_span.hi()), - format!("you might have meant to use the `{name}` associated function",), - format!( - "::{name}({})", - std::iter::repeat("_").take(*args).collect::>().join(", ") - ), + format!("you might have meant to use the `{name}` associated function"), + suggestion(name, *args), Applicability::MaybeIncorrect, ); } @@ -2151,15 +2149,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { "you might have meant to use an associated function to build this type", items .iter() - .map(|(_, name, args)| { - format!( - "::{name}({})", - std::iter::repeat("_") - .take(*args) - .collect::>() - .join(", ") - ) - }) + .map(|(_, name, args)| suggestion(name, *args)) .collect::>(), Applicability::MaybeIncorrect, ); diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index b0bb9a383db92..a4f2446cc740f 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1732,6 +1732,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { args: &[P], ) { if def_id.is_local() { + // Doing analysis on local `DefId`s would cause infinite recursion. return; } // Look at all the associated functions without receivers in the type's @@ -1752,23 +1753,21 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { 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, input_len)) - } else { - None + if def.did() != def_id { + return None; } + let order = !item.name.as_str().starts_with("new"); + Some((order, item.name, input_len)) }) .collect::>(); items.sort_by_key(|(order, _, _)| *order); + let suggestion = |name, args| { + format!( + "::{name}({})", + std::iter::repeat("_").take(args).collect::>().join(", ") + ) + }; match &items[..] { [] => {} [(_, name, len)] if *len == args.len() => { @@ -1783,10 +1782,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { err.span_suggestion_verbose( path_span.shrink_to_hi().with_hi(call_span.hi()), format!("you might have meant to use the `{name}` associated function",), - format!( - "::{name}({})", - std::iter::repeat("_").take(*len).collect::>().join(", ") - ), + suggestion(name, *len), Applicability::MaybeIncorrect, ); } @@ -1796,12 +1792,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { "you might have meant to use an associated function to build this type", items .iter() - .map(|(_, name, len)| { - format!( - "::{name}({})", - std::iter::repeat("_").take(*len).collect::>().join(", ") - ) - }) + .map(|(_, name, len)| suggestion(name, *len)) .collect::>(), Applicability::MaybeIncorrect, SuggestionStyle::ShowAlways, From b0d7ccb133e5b254f7d9c317ad566903dc1c4b38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 16 Nov 2023 16:42:06 +0000 Subject: [PATCH 13/14] fmt --- compiler/rustc_hir_typeck/src/expr.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index ab567edd0626d..4e5a9ce89a941 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -2156,7 +2156,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } if let Some(default_trait) = self.tcx.get_diagnostic_item(sym::Default) - && self.infcx + && self + .infcx .type_implements_trait(default_trait, [adt_ty], self.param_env) .may_apply() { From ac56b06b44c3aeb2d64c7ea6caf08973e4892637 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 19 Nov 2023 18:07:22 +0000 Subject: [PATCH 14/14] fix rebase --- compiler/rustc_hir_typeck/src/expr.rs | 3 ++- compiler/rustc_hir_typeck/src/method/suggest.rs | 1 + compiler/rustc_resolve/src/late/diagnostics.rs | 5 ++++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 4e5a9ce89a941..d9b27fc13182c 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -2117,7 +2117,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Only assoc fns that return `Self` let fn_sig = self.tcx.fn_sig(item.def_id).skip_binder(); let ret_ty = fn_sig.output(); - let ret_ty = self.tcx.erase_late_bound_regions(ret_ty); + let ret_ty = + self.tcx.normalize_erasing_late_bound_regions(self.param_env, ret_ty); if !self.can_eq(self.param_env, ret_ty, adt_ty) { return None; } diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs index cdb08cfd7d966..db434636f5951 100644 --- a/compiler/rustc_hir_typeck/src/method/suggest.rs +++ b/compiler/rustc_hir_typeck/src/method/suggest.rs @@ -2002,6 +2002,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.tcx.get_diagnostic_item(sym::BorrowMut), ]; let mut candidate_fields: Vec<_> = fields + .into_iter() .filter_map(|candidate_field| { self.check_for_nested_field_satisfying( span, diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index a4f2446cc740f..d62d7fdcae0c2 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1749,7 +1749,10 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { // 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 ret_ty = self + .r + .tcx + .normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), ret_ty); let ty::Adt(def, _args) = ret_ty.kind() else { return None; };