Skip to content

Commit

Permalink
Suggest struct pat on incorrect unit or tuple pat
Browse files Browse the repository at this point in the history 
When encountering a unit or tuple pattern for a struct-like item, suggest
using the correct pattern.

Use `insert_field_names_local` when evaluating variants and store field
names even when the list is empty in order to produce accurate
structured suggestions.
  • Loading branch information
@estebank
estebank committed Jul 14, 2020
1 parent 346aec9 commit 0429820
Show file tree
Hide file tree
Showing 23 changed files with 239 additions and 139 deletions.
6 changes: 3 additions & 3 deletions src/librustc_resolve/build_reduced_graph.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,9 +300,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
}

fn insert_field_names(&mut self, def_id: DefId, field_names: Vec<Spanned<Symbol>>) {
if !field_names.is_empty() {
self.r.field_names.insert(def_id, field_names);
}
self.r.field_names.insert(def_id, field_names);
}

fn block_needs_anonymous_module(&mut self, block: &Block) -> bool {
Expand Down Expand Up @@ -1428,6 +1426,8 @@ impl<'a, 'b> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b> {
let ctor_kind = CtorKind::from_ast(&variant.data);
let ctor_res = Res::Def(DefKind::Ctor(CtorOf::Variant, ctor_kind), ctor_def_id);
self.r.define(parent, ident, ValueNS, (ctor_res, ctor_vis, variant.span, expn_id));
// Record field names for error reporting.
self.insert_field_names_local(ctor_def_id, &variant.data);

visit::walk_variant(self, variant);
}
Expand Down
16 changes: 8 additions & 8 deletions src/librustc_resolve/late.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ crate enum PathSource<'a> {
// Paths in struct expressions and patterns `Path { .. }`.
Struct,
// Paths in tuple struct patterns `Path(..)`.
TupleStruct,
TupleStruct(Span),
// `m::A::B` in `<T as m::A>::B::C`.
TraitItem(Namespace),
}
Expand All @@ -193,7 +193,7 @@ impl<'a> PathSource<'a> {
fn namespace(self) -> Namespace {
match self {
PathSource::Type | PathSource::Trait(_) | PathSource::Struct => TypeNS,
PathSource::Expr(..) | PathSource::Pat | PathSource::TupleStruct => ValueNS,
PathSource::Expr(..) | PathSource::Pat | PathSource::TupleStruct(_) => ValueNS,
PathSource::TraitItem(ns) => ns,
}
}
Expand All @@ -204,7 +204,7 @@ impl<'a> PathSource<'a> {
| PathSource::Expr(..)
| PathSource::Pat
| PathSource::Struct
| PathSource::TupleStruct => true,
| PathSource::TupleStruct(_) => true,
PathSource::Trait(_) | PathSource::TraitItem(..) => false,
}
}
Expand All @@ -215,7 +215,7 @@ impl<'a> PathSource<'a> {
PathSource::Trait(_) => "trait",
PathSource::Pat => "unit struct, unit variant or constant",
PathSource::Struct => "struct, variant or union type",
PathSource::TupleStruct => "tuple struct or tuple variant",
PathSource::TupleStruct(_) => "tuple struct or tuple variant",
PathSource::TraitItem(ns) => match ns {
TypeNS => "associated type",
ValueNS => "method or associated constant",
Expand Down Expand Up @@ -301,7 +301,7 @@ impl<'a> PathSource<'a> {
| Res::SelfCtor(..) => true,
_ => false,
},
PathSource::TupleStruct => match res {
PathSource::TupleStruct(_) => match res {
Res::Def(DefKind::Ctor(_, CtorKind::Fn), _) | Res::SelfCtor(..) => true,
_ => false,
},
Expand Down Expand Up @@ -336,8 +336,8 @@ impl<'a> PathSource<'a> {
(PathSource::Struct, false) => error_code!(E0422),
(PathSource::Expr(..), true) => error_code!(E0423),
(PathSource::Expr(..), false) => error_code!(E0425),
(PathSource::Pat | PathSource::TupleStruct, true) => error_code!(E0532),
(PathSource::Pat | PathSource::TupleStruct, false) => error_code!(E0531),
(PathSource::Pat | PathSource::TupleStruct(_), true) => error_code!(E0532),
(PathSource::Pat | PathSource::TupleStruct(_), false) => error_code!(E0531),
(PathSource::TraitItem(..), true) => error_code!(E0575),
(PathSource::TraitItem(..), false) => error_code!(E0576),
}
Expand Down Expand Up @@ -1483,7 +1483,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
self.r.record_partial_res(pat.id, PartialRes::new(res));
}
PatKind::TupleStruct(ref path, ..) => {
self.smart_resolve_path(pat.id, None, path, PathSource::TupleStruct);
self.smart_resolve_path(pat.id, None, path, PathSource::TupleStruct(pat.span));
}
PatKind::Path(ref qself, ref path) => {
self.smart_resolve_path(pat.id, qself.as_ref(), path, PathSource::Pat);
Expand Down
68 changes: 57 additions & 11 deletions src/librustc_resolve/late/diagnostics.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -482,10 +482,12 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {

let mut bad_struct_syntax_suggestion = |def_id: DefId| {
let (followed_by_brace, closing_brace) = self.followed_by_brace(span);
let mut suggested = false;

match source {
PathSource::Expr(Some(parent)) => {
suggested = path_sep(err, &parent);
PathSource::Expr(Some(
parent @ Expr { kind: ExprKind::Field(..) | ExprKind::MethodCall(..), .. },
)) => {
path_sep(err, &parent);
}
PathSource::Expr(None) if followed_by_brace => {
if let Some(sp) = closing_brace {
Expand All @@ -507,15 +509,56 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
),
);
}
suggested = true;
}
_ => {}
}
if !suggested {
if let Some(span) = self.r.opt_span(def_id) {
err.span_label(span, &format!("`{}` defined here", path_str));
PathSource::Expr(
None | Some(Expr { kind: ExprKind::Call(..) | ExprKind::Path(..), .. }),
)
| PathSource::TupleStruct(_)
| PathSource::Pat => {
let span = match &source {
PathSource::Expr(Some(Expr {
span, kind: ExprKind::Call(_, _), ..
}))
| PathSource::TupleStruct(span) => {
// We want the main underline to cover the suggested code as well for
// cleaner output.
err.set_span(*span);
*span
}
_ => span,
};
if let Some(span) = self.r.opt_span(def_id) {
err.span_label(span, &format!("`{}` defined here", path_str));
}
let (tail, descr, applicability) = match source {
PathSource::Pat | PathSource::TupleStruct(_) => {
("", "pattern", Applicability::MachineApplicable)
}
_ => (": val", "literal", Applicability::HasPlaceholders),
};
let (fields, applicability) = match self.r.field_names.get(&def_id) {
Some(fields) => (
fields
.iter()
.map(|f| format!("{}{}", f.node, tail))
.collect::<Vec<String>>()
.join(", "),
applicability,
),
None => ("/* fields */".to_string(), Applicability::HasPlaceholders),
};
let pad = match self.r.field_names.get(&def_id) {
Some(fields) if fields.is_empty() => "",
_ => " ",
};
err.span_suggestion(
span,
&format!("use struct {} syntax instead", descr),
format!("{} {{{pad}{}{pad}}}", path_str, fields, pad = pad),
applicability,
);
}
err.span_label(span, format!("did you mean `{} {{ /* fields */ }}`?", path_str));
_ => {}
}
};

Expand Down Expand Up @@ -548,7 +591,10 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
return false;
}
}
(Res::Def(DefKind::Enum, def_id), PathSource::TupleStruct | PathSource::Expr(..)) => {
(
Res::Def(DefKind::Enum, def_id),
PathSource::TupleStruct(_) | PathSource::Expr(..),
) => {
if let Some(variants) = self.collect_enum_variants(def_id) {
if !variants.is_empty() {
let msg = if variants.len() == 1 {
Expand Down
60 changes: 37 additions & 23 deletions src/test/ui/empty/empty-struct-braces-expr.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,20 @@ LL | struct Empty1 {}
...
LL | let e1 = Empty1;
| ^^^^^^
| |
| did you mean `Empty1 { /* fields */ }`?
| help: a unit struct with a similar name exists: `XEmpty2`
|
::: $DIR/auxiliary/empty-struct.rs:2:1
|
LL | pub struct XEmpty2;
| ------------------- similarly named unit struct `XEmpty2` defined here
|
help: a unit struct with a similar name exists
|
LL | let e1 = XEmpty2;
| ^^^^^^^
help: use struct literal syntax instead
|
LL | let e1 = Empty1 {};
| ^^^^^^^^^

error[E0423]: expected function, tuple struct or tuple variant, found struct `Empty1`
--> $DIR/empty-struct-braces-expr.rs:16:14
Expand All @@ -22,15 +28,16 @@ LL | struct Empty1 {}
| ---------------- `Empty1` defined here
...
LL | let e1 = Empty1();
| ^^^^^^
| |
| did you mean `Empty1 { /* fields */ }`?
| help: a unit struct with a similar name exists: `XEmpty2`
|
::: $DIR/auxiliary/empty-struct.rs:2:1
| ^^^^^^^^
|
LL | pub struct XEmpty2;
| ------------------- similarly named unit struct `XEmpty2` defined here
help: a unit struct with a similar name exists
|
LL | let e1 = XEmpty2();
| ^^^^^^^
help: use struct literal syntax instead
|
LL | let e1 = Empty1 {};
| ^^^^^^^^^

error[E0423]: expected value, found struct variant `E::Empty3`
--> $DIR/empty-struct-braces-expr.rs:18:14
Expand All @@ -39,7 +46,7 @@ LL | Empty3 {}
| --------- `E::Empty3` defined here
...
LL | let e3 = E::Empty3;
| ^^^^^^^^^ did you mean `E::Empty3 { /* fields */ }`?
| ^^^^^^^^^ help: use struct literal syntax instead: `E::Empty3 {}`

error[E0423]: expected function, tuple struct or tuple variant, found struct variant `E::Empty3`
--> $DIR/empty-struct-braces-expr.rs:19:14
Expand All @@ -48,35 +55,42 @@ LL | Empty3 {}
| --------- `E::Empty3` defined here
...
LL | let e3 = E::Empty3();
| ^^^^^^^^^ did you mean `E::Empty3 { /* fields */ }`?
| ^^^^^^^^^^^ help: use struct literal syntax instead: `E::Empty3 {}`

error[E0423]: expected value, found struct `XEmpty1`
--> $DIR/empty-struct-braces-expr.rs:22:15
|
LL | let xe1 = XEmpty1;
| ^^^^^^^
| |
| did you mean `XEmpty1 { /* fields */ }`?
| help: a unit struct with a similar name exists: `XEmpty2`
|
::: $DIR/auxiliary/empty-struct.rs:2:1
|
LL | pub struct XEmpty2;
| ------------------- similarly named unit struct `XEmpty2` defined here
|
help: a unit struct with a similar name exists
|
LL | let xe1 = XEmpty2;
| ^^^^^^^
help: use struct literal syntax instead
|
LL | let xe1 = XEmpty1 {};
| ^^^^^^^^^^

error[E0423]: expected function, tuple struct or tuple variant, found struct `XEmpty1`
--> $DIR/empty-struct-braces-expr.rs:23:15
|
LL | let xe1 = XEmpty1();
| ^^^^^^^^^
|
help: a unit struct with a similar name exists
|
LL | let xe1 = XEmpty2();
| ^^^^^^^
| |
| did you mean `XEmpty1 { /* fields */ }`?
| help: a unit struct with a similar name exists: `XEmpty2`
|
::: $DIR/auxiliary/empty-struct.rs:2:1
help: use struct literal syntax instead
|
LL | pub struct XEmpty2;
| ------------------- similarly named unit struct `XEmpty2` defined here
LL | let xe1 = XEmpty1 {};
| ^^^^^^^^^^

error[E0599]: no variant or associated item named `Empty3` found for enum `empty_struct::XE` in the current scope
--> $DIR/empty-struct-braces-expr.rs:25:19
Expand Down
16 changes: 11 additions & 5 deletions src/test/ui/empty/empty-struct-braces-pat-1.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,27 @@ LL | Empty3 {}
| --------- `E::Empty3` defined here
...
LL | E::Empty3 => ()
| ^^^^^^^^^ did you mean `E::Empty3 { /* fields */ }`?
| ^^^^^^^^^ help: use struct pattern syntax instead: `E::Empty3 {}`

error[E0532]: expected unit struct, unit variant or constant, found struct variant `XE::XEmpty3`
--> $DIR/empty-struct-braces-pat-1.rs:31:9
|
LL | XE::XEmpty3 => ()
| ^^^^-------
| | |
| | help: a unit variant with a similar name exists: `XEmpty4`
| did you mean `XE::XEmpty3 { /* fields */ }`?
| ^^^^^^^^^^^
|
::: $DIR/auxiliary/empty-struct.rs:7:5
|
LL | XEmpty4,
| ------- similarly named unit variant `XEmpty4` defined here
|
help: a unit variant with a similar name exists
|
LL | XE::XEmpty4 => ()
| ^^^^^^^
help: use struct pattern syntax instead
|
LL | XE::XEmpty3 { /* fields */ } => ()
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors

Expand Down
Loading

0 comments on commit 0429820

Please sign in to comment.