Skip to content

Suggest let or == on typo'd let-chain #118191

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

Merged
merged 1 commit into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions compiler/rustc_parse/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -492,9 +492,13 @@ parse_match_arm_body_without_braces = `match` arm body without braces
} with a body
.suggestion_use_comma_not_semicolon = replace `;` with `,` to end a `match` arm expression
parse_maybe_comparison = you might have meant to compare for equality
parse_maybe_fn_typo_with_impl = you might have meant to write `impl` instead of `fn`
.suggestion = replace `fn` with `impl` here
parse_maybe_missing_let = you might have meant to continue the let-chain
parse_maybe_recover_from_bad_qpath_stage_2 =
missing angle brackets in associated item path
.suggestion = types that don't start with an identifier need to be surrounded with angle brackets in qualified paths
Expand Down
26 changes: 26 additions & 0 deletions compiler/rustc_parse/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,32 @@ pub(crate) struct ExpectedExpressionFoundLet {
pub span: Span,
#[subdiagnostic]
pub reason: ForbiddenLetReason,
#[subdiagnostic]
pub missing_let: Option<MaybeMissingLet>,
#[subdiagnostic]
pub comparison: Option<MaybeComparison>,
}

#[derive(Subdiagnostic, Clone, Copy)]
#[multipart_suggestion(
parse_maybe_missing_let,
applicability = "maybe-incorrect",
style = "verbose"
)]
pub(crate) struct MaybeMissingLet {
#[suggestion_part(code = "let ")]
pub span: Span,
}

#[derive(Subdiagnostic, Clone, Copy)]
#[multipart_suggestion(
parse_maybe_comparison,
applicability = "maybe-incorrect",
style = "verbose"
)]
pub(crate) struct MaybeComparison {
#[suggestion_part(code = "=")]
pub span: Span,
}

#[derive(Diagnostic)]
Expand Down
48 changes: 40 additions & 8 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// ignore-tidy-filelength
use super::diagnostics::SnapshotParser;
use super::pat::{CommaRecoveryMode, Expected, RecoverColon, RecoverComma};
use super::ty::{AllowPlus, RecoverQPath, RecoverReturnSign};
Expand Down Expand Up @@ -2477,7 +2478,7 @@ impl<'a> Parser<'a> {
let mut cond =
self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL | Restrictions::ALLOW_LET, None)?;

CondChecker { parser: self, forbid_let_reason: None }.visit_expr(&mut cond);
CondChecker::new(self).visit_expr(&mut cond);

if let ExprKind::Let(_, _, _, None) = cond.kind {
// Remove the last feature gating of a `let` expression since it's stable.
Expand All @@ -2493,6 +2494,8 @@ impl<'a> Parser<'a> {
let err = errors::ExpectedExpressionFoundLet {
span: self.token.span,
reason: ForbiddenLetReason::OtherForbidden,
missing_let: None,
comparison: None,
};
if self.prev_token.kind == token::BinOp(token::Or) {
// This was part of a closure, the that part of the parser recover.
Expand Down Expand Up @@ -2876,7 +2879,7 @@ impl<'a> Parser<'a> {
let if_span = this.prev_token.span;
let mut cond = this.parse_match_guard_condition()?;

CondChecker { parser: this, forbid_let_reason: None }.visit_expr(&mut cond);
CondChecker::new(this).visit_expr(&mut cond);

let (has_let_expr, does_not_have_bin_op) = check_let_expr(&cond);
if has_let_expr {
Expand Down Expand Up @@ -3552,6 +3555,14 @@ pub(crate) enum ForbiddenLetReason {
struct CondChecker<'a> {
parser: &'a Parser<'a>,
forbid_let_reason: Option<ForbiddenLetReason>,
missing_let: Option<errors::MaybeMissingLet>,
comparison: Option<errors::MaybeComparison>,
}

impl<'a> CondChecker<'a> {
fn new(parser: &'a Parser<'a>) -> Self {
CondChecker { parser, forbid_let_reason: None, missing_let: None, comparison: None }
}
}

impl MutVisitor for CondChecker<'_> {
Expand All @@ -3562,11 +3573,13 @@ impl MutVisitor for CondChecker<'_> {
match e.kind {
ExprKind::Let(_, _, _, ref mut is_recovered @ None) => {
if let Some(reason) = self.forbid_let_reason {
*is_recovered = Some(
self.parser
.sess
.emit_err(errors::ExpectedExpressionFoundLet { span, reason }),
);
*is_recovered =
Some(self.parser.sess.emit_err(errors::ExpectedExpressionFoundLet {
span,
reason,
missing_let: self.missing_let,
comparison: self.comparison,
}));
} else {
self.parser.sess.gated_spans.gate(sym::let_chains, span);
}
Expand All @@ -3590,9 +3603,28 @@ impl MutVisitor for CondChecker<'_> {
noop_visit_expr(e, self);
self.forbid_let_reason = forbid_let_reason;
}
ExprKind::Assign(ref lhs, _, span) => {
let forbid_let_reason = self.forbid_let_reason;
self.forbid_let_reason = Some(OtherForbidden);
let missing_let = self.missing_let;
if let ExprKind::Binary(_, _, rhs) = &lhs.kind
&& let ExprKind::Path(_, _)
| ExprKind::Struct(_)
| ExprKind::Call(_, _)
| ExprKind::Array(_) = rhs.kind
{
self.missing_let =
Some(errors::MaybeMissingLet { span: rhs.span.shrink_to_lo() });
}
let comparison = self.comparison;
self.comparison = Some(errors::MaybeComparison { span: span.shrink_to_hi() });
noop_visit_expr(e, self);
self.forbid_let_reason = forbid_let_reason;
self.missing_let = missing_let;
self.comparison = comparison;
}
ExprKind::Unary(_, _)
| ExprKind::Await(_, _)
| ExprKind::Assign(_, _, _)
| ExprKind::AssignOp(_, _, _)
| ExprKind::Range(_, _, _)
| ExprKind::Try(_)
Expand Down
3 changes: 0 additions & 3 deletions tests/ui/expr/if/bad-if-let-suggestion.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
// FIXME(compiler-errors): This really should suggest `let` on the RHS of the
// `&&` operator, but that's kinda hard to do because of precedence.
// Instead, for now we just make sure not to suggest `if let let`.
fn a() {
if let x = 1 && i = 2 {}
//~^ ERROR cannot find value `i` in this scope
Expand Down
22 changes: 15 additions & 7 deletions tests/ui/expr/if/bad-if-let-suggestion.stderr
Original file line number Diff line number Diff line change
@@ -1,19 +1,27 @@
error: expected expression, found `let` statement
--> $DIR/bad-if-let-suggestion.rs:5:8
--> $DIR/bad-if-let-suggestion.rs:2: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 {}
| +

error[E0425]: cannot find value `i` in this scope
--> $DIR/bad-if-let-suggestion.rs:5:21
--> $DIR/bad-if-let-suggestion.rs:2:21
|
LL | if let x = 1 && i = 2 {}
| ^ not found in this scope

error[E0425]: cannot find value `i` in this scope
--> $DIR/bad-if-let-suggestion.rs:12:9
--> $DIR/bad-if-let-suggestion.rs:9:9
|
LL | fn a() {
| ------ similarly named function `a` defined here
Expand All @@ -22,7 +30,7 @@ LL | if (i + j) = i {}
| ^ help: a function with a similar name exists: `a`

error[E0425]: cannot find value `j` in this scope
--> $DIR/bad-if-let-suggestion.rs:12:13
--> $DIR/bad-if-let-suggestion.rs:9:13
|
LL | fn a() {
| ------ similarly named function `a` defined here
Expand All @@ -31,7 +39,7 @@ LL | if (i + j) = i {}
| ^ help: a function with a similar name exists: `a`

error[E0425]: cannot find value `i` in this scope
--> $DIR/bad-if-let-suggestion.rs:12:18
--> $DIR/bad-if-let-suggestion.rs:9:18
|
LL | fn a() {
| ------ similarly named function `a` defined here
Expand All @@ -40,7 +48,7 @@ LL | if (i + j) = i {}
| ^ help: a function with a similar name exists: `a`

error[E0425]: cannot find value `x` in this scope
--> $DIR/bad-if-let-suggestion.rs:19:8
--> $DIR/bad-if-let-suggestion.rs:16:8
|
LL | fn a() {
| ------ similarly named function `a` defined here
Expand All @@ -49,7 +57,7 @@ LL | if x[0] = 1 {}
| ^ help: a function with a similar name exists: `a`

error[E0308]: mismatched types
--> $DIR/bad-if-let-suggestion.rs:5:8
--> $DIR/bad-if-let-suggestion.rs:2:8
|
LL | if let x = 1 && i = 2 {}
| ^^^^^^^^^^^^^^^^^^ expected `bool`, found `()`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ LL | if let Some(elem) = _opt && [1, 2, 3][let _ = &&let Some(x) = Some(
| ^^^^^^^^^^^^^^^^^^^^^
|
= note: only supported directly in conditions of `if` and `while` expressions
help: you might have meant to compare for equality
|
LL | if let Some(elem) = _opt && [1, 2, 3][let _ = &&let Some(x) = Some(42)] == 1 {
| +

error: expected expression, found `let` statement
--> $DIR/invalid-let-in-a-valid-let-context.rs:24:23
Expand All @@ -53,6 +57,10 @@ LL | if let Some(elem) = _opt && [1, 2, 3][let _ = ()] = 1 {
| ^^^^^^^^^^^^^^^^^^^^^
|
= note: only supported directly in conditions of `if` and `while` expressions
help: you might have meant to compare for equality
|
LL | if let Some(elem) = _opt && [1, 2, 3][let _ = ()] == 1 {
| +

error: expected expression, found `let` statement
--> $DIR/invalid-let-in-a-valid-let-context.rs:42:21
Expand Down