From 0bb43c63c33bf29e7a24e696f619be8e55e62f37 Mon Sep 17 00:00:00 2001 From: yukang Date: Wed, 3 May 2023 11:46:34 +0800 Subject: [PATCH 1/5] Suggest let for possible binding with ty --- compiler/rustc_macros/src/diagnostics/mod.rs | 2 +- compiler/rustc_parse/src/parser/diagnostics.rs | 17 +++++++++++++++++ compiler/rustc_parse/src/parser/stmt.rs | 9 --------- .../type-ascription-instead-of-let.fixed | 11 +++++++++++ .../type-ascription-instead-of-let.rs | 4 +++- .../type-ascription-instead-of-let.stderr | 7 ++++++- tests/ui/type/missing-let-in-binding-2.fixed | 5 +++++ tests/ui/type/missing-let-in-binding-2.rs | 5 +++++ tests/ui/type/missing-let-in-binding-2.stderr | 13 +++++++++++++ 9 files changed, 61 insertions(+), 12 deletions(-) create mode 100644 tests/ui/suggestions/type-ascription-instead-of-let.fixed create mode 100644 tests/ui/type/missing-let-in-binding-2.fixed create mode 100644 tests/ui/type/missing-let-in-binding-2.rs create mode 100644 tests/ui/type/missing-let-in-binding-2.stderr diff --git a/compiler/rustc_macros/src/diagnostics/mod.rs b/compiler/rustc_macros/src/diagnostics/mod.rs index bd84681cbb44b..a536eb3b04e69 100644 --- a/compiler/rustc_macros/src/diagnostics/mod.rs +++ b/compiler/rustc_macros/src/diagnostics/mod.rs @@ -140,7 +140,7 @@ pub fn lint_diagnostic_derive(s: Structure<'_>) -> TokenStream { /// ```fluent /// parser_expected_identifier = expected identifier /// -/// parser_expected_identifier-found = expected identifier, found {$found} +/// parser_expected_identifier_found = expected identifier, found {$found} /// /// parser_raw_identifier = escape `{$ident}` to use it as an identifier /// ``` diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 36883bd217211..e8f47346fa7cb 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -399,6 +399,23 @@ impl<'a> Parser<'a> { } } } + // we suggest add the missing `let` before the identifier + // `a: Ty = 1` -> `let a: Ty = 1` + if self.token == token::Colon { + let prev_span = self.prev_token.span.shrink_to_lo(); + let snapshot = self.create_snapshot_for_diagnostic(); + self.bump(); + let res = self.parse_ty(); + if res.is_ok() && self.token == token::Eq { + err.span_suggestion_verbose( + prev_span, + "you might have meant to introduce a new binding", + "let ".to_string(), + Applicability::MaybeIncorrect, + ); + } + self.restore_snapshot(snapshot); + } if let Some(recovered_ident) = recovered_ident && recover { err.emit(); diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index 1c17de337e833..ab04219b1772b 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -555,7 +555,6 @@ impl<'a> Parser<'a> { if self.token == token::Colon { // if next token is following a colon, it's likely a path // and we can suggest a path separator - let ident_span = self.prev_token.span; self.bump(); if self.token.span.lo() == self.prev_token.span.hi() { err.span_suggestion_verbose( @@ -565,14 +564,6 @@ impl<'a> Parser<'a> { Applicability::MaybeIncorrect, ); } - if self.look_ahead(1, |token| token == &token::Eq) { - err.span_suggestion_verbose( - ident_span.shrink_to_lo(), - "you might have meant to introduce a new binding", - "let ", - Applicability::MaybeIncorrect, - ); - } if self.sess.unstable_features.is_nightly_build() { // FIXME(Nilstrieb): Remove this again after a few months. err.note("type ascription syntax has been removed, see issue #101728 "); diff --git a/tests/ui/suggestions/type-ascription-instead-of-let.fixed b/tests/ui/suggestions/type-ascription-instead-of-let.fixed new file mode 100644 index 0000000000000..e3d03b6f22ad6 --- /dev/null +++ b/tests/ui/suggestions/type-ascription-instead-of-let.fixed @@ -0,0 +1,11 @@ +// run-rustfix + +fn fun(x: i32) -> i32 { x } + +fn main() { + let _closure_annotated = |value: i32| -> i32 { + let temp: i32 = fun(5i32); + //~^ ERROR expected identifier, found `:` + temp + value + 1 + }; +} diff --git a/tests/ui/suggestions/type-ascription-instead-of-let.rs b/tests/ui/suggestions/type-ascription-instead-of-let.rs index 5ad6024329861..6e1c86f967119 100644 --- a/tests/ui/suggestions/type-ascription-instead-of-let.rs +++ b/tests/ui/suggestions/type-ascription-instead-of-let.rs @@ -1,7 +1,9 @@ +// run-rustfix + fn fun(x: i32) -> i32 { x } fn main() { - let closure_annotated = |value: i32| -> i32 { + let _closure_annotated = |value: i32| -> i32 { temp: i32 = fun(5i32); //~^ ERROR expected identifier, found `:` temp + value + 1 diff --git a/tests/ui/suggestions/type-ascription-instead-of-let.stderr b/tests/ui/suggestions/type-ascription-instead-of-let.stderr index fb697b0ccfd5e..065b1f4d3538e 100644 --- a/tests/ui/suggestions/type-ascription-instead-of-let.stderr +++ b/tests/ui/suggestions/type-ascription-instead-of-let.stderr @@ -1,8 +1,13 @@ error: expected identifier, found `:` - --> $DIR/type-ascription-instead-of-let.rs:5:13 + --> $DIR/type-ascription-instead-of-let.rs:7:13 | LL | temp: i32 = fun(5i32); | ^ expected identifier + | +help: you might have meant to introduce a new binding + | +LL | let temp: i32 = fun(5i32); + | +++ error: aborting due to previous error diff --git a/tests/ui/type/missing-let-in-binding-2.fixed b/tests/ui/type/missing-let-in-binding-2.fixed new file mode 100644 index 0000000000000..d64013c8c8385 --- /dev/null +++ b/tests/ui/type/missing-let-in-binding-2.fixed @@ -0,0 +1,5 @@ +// run-rustfix + +fn main() { + let _v: Vec = vec![1, 2, 3]; //~ ERROR expected identifier, found `:` +} diff --git a/tests/ui/type/missing-let-in-binding-2.rs b/tests/ui/type/missing-let-in-binding-2.rs new file mode 100644 index 0000000000000..f95f7bef21585 --- /dev/null +++ b/tests/ui/type/missing-let-in-binding-2.rs @@ -0,0 +1,5 @@ +// run-rustfix + +fn main() { + _v: Vec = vec![1, 2, 3]; //~ ERROR expected identifier, found `:` +} diff --git a/tests/ui/type/missing-let-in-binding-2.stderr b/tests/ui/type/missing-let-in-binding-2.stderr new file mode 100644 index 0000000000000..2e10125943e75 --- /dev/null +++ b/tests/ui/type/missing-let-in-binding-2.stderr @@ -0,0 +1,13 @@ +error: expected identifier, found `:` + --> $DIR/missing-let-in-binding-2.rs:4:7 + | +LL | _v: Vec = vec![1, 2, 3]; + | ^ expected identifier + | +help: you might have meant to introduce a new binding + | +LL | let _v: Vec = vec![1, 2, 3]; + | +++ + +error: aborting due to previous error + From 20e6e6a4932991c4b53605154868cfa645b04998 Mon Sep 17 00:00:00 2001 From: yukang Date: Wed, 3 May 2023 11:56:19 +0800 Subject: [PATCH 2/5] cleanup --- compiler/rustc_parse/src/parser/diagnostics.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index e8f47346fa7cb..bd0ea50b4d8c1 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -405,8 +405,7 @@ impl<'a> Parser<'a> { let prev_span = self.prev_token.span.shrink_to_lo(); let snapshot = self.create_snapshot_for_diagnostic(); self.bump(); - let res = self.parse_ty(); - if res.is_ok() && self.token == token::Eq { + if self.parse_ty().is_ok() && self.token == token::Eq { err.span_suggestion_verbose( prev_span, "you might have meant to introduce a new binding", From a7fc32ceaf4708a26a992f61c4ac4ead1555c8eb Mon Sep 17 00:00:00 2001 From: yukang Date: Mon, 8 May 2023 11:16:17 +0800 Subject: [PATCH 3/5] fix ice in suggesting --- .../rustc_parse/src/parser/diagnostics.rs | 21 ++++++++++++------- tests/ui/type/missing-let-in-binding-3.rs | 5 +++++ tests/ui/type/missing-let-in-binding-3.stderr | 10 +++++++++ 3 files changed, 29 insertions(+), 7 deletions(-) create mode 100644 tests/ui/type/missing-let-in-binding-3.rs create mode 100644 tests/ui/type/missing-let-in-binding-3.stderr diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index bd0ea50b4d8c1..456c6243bbbb7 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -405,13 +405,20 @@ impl<'a> Parser<'a> { let prev_span = self.prev_token.span.shrink_to_lo(); let snapshot = self.create_snapshot_for_diagnostic(); self.bump(); - if self.parse_ty().is_ok() && self.token == token::Eq { - err.span_suggestion_verbose( - prev_span, - "you might have meant to introduce a new binding", - "let ".to_string(), - Applicability::MaybeIncorrect, - ); + match self.parse_ty() { + Ok(_) => { + if self.token == token::Eq { + err.span_suggestion_verbose( + prev_span, + "you might have meant to introduce a new binding", + "let ".to_string(), + Applicability::MaybeIncorrect, + ); + } + } + Err(err) => { + err.cancel(); + } } self.restore_snapshot(snapshot); } diff --git a/tests/ui/type/missing-let-in-binding-3.rs b/tests/ui/type/missing-let-in-binding-3.rs new file mode 100644 index 0000000000000..d56b1393336b1 --- /dev/null +++ b/tests/ui/type/missing-let-in-binding-3.rs @@ -0,0 +1,5 @@ +struct A { + : :u8, //~ ERROR expected identifier, found `:` +} + +fn main() {} diff --git a/tests/ui/type/missing-let-in-binding-3.stderr b/tests/ui/type/missing-let-in-binding-3.stderr new file mode 100644 index 0000000000000..ca828ce37eb7a --- /dev/null +++ b/tests/ui/type/missing-let-in-binding-3.stderr @@ -0,0 +1,10 @@ +error: expected identifier, found `:` + --> $DIR/missing-let-in-binding-3.rs:2:5 + | +LL | struct A { + | - while parsing this struct +LL | : :u8, + | ^ expected identifier + +error: aborting due to previous error + From 5e94b5faf1f0812207a9128e2754bd146210b16e Mon Sep 17 00:00:00 2001 From: yukang Date: Mon, 8 May 2023 14:52:52 +0800 Subject: [PATCH 4/5] code refactor and fix wrong suggestion --- .../rustc_parse/src/parser/diagnostics.rs | 52 +++++++++++-------- compiler/rustc_parse/src/parser/stmt.rs | 8 ++- tests/ui/type/missing-let-in-binding-4.rs | 5 ++ tests/ui/type/missing-let-in-binding-4.stderr | 10 ++++ 4 files changed, 51 insertions(+), 24 deletions(-) create mode 100644 tests/ui/type/missing-let-in-binding-4.rs create mode 100644 tests/ui/type/missing-let-in-binding-4.stderr diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 456c6243bbbb7..b0822fe795650 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -399,29 +399,6 @@ impl<'a> Parser<'a> { } } } - // we suggest add the missing `let` before the identifier - // `a: Ty = 1` -> `let a: Ty = 1` - if self.token == token::Colon { - let prev_span = self.prev_token.span.shrink_to_lo(); - let snapshot = self.create_snapshot_for_diagnostic(); - self.bump(); - match self.parse_ty() { - Ok(_) => { - if self.token == token::Eq { - err.span_suggestion_verbose( - prev_span, - "you might have meant to introduce a new binding", - "let ".to_string(), - Applicability::MaybeIncorrect, - ); - } - } - Err(err) => { - err.cancel(); - } - } - self.restore_snapshot(snapshot); - } if let Some(recovered_ident) = recovered_ident && recover { err.emit(); @@ -1029,6 +1006,35 @@ impl<'a> Parser<'a> { Err(e) } + /// Suggest add the missing `let` before the identifier in stmt + /// `a: Ty = 1` -> `let a: Ty = 1` + pub(super) fn suggest_add_missing_let_for_stmt( + &mut self, + err: &mut DiagnosticBuilder<'a, ErrorGuaranteed>, + ) { + if self.token == token::Colon { + let prev_span = self.prev_token.span.shrink_to_lo(); + let snapshot = self.create_snapshot_for_diagnostic(); + self.bump(); + match self.parse_ty() { + Ok(_) => { + if self.token == token::Eq { + err.span_suggestion_verbose( + prev_span, + "you might have meant to introduce a new binding", + "let ".to_string(), + Applicability::MaybeIncorrect, + ); + } + } + Err(e) => { + e.cancel(); + } + } + self.restore_snapshot(snapshot); + } + } + /// Check to see if a pair of chained operators looks like an attempt at chained comparison, /// e.g. `1 < x <= 3`. If so, suggest either splitting the comparison into two, or /// parenthesising the leftmost comparison. diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index ab04219b1772b..c35c32f272a93 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -99,7 +99,13 @@ impl<'a> Parser<'a> { ForceCollect::Yes => { self.collect_tokens_no_attrs(|this| this.parse_stmt_path_start(lo, attrs))? } - ForceCollect::No => self.parse_stmt_path_start(lo, attrs)?, + ForceCollect::No => match self.parse_stmt_path_start(lo, attrs) { + Ok(stmt) => stmt, + Err(mut err) => { + self.suggest_add_missing_let_for_stmt(&mut err); + return Err(err); + } + }, } } else if let Some(item) = self.parse_item_common( attrs.clone(), diff --git a/tests/ui/type/missing-let-in-binding-4.rs b/tests/ui/type/missing-let-in-binding-4.rs new file mode 100644 index 0000000000000..879a6fedcd677 --- /dev/null +++ b/tests/ui/type/missing-let-in-binding-4.rs @@ -0,0 +1,5 @@ +struct A { + : u8 =, //~ ERROR expected identifier, found `:` +} + +fn main() {} diff --git a/tests/ui/type/missing-let-in-binding-4.stderr b/tests/ui/type/missing-let-in-binding-4.stderr new file mode 100644 index 0000000000000..e6f173a665870 --- /dev/null +++ b/tests/ui/type/missing-let-in-binding-4.stderr @@ -0,0 +1,10 @@ +error: expected identifier, found `:` + --> $DIR/missing-let-in-binding-4.rs:2:5 + | +LL | struct A { + | - while parsing this struct +LL | : u8 =, + | ^ expected identifier + +error: aborting due to previous error + From 4d219d066626c49fdec3d8fa143a94c81150b633 Mon Sep 17 00:00:00 2001 From: yukang Date: Tue, 9 May 2023 11:51:04 +0800 Subject: [PATCH 5/5] move sugg to derive session diagnostic --- compiler/rustc_parse/messages.ftl | 1 + compiler/rustc_parse/src/errors.rs | 12 ++++++++++++ compiler/rustc_parse/src/parser/diagnostics.rs | 14 +++++--------- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_parse/messages.ftl b/compiler/rustc_parse/messages.ftl index cd296dca133f5..a01e6f5ba113c 100644 --- a/compiler/rustc_parse/messages.ftl +++ b/compiler/rustc_parse/messages.ftl @@ -339,6 +339,7 @@ parse_expected_identifier = expected identifier parse_sugg_escape_identifier = escape `{$ident_name}` to use it as an identifier parse_sugg_remove_comma = remove this comma +parse_sugg_add_let_for_stmt = you might have meant to introduce a new binding parse_expected_semi_found_reserved_identifier_str = expected `;`, found reserved identifier `{$token}` parse_expected_semi_found_keyword_str = expected `;`, found keyword `{$token}` diff --git a/compiler/rustc_parse/src/errors.rs b/compiler/rustc_parse/src/errors.rs index 010a13aefa420..4dffdae1dc9bf 100644 --- a/compiler/rustc_parse/src/errors.rs +++ b/compiler/rustc_parse/src/errors.rs @@ -906,6 +906,18 @@ pub(crate) struct SuggRemoveComma { pub span: Span, } +#[derive(Subdiagnostic)] +#[suggestion( + parse_sugg_add_let_for_stmt, + style = "verbose", + applicability = "maybe-incorrect", + code = "let " +)] +pub(crate) struct SuggAddMissingLetStmt { + #[primary_span] + pub span: Span, +} + #[derive(Subdiagnostic)] pub(crate) enum ExpectedIdentifierFound { #[label(parse_expected_identifier_found_reserved_identifier)] diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index b0822fe795650..3002f23da75cc 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -13,7 +13,7 @@ use crate::errors::{ IncorrectUseOfAwait, ParenthesesInForHead, ParenthesesInForHeadSugg, PatternMethodParamWithoutBody, QuestionMarkInType, QuestionMarkInTypeSugg, SelfParamNotFirst, StructLiteralBodyWithoutPath, StructLiteralBodyWithoutPathSugg, StructLiteralNeedingParens, - StructLiteralNeedingParensSugg, SuggEscapeIdentifier, SuggRemoveComma, + StructLiteralNeedingParensSugg, SuggAddMissingLetStmt, SuggEscapeIdentifier, SuggRemoveComma, UnexpectedConstInGenericParam, UnexpectedConstParamDeclaration, UnexpectedConstParamDeclarationSugg, UnmatchedAngleBrackets, UseEqInstead, }; @@ -32,8 +32,8 @@ use rustc_ast::{ use rustc_ast_pretty::pprust; use rustc_data_structures::fx::FxHashSet; use rustc_errors::{ - pluralize, Applicability, Diagnostic, DiagnosticBuilder, DiagnosticMessage, ErrorGuaranteed, - FatalError, Handler, IntoDiagnostic, MultiSpan, PResult, + pluralize, AddToDiagnostic, Applicability, Diagnostic, DiagnosticBuilder, DiagnosticMessage, + ErrorGuaranteed, FatalError, Handler, IntoDiagnostic, MultiSpan, PResult, }; use rustc_session::errors::ExprParenthesesNeeded; use rustc_span::source_map::Spanned; @@ -1019,12 +1019,8 @@ impl<'a> Parser<'a> { match self.parse_ty() { Ok(_) => { if self.token == token::Eq { - err.span_suggestion_verbose( - prev_span, - "you might have meant to introduce a new binding", - "let ".to_string(), - Applicability::MaybeIncorrect, - ); + let sugg = SuggAddMissingLetStmt { span: prev_span }; + sugg.add_to_diagnostic(err); } } Err(e) => {