From 952df48948438a67a8130137a120e515897cde87 Mon Sep 17 00:00:00 2001 From: yukang <moorekang@gmail.com> Date: Wed, 9 Nov 2022 16:43:37 +0800 Subject: [PATCH 1/3] Fix #104086, Tighten the 'introduce new binding' suggestion --- .../rustc_resolve/src/late/diagnostics.rs | 25 ++++-- .../suggestions/issue-104086-suggest-let.rs | 10 +++ .../issue-104086-suggest-let.stderr | 81 +++++++++++++++++++ 3 files changed, 109 insertions(+), 7 deletions(-) create mode 100644 src/test/ui/suggestions/issue-104086-suggest-let.rs create mode 100644 src/test/ui/suggestions/issue-104086-suggest-let.stderr diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index a1338dcd4771b..45c11fc9eb00d 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1810,18 +1810,27 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { false } - fn let_binding_suggestion(&self, err: &mut Diagnostic, ident_span: Span) -> bool { + fn let_binding_suggestion(&mut self, err: &mut Diagnostic, ident_span: Span) -> bool { // try to give a suggestion for this pattern: `name = 1`, which is common in other languages let mut added_suggestion = false; - if let Some(Expr { kind: ExprKind::Assign(lhs, _rhs, _), .. }) = self.diagnostic_metadata.in_assignment && - let ast::ExprKind::Path(None, _) = lhs.kind { + if let Some(Expr { kind: ExprKind::Assign(lhs, rhs, _), .. }) = + self.diagnostic_metadata.in_assignment + { + let is_rhs_assign = match rhs.kind { + ExprKind::Assign(..) => true, + _ => false, + }; + + if let ast::ExprKind::Path(None, _) = lhs.kind && !is_rhs_assign { let sm = self.r.session.source_map(); let line_span = sm.span_extend_to_line(ident_span); let ident_name = sm.span_to_snippet(ident_span).unwrap(); - // HACK(chenyukang): make sure ident_name is at the starting of the line to protect against macros - if sm - .span_to_snippet(line_span) - .map_or(false, |s| s.trim().starts_with(&ident_name)) + // HACK(chenyukang): make sure ident_name is at the starting of the line to protect against macros, + // and avoid some special cases like `x = x = x` + if let Ok(line) = sm.span_to_snippet(line_span) && + let stripped = line.split_whitespace().collect::<String>() && + stripped.trim().starts_with(&ident_name) && + stripped.matches(&format!("{}=", &ident_name)).count() == 1 { err.span_suggestion_verbose( ident_span.shrink_to_lo(), @@ -1832,6 +1841,8 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { added_suggestion = true; } } + self.diagnostic_metadata.in_assignment = None; + } added_suggestion } diff --git a/src/test/ui/suggestions/issue-104086-suggest-let.rs b/src/test/ui/suggestions/issue-104086-suggest-let.rs new file mode 100644 index 0000000000000..e23109a3be69c --- /dev/null +++ b/src/test/ui/suggestions/issue-104086-suggest-let.rs @@ -0,0 +1,10 @@ +// error-pattern: not found in this scope + +fn main() { + x = x = x; + x = y = y = y; + x = y = y; + x = x = y; + x = x; // will suggest add `let` + x = y // will suggest add `let` +} diff --git a/src/test/ui/suggestions/issue-104086-suggest-let.stderr b/src/test/ui/suggestions/issue-104086-suggest-let.stderr new file mode 100644 index 0000000000000..2af9c142ec69e --- /dev/null +++ b/src/test/ui/suggestions/issue-104086-suggest-let.stderr @@ -0,0 +1,81 @@ +error[E0425]: cannot find value `x` in this scope + --> $DIR/issue-104086-suggest-let.rs:4:5 + | +LL | x = x = x; + | ^ not found in this scope + +error[E0425]: cannot find value `x` in this scope + --> $DIR/issue-104086-suggest-let.rs:4:9 + | +LL | x = x = x; + | ^ not found in this scope + +error[E0425]: cannot find value `x` in this scope + --> $DIR/issue-104086-suggest-let.rs:4:13 + | +LL | x = x = x; + | ^ not found in this scope + +error[E0425]: cannot find value `x` in this scope + --> $DIR/issue-104086-suggest-let.rs:5:5 + | +LL | x = y = y = y; + | ^ not found in this scope + +error[E0425]: cannot find value `y` in this scope + --> $DIR/issue-104086-suggest-let.rs:5:9 + | +LL | x = y = y = y; + | ^ not found in this scope + +error[E0425]: cannot find value `y` in this scope + --> $DIR/issue-104086-suggest-let.rs:5:13 + | +LL | x = y = y = y; + | ^ not found in this scope + +error[E0425]: cannot find value `y` in this scope + --> $DIR/issue-104086-suggest-let.rs:5:17 + | +LL | x = y = y = y; + | ^ not found in this scope + +error[E0425]: cannot find value `x` in this scope + --> $DIR/issue-104086-suggest-let.rs:6:5 + | +LL | x = y = y; + | ^ not found in this scope + +error[E0425]: cannot find value `y` in this scope + --> $DIR/issue-104086-suggest-let.rs:6:9 + | +LL | x = y = y; + | ^ not found in this scope + +error[E0425]: cannot find value `y` in this scope + --> $DIR/issue-104086-suggest-let.rs:6:13 + | +LL | x = y = y; + | ^ not found in this scope + +error[E0425]: cannot find value `x` in this scope + --> $DIR/issue-104086-suggest-let.rs:7:5 + | +LL | x = x = y; + | ^ not found in this scope + +error[E0425]: cannot find value `x` in this scope + --> $DIR/issue-104086-suggest-let.rs:7:9 + | +LL | x = x = y; + | ^ not found in this scope + +error[E0425]: cannot find value `y` in this scope + --> $DIR/issue-104086-suggest-let.rs:7:13 + | +LL | x = x = y; + | ^ not found in this scope + +error: aborting due to 13 previous errors + +For more information about this error, try `rustc --explain E0425`. From 5689f9c679838a921be5a0ac64e0cd13637efc35 Mon Sep 17 00:00:00 2001 From: yukang <moorekang@gmail.com> Date: Wed, 9 Nov 2022 17:10:33 +0800 Subject: [PATCH 2/3] fix tests and code cleanup --- .../rustc_resolve/src/late/diagnostics.rs | 6 +- .../suggestions/issue-104086-suggest-let.rs | 24 ++++++- .../issue-104086-suggest-let.stderr | 62 ++++++++++++++----- 3 files changed, 71 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 45c11fc9eb00d..9e47f4bc6c1c5 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1816,11 +1816,7 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { if let Some(Expr { kind: ExprKind::Assign(lhs, rhs, _), .. }) = self.diagnostic_metadata.in_assignment { - let is_rhs_assign = match rhs.kind { - ExprKind::Assign(..) => true, - _ => false, - }; - + let is_rhs_assign = matches!(rhs.kind, ExprKind::Assign(..)); if let ast::ExprKind::Path(None, _) = lhs.kind && !is_rhs_assign { let sm = self.r.session.source_map(); let line_span = sm.span_extend_to_line(ident_span); diff --git a/src/test/ui/suggestions/issue-104086-suggest-let.rs b/src/test/ui/suggestions/issue-104086-suggest-let.rs index e23109a3be69c..d22ad27d0e031 100644 --- a/src/test/ui/suggestions/issue-104086-suggest-let.rs +++ b/src/test/ui/suggestions/issue-104086-suggest-let.rs @@ -1,10 +1,30 @@ -// error-pattern: not found in this scope - fn main() { x = x = x; + //~^ ERROR cannot find value `x` in this scope + //~| ERROR cannot find value `x` in this scope + //~| ERROR cannot find value `x` in this scope + x = y = y = y; + //~^ ERROR cannot find value `y` in this scope + //~| ERROR cannot find value `y` in this scope + //~| ERROR cannot find value `y` in this scope + //~| ERROR cannot find value `x` in this scope + x = y = y; + //~^ ERROR cannot find value `x` in this scope + //~| ERROR cannot find value `y` in this scope + //~| ERROR cannot find value `y` in this scope + x = x = y; + //~^ ERROR cannot find value `x` in this scope + //~| ERROR cannot find value `x` in this scope + //~| ERROR cannot find value `y` in this scope + x = x; // will suggest add `let` + //~^ ERROR cannot find value `x` in this scope + //~| ERROR cannot find value `x` in this scope + x = y // will suggest add `let` + //~^ ERROR cannot find value `x` in this scope + //~| ERROR cannot find value `y` in this scope } diff --git a/src/test/ui/suggestions/issue-104086-suggest-let.stderr b/src/test/ui/suggestions/issue-104086-suggest-let.stderr index 2af9c142ec69e..d37a18f005fda 100644 --- a/src/test/ui/suggestions/issue-104086-suggest-let.stderr +++ b/src/test/ui/suggestions/issue-104086-suggest-let.stderr @@ -1,81 +1,115 @@ error[E0425]: cannot find value `x` in this scope - --> $DIR/issue-104086-suggest-let.rs:4:5 + --> $DIR/issue-104086-suggest-let.rs:2:5 | LL | x = x = x; | ^ not found in this scope error[E0425]: cannot find value `x` in this scope - --> $DIR/issue-104086-suggest-let.rs:4:9 + --> $DIR/issue-104086-suggest-let.rs:2:9 | LL | x = x = x; | ^ not found in this scope error[E0425]: cannot find value `x` in this scope - --> $DIR/issue-104086-suggest-let.rs:4:13 + --> $DIR/issue-104086-suggest-let.rs:2:13 | LL | x = x = x; | ^ not found in this scope error[E0425]: cannot find value `x` in this scope - --> $DIR/issue-104086-suggest-let.rs:5:5 + --> $DIR/issue-104086-suggest-let.rs:7:5 | LL | x = y = y = y; | ^ not found in this scope error[E0425]: cannot find value `y` in this scope - --> $DIR/issue-104086-suggest-let.rs:5:9 + --> $DIR/issue-104086-suggest-let.rs:7:9 | LL | x = y = y = y; | ^ not found in this scope error[E0425]: cannot find value `y` in this scope - --> $DIR/issue-104086-suggest-let.rs:5:13 + --> $DIR/issue-104086-suggest-let.rs:7:13 | LL | x = y = y = y; | ^ not found in this scope error[E0425]: cannot find value `y` in this scope - --> $DIR/issue-104086-suggest-let.rs:5:17 + --> $DIR/issue-104086-suggest-let.rs:7:17 | LL | x = y = y = y; | ^ not found in this scope error[E0425]: cannot find value `x` in this scope - --> $DIR/issue-104086-suggest-let.rs:6:5 + --> $DIR/issue-104086-suggest-let.rs:13:5 | LL | x = y = y; | ^ not found in this scope error[E0425]: cannot find value `y` in this scope - --> $DIR/issue-104086-suggest-let.rs:6:9 + --> $DIR/issue-104086-suggest-let.rs:13:9 | LL | x = y = y; | ^ not found in this scope error[E0425]: cannot find value `y` in this scope - --> $DIR/issue-104086-suggest-let.rs:6:13 + --> $DIR/issue-104086-suggest-let.rs:13:13 | LL | x = y = y; | ^ not found in this scope error[E0425]: cannot find value `x` in this scope - --> $DIR/issue-104086-suggest-let.rs:7:5 + --> $DIR/issue-104086-suggest-let.rs:18:5 | LL | x = x = y; | ^ not found in this scope error[E0425]: cannot find value `x` in this scope - --> $DIR/issue-104086-suggest-let.rs:7:9 + --> $DIR/issue-104086-suggest-let.rs:18:9 | LL | x = x = y; | ^ not found in this scope error[E0425]: cannot find value `y` in this scope - --> $DIR/issue-104086-suggest-let.rs:7:13 + --> $DIR/issue-104086-suggest-let.rs:18:13 | LL | x = x = y; | ^ not found in this scope -error: aborting due to 13 previous errors +error[E0425]: cannot find value `x` in this scope + --> $DIR/issue-104086-suggest-let.rs:23:5 + | +LL | x = x; // will suggest add `let` + | ^ + | +help: you might have meant to introduce a new binding + | +LL | let x = x; // will suggest add `let` + | +++ + +error[E0425]: cannot find value `x` in this scope + --> $DIR/issue-104086-suggest-let.rs:23:9 + | +LL | x = x; // will suggest add `let` + | ^ not found in this scope + +error[E0425]: cannot find value `x` in this scope + --> $DIR/issue-104086-suggest-let.rs:27:5 + | +LL | x = y // will suggest add `let` + | ^ + | +help: you might have meant to introduce a new binding + | +LL | let x = y // will suggest add `let` + | +++ + +error[E0425]: cannot find value `y` in this scope + --> $DIR/issue-104086-suggest-let.rs:27:9 + | +LL | x = y // will suggest add `let` + | ^ not found in this scope + +error: aborting due to 17 previous errors For more information about this error, try `rustc --explain E0425`. From c69872bb6c0657241a5b3cf8944861dbd3ff7468 Mon Sep 17 00:00:00 2001 From: yukang <moorekang@gmail.com> Date: Wed, 9 Nov 2022 22:00:22 +0800 Subject: [PATCH 3/3] add 'is_assign_rhs' to avoid weird suggesting 'let' --- compiler/rustc_resolve/src/late.rs | 14 +++++++--- .../rustc_resolve/src/late/diagnostics.rs | 28 +++++-------------- .../issue-104086-suggest-let.stderr | 28 ++++++++++++++++--- 3 files changed, 41 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 6d2ee25df320d..ede67813883d6 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -527,6 +527,7 @@ struct DiagnosticMetadata<'ast> { /// Used to detect possible new binding written without `let` and to provide structured suggestion. in_assignment: Option<&'ast Expr>, + is_assign_rhs: bool, /// If we are currently in a trait object definition. Used to point at the bounds when /// encountering a struct or enum. @@ -3963,10 +3964,15 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { self.resolve_expr(elem, Some(expr)); self.visit_expr(idx); } - ExprKind::Assign(..) => { - let old = self.diagnostic_metadata.in_assignment.replace(expr); - visit::walk_expr(self, expr); - self.diagnostic_metadata.in_assignment = old; + ExprKind::Assign(ref lhs, ref rhs, _) => { + if !self.diagnostic_metadata.is_assign_rhs { + self.diagnostic_metadata.in_assignment = Some(expr); + } + self.visit_expr(lhs); + self.diagnostic_metadata.is_assign_rhs = true; + self.diagnostic_metadata.in_assignment = None; + self.visit_expr(rhs); + self.diagnostic_metadata.is_assign_rhs = false; } _ => { visit::walk_expr(self, expr); diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 9e47f4bc6c1c5..ec1ac015daf21 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1810,36 +1810,22 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { false } + // try to give a suggestion for this pattern: `name = blah`, which is common in other languages + // suggest `let name = blah` to introduce a new binding fn let_binding_suggestion(&mut self, err: &mut Diagnostic, ident_span: Span) -> bool { - // try to give a suggestion for this pattern: `name = 1`, which is common in other languages - let mut added_suggestion = false; - if let Some(Expr { kind: ExprKind::Assign(lhs, rhs, _), .. }) = - self.diagnostic_metadata.in_assignment - { - let is_rhs_assign = matches!(rhs.kind, ExprKind::Assign(..)); - if let ast::ExprKind::Path(None, _) = lhs.kind && !is_rhs_assign { - let sm = self.r.session.source_map(); - let line_span = sm.span_extend_to_line(ident_span); - let ident_name = sm.span_to_snippet(ident_span).unwrap(); - // HACK(chenyukang): make sure ident_name is at the starting of the line to protect against macros, - // and avoid some special cases like `x = x = x` - if let Ok(line) = sm.span_to_snippet(line_span) && - let stripped = line.split_whitespace().collect::<String>() && - stripped.trim().starts_with(&ident_name) && - stripped.matches(&format!("{}=", &ident_name)).count() == 1 - { + if let Some(Expr { kind: ExprKind::Assign(lhs, .. ), .. }) = self.diagnostic_metadata.in_assignment && + let ast::ExprKind::Path(None, _) = lhs.kind { + if !ident_span.from_expansion() { err.span_suggestion_verbose( ident_span.shrink_to_lo(), "you might have meant to introduce a new binding", "let ".to_string(), Applicability::MaybeIncorrect, ); - added_suggestion = true; + return true; } } - self.diagnostic_metadata.in_assignment = None; - } - added_suggestion + false } fn find_module(&mut self, def_id: DefId) -> Option<(Module<'a>, ImportSuggestion)> { diff --git a/src/test/ui/suggestions/issue-104086-suggest-let.stderr b/src/test/ui/suggestions/issue-104086-suggest-let.stderr index d37a18f005fda..fb4ea3121ac67 100644 --- a/src/test/ui/suggestions/issue-104086-suggest-let.stderr +++ b/src/test/ui/suggestions/issue-104086-suggest-let.stderr @@ -2,7 +2,12 @@ error[E0425]: cannot find value `x` in this scope --> $DIR/issue-104086-suggest-let.rs:2:5 | LL | x = x = x; - | ^ not found in this scope + | ^ + | +help: you might have meant to introduce a new binding + | +LL | let x = x = x; + | +++ error[E0425]: cannot find value `x` in this scope --> $DIR/issue-104086-suggest-let.rs:2:9 @@ -20,7 +25,12 @@ error[E0425]: cannot find value `x` in this scope --> $DIR/issue-104086-suggest-let.rs:7:5 | LL | x = y = y = y; - | ^ not found in this scope + | ^ + | +help: you might have meant to introduce a new binding + | +LL | let x = y = y = y; + | +++ error[E0425]: cannot find value `y` in this scope --> $DIR/issue-104086-suggest-let.rs:7:9 @@ -44,7 +54,12 @@ error[E0425]: cannot find value `x` in this scope --> $DIR/issue-104086-suggest-let.rs:13:5 | LL | x = y = y; - | ^ not found in this scope + | ^ + | +help: you might have meant to introduce a new binding + | +LL | let x = y = y; + | +++ error[E0425]: cannot find value `y` in this scope --> $DIR/issue-104086-suggest-let.rs:13:9 @@ -62,7 +77,12 @@ error[E0425]: cannot find value `x` in this scope --> $DIR/issue-104086-suggest-let.rs:18:5 | LL | x = x = y; - | ^ not found in this scope + | ^ + | +help: you might have meant to introduce a new binding + | +LL | let x = x = y; + | +++ error[E0425]: cannot find value `x` in this scope --> $DIR/issue-104086-suggest-let.rs:18:9