Skip to content

Commit

Permalink
fix #101880: suggest let for assignment, and some code refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
chenyukang committed Oct 20, 2022
1 parent dcb3761 commit eb68e27
Show file tree
Hide file tree
Showing 6 changed files with 165 additions and 53 deletions.
8 changes: 8 additions & 0 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,9 @@ struct DiagnosticMetadata<'ast> {
/// Used to detect possible `if let` written without `let` and to provide structured suggestion.
in_if_condition: Option<&'ast Expr>,

/// Used to detect possible new binding written without `let` and to provide structured suggestion.
in_assignment: Option<&'ast Expr>,

/// If we are currently in a trait object definition. Used to point at the bounds when
/// encountering a struct or enum.
current_trait_object: Option<&'ast [ast::GenericBound]>,
Expand Down Expand Up @@ -3905,6 +3908,11 @@ 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;
}
_ => {
visit::walk_expr(self, expr);
}
Expand Down
96 changes: 43 additions & 53 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,9 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {

// If the trait has a single item (which wasn't matched by Levenshtein), suggest it
let suggestion = self.get_single_associated_item(&path, &source, is_expected);
self.r.add_typo_suggestion(err, suggestion, ident_span);
if !self.r.add_typo_suggestion(err, suggestion, ident_span) {
fallback = !self.let_binding_suggestion(err, ident_span);
}
}
fallback
}
Expand Down Expand Up @@ -1076,41 +1078,14 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
// where a brace being opened means a block is being started. Look
// ahead for the next text to see if `span` is followed by a `{`.
let sm = self.r.session.source_map();
let mut sp = span;
loop {
sp = sm.next_point(sp);
match sm.span_to_snippet(sp) {
Ok(ref snippet) => {
if snippet.chars().any(|c| !c.is_whitespace()) {
break;
}
}
_ => break,
}
}
let sp = sm.span_look_ahead(span, None, Some(50));
let followed_by_brace = matches!(sm.span_to_snippet(sp), Ok(ref snippet) if snippet == "{");
// In case this could be a struct literal that needs to be surrounded
// by parentheses, find the appropriate span.
let mut i = 0;
let mut closing_brace = None;
loop {
sp = sm.next_point(sp);
match sm.span_to_snippet(sp) {
Ok(ref snippet) => {
if snippet == "}" {
closing_brace = Some(span.to(sp));
break;
}
}
_ => break,
}
i += 1;
// The bigger the span, the more likely we're incorrect --
// bound it to 100 chars long.
if i > 100 {
break;
}
}
let closing_span = sm.span_look_ahead(span, Some("}"), Some(50));
let closing_brace: Option<Span> = sm
.span_to_snippet(closing_span)
.map_or(None, |s| if s == "}" { Some(span.to(closing_span)) } else { None });
(followed_by_brace, closing_brace)
}

Expand Down Expand Up @@ -1727,26 +1702,16 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
}
}
if let Ok(base_snippet) = base_snippet {
let mut sp = after_colon_sp;
for _ in 0..100 {
// Try to find an assignment
sp = sm.next_point(sp);
let snippet = sm.span_to_snippet(sp);
match snippet {
Ok(ref x) if x.as_str() == "=" => {
err.span_suggestion(
base_span,
"maybe you meant to write an assignment here",
format!("let {}", base_snippet),
Applicability::MaybeIncorrect,
);
show_label = false;
break;
}
Ok(ref x) if x.as_str() == "\n" => break,
Err(_) => break,
Ok(_) => {}
}
// Try to find an assignment
let eq_span = sm.span_look_ahead(after_colon_sp, Some("="), Some(50));
if let Ok(ref snippet) = sm.span_to_snippet(eq_span) && snippet == "=" {
err.span_suggestion(
base_span,
"maybe you meant to write an assignment here",
format!("let {}", base_snippet),
Applicability::MaybeIncorrect,
);
show_label = false;
}
}
}
Expand All @@ -1763,6 +1728,31 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
false
}

fn let_binding_suggestion(&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 {
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))
{
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;
}
}
added_suggestion
}

fn find_module(&mut self, def_id: DefId) -> Option<(Module<'a>, ImportSuggestion)> {
let mut result = None;
let mut seen_modules = FxHashSet::default();
Expand Down
20 changes: 20 additions & 0 deletions compiler/rustc_span/src/source_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -877,6 +877,26 @@ impl SourceMap {
Span::new(BytePos(start_of_next_point), end_of_next_point, sp.ctxt(), None)
}

/// Returns a new span to check next none-whitespace character or some specified expected character
/// If `expect` is none, the first span of non-whitespace character is returned.
/// If `expect` presented, the first span of the character `expect` is returned
/// Otherwise, the span reached to limit is returned.
pub fn span_look_ahead(&self, span: Span, expect: Option<&str>, limit: Option<usize>) -> Span {
let mut sp = span;
for _ in 0..limit.unwrap_or(100 as usize) {
sp = self.next_point(sp);
if let Ok(ref snippet) = self.span_to_snippet(sp) {
if expect.map_or(false, |es| snippet == es) {
break;
}
if expect.is_none() && snippet.chars().any(|c| !c.is_whitespace()) {
break;
}
}
}
sp
}

/// Finds the width of the character, either before or after the end of provided span,
/// depending on the `forwards` parameter.
fn find_width_of_character_at_span(&self, sp: Span, forwards: bool) -> u32 {
Expand Down
17 changes: 17 additions & 0 deletions src/test/ui/suggestions/suggest-let-for-assignment.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// run-rustfix

fn main() {
let demo = 1; //~ ERROR cannot find value `demo` in this scope
dbg!(demo); //~ ERROR cannot find value `demo` in this scope

let x = "x"; //~ ERROR cannot find value `x` in this scope
println!("x: {}", x); //~ ERROR cannot find value `x` in this scope

if x == "x" {
//~^ ERROR cannot find value `x` in this scope
println!("x is 1");
}

let y = 1 + 2; //~ ERROR cannot find value `y` in this scope
println!("y: {}", y); //~ ERROR cannot find value `y` in this scope
}
17 changes: 17 additions & 0 deletions src/test/ui/suggestions/suggest-let-for-assignment.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// run-rustfix

fn main() {
demo = 1; //~ ERROR cannot find value `demo` in this scope
dbg!(demo); //~ ERROR cannot find value `demo` in this scope

x = "x"; //~ ERROR cannot find value `x` in this scope
println!("x: {}", x); //~ ERROR cannot find value `x` in this scope

if x == "x" {
//~^ ERROR cannot find value `x` in this scope
println!("x is 1");
}

y = 1 + 2; //~ ERROR cannot find value `y` in this scope
println!("y: {}", y); //~ ERROR cannot find value `y` in this scope
}
60 changes: 60 additions & 0 deletions src/test/ui/suggestions/suggest-let-for-assignment.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
error[E0425]: cannot find value `demo` in this scope
--> $DIR/suggest-let-for-assignment.rs:4:5
|
LL | demo = 1;
| ^^^^
|
help: you might have meant to introduce a new binding
|
LL | let demo = 1;
| +++

error[E0425]: cannot find value `demo` in this scope
--> $DIR/suggest-let-for-assignment.rs:5:10
|
LL | dbg!(demo);
| ^^^^ not found in this scope

error[E0425]: cannot find value `x` in this scope
--> $DIR/suggest-let-for-assignment.rs:7:5
|
LL | x = "x";
| ^
|
help: you might have meant to introduce a new binding
|
LL | let x = "x";
| +++

error[E0425]: cannot find value `x` in this scope
--> $DIR/suggest-let-for-assignment.rs:8:23
|
LL | println!("x: {}", x);
| ^ not found in this scope

error[E0425]: cannot find value `x` in this scope
--> $DIR/suggest-let-for-assignment.rs:10:8
|
LL | if x == "x" {
| ^ not found in this scope

error[E0425]: cannot find value `y` in this scope
--> $DIR/suggest-let-for-assignment.rs:15:5
|
LL | y = 1 + 2;
| ^
|
help: you might have meant to introduce a new binding
|
LL | let y = 1 + 2;
| +++

error[E0425]: cannot find value `y` in this scope
--> $DIR/suggest-let-for-assignment.rs:16:23
|
LL | println!("y: {}", y);
| ^ not found in this scope

error: aborting due to 7 previous errors

For more information about this error, try `rustc --explain E0425`.

0 comments on commit eb68e27

Please sign in to comment.