From 04fe8391775683e58d861f28678bf80940c91f44 Mon Sep 17 00:00:00 2001 From: Esteban Küber Date: Thu, 14 Nov 2024 20:29:24 +0000 Subject: Increase accuracy of `if` condition misparse suggestion Look at the expression that was parsed when trying to recover from a bad `if` condition to determine what was likely intended by the user beyond "maybe this was meant to be an `else` body". ``` error: expected `{`, found `map` --> $DIR/missing-dot-on-if-condition-expression-fixable.rs:4:30 | LL | for _ in [1, 2, 3].iter()map(|x| x) {} | ^^^ expected `{` | help: you might have meant to write a method call | LL | for _ in [1, 2, 3].iter().map(|x| x) {} | + ``` --- compiler/rustc_parse/src/parser/stmt.rs | 98 ++++++++++++++++++++++++++++++--- 1 file changed, 89 insertions(+), 9 deletions(-) (limited to 'compiler/rustc_parse/src/parser') diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index b7cdae3e3e1..e8877b8c541 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -475,6 +475,7 @@ impl<'a> Parser<'a> { } fn error_block_no_opening_brace_msg(&mut self, msg: Cow<'static, str>) -> Diag<'a> { + let prev = self.prev_token.span; let sp = self.token.span; let mut e = self.dcx().struct_span_err(sp, msg); let do_not_suggest_help = self.token.is_keyword(kw::In) || self.token == token::Colon; @@ -514,15 +515,94 @@ impl<'a> Parser<'a> { } else { stmt.span }; - e.multipart_suggestion( - "try placing this code inside a block", - vec![ - (stmt_span.shrink_to_lo(), "{ ".to_string()), - (stmt_span.shrink_to_hi(), " }".to_string()), - ], - // Speculative; has been misleading in the past (#46836). - Applicability::MaybeIncorrect, - ); + match (&self.token.kind, &stmt.kind) { + (token::OpenDelim(Delimiter::Brace), StmtKind::Expr(expr)) + if let ExprKind::Call(..) = expr.kind => + { + // for _ in x y() {} + e.span_suggestion_verbose( + prev.between(sp), + "you might have meant to write a method call", + ".".to_string(), + Applicability::MaybeIncorrect, + ); + } + (token::OpenDelim(Delimiter::Brace), StmtKind::Expr(expr)) + if let ExprKind::Field(..) = expr.kind => + { + // for _ in x y.z {} + e.span_suggestion_verbose( + prev.between(sp), + "you might have meant to write a field access", + ".".to_string(), + Applicability::MaybeIncorrect, + ); + } + (token::CloseDelim(Delimiter::Brace), StmtKind::Expr(expr)) + if let ExprKind::Struct(expr) = &expr.kind + && let None = expr.qself + && expr.path.segments.len() == 1 => + { + // This is specific to "mistyped `if` condition followed by empty body" + // + // for _ in x y {} + e.span_suggestion_verbose( + prev.between(sp), + "you might have meant to write a field access", + ".".to_string(), + Applicability::MaybeIncorrect, + ); + } + (token::OpenDelim(Delimiter::Brace), StmtKind::Expr(expr)) + if let ExprKind::Lit(lit) = expr.kind + && let None = lit.suffix + && let token::LitKind::Integer | token::LitKind::Float = lit.kind => + { + // for _ in x 0 {} + // for _ in x 0.0 {} + e.span_suggestion_verbose( + prev.between(sp), + format!("you might have meant to write a field access"), + ".".to_string(), + Applicability::MaybeIncorrect, + ); + } + (token::OpenDelim(Delimiter::Brace), StmtKind::Expr(expr)) + if let ExprKind::Loop(..) + | ExprKind::If(..) + | ExprKind::While(..) + | ExprKind::Match(..) + | ExprKind::ForLoop { .. } + | ExprKind::TryBlock(..) + | ExprKind::Ret(..) + | ExprKind::Closure(..) + | ExprKind::Struct(..) + | ExprKind::Try(..) = expr.kind => + { + // These are more likely to have been meant as a block body. + e.multipart_suggestion( + "try placing this code inside a block", + vec![ + (stmt_span.shrink_to_lo(), "{ ".to_string()), + (stmt_span.shrink_to_hi(), " }".to_string()), + ], + // Speculative; has been misleading in the past (#46836). + Applicability::MaybeIncorrect, + ); + } + (token::OpenDelim(Delimiter::Brace), _) => {} + (_, _) => { + e.multipart_suggestion( + "try placing this code inside a block", + vec![ + (stmt_span.shrink_to_lo(), "{ ".to_string()), + (stmt_span.shrink_to_hi(), " }".to_string()), + ], + // Speculative; has been misleading in the past (#46836). + Applicability::MaybeIncorrect, + ); + } + } } Err(e) => { self.recover_stmt_(SemiColonMode::Break, BlockMode::Ignore); -- cgit 1.4.1-3-g733a5 From 629a69f3e2728251774f825ff54cb642b38249df Mon Sep 17 00:00:00 2001 From: Esteban Küber Date: Thu, 14 Nov 2024 20:39:25 +0000 Subject: Better account for `else if` macro conditions mising an `if` If a macro statement has been parsed after `else`, suggest a missing `if`: ``` error: expected `{`, found `falsy` --> $DIR/else-no-if.rs:47:12 | LL | } else falsy! {} { | ---- ^^^^^ | | | expected an `if` or a block after this `else` | help: add an `if` if this is the condition of a chained `else if` statement | LL | } else if falsy! {} { | ++ ``` --- compiler/rustc_parse/src/parser/expr.rs | 11 ++++++++++- tests/ui/parser/else-no-if.stderr | 9 ++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) (limited to 'compiler/rustc_parse/src/parser') diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 0ac6133e828..0012db471ef 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -2683,6 +2683,13 @@ impl<'a> Parser<'a> { // ^^ // } // + // We account for macro calls that were meant as conditions as well. + // + // if ... { + // } else if macro! { foo bar } { + // ^^ + // } + // // If $cond is "statement-like" such as ExprKind::While then we // want to suggest wrapping in braces. // @@ -2693,7 +2700,9 @@ impl<'a> Parser<'a> { // } // ^ if self.check(&TokenKind::OpenDelim(Delimiter::Brace)) - && classify::expr_requires_semi_to_be_stmt(&cond) => + && (classify::expr_requires_semi_to_be_stmt(&cond) + || matches!(cond.kind, ExprKind::MacCall(..))) + => { self.dcx().emit_err(errors::ExpectedElseBlock { first_tok_span, diff --git a/tests/ui/parser/else-no-if.stderr b/tests/ui/parser/else-no-if.stderr index c86791ec896..9954505e7c8 100644 --- a/tests/ui/parser/else-no-if.stderr +++ b/tests/ui/parser/else-no-if.stderr @@ -74,7 +74,14 @@ error: expected `{`, found `falsy` --> $DIR/else-no-if.rs:47:12 | LL | } else falsy! {} { - | ^^^^^ expected `{` + | ---- ^^^^^ + | | + | expected an `if` or a block after this `else` + | +help: add an `if` if this is the condition of a chained `else if` statement + | +LL | } else if falsy! {} { + | ++ error: expected `{`, found `falsy` --> $DIR/else-no-if.rs:54:12 -- cgit 1.4.1-3-g733a5 From c09c73b99622905e8df074b81377c9ca82467dce Mon Sep 17 00:00:00 2001 From: Esteban Küber Date: Sat, 16 Nov 2024 00:22:54 +0000 Subject: Reword suggestion message --- compiler/rustc_parse/src/parser/stmt.rs | 4 ++-- tests/ui/let-else/let-else-if.stderr | 2 +- tests/ui/lint/issue-104392.stderr | 2 +- tests/ui/missing/missing-block-hint.stderr | 2 +- tests/ui/parser/bad-if-statements.stderr | 4 ++-- tests/ui/parser/block-no-opening-brace.stderr | 6 +++--- tests/ui/parser/closure-return-syntax.stderr | 2 +- tests/ui/parser/else-no-if.stderr | 8 ++++---- tests/ui/parser/label-after-block-like.stderr | 14 +++++++------- tests/ui/unsafe/unsafe-block-without-braces.stderr | 2 +- 10 files changed, 23 insertions(+), 23 deletions(-) (limited to 'compiler/rustc_parse/src/parser') diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index e8877b8c541..4ab056a4e13 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -581,7 +581,7 @@ impl<'a> Parser<'a> { { // These are more likely to have been meant as a block body. e.multipart_suggestion( - "try placing this code inside a block", + "you might have meant to write this as part of a block", vec![ (stmt_span.shrink_to_lo(), "{ ".to_string()), (stmt_span.shrink_to_hi(), " }".to_string()), @@ -593,7 +593,7 @@ impl<'a> Parser<'a> { (token::OpenDelim(Delimiter::Brace), _) => {} (_, _) => { e.multipart_suggestion( - "try placing this code inside a block", + "you might have meant to write this as part of a block", vec![ (stmt_span.shrink_to_lo(), "{ ".to_string()), (stmt_span.shrink_to_hi(), " }".to_string()), diff --git a/tests/ui/let-else/let-else-if.stderr b/tests/ui/let-else/let-else-if.stderr index 7e2215c8c05..ad36b423150 100644 --- a/tests/ui/let-else/let-else-if.stderr +++ b/tests/ui/let-else/let-else-if.stderr @@ -4,7 +4,7 @@ error: conditional `else if` is not supported for `let...else` LL | let Some(_) = Some(()) else if true { | ^^ expected `{` | -help: try placing this code inside a block +help: you might have meant to write this as part of a block | LL ~ let Some(_) = Some(()) else { if true { LL | diff --git a/tests/ui/lint/issue-104392.stderr b/tests/ui/lint/issue-104392.stderr index 8e466439ae6..4d8d8c56d41 100644 --- a/tests/ui/lint/issue-104392.stderr +++ b/tests/ui/lint/issue-104392.stderr @@ -6,7 +6,7 @@ LL | { unsafe 92 } | | | while parsing this `unsafe` expression | -help: try placing this code inside a block +help: you might have meant to write this as part of a block | LL | { unsafe { 92 } } | + + diff --git a/tests/ui/missing/missing-block-hint.stderr b/tests/ui/missing/missing-block-hint.stderr index 18719289abd..7a08d70d0ce 100644 --- a/tests/ui/missing/missing-block-hint.stderr +++ b/tests/ui/missing/missing-block-hint.stderr @@ -25,7 +25,7 @@ note: the `if` expression is missing a block after this condition | LL | if (foo) | ^^^^^ -help: try placing this code inside a block +help: you might have meant to write this as part of a block | LL | { bar; } | + + diff --git a/tests/ui/parser/bad-if-statements.stderr b/tests/ui/parser/bad-if-statements.stderr index ee839db6455..320b1176993 100644 --- a/tests/ui/parser/bad-if-statements.stderr +++ b/tests/ui/parser/bad-if-statements.stderr @@ -29,7 +29,7 @@ note: the `if` expression is missing a block after this condition | LL | if true x | ^^^^ -help: try placing this code inside a block +help: you might have meant to write this as part of a block | LL | if true { x } | + + @@ -65,7 +65,7 @@ note: the `if` expression is missing a block after this condition | LL | if true x else {} | ^^^^ -help: try placing this code inside a block +help: you might have meant to write this as part of a block | LL | if true { x } else {} | + + diff --git a/tests/ui/parser/block-no-opening-brace.stderr b/tests/ui/parser/block-no-opening-brace.stderr index 83360944ed5..b65de4eac3f 100644 --- a/tests/ui/parser/block-no-opening-brace.stderr +++ b/tests/ui/parser/block-no-opening-brace.stderr @@ -6,7 +6,7 @@ LL | loop LL | let x = 0; | ^^^ expected `{` | -help: try placing this code inside a block +help: you might have meant to write this as part of a block | LL | { let x = 0; } | + + @@ -21,7 +21,7 @@ LL | while true LL | let x = 0; | ^^^ expected `{` | -help: try placing this code inside a block +help: you might have meant to write this as part of a block | LL | { let x = 0; } | + + @@ -32,7 +32,7 @@ error: expected `{`, found keyword `let` LL | let x = 0; | ^^^ expected `{` | -help: try placing this code inside a block +help: you might have meant to write this as part of a block | LL | { let x = 0; } | + + diff --git a/tests/ui/parser/closure-return-syntax.stderr b/tests/ui/parser/closure-return-syntax.stderr index eb8428854af..aacc31ed871 100644 --- a/tests/ui/parser/closure-return-syntax.stderr +++ b/tests/ui/parser/closure-return-syntax.stderr @@ -4,7 +4,7 @@ error: expected `{`, found `22` LL | let x = || -> i32 22; | ^^ expected `{` | -help: try placing this code inside a block +help: you might have meant to write this as part of a block | LL | let x = || -> i32 { 22 }; | + + diff --git a/tests/ui/parser/else-no-if.stderr b/tests/ui/parser/else-no-if.stderr index 9954505e7c8..eec64b0f4bc 100644 --- a/tests/ui/parser/else-no-if.stderr +++ b/tests/ui/parser/else-no-if.stderr @@ -30,7 +30,7 @@ error: expected `{`, found `falsy` LL | } else falsy(); | ^^^^^ expected `{` | -help: try placing this code inside a block +help: you might have meant to write this as part of a block | LL | } else { falsy() }; | + + @@ -41,7 +41,7 @@ error: expected `{`, found keyword `loop` LL | } else loop{} | ^^^^ expected `{` | -help: try placing this code inside a block +help: you might have meant to write this as part of a block | LL | } else { loop{} } | + + @@ -65,7 +65,7 @@ error: expected `{`, found `falsy` LL | } else falsy!(); | ^^^^^ expected `{` | -help: try placing this code inside a block +help: you might have meant to write this as part of a block | LL | } else { falsy!() }; | + + @@ -89,7 +89,7 @@ error: expected `{`, found `falsy` LL | } else falsy! {}; | ^^^^^ expected `{` | -help: try placing this code inside a block +help: you might have meant to write this as part of a block | LL | } else { falsy! {} }; | + + diff --git a/tests/ui/parser/label-after-block-like.stderr b/tests/ui/parser/label-after-block-like.stderr index be8c679d8ce..4dea225e3f3 100644 --- a/tests/ui/parser/label-after-block-like.stderr +++ b/tests/ui/parser/label-after-block-like.stderr @@ -23,7 +23,7 @@ note: the `if` expression is missing a block after this condition | LL | if let () = () 'a {} | ^^^^^^^^^^^ -help: try placing this code inside a block +help: you might have meant to write this as part of a block | LL | if let () = () { 'a {} } | + + @@ -53,7 +53,7 @@ note: the `if` expression is missing a block after this condition | LL | if true 'a {} | ^^^^ -help: try placing this code inside a block +help: you might have meant to write this as part of a block | LL | if true { 'a {} } | + + @@ -80,7 +80,7 @@ LL | loop 'a {} | | | while parsing this `loop` expression | -help: try placing this code inside a block +help: you might have meant to write this as part of a block | LL | loop { 'a {} } | + + @@ -108,7 +108,7 @@ LL | while true 'a {} | | this `while` condition successfully parsed | while parsing the body of this `while` expression | -help: try placing this code inside a block +help: you might have meant to write this as part of a block | LL | while true { 'a {} } | + + @@ -136,7 +136,7 @@ LL | while let () = () 'a {} | | this `while` condition successfully parsed | while parsing the body of this `while` expression | -help: try placing this code inside a block +help: you might have meant to write this as part of a block | LL | while let () = () { 'a {} } | + + @@ -161,7 +161,7 @@ error: expected `{`, found `'a` LL | for _ in 0..0 'a {} | ^^ expected `{` | -help: try placing this code inside a block +help: you might have meant to write this as part of a block | LL | for _ in 0..0 { 'a {} } | + + @@ -188,7 +188,7 @@ LL | unsafe 'a {} | | | while parsing this `unsafe` expression | -help: try placing this code inside a block +help: you might have meant to write this as part of a block | LL | unsafe { 'a {} } | + + diff --git a/tests/ui/unsafe/unsafe-block-without-braces.stderr b/tests/ui/unsafe/unsafe-block-without-braces.stderr index d29e49d73a6..3d8234c15e7 100644 --- a/tests/ui/unsafe/unsafe-block-without-braces.stderr +++ b/tests/ui/unsafe/unsafe-block-without-braces.stderr @@ -6,7 +6,7 @@ LL | unsafe //{ LL | std::mem::transmute::(1.0); | ^^^ expected `{` | -help: try placing this code inside a block +help: you might have meant to write this as part of a block | LL | { std::mem::transmute::(1.0); } | + + -- cgit 1.4.1-3-g733a5 From 6913194b8ea47ebe8e65b627e83f029d0968d8f8 Mon Sep 17 00:00:00 2001 From: Esteban Küber Date: Sat, 16 Nov 2024 20:02:27 +0000 Subject: review comment: move logic to new method --- compiler/rustc_parse/src/parser/stmt.rs | 191 +++++++++++++++++--------------- 1 file changed, 103 insertions(+), 88 deletions(-) (limited to 'compiler/rustc_parse/src/parser') diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index 4ab056a4e13..190cd9ed061 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -515,94 +515,12 @@ impl<'a> Parser<'a> { } else { stmt.span }; - match (&self.token.kind, &stmt.kind) { - (token::OpenDelim(Delimiter::Brace), StmtKind::Expr(expr)) - if let ExprKind::Call(..) = expr.kind => - { - // for _ in x y() {} - e.span_suggestion_verbose( - prev.between(sp), - "you might have meant to write a method call", - ".".to_string(), - Applicability::MaybeIncorrect, - ); - } - (token::OpenDelim(Delimiter::Brace), StmtKind::Expr(expr)) - if let ExprKind::Field(..) = expr.kind => - { - // for _ in x y.z {} - e.span_suggestion_verbose( - prev.between(sp), - "you might have meant to write a field access", - ".".to_string(), - Applicability::MaybeIncorrect, - ); - } - (token::CloseDelim(Delimiter::Brace), StmtKind::Expr(expr)) - if let ExprKind::Struct(expr) = &expr.kind - && let None = expr.qself - && expr.path.segments.len() == 1 => - { - // This is specific to "mistyped `if` condition followed by empty body" - // - // for _ in x y {} - e.span_suggestion_verbose( - prev.between(sp), - "you might have meant to write a field access", - ".".to_string(), - Applicability::MaybeIncorrect, - ); - } - (token::OpenDelim(Delimiter::Brace), StmtKind::Expr(expr)) - if let ExprKind::Lit(lit) = expr.kind - && let None = lit.suffix - && let token::LitKind::Integer | token::LitKind::Float = lit.kind => - { - // for _ in x 0 {} - // for _ in x 0.0 {} - e.span_suggestion_verbose( - prev.between(sp), - format!("you might have meant to write a field access"), - ".".to_string(), - Applicability::MaybeIncorrect, - ); - } - (token::OpenDelim(Delimiter::Brace), StmtKind::Expr(expr)) - if let ExprKind::Loop(..) - | ExprKind::If(..) - | ExprKind::While(..) - | ExprKind::Match(..) - | ExprKind::ForLoop { .. } - | ExprKind::TryBlock(..) - | ExprKind::Ret(..) - | ExprKind::Closure(..) - | ExprKind::Struct(..) - | ExprKind::Try(..) = expr.kind => - { - // These are more likely to have been meant as a block body. - e.multipart_suggestion( - "you might have meant to write this as part of a block", - vec![ - (stmt_span.shrink_to_lo(), "{ ".to_string()), - (stmt_span.shrink_to_hi(), " }".to_string()), - ], - // Speculative; has been misleading in the past (#46836). - Applicability::MaybeIncorrect, - ); - } - (token::OpenDelim(Delimiter::Brace), _) => {} - (_, _) => { - e.multipart_suggestion( - "you might have meant to write this as part of a block", - vec![ - (stmt_span.shrink_to_lo(), "{ ".to_string()), - (stmt_span.shrink_to_hi(), " }".to_string()), - ], - // Speculative; has been misleading in the past (#46836). - Applicability::MaybeIncorrect, - ); - } - } + self.suggest_fixes_misparsed_for_loop_head( + &mut e, + prev.between(sp), + stmt_span, + &stmt.kind, + ); } Err(e) => { self.recover_stmt_(SemiColonMode::Break, BlockMode::Ignore); @@ -614,6 +532,103 @@ impl<'a> Parser<'a> { e } + fn suggest_fixes_misparsed_for_loop_head( + &self, + e: &mut Diag<'_>, + between: Span, + stmt_span: Span, + stmt_kind: &StmtKind, + ) { + match (&self.token.kind, &stmt_kind) { + (token::OpenDelim(Delimiter::Brace), StmtKind::Expr(expr)) + if let ExprKind::Call(..) = expr.kind => + { + // for _ in x y() {} + e.span_suggestion_verbose( + between, + "you might have meant to write a method call", + ".".to_string(), + Applicability::MaybeIncorrect, + ); + } + (token::OpenDelim(Delimiter::Brace), StmtKind::Expr(expr)) + if let ExprKind::Field(..) = expr.kind => + { + // for _ in x y.z {} + e.span_suggestion_verbose( + between, + "you might have meant to write a field access", + ".".to_string(), + Applicability::MaybeIncorrect, + ); + } + (token::CloseDelim(Delimiter::Brace), StmtKind::Expr(expr)) + if let ExprKind::Struct(expr) = &expr.kind + && let None = expr.qself + && expr.path.segments.len() == 1 => + { + // This is specific to "mistyped `if` condition followed by empty body" + // + // for _ in x y {} + e.span_suggestion_verbose( + between, + "you might have meant to write a field access", + ".".to_string(), + Applicability::MaybeIncorrect, + ); + } + (token::OpenDelim(Delimiter::Brace), StmtKind::Expr(expr)) + if let ExprKind::Lit(lit) = expr.kind + && let None = lit.suffix + && let token::LitKind::Integer | token::LitKind::Float = lit.kind => + { + // for _ in x 0 {} + // for _ in x 0.0 {} + e.span_suggestion_verbose( + between, + format!("you might have meant to write a field access"), + ".".to_string(), + Applicability::MaybeIncorrect, + ); + } + (token::OpenDelim(Delimiter::Brace), StmtKind::Expr(expr)) + if let ExprKind::Loop(..) + | ExprKind::If(..) + | ExprKind::While(..) + | ExprKind::Match(..) + | ExprKind::ForLoop { .. } + | ExprKind::TryBlock(..) + | ExprKind::Ret(..) + | ExprKind::Closure(..) + | ExprKind::Struct(..) + | ExprKind::Try(..) = expr.kind => + { + // These are more likely to have been meant as a block body. + e.multipart_suggestion( + "you might have meant to write this as part of a block", + vec![ + (stmt_span.shrink_to_lo(), "{ ".to_string()), + (stmt_span.shrink_to_hi(), " }".to_string()), + ], + // Speculative; has been misleading in the past (#46836). + Applicability::MaybeIncorrect, + ); + } + (token::OpenDelim(Delimiter::Brace), _) => {} + (_, _) => { + e.multipart_suggestion( + "you might have meant to write this as part of a block", + vec![ + (stmt_span.shrink_to_lo(), "{ ".to_string()), + (stmt_span.shrink_to_hi(), " }".to_string()), + ], + // Speculative; has been misleading in the past (#46836). + Applicability::MaybeIncorrect, + ); + } + } + } + fn error_block_no_opening_brace(&mut self) -> PResult<'a, T> { let tok = super::token_descr(&self.token); let msg = format!("expected `{{`, found {tok}"); -- cgit 1.4.1-3-g733a5