Increase accuracy of if condition misparse suggestion
#133051
Conversation
|
r? @Nadrieril rustbot has assigned @Nadrieril. Use |
rustbot
added
S-waiting-on-review
There was a problem hiding this comment.
Thanks! These suggestions are best-effort so that looks good to me. I have one uncertainty (heh) regarding how certain the "try using xxx" diagnostics sounds, but otherwise LGTM. The wording discussions are not blocking, if you prefer the current wording that's fine with me too, then just r=me.
| { | ||
| // These are more likely to have been meant as a block body. | ||
| e.multipart_suggestion( | ||
| "try placing this code inside a block", |
There was a problem hiding this comment.
Discussion [UNCERTAINTY 1/2]: "try placing this code inside a block" -> "maybe try placing this code inside a block"? My first reaction when reading this immediately is that it feels like it sounds too certain.
There was a problem hiding this comment.
I think the "try..." wording is what we used to use back in the pre-2018 era, and had slowly tried to move away from it. Looking at this again, what do you think of "you might have meant to write this statement/expression as part of a new block" or similar?
| (token::OpenDelim(Delimiter::Brace), _) => {} | ||
| (_, _) => { | ||
| e.multipart_suggestion( | ||
| "try placing this code inside a block", |
There was a problem hiding this comment.
Discussion [UNCERTAINTY 2/2]: ditto
|
r? jieyouxu |
|
@rustbot author |
rustbot
added
S-waiting-on-author
| // Speculative; has been misleading in the past (#46836). | ||
| Applicability::MaybeIncorrect, | ||
| ); | ||
| match (&self.token.kind, &stmt.kind) { |
There was a problem hiding this comment.
That's 70 new lines of code, more than doubling the size of a nontrivial function. Maybe split parts out into appropriately named function(s)?
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) {}
| +
```
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! {} {
| ++
```
estebank
force-pushed
the
cond-misparse
branch
from
fcc3bc5
to
6913194
Compare
|
@bors r=jieyouxu |
|
🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened. |
bors
added
S-waiting-on-bors
Rollup of 6 pull requests Successful merges: - rust-lang#133029 (ABI checks: add support for some tier3 arches, warn on others.) - rust-lang#133051 (Increase accuracy of `if` condition misparse suggestion) - rust-lang#133060 (Trim whitespace in RemoveLet primary span) - rust-lang#133093 (Let chains tests) - rust-lang#133116 (stabilize const_ptr_is_null) - rust-lang#133126 (alloc: fix `String`'s doc) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#133051 - estebank:cond-misparse, r=jieyouxu Increase accuracy of `if` condition misparse suggestion Fix rust-lang#132656. 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) {} | + ``` 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! {} { | ++ ```
Fix #132656.
Look at the expression that was parsed when trying to recover from a bad
ifcondition to determine what was likely intended by the user beyond "maybe this was meant to be anelsebody".If a macro statement has been parsed after
else, suggest a missingif: