Parse expression after `else` as a condition if followed by `{` #97298

compiler-errors · 2022-05-22T23:07:59Z

Fixes #49361.

Two things:

This wording needs help. I can never find a natural/intuitive phrasing when I write diagnostics 😅
Do we even want to show the "wrap in braces" case? I would assume most of the time the "add an if" case is the right one.

rust-highfive · 2022-05-22T23:08:02Z

r? @davidtwco

(rust-highfive has picked a reviewer for you, use r? to override)

davidtwco

One suggestion, otherwise looks good to me.

compiler/rustc_parse/src/parser/expr.rs

src/test/ui/parser/else-no-if.stderr

compiler-errors · 2022-05-24T04:10:32Z

@rustbot ready

davidtwco · 2022-05-24T07:31:28Z

@bors r+

bors · 2022-05-24T07:31:29Z

📌 Commit 9be37b2 has been approved by davidtwco

…, r=davidtwco Parse expression after `else` as a condition if followed by `{` Fixes rust-lang#49361. Two things: 1. This wording needs help. I can never find a natural/intuitive phrasing when I write diagnostics 😅 2. Do we even want to show the "wrap in braces" case? I would assume most of the time the "add an `if`" case is the right one.

Rollup of 4 pull requests Successful merges: - rust-lang#97288 (Lifetime variance fixes for rustdoc) - rust-lang#97298 (Parse expression after `else` as a condition if followed by `{`) - rust-lang#97308 (Stabilize `cell_filter_map`) - rust-lang#97321 (explain how to turn integers into fn ptrs) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

estebank · 2022-05-24T20:42:07Z

compiler/rustc_parse/src/parser/expr.rs

+                Ok(cond)
+                // If it's not a free-standing expression, and is followed by a block,
+                // then it's very likely the condition to an `else if`.
+                    if self.check(&TokenKind::OpenDelim(Delimiter::Brace))
+                        && classify::expr_requires_semi_to_be_stmt(&cond) =>
+                {


Given all the checks you have here...

estebank · 2022-05-24T20:42:22Z

compiler/rustc_parse/src/parser/expr.rs

+                        ).multipart_suggestion(
+                            "... otherwise, place this expression inside of a block if it is not an `if` condition",
+                            vec![
+                                (cond.span.shrink_to_lo(), "{ ".to_string()),
+                                (cond.span.shrink_to_hi(), " }".to_string()),
+                            ],
+                            Applicability::MaybeIncorrect,
+                        )


...I would have likely skipped this suggestion :)

neoeinstein · 2022-05-25T00:31:47Z

compiler/rustc_parse/src/parser/expr.rs

+                        .span_label(else_span, "expected an `if` or a block after this `else`")
+                        .span_suggestion(
+                            cond.span.shrink_to_lo(),
+                            "add an `if` if this is the condition to an chained `if` statement after the `else`",


Suggested change

"add an `if` if this is the condition to an chained `if` statement after the `else`",

"add an `if` if this is the condition to a chained `if` statement after the `else`",

#97370 addresses this suggestion and @estebank's above.

…an-DPC Minor improvement on else-no-if diagnostic Don't suggest wrapping in block since it's highly likely to be a missing `if` after `else`. Also rework message a bit (open to further suggestions). cc: rust-lang#97298 (comment) r? `@estebank`

rust-highfive assigned davidtwco May 22, 2022

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 22, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 22, 2022

davidtwco requested changes May 23, 2022

View reviewed changes

compiler/rustc_parse/src/parser/expr.rs Outdated Show resolved Hide resolved

davidtwco added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 23, 2022

fmease reviewed May 23, 2022

View reviewed changes

src/test/ui/parser/else-no-if.stderr Outdated Show resolved Hide resolved

src/test/ui/parser/else-no-if.stderr Outdated Show resolved Hide resolved

Parse expression after else as a condition if followed by {

9be37b2

compiler-errors force-pushed the if-else-stmt-braces branch from 0246585 to 9be37b2 Compare May 24, 2022 04:10

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 24, 2022

davidtwco approved these changes May 24, 2022

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 24, 2022

Dylan-DPC mentioned this pull request May 24, 2022

Rollup of 5 pull requests #97347

Closed

Dylan-DPC mentioned this pull request May 24, 2022

Rollup of 4 pull requests #97349

Closed

Dylan-DPC mentioned this pull request May 24, 2022

Rollup of 4 pull requests #97356

Merged

bors merged commit 0531521 into rust-lang:master May 24, 2022

rustbot added this to the 1.63.0 milestone May 24, 2022

estebank reviewed May 24, 2022

View reviewed changes

compiler-errors mentioned this pull request May 24, 2022

Minor improvement on else-no-if diagnostic #97370

Merged

neoeinstein reviewed May 25, 2022

View reviewed changes

compiler-errors deleted the if-else-stmt-braces branch August 11, 2023 20:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Parse expression after `else` as a condition if followed by `{` #97298

Parse expression after `else` as a condition if followed by `{` #97298

Uh oh!

compiler-errors commented May 22, 2022 •

edited

Loading

Uh oh!

rust-highfive commented May 22, 2022

Uh oh!

davidtwco left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

compiler-errors commented May 24, 2022

Uh oh!

davidtwco commented May 24, 2022

Uh oh!

bors commented May 24, 2022

Uh oh!

estebank May 24, 2022

Uh oh!

estebank May 24, 2022

Uh oh!

neoeinstein May 25, 2022

Uh oh!

compiler-errors May 25, 2022

Uh oh!

Uh oh!

	"add an `if` if this is the condition to an chained `if` statement after the `else`",
	"add an `if` if this is the condition to a chained `if` statement after the `else`",

Parse expression after else as a condition if followed by { #97298

Parse expression after else as a condition if followed by { #97298

Uh oh!

Conversation

compiler-errors commented May 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented May 22, 2022

Uh oh!

davidtwco left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

compiler-errors commented May 24, 2022

Uh oh!

davidtwco commented May 24, 2022

Uh oh!

bors commented May 24, 2022

Uh oh!

estebank May 24, 2022

Choose a reason for hiding this comment

Uh oh!

estebank May 24, 2022

Choose a reason for hiding this comment

Uh oh!

neoeinstein May 25, 2022

Choose a reason for hiding this comment

Uh oh!

compiler-errors May 25, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Parse expression after `else` as a condition if followed by `{` #97298

Parse expression after `else` as a condition if followed by `{` #97298

compiler-errors commented May 22, 2022 •

edited

Loading