From 1267333ef102d854cf0cefef877ba0d9adb07107 Mon Sep 17 00:00:00 2001 From: Jamie Date: Wed, 14 May 2025 13:32:41 +0100 Subject: [PATCH] Improve ternary operator recovery --- compiler/rustc_parse/messages.ftl | 3 +- compiler/rustc_parse/src/errors.rs | 20 ++++++++++- .../rustc_parse/src/parser/diagnostics.rs | 34 ++++++++++++++----- compiler/rustc_parse/src/parser/stmt.rs | 7 +++- tests/ui/parser/ternary_operator.rs | 6 ++++ tests/ui/parser/ternary_operator.stderr | 22 +++++++++--- 6 files changed, 75 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_parse/messages.ftl b/compiler/rustc_parse/messages.ftl index f88c15785d336..a6919afef12cf 100644 --- a/compiler/rustc_parse/messages.ftl +++ b/compiler/rustc_parse/messages.ftl @@ -815,7 +815,6 @@ parse_switch_ref_box_order = switch the order of `ref` and `box` .suggestion = swap them parse_ternary_operator = Rust has no ternary operator - .help = use an `if-else` expression instead parse_tilde_is_not_unary_operator = `~` cannot be used as a unary operator .suggestion = use `!` to perform bitwise not @@ -963,6 +962,8 @@ parse_use_empty_block_not_semi = expected { "`{}`" }, found `;` parse_use_eq_instead = unexpected `==` .suggestion = try using `=` instead +parse_use_if_else = use an `if-else` expression instead + parse_use_let_not_auto = write `let` instead of `auto` to introduce a new variable parse_use_let_not_var = write `let` instead of `var` to introduce a new variable diff --git a/compiler/rustc_parse/src/errors.rs b/compiler/rustc_parse/src/errors.rs index 766baf6f80c75..31a48b22cfee1 100644 --- a/compiler/rustc_parse/src/errors.rs +++ b/compiler/rustc_parse/src/errors.rs @@ -436,10 +436,28 @@ pub(crate) enum IfExpressionMissingThenBlockSub { #[derive(Diagnostic)] #[diag(parse_ternary_operator)] -#[help] pub(crate) struct TernaryOperator { #[primary_span] pub span: Span, + /// If we have a span for the condition expression, suggest the if/else + #[subdiagnostic] + pub sugg: Option, + /// Otherwise, just print the suggestion message + #[help(parse_use_if_else)] + pub no_sugg: bool, +} + +#[derive(Subdiagnostic, Copy, Clone)] +#[multipart_suggestion(parse_use_if_else, applicability = "maybe-incorrect", style = "verbose")] +pub(crate) struct TernaryOperatorSuggestion { + #[suggestion_part(code = "if ")] + pub before_cond: Span, + #[suggestion_part(code = "{{")] + pub question: Span, + #[suggestion_part(code = "}} else {{")] + pub colon: Span, + #[suggestion_part(code = " }}")] + pub end: Span, } #[derive(Subdiagnostic)] diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 23c8db7bca784..6277dde7c974c 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -41,8 +41,9 @@ use crate::errors::{ IncorrectSemicolon, IncorrectUseOfAwait, IncorrectUseOfUse, PatternMethodParamWithoutBody, QuestionMarkInType, QuestionMarkInTypeSugg, SelfParamNotFirst, StructLiteralBodyWithoutPath, StructLiteralBodyWithoutPathSugg, SuggAddMissingLetStmt, SuggEscapeIdentifier, SuggRemoveComma, - TernaryOperator, UnexpectedConstInGenericParam, UnexpectedConstParamDeclaration, - UnexpectedConstParamDeclarationSugg, UnmatchedAngleBrackets, UseEqInstead, WrapType, + TernaryOperator, TernaryOperatorSuggestion, UnexpectedConstInGenericParam, + UnexpectedConstParamDeclaration, UnexpectedConstParamDeclarationSugg, UnmatchedAngleBrackets, + UseEqInstead, WrapType, }; use crate::parser::attr::InnerAttrPolicy; use crate::{exp, fluent_generated as fluent}; @@ -497,7 +498,7 @@ impl<'a> Parser<'a> { // If the user is trying to write a ternary expression, recover it and // return an Err to prevent a cascade of irrelevant diagnostics. if self.prev_token == token::Question - && let Err(e) = self.maybe_recover_from_ternary_operator() + && let Err(e) = self.maybe_recover_from_ternary_operator(None) { return Err(e); } @@ -1602,12 +1603,18 @@ impl<'a> Parser<'a> { /// Rust has no ternary operator (`cond ? then : else`). Parse it and try /// to recover from it if `then` and `else` are valid expressions. Returns /// an err if this appears to be a ternary expression. - pub(super) fn maybe_recover_from_ternary_operator(&mut self) -> PResult<'a, ()> { + /// If we have the span of the condition, we can provide a better error span + /// and code suggestion. + pub(super) fn maybe_recover_from_ternary_operator( + &mut self, + cond: Option, + ) -> PResult<'a, ()> { if self.prev_token != token::Question { return PResult::Ok(()); } - let lo = self.prev_token.span.lo(); + let question = self.prev_token.span; + let lo = cond.unwrap_or(question).lo(); let snapshot = self.create_snapshot_for_diagnostic(); if match self.parse_expr() { @@ -1620,11 +1627,20 @@ impl<'a> Parser<'a> { } } { if self.eat_noexpect(&token::Colon) { + let colon = self.prev_token.span; match self.parse_expr() { - Ok(_) => { - return Err(self - .dcx() - .create_err(TernaryOperator { span: self.token.span.with_lo(lo) })); + Ok(expr) => { + let sugg = cond.map(|cond| TernaryOperatorSuggestion { + before_cond: cond.shrink_to_lo(), + question, + colon, + end: expr.span.shrink_to_hi(), + }); + return Err(self.dcx().create_err(TernaryOperator { + span: self.prev_token.span.with_lo(lo), + sugg, + no_sugg: sugg.is_none(), + })); } Err(err) => { err.cancel(); diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index 885a65d4de7a1..396ded96bde13 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -879,7 +879,12 @@ impl<'a> Parser<'a> { { // Just check for errors and recover; do not eat semicolon yet. - let expect_result = self.expect_one_of(&[], &[exp!(Semi), exp!(CloseBrace)]); + let expect_result = + if let Err(e) = self.maybe_recover_from_ternary_operator(Some(expr.span)) { + Err(e) + } else { + self.expect_one_of(&[], &[exp!(Semi), exp!(CloseBrace)]) + }; // Try to both emit a better diagnostic, and avoid further errors by replacing // the `expr` with `ExprKind::Err`. diff --git a/tests/ui/parser/ternary_operator.rs b/tests/ui/parser/ternary_operator.rs index c8810781b3d84..08f6a4b2a244f 100644 --- a/tests/ui/parser/ternary_operator.rs +++ b/tests/ui/parser/ternary_operator.rs @@ -28,3 +28,9 @@ fn main() { //~| HELP use an `if-else` expression instead //~| ERROR expected one of `.`, `;`, `?`, `else`, or an operator, found `:` } + +fn expr(a: u64, b: u64) -> u64 { + a > b ? a : b + //~^ ERROR Rust has no ternary operator + //~| HELP use an `if-else` expression instead +} diff --git a/tests/ui/parser/ternary_operator.stderr b/tests/ui/parser/ternary_operator.stderr index e12a7ff3718e0..d4a633e5e5571 100644 --- a/tests/ui/parser/ternary_operator.stderr +++ b/tests/ui/parser/ternary_operator.stderr @@ -2,7 +2,7 @@ error: Rust has no ternary operator --> $DIR/ternary_operator.rs:2:19 | LL | let x = 5 > 2 ? true : false; - | ^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^ | = help: use an `if-else` expression instead @@ -10,7 +10,7 @@ error: Rust has no ternary operator --> $DIR/ternary_operator.rs:8:19 | LL | let x = 5 > 2 ? { true } : { false }; - | ^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^ | = help: use an `if-else` expression instead @@ -18,7 +18,7 @@ error: Rust has no ternary operator --> $DIR/ternary_operator.rs:14:19 | LL | let x = 5 > 2 ? f32::MAX : f32::MIN; - | ^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^ | = help: use an `if-else` expression instead @@ -38,9 +38,21 @@ error: Rust has no ternary operator --> $DIR/ternary_operator.rs:26:19 | LL | let x = 5 > 2 ? { let x = vec![]: Vec; x } : { false }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: use an `if-else` expression instead -error: aborting due to 6 previous errors +error: Rust has no ternary operator + --> $DIR/ternary_operator.rs:33:5 + | +LL | a > b ? a : b + | ^^^^^^^^^^^^^ + | +help: use an `if-else` expression instead + | +LL - a > b ? a : b +LL + if a > b { a } else { b } + | + +error: aborting due to 7 previous errors