Suggest using builder on curly brace struct called as fn

This commit is contained in:
Esteban Küber 2023-11-08 23:18:00 +00:00
parent 12a8bb8d9b
commit 987155f35d
3 changed files with 211 additions and 172 deletions

View File

@ -7,6 +7,7 @@
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 @@ fn smart_resolve_context_dependent_help(
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 @@ fn smart_resolve_context_dependent_help(
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 @@ fn smart_resolve_context_dependent_help(
}
};
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 @@ fn smart_resolve_context_dependent_help(
}
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 @@ fn smart_resolve_context_dependent_help(
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 @@ fn smart_resolve_context_dependent_help(
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 @@ fn smart_resolve_context_dependent_help(
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 @@ fn smart_resolve_context_dependent_help(
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 @@ fn smart_resolve_context_dependent_help(
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,
self.suggest_alternative_construction_methods(
def_id,
err,
path.span,
*call_span,
&args[..],
);
}
_ => {
// 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,
);
}
}
// Use spans of the tuple struct definition.
self.r.field_def_ids(def_id).map(|field_ids| {
field_ids
@ -1806,7 +1677,7 @@ fn smart_resolve_context_dependent_help(
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 @@ fn smart_resolve_context_dependent_help(
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(

View File

@ -11,4 +11,6 @@ fn main() {
})),
x: ()
};
let _ = std::collections::HashMap();
//~^ ERROR expected function, tuple struct or tuple variant, found struct `std::collections::HashMap`
}

View File

@ -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`.