Skip to content

Commit c7623f5

Browse files
estebankestebank
committed
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<T>`.
1 parent 187d1af commit c7623f5

File tree

9 files changed

+98
-43
lines changed

9 files changed

+98
-43
lines changed

‎compiler/rustc_errors/src/diagnostic.rs

+7-4
Original file line numberDiff line numberDiff line change
@@ -759,6 +759,9 @@ impl Diagnostic {
759759
suggestions: impl IntoIterator<Item = String>,
760760
applicability: Applicability,
761761
) -> &mut Self {
762+
let mut suggestions: Vec<_> = suggestions.into_iter().collect();
763+
suggestions.sort();
764+
762765
self.span_suggestions_with_style(
763766
sp,
764767
msg,
@@ -768,7 +771,9 @@ impl Diagnostic {
768771
)
769772
}
770773

771-
/// [`Diagnostic::span_suggestions()`] but you can set the [`SuggestionStyle`].
774+
/// [`Diagnostic::span_suggestions()`] but you can set the [`SuggestionStyle`]. This version
775+
/// *doesn't* sort the suggestions, so the caller has control of the order in which they are
776+
/// presented.
772777
pub fn span_suggestions_with_style(
773778
&mut self,
774779
sp: Span,
@@ -777,9 +782,7 @@ impl Diagnostic {
777782
applicability: Applicability,
778783
style: SuggestionStyle,
779784
) -> &mut Self {
780-
let mut suggestions: Vec<_> = suggestions.into_iter().collect();
781-
suggestions.sort();
782-
785+
let suggestions: Vec<_> = suggestions.into_iter().collect();
783786
debug_assert!(
784787
!(sp.is_empty() && suggestions.iter().any(|suggestion| suggestion.is_empty())),
785788
"Span must not be empty and have no suggestion"

‎compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -441,10 +441,12 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> {
441441

442442
self.formatting_init.extend(code_init);
443443
Ok(quote! {
444+
let mut code: Vec<_> = #code_field.into_iter().collect();
445+
code.sort();
444446
#diag.span_suggestions_with_style(
445447
#span_field,
446448
crate::fluent_generated::#slug,
447-
#code_field,
449+
code,
448450
#applicability,
449451
#style
450452
);

‎compiler/rustc_resolve/src/diagnostics.rs

+1
Original file line numberDiff line numberDiff line change
@@ -2596,6 +2596,7 @@ fn show_candidates(
25962596
path_strings.extend(core_path_strings);
25972597
path_strings.dedup_by(|a, b| a.0 == b.0);
25982598
}
2599+
accessible_path_strings.sort();
25992600

26002601
if !accessible_path_strings.is_empty() {
26012602
let (determiner, kind, name, through) =

‎compiler/rustc_resolve/src/late/diagnostics.rs

+74-16
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use rustc_ast_pretty::pprust::where_bound_predicate_to_string;
1515
use rustc_data_structures::fx::FxHashSet;
1616
use rustc_errors::{
1717
pluralize, struct_span_err, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed,
18-
MultiSpan,
18+
MultiSpan, SuggestionStyle,
1919
};
2020
use rustc_hir as hir;
2121
use rustc_hir::def::{self, CtorKind, CtorOf, DefKind};
@@ -29,6 +29,8 @@ use rustc_span::hygiene::MacroKind;
2929
use rustc_span::symbol::{kw, sym, Ident, Symbol};
3030
use rustc_span::Span;
3131

32+
use rustc_middle::ty;
33+
3234
use std::borrow::Cow;
3335
use std::iter;
3436
use std::ops::Deref;
@@ -1566,29 +1568,85 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
15661568
Some(Vec::from(pattern_spans))
15671569
}
15681570
// e.g. `let _ = Enum::TupleVariant(field1, field2);`
1569-
_ if source.is_call() => {
1571+
PathSource::Expr(Some(Expr { kind: ExprKind::Call(_, ref args), .. })) => {
15701572
err.set_primary_message(
15711573
"cannot initialize a tuple struct which contains private fields",
15721574
);
1573-
if !def_id.is_local()
1574-
&& self
1575+
if !def_id.is_local() {
1576+
// Look at all the associated functions without receivers in the type's
1577+
// inherent impls to look for builders that return `Self`
1578+
let mut items = self
15751579
.r
15761580
.tcx
15771581
.inherent_impls(def_id)
15781582
.iter()
1579-
.flat_map(|impl_def_id| {
1580-
self.r.tcx.provided_trait_methods(*impl_def_id)
1583+
.flat_map(|i| self.r.tcx.associated_items(i).in_definition_order())
1584+
// Only assoc fn with no receivers.
1585+
.filter(|item| {
1586+
matches!(item.kind, ty::AssocKind::Fn)
1587+
&& !item.fn_has_self_parameter
15811588
})
1582-
.any(|assoc| !assoc.fn_has_self_parameter && assoc.name == sym::new)
1583-
{
1584-
// FIXME: look for associated functions with Self return type,
1585-
// instead of relying only on the name and lack of self receiver.
1586-
err.span_suggestion_verbose(
1587-
span.shrink_to_hi(),
1588-
"you might have meant to use the `new` associated function",
1589-
"::new".to_string(),
1590-
Applicability::MaybeIncorrect,
1591-
);
1589+
.filter_map(|item| {
1590+
// Only assoc fns that return `Self`
1591+
let fn_sig = self.r.tcx.fn_sig(item.def_id).skip_binder();
1592+
let ret_ty = fn_sig.output();
1593+
let ret_ty = self.r.tcx.erase_late_bound_regions(ret_ty);
1594+
let ty::Adt(def, _args) = ret_ty.kind() else {
1595+
return None;
1596+
};
1597+
// Check for `-> Self`
1598+
if def.did() == def_id {
1599+
let order = if item.name.as_str().starts_with("new")
1600+
&& fn_sig.inputs().skip_binder().len() == args.len()
1601+
{
1602+
0
1603+
} else if item.name.as_str().starts_with("new")
1604+
|| item.name.as_str().starts_with("default")
1605+
{
1606+
// Give higher precedence to functions with a name that
1607+
// imply construction.
1608+
1
1609+
} else if fn_sig.inputs().skip_binder().len() == args.len()
1610+
{
1611+
2
1612+
} else {
1613+
3
1614+
};
1615+
return Some((order, item.name));
1616+
}
1617+
None
1618+
})
1619+
.collect::<Vec<_>>();
1620+
items.sort_by_key(|(order, _)| *order);
1621+
match &items[..] {
1622+
[] => {}
1623+
[(_, name)] => {
1624+
err.span_suggestion_verbose(
1625+
span.shrink_to_hi(),
1626+
format!(
1627+
"you might have meant to use the `{name}` associated \
1628+
function",
1629+
),
1630+
format!("::{name}"),
1631+
Applicability::MaybeIncorrect,
1632+
);
1633+
}
1634+
_ => {
1635+
// We use this instead of `span_suggestions` to retain output
1636+
// sort order.
1637+
err.span_suggestions_with_style(
1638+
span.shrink_to_hi(),
1639+
"you might have meant to use an associated function to \
1640+
build this type",
1641+
items
1642+
.iter()
1643+
.map(|(_, name)| format!("::{name}"))
1644+
.collect::<Vec<String>>(),
1645+
Applicability::MaybeIncorrect,
1646+
SuggestionStyle::ShowAlways,
1647+
);
1648+
}
1649+
}
15921650
}
15931651
// Use spans of the tuple struct definition.
15941652
self.r.field_def_ids(def_id).map(|field_ids| {

‎tests/ui/hygiene/extern-prelude-from-opaque-fail-2018.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ LL | a!();
2525
| ---- in this macro invocation
2626
|
2727
= help: consider importing one of these items:
28-
std::mem
2928
core::mem
29+
std::mem
3030
= note: this error originates in the macro `a` (in Nightly builds, run with -Z macro-backtrace for more info)
3131

3232
error[E0433]: failed to resolve: use of undeclared crate or module `my_core`

‎tests/ui/privacy/suggest-box-new.fixed

-15
This file was deleted.

‎tests/ui/privacy/suggest-box-new.rs

-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
// run-rustfix
21
#![allow(dead_code)]
32
struct U <T> {
43
wtf: Option<Box<U<T>>>,

‎tests/ui/privacy/suggest-box-new.stderr

+9-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error[E0423]: cannot initialize a tuple struct which contains private fields
2-
--> $DIR/suggest-box-new.rs:9:19
2+
--> $DIR/suggest-box-new.rs:8:19
33
|
44
LL | wtf: Some(Box(U {
55
| ^^^
@@ -10,10 +10,17 @@ note: constructor is not visible here due to private fields
1010
= note: private field
1111
|
1212
= note: private field
13-
help: you might have meant to use the `new` associated function
13+
help: you might have meant to use an associated function to build this type
1414
|
1515
LL | wtf: Some(Box::new(U {
1616
| +++++
17+
LL | wtf: Some(Box::new_uninit_in(U {
18+
| +++++++++++++++
19+
LL | wtf: Some(Box::new_zeroed_in(U {
20+
| +++++++++++++++
21+
LL | wtf: Some(Box::new_uninit_slice(U {
22+
| ++++++++++++++++++
23+
and 10 other candidates
1724

1825
error: aborting due to previous error
1926

‎tests/ui/suggestions/suggest-tryinto-edition-change.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ error[E0433]: failed to resolve: use of undeclared type `TryFrom`
44
LL | let _i: i16 = TryFrom::try_from(0_i32).unwrap();
55
| ^^^^^^^ use of undeclared type `TryFrom`
66
|
7-
= note: 'std::convert::TryFrom' is included in the prelude starting in Edition 2021
87
= note: 'core::convert::TryFrom' is included in the prelude starting in Edition 2021
8+
= note: 'std::convert::TryFrom' is included in the prelude starting in Edition 2021
99
help: consider importing one of these items
1010
|
1111
LL + use core::convert::TryFrom;
@@ -19,8 +19,8 @@ error[E0433]: failed to resolve: use of undeclared type `TryInto`
1919
LL | let _i: i16 = TryInto::try_into(0_i32).unwrap();
2020
| ^^^^^^^ use of undeclared type `TryInto`
2121
|
22-
= note: 'std::convert::TryInto' is included in the prelude starting in Edition 2021
2322
= note: 'core::convert::TryInto' is included in the prelude starting in Edition 2021
23+
= note: 'std::convert::TryInto' is included in the prelude starting in Edition 2021
2424
help: consider importing one of these items
2525
|
2626
LL + use core::convert::TryInto;
@@ -34,8 +34,8 @@ error[E0433]: failed to resolve: use of undeclared type `FromIterator`
3434
LL | let _v: Vec<_> = FromIterator::from_iter(&[1]);
3535
| ^^^^^^^^^^^^ use of undeclared type `FromIterator`
3636
|
37-
= note: 'std::iter::FromIterator' is included in the prelude starting in Edition 2021
3837
= note: 'core::iter::FromIterator' is included in the prelude starting in Edition 2021
38+
= note: 'std::iter::FromIterator' is included in the prelude starting in Edition 2021
3939
help: a trait with a similar name exists
4040
|
4141
LL | let _v: Vec<_> = IntoIterator::from_iter(&[1]);

0 commit comments

Comments
 (0)