about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMatthias Krüger <476013+matthiaskrgr@users.noreply.github.com>2025-05-16 07:19:41 +0200
committerGitHub <noreply@github.com>2025-05-16 07:19:41 +0200
commite53b9f8fdd7be3357063cc82abe9a12f9014c13f (patch)
treebdbd8bf36002dce59088cb2ee0134c2de9186048
parent953905fd9e567679e868c50a734debc872815f81 (diff)
parent1267333ef102d854cf0cefef877ba0d9adb07107 (diff)
downloadrust-e53b9f8fdd7be3357063cc82abe9a12f9014c13f.tar.gz
rust-e53b9f8fdd7be3357063cc82abe9a12f9014c13f.zip
Rollup merge of #141003 - clubby789:ternary-improve, r=compiler-errors
Improve ternary operator recovery

This
- Improves the span of the error to not point at the next token
- Where possible, we use the span of the condition to further improve the span of the error to include the cond, and suggest a maybe-incorrect fix

Currently this works on free expressions, not let statements; some more refactoring would be needed to pass the span down, which I'm not sure is worth doing.

### Old
![image](https://github.com/user-attachments/assets/5688cefc-e4ef-4135-a5ba-340ce05ae6f3)

### New
![image](https://github.com/user-attachments/assets/154f5380-e0c8-42c7-9bf8-0adb3d0433fa)
-rw-r--r--compiler/rustc_parse/messages.ftl3
-rw-r--r--compiler/rustc_parse/src/errors.rs20
-rw-r--r--compiler/rustc_parse/src/parser/diagnostics.rs34
-rw-r--r--compiler/rustc_parse/src/parser/stmt.rs7
-rw-r--r--tests/ui/parser/ternary_operator.rs6
-rw-r--r--tests/ui/parser/ternary_operator.stderr22
6 files changed, 75 insertions, 17 deletions
diff --git a/compiler/rustc_parse/messages.ftl b/compiler/rustc_parse/messages.ftl
index f88c15785d3..a6919afef12 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 766baf6f80c..31a48b22cfe 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<TernaryOperatorSuggestion>,
+    /// 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 23c8db7bca7..6277dde7c97 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<Span>,
+    ) -> 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 885a65d4de7..396ded96bde 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 c8810781b3d..08f6a4b2a24 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 e12a7ff3718..d4a633e5e55 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<u16>; 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