Skip to content

Commit

Permalink
don't suggest erroneous trailing comma after ..
Browse files Browse the repository at this point in the history
In #76612, suggestions were added for missing fields in
patterns. However, the suggestions are being inserted just at the end
of the last field in the pattern—before any trailing comma after the
last field. This resulted in the "if you don't care about missing
fields" suggestion to recommend code with a trailing comma after the
field ellipsis (`..,`), which is actually not legal ("`..` must be at
the end and cannot have a trailing comma")!

Incidentally, the doc-comment on `error_unmentioned_fields` was using
`you_cant_use_this_field` as an example field name (presumably
copy-paste inherited from the description of Issue #76077), but
the present author found this confusing, because unmentioned fields
aren't necessarily unusable.

The suggested code in the diff this commit introduces to
`destructuring-assignment/struct_destructure_fail.stderr` doesn't
work, but it didn't work beforehand, either (because of the "found
reserved identifier `_`" thing), so you can't really call it a
regression; it could be fixed in a separate PR.

Resolves #78511.
  • Loading branch information
zackmdavis committed Jan 17, 2021
1 parent 63a83c5 commit 14eb94f
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 15 deletions.
32 changes: 22 additions & 10 deletions compiler/rustc_typeck/src/check/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1486,11 +1486,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// Returns a diagnostic reporting a struct pattern which does not mention some fields.
///
/// ```text
/// error[E0027]: pattern does not mention field `you_cant_use_this_field`
/// error[E0027]: pattern does not mention field `bar`
/// --> src/main.rs:15:9
/// |
/// LL | let foo::Foo {} = foo::Foo::new();
/// | ^^^^^^^^^^^ missing field `you_cant_use_this_field`
/// | ^^^^^^^^^^^ missing field `bar`
/// ```
fn error_unmentioned_fields(
&self,
Expand Down Expand Up @@ -1524,14 +1524,26 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
_ => return err,
},
[.., field] => (
match pat.kind {
PatKind::Struct(_, [_, ..], _) => ", ",
_ => "",
},
"",
field.span.shrink_to_hi(),
),
[.., field] => {
// if last field has a trailing comma, use the comma
// as the span to avoid trailing comma in ultimate
// suggestion (Issue #78511)
let tail = field.span.shrink_to_hi().until(pat.span.shrink_to_hi());
let tail_through_comma = self.tcx.sess.source_map().span_through_char(tail, ',');
let sp = if tail_through_comma == tail {
field.span.shrink_to_hi()
} else {
tail_through_comma
};
(
match pat.kind {
PatKind::Struct(_, [_, ..], _) => ", ",
_ => "",
},
"",
sp,
)
}
};
err.span_suggestion(
sp,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ LL | Struct { a, _ } = Struct { a: 1, b: 2 };
|
help: include the missing field in the pattern
|
LL | Struct { a, b, _ } = Struct { a: 1, b: 2 };
LL | Struct { a, b _ } = Struct { a: 1, b: 2 };
| ^^^
help: if you don't care about this missing field, you can explicitly ignore it
|
LL | Struct { a, .., _ } = Struct { a: 1, b: 2 };
LL | Struct { a, .. _ } = Struct { a: 1, b: 2 };
| ^^^^

error: aborting due to 5 previous errors
Expand Down
9 changes: 9 additions & 0 deletions src/test/ui/error-codes/E0027.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,21 @@ struct Dog {
age: u32,
}


fn main() {
let d = Dog { name: "Rusty".to_string(), age: 8 };

match d {
Dog { age: x } => {} //~ ERROR pattern does not mention field `name`
}
match d {
// trailing comma
Dog { name: x, } => {} //~ ERROR pattern does not mention field `age`
}
match d {
// trailing comma with weird whitespace
Dog { name: x , } => {} //~ ERROR pattern does not mention field `age`
}
match d {
Dog {} => {} //~ ERROR pattern does not mention fields `name`, `age`
}
Expand Down
36 changes: 33 additions & 3 deletions src/test/ui/error-codes/E0027.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0027]: pattern does not mention field `name`
--> $DIR/E0027.rs:10:9
--> $DIR/E0027.rs:11:9
|
LL | Dog { age: x } => {}
| ^^^^^^^^^^^^^^ missing field `name`
Expand All @@ -13,8 +13,38 @@ help: if you don't care about this missing field, you can explicitly ignore it
LL | Dog { age: x, .. } => {}
| ^^^^

error[E0027]: pattern does not mention field `age`
--> $DIR/E0027.rs:15:9
|
LL | Dog { name: x, } => {}
| ^^^^^^^^^^^^^^^^ missing field `age`
|
help: include the missing field in the pattern
|
LL | Dog { name: x, age } => {}
| ^^^^^
help: if you don't care about this missing field, you can explicitly ignore it
|
LL | Dog { name: x, .. } => {}
| ^^^^

error[E0027]: pattern does not mention field `age`
--> $DIR/E0027.rs:19:9
|
LL | Dog { name: x , } => {}
| ^^^^^^^^^^^^^^^^^^ missing field `age`
|
help: include the missing field in the pattern
|
LL | Dog { name: x, age } => {}
| ^^^^^
help: if you don't care about this missing field, you can explicitly ignore it
|
LL | Dog { name: x, .. } => {}
| ^^^^

error[E0027]: pattern does not mention fields `name`, `age`
--> $DIR/E0027.rs:13:9
--> $DIR/E0027.rs:22:9
|
LL | Dog {} => {}
| ^^^^^^ missing fields `name`, `age`
Expand All @@ -28,6 +58,6 @@ help: if you don't care about these missing fields, you can explicitly ignore th
LL | Dog { .. } => {}
| ^^^^^^

error: aborting due to 2 previous errors
error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0027`.

0 comments on commit 14eb94f

Please sign in to comment.