From 14eb94fe7a3ca3a92d5d7e42744d105c0fc6d527 Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Sat, 16 Jan 2021 15:41:52 -0800 Subject: [PATCH] don't suggest erroneous trailing comma after `..` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- compiler/rustc_typeck/src/check/pat.rs | 32 +++++++++++------ .../struct_destructure_fail.stderr | 4 +-- src/test/ui/error-codes/E0027.rs | 9 +++++ src/test/ui/error-codes/E0027.stderr | 36 +++++++++++++++++-- 4 files changed, 66 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_typeck/src/check/pat.rs b/compiler/rustc_typeck/src/check/pat.rs index ecc6e8599ad01..79234f076acd1 100644 --- a/compiler/rustc_typeck/src/check/pat.rs +++ b/compiler/rustc_typeck/src/check/pat.rs @@ -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, @@ -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, diff --git a/src/test/ui/destructuring-assignment/struct_destructure_fail.stderr b/src/test/ui/destructuring-assignment/struct_destructure_fail.stderr index 81661a357e750..c0955ef8b065a 100644 --- a/src/test/ui/destructuring-assignment/struct_destructure_fail.stderr +++ b/src/test/ui/destructuring-assignment/struct_destructure_fail.stderr @@ -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 diff --git a/src/test/ui/error-codes/E0027.rs b/src/test/ui/error-codes/E0027.rs index 8d08e17893480..e7eca1ce5af97 100644 --- a/src/test/ui/error-codes/E0027.rs +++ b/src/test/ui/error-codes/E0027.rs @@ -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` } diff --git a/src/test/ui/error-codes/E0027.stderr b/src/test/ui/error-codes/E0027.stderr index cf0ff6311483c..694bbc358fee2 100644 --- a/src/test/ui/error-codes/E0027.stderr +++ b/src/test/ui/error-codes/E0027.stderr @@ -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` @@ -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` @@ -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`.