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] 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 470f318eb33..b379d17dc22 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 2755a161d91..a078a9e00ae 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 28e6fe9b4b7..84225bbe39b 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 6aecd3610c6..3b59af89a98 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 7ed15e89caa..78e6376bca2 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 f5ae5c2abfd..00000000000 --- 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 2e18dba8b9f..e483d3f3af0 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 ed7fa036408..dbed66d0ae4 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 671f5efddd9..057e37dbe10 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]);