-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Suggest let
or ==
on typo'd let-chain
#118191
Conversation
r? @cjgillot (rustbot has picked a reviewer for you, use r? to override) |
CondChecker { parser: self, forbid_let_reason: None }.visit_expr(&mut cond); | ||
let mut checker = CondChecker::new(self); | ||
checker.visit_expr(&mut cond); | ||
if checker.seen_missing_let { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of maintaining global state here, why don't we just bubble up a parse error to stop compilation, or taint the AST with an ExprKind::Error
?
compiler/rustc_session/src/parse.rs
Outdated
/// we should do something that is scope-based, instead of crate-global. | ||
pub silence_resolve_errors: Lock<bool>, | ||
/// Track parse errors that suggests changing an assignment to be an equality check. | ||
pub silence_missing_comparison: Lock<Vec<Span>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one will be trickier to write in a nice way :-/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.any(|(sp, _)| sp == span)
, or maybe use a HashMap
?
This comment has been minimized.
This comment has been minimized.
Also, this PR silences type errors along with resolver errors. Can the title of the PR be updated to make this apparent? |
let
or ==
and silence resolve errors on typo'd let-chainlet
or ==
and silence resolve and type errors on typo'd let-chain
checker.visit_expr(&mut cond); | ||
if checker.seen_missing_let.is_some() { | ||
// Avoid unnecessary resolve errors as we know we'll have some. | ||
*self.sess.silence_resolve_errors.borrow_mut() = checker.seen_missing_let; | ||
} | ||
self.sess.silence_missing_comparison.borrow_mut().append(&mut checker.seen_comparison); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of having global state here, why not just return Err(..)
? That should halt parsing.
When encountering a bare assignment in a let-chain, suggest turning the assignment into a `let` expression or an equality check. ``` error: expected expression, found `let` statement --> $DIR/bad-if-let-suggestion.rs:5:8 | LL | if let x = 1 && i = 2 {} | ^^^^^^^^^ | = note: only supported directly in conditions of `if` and `while` expressions help: you might have meant to continue the let-chain | LL | if let x = 1 && let i = 2 {} | +++ help: you might have meant to compare for equality | LL | if let x = 1 && i == 2 {} | + ```
6944bff
to
55e4e3e
Compare
let
or ==
and silence resolve and type errors on typo'd let-chainlet
or ==
on typo'd let-chain
Removed the subsequent silencing logic. There are several cases of resolve errors that I want to address and it's likely better to do that in a different PR. |
@bors r+ |
@bors rollup |
…r-errors Suggest `let` or `==` on typo'd let-chain When encountering a bare assignment in a let-chain, suggest turning the assignment into a `let` expression or an equality check. ``` error: expected expression, found `let` statement --> $DIR/bad-if-let-suggestion.rs:5:8 | LL | if let x = 1 && i = 2 {} | ^^^^^^^^^ | = note: only supported directly in conditions of `if` and `while` expressions help: you might have meant to continue the let-chain | LL | if let x = 1 && let i = 2 {} | +++ help: you might have meant to compare for equality | LL | if let x = 1 && i == 2 {} | + ```
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#118157 (Add `never_patterns` feature gate) - rust-lang#118191 (Suggest `let` or `==` on typo'd let-chain) - rust-lang#118231 (also add is_empty to const raw slices) - rust-lang#118333 (Print list of missing target features when calling a function with target features outside an unsafe block) - rust-lang#118426 (ConstProp: Correctly remove const if unknown value assigned to it.) - rust-lang#118428 (rustdoc: Move `AssocItemRender` and `RenderMode` to `html::render`.) - rust-lang#118438 (Update nto-qnx.md) Failed merges: - rust-lang#118268 (Pretty print `Fn<(..., ...)>` trait refs with parentheses (almost) always) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#118191 - estebank:let-chain-typo, r=compiler-errors Suggest `let` or `==` on typo'd let-chain When encountering a bare assignment in a let-chain, suggest turning the assignment into a `let` expression or an equality check. ``` error: expected expression, found `let` statement --> $DIR/bad-if-let-suggestion.rs:5:8 | LL | if let x = 1 && i = 2 {} | ^^^^^^^^^ | = note: only supported directly in conditions of `if` and `while` expressions help: you might have meant to continue the let-chain | LL | if let x = 1 && let i = 2 {} | +++ help: you might have meant to compare for equality | LL | if let x = 1 && i == 2 {} | + ```
When encountering a bare assignment in a let-chain, suggest turning the
assignment into a
let
expression or an equality check.