Add suggestions for misspelled method names #44297

laumann · 2017-09-03T19:36:45Z

Use the syntax::util::lev_distance module to provide suggestions when a
named method cannot be found.

Part of #30197

rust-highfive · 2017-09-03T19:36:50Z

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

laumann · 2017-09-03T19:42:15Z

I had some concerns when submitting this - not sure if this is the best approach.

Trait methods are not suggested
lev_candidates should maybe be Vec<CandidateSource> instead of Vec<DefId>

eddyb · 2017-09-04T02:26:45Z

r? @nikomatsakis

arielb1 · 2017-09-04T11:20:12Z

If you want to integrate traits, to avoid issues like #43149 you want to

add an allow_similar_names flag to ProbeContext

if it's present, make ProbeContext::impl_or_trait_item pick items with names similar enough to this name:

rust/src/librustc_typeck/check/method/probe.rs

Lines 1255 to 1261 in a36d93d

    
               fn impl_or_trait_item(&self, def_id: DefId) -> Vec<ty::AssociatedItem> { 
        
                   if let Some(name) = self.method_name { 
        
                       self.fcx.associated_item(def_id, name).map_or(Vec::new(), |x| vec![x]) 
        
                   } else { 
        
                       self.tcx.associated_items(def_id).collect() 
        
                   } 
        
               }

Do something like ProbeContext::probe_for_return_type to find all similar methods that can be used:

rust/src/librustc_typeck/check/method/probe.rs

Lines 166 to 192 in a36d93d

    
               pub fn probe_for_return_type(&self, 
        
                                            span: Span, 
        
                                            mode: Mode, 
        
                                            return_type: Ty<'tcx>, 
        
                                            self_ty: Ty<'tcx>, 
        
                                            scope_expr_id: ast::NodeId) 
        
                                            -> Vec<ty::AssociatedItem> { 
        
                   debug!("probe(self_ty={:?}, return_type={}, scope_expr_id={})", 
        
                          self_ty, 
        
                          return_type, 
        
                          scope_expr_id); 
        
                   let method_names = 
        
                       self.probe_op(span, mode, None, Some(return_type), IsSuggestion(true), 
        
                                     self_ty, scope_expr_id, ProbeScope::TraitsInScope, 
        
                                     |probe_cx| Ok(probe_cx.candidate_method_names())) 
        
                           .unwrap_or(vec![]); 
        
                    method_names 
        
                        .iter() 
        
                        .flat_map(|&method_name| { 
        
                            self.probe_op( 
        
                                span, mode, Some(method_name), Some(return_type), 
        
                                IsSuggestion(true), self_ty, scope_expr_id, 
        
                                ProbeScope::TraitsInScope, |probe_cx| probe_cx.pick() 
        
                            ).ok().map(|pick| pick.item) 
        
                        }) 
        
                       .collect() 
        
               }

laumann · 2017-09-05T05:52:21Z

@arielb1 Thanks for the suggestion! I'll try to do that...

nikomatsakis · 2017-09-05T16:20:21Z

r? @arielb1 -- you seem to be on top of this =) feel free to re-assign me though...

aidanhs · 2017-09-13T21:56:38Z

Hi @laumann, just wanted to check in and make sure this is still on your radar!

laumann · 2017-09-14T06:26:23Z

@aidanhs Hi, yes, it is, I'm sorry for the delay, but I've had little time to work on it, since submitting the PR. I was looking at it last night, implementing @arielb1's suggestions and I have it working to the point where the suggestions are found, they just need to be emitted.

I got stuck at how to turn a ty::AssociatedItem into a CandidateSource - I don't think it's hard, I just need to look at it :-)

Sorry again!

arielb1 · 2017-09-14T10:18:12Z

@laumann

You are supposed to have the Candidate that gave rise to the AssociatedItem, and use it to call candidate_source.

arielb1 · 2017-09-14T10:30:08Z

And actually you don't want a CandidateSource here - an AssociatedItem is enough.

laumann · 2017-09-14T20:29:23Z

@arielb1 I've amended the commit and rebased it on top of master, but haven't yet run the tests. I'm really not sure if I've taken the right approach though :-/ It seems to be the right point to hook in, but I suspect that candidate assembly might include too many things that pick_core() would then get rid of.

arielb1 · 2017-09-14T21:32:29Z

src/librustc_typeck/check/method/probe.rs

+        // Toggle the flag that allows for similar names to be found
+        // and search again
+        self.allow_similar_names = true;
+        self.inherent_candidates.clear();


should be self.reset();

arielb1 · 2017-09-14T21:33:05Z

src/librustc_typeck/check/method/probe.rs

+        let mut lev_candidates = Vec::with_capacity(self.inherent_candidates.len() +
+                                                    self.extension_candidates.len());
+        for lev_cand in self.inherent_candidates.iter().chain(self.extension_candidates.iter()) {
+            lev_candidates.push(lev_cand.item);


You might want to try to pick each candidate, to avoid showing unusable candidates.

I'm not sure how to do this best. To pick, them I'd have to extract all the close matching names, run a probe for each and call pick() ?

laumann · 2017-09-17T20:47:37Z

src/librustc_typeck/check/method/probe.rs

+            let mut pcx = ProbeContext::new(self.fcx, span, self.mode, self.method_name,
+                                            self.return_type, vec![]);
+            pcx.allow_similar_names = true;
+            pcx.steps = steps;


I don't know if this might cause issues. The constructor for ProbeContext wants a Vec, but internally it's wrapped in an Rc.

that shouldn't be a problem. maybe change the constructor to take an Rc for cleanness?

laumann · 2017-09-19T08:52:48Z

@arielb1 I think it's ready for review now.

laumann · 2017-09-19T08:53:44Z

src/librustc_typeck/check/method/suggest.rs

+
+                if !lev_candidates.is_empty() {
+                    use rustc_data_structures::fx::FxHashSet;
+                    let mut dedup = FxHashSet();


I think this dedup thing is redundant now...

It should be - candidate_method_names performs deduplication.

arielb1 · 2017-09-20T10:52:06Z

src/librustc_typeck/check/method/probe.rs

@@ -799,7 +807,38 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> {
            return Err(MethodError::PrivateMatch(def, out_of_scope_traits));
        }

+        debug!("Probing for method names similar to {:?}",


It might be better to extract this section to a separate method?

laumann · 2017-09-20T14:01:31Z

Should I do another rebase?

Use the syntax::util::lev_distance module to provide suggestions when a named method cannot be found. Part of rust-lang#30197

arielb1 · 2017-09-21T10:06:41Z

@laumann

There's no need to - bors will automatically merge - but you can rebase if you want.

It turns out that when working with levenshtein distance, we report the 1 best name that matches, as in:

rust/src/librustc_typeck/check/mod.rs

Lines 3038 to 3054 in e2504cf

    
           fn suggest_field_name(variant: &'tcx ty::VariantDef, 
        
                                 field: &Spanned<ast::Name>, 
        
                                 skip: Vec<InternedString>) 
        
                                 -> Option<Symbol> { 
        
               let name = field.node.as_str(); 
        
               let names = variant.fields.iter().filter_map(|field| { 
        
                   // ignore already set fields and private fields from non-local crates 
        
                   if skip.iter().any(|x| *x == field.name.as_str()) || 
        
                      (variant.did.krate != LOCAL_CRATE && field.vis != Visibility::Public) { 
        
                       None 
        
                   } else { 
        
                       Some(&field.name) 
        
                   } 
        
               }); 
        
               find_best_match_for_name(names, &name, None) 
        
           }

arielb1 · 2017-09-21T11:10:19Z

@bors r+

bors · 2017-09-21T11:10:20Z

📌 Commit 8eff63f has been approved by arielb1

bors · 2017-09-21T12:03:22Z

⌛ Testing commit 8eff63fdc12d153dd3301f0743a9f62204de64dd with merge a5c611a02164c07d86eccc2ddc99c4012aa5cffe...

bors · 2017-09-21T12:09:57Z

💔 Test failed - status-appveyor

arielb1 · 2017-09-21T12:22:37Z

[00:03:11] tidy error: /checkout/src/librustc_typeck/check/method/probe.rs:1136: line longer than 100 chars

arielb1 · 2017-09-21T16:37:36Z

test needs updating:

[00:39:41] ---- [ui] ui/suggestions/suggest-methods.rs stdout ----
[00:39:41] 	normalized stderr:
[00:39:41] error[E0599]: no method named `bat` found for type `Foo` in the current scope
[00:39:41]   --> $DIR/suggest-methods.rs:28:7
[00:39:41]    |
[00:39:41] 28 |     f.bat(1.0);
[00:39:41]    |       ^^^
[00:39:41]    |
[00:39:41]    = help: did you mean `bar`?
[00:39:41] 
[00:39:41] error[E0599]: no method named `is_emtpy` found for type `std::string::String` in the current scope
[00:39:41]   --> $DIR/suggest-methods.rs:31:15
[00:39:41]    |
[00:39:41] 31 |     let _ = s.is_emtpy();
[00:39:41]    |               ^^^^^^^^
[00:39:41]    |
[00:39:41]    = help: did you mean `is_empty`?
[00:39:41] 
[00:39:41] error[E0599]: no method named `count_eos` found for type `u32` in the current scope
[00:39:41]   --> $DIR/suggest-methods.rs:35:19
[00:39:41]    |
[00:39:41] 35 |     let _ = 63u32.count_eos();
[00:39:41]    |                   ^^^^^^^^^
[00:39:41]    |
[00:39:41]    = help: did you mean `count_zeros`?
[00:39:41] 
[00:39:41] error[E0599]: no method named `count_o` found for type `u32` in the current scope
[00:39:41]   --> $DIR/suggest-methods.rs:38:19
[00:39:41]    |
[00:39:41] 38 |     let _ = 63u32.count_o();
[00:39:41]    |                   ^^^^^^^
[00:39:41] 
[00:39:41] error: aborting due to 4 previous errors
[00:39:41] 
[00:39:41] 
[00:39:41] 
[00:39:41] expected stderr:
[00:39:41] error[E0599]: no method named `bat` found for type `Foo` in the current scope
[00:39:41]   --> $DIR/suggest-methods.rs:28:7
[00:39:41]    |
[00:39:41] 28 |     f.bat(1.0);
[00:39:41]    |       ^^^
[00:39:41]    |
[00:39:41]    = help: did you mean `bar`?
[00:39:41] 
[00:39:41] error[E0599]: no method named `is_emtpy` found for type `std::string::String` in the current scope
[00:39:41]   --> $DIR/suggest-methods.rs:31:15
[00:39:41]    |
[00:39:41] 31 |     let _ = s.is_emtpy();
[00:39:41]    |               ^^^^^^^^
[00:39:41]    |
[00:39:41]    = help: did you mean `is_empty`?
[00:39:41] 
[00:39:41] error[E0599]: no method named `count_eos` found for type `u32` in the current scope
[00:39:41]   --> $DIR/suggest-methods.rs:34:19
[00:39:41]    |
[00:39:41] 35 |     let _ = 63u32.count_eos();
[00:39:41]    |                   ^^^^^^^^^
[00:39:41]    |
[00:39:41]    = help: did you mean `count_ones`?
[00:39:41] 
[00:39:41] error[E0599]: no method named `count_o` found for type `u32` in the current scope
[00:39:41]   --> $DIR/suggest-methods.rs:35:19
[00:39:41]    |
[00:39:41] 38 |     let _ = 63u32.count_o();
[00:39:41]    |                   ^^^^^^^
[00:39:41] 
[00:39:41] error: aborting due to 4 previous errors
[00:39:41] 
[00:39:41] 
[00:39:41] 
[00:39:41] diff of stderr:
[00:39:41] 
[00:39:41]  error[E0599]: no method named `bat` found for type `Foo` in the current scope
[00:39:41]    --> $DIR/suggest-methods.rs:28:7
[00:39:41]     |
[00:39:41]  28 |     f.bat(1.0);
[00:39:41]     |       ^^^
[00:39:41]     |
[00:39:41]     = help: did you mean `bar`?
[00:39:41]  
[00:39:41]  error[E0599]: no method named `is_emtpy` found for type `std::string::String` in the current scope
[00:39:41]    --> $DIR/suggest-methods.rs:31:15
[00:39:41]     |
[00:39:41]  31 |     let _ = s.is_emtpy();
[00:39:41]     |               ^^^^^^^^
[00:39:41]     |
[00:39:41]     = help: did you mean `is_empty`?
[00:39:41]  
[00:39:41]  error[E0599]: no method named `count_eos` found for type `u32` in the current scope
[00:39:41] -  --> $DIR/suggest-methods.rs:34:19
[00:39:41] +  --> $DIR/suggest-methods.rs:35:19
[00:39:41]     |
[00:39:41]  35 |     let _ = 63u32.count_eos();
[00:39:41]     |                   ^^^^^^^^^
[00:39:41]     |
[00:39:41] -   = help: did you mean `count_ones`?
[00:39:41] +   = help: did you mean `count_zeros`?
[00:39:41]  
[00:39:41]  error[E0599]: no method named `count_o` found for type `u32` in the current scope
[00:39:41] -  --> $DIR/suggest-methods.rs:35:19
[00:39:41] +  --> $DIR/suggest-methods.rs:38:19
[00:39:41]     |
[00:39:41]  38 |     let _ = 63u32.count_o();
[00:39:41]     |                   ^^^^^^^
[00:39:41]  
[00:39:41]  error: aborting due to 4 previous errors
[00:39:41]  
[00:39:41] 
[00:39:41] The actual stderr differed from the expected stderr.
[00:39:41] Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/suggestions/suggest-methods.stderr
[00:39:41] To update references, run this command from build directory:
[00:39:41] /checkout/src/test/ui/update-references.sh '/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui' 'suggestions/suggest-methods.rs'
[00:39:41] 
[00:39:41] error: 1 errors occurred comparing output.
[00:39:41] status: exit code: 101
[00:39:41] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/suggestions/suggest-methods.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--target=x86_64-unknown-linux-gnu" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/suggestions/suggest-methods.stage2-x86_64-unknown-linux-gnu" "-Crpath" "-O" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/suggestions/suggest-methods.stage2-x86_64-unknown-linux-gnu.ui.libaux" "-A" "unused"
[00:39:41] stdout:
[00:39:41] ------------------------------------------
[00:39:41] 
[00:39:41] ------------------------------------------
[00:39:41] stderr:
[00:39:41] ------------------------------------------
[00:39:41] error[E0599]: no method named `bat` found for type `Foo` in the current scope
[00:39:41]   --> /checkout/src/test/ui/suggestions/suggest-methods.rs:28:7
[00:39:41]    |
[00:39:41] 28 |     f.bat(1.0);
[00:39:41]    |       ^^^
[00:39:41]    |
[00:39:41]    = help: did you mean `bar`?
[00:39:41] 
[00:39:41] error[E0599]: no method named `is_emtpy` found for type `std::string::String` in the current scope
[00:39:41]   --> /checkout/src/test/ui/suggestions/suggest-methods.rs:31:15
[00:39:41]    |
[00:39:41] 31 |     let _ = s.is_emtpy();
[00:39:41]    |               ^^^^^^^^
[00:39:41]    |
[00:39:41]    = help: did you mean `is_empty`?
[00:39:41] 
[00:39:41] error[E0599]: no method named `count_eos` found for type `u32` in the current scope
[00:39:41]   --> /checkout/src/test/ui/suggestions/suggest-methods.rs:35:19
[00:39:41]    |
[00:39:41] 35 |     let _ = 63u32.count_eos();
[00:39:41]    |                   ^^^^^^^^^
[00:39:41]    |
[00:39:41]    = help: did you mean `count_zeros`?
[00:39:41] 
[00:39:41] error[E0599]: no method named `count_o` found for type `u32` in the current scope
[00:39:41]   --> /checkout/src/test/ui/suggestions/suggest-methods.rs:38:19
[00:39:41]    |
[00:39:41] 38 |     let _ = 63u32.count_o();
[00:39:41]    |                   ^^^^^^^
[00:39:41] 
[00:39:41] error: aborting due to 4 previous errors
[00:39:41] 
[00:39:41] 
[00:39:41] ------------------------------------------
[00:39:41] 
[00:39:41] thread '[ui] ui/suggestions/suggest-methods.rs' panicked at 'explicit panic', /checkout/src/tools/compiletest/src/runtest.rs:2433:8
[00:39:41] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:39:41] 
[00:39:41] 
[00:39:41] failures:
[00:39:41]     [ui] ui/suggestions/suggest-methods.rs
[00:39:41] 
[00:39:41] test result: �[31mFAILED�(B�[m. 400 passed; 1 failed; 1 ignored; 0 measured; 0 filtered out

arielb1 · 2017-09-24T15:39:57Z

@bors r+

bors · 2017-09-24T15:39:58Z

📌 Commit 10098e2 has been approved by arielb1

src/test/ui/suggestions/suggest-methods.rs

The convention for suggesting close matches is to provide at most one match (the closest one). Change the suggestions for misspelt method names to obey that.

laumann · 2017-09-24T20:28:07Z

@arielb1 @goffrie It should be OK now, I think.

arielb1 · 2017-09-25T09:37:27Z

@bors r+

bors · 2017-09-25T09:37:28Z

📌 Commit 4963394 has been approved by arielb1

bors · 2017-09-25T23:09:11Z

⌛ Testing commit 4963394 with merge 82ae968...

Add suggestions for misspelled method names Use the syntax::util::lev_distance module to provide suggestions when a named method cannot be found. Part of #30197

bors · 2017-09-26T02:44:41Z

☀️ Test successful - status-appveyor, status-travis
Approved by: arielb1
Pushing 82ae968 to master...

rust-highfive assigned eddyb Sep 3, 2017

rust-highfive assigned nikomatsakis and unassigned eddyb Sep 4, 2017

rust-highfive assigned arielb1 and unassigned nikomatsakis Sep 5, 2017

arielb1 added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 5, 2017

laumann force-pushed the suggest-misspelt-methods branch from a54f408 to 4a84be4 Compare September 14, 2017 20:26

arielb1 reviewed Sep 14, 2017

View reviewed changes

laumann force-pushed the suggest-misspelt-methods branch 4 times, most recently from a3768c2 to 6fbf991 Compare September 17, 2017 20:19

laumann commented Sep 17, 2017

View reviewed changes

laumann commented Sep 19, 2017

View reviewed changes

arielb1 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 19, 2017

arielb1 reviewed Sep 20, 2017

View reviewed changes

laumann force-pushed the suggest-misspelt-methods branch from 68a6212 to 452524e Compare September 20, 2017 13:59

Add suggestions for misspelled method names

09defbc

Use the syntax::util::lev_distance module to provide suggestions when a named method cannot be found. Part of rust-lang#30197

laumann force-pushed the suggest-misspelt-methods branch from 452524e to 09defbc Compare September 21, 2017 10:06

laumann force-pushed the suggest-misspelt-methods branch from 24a0af8 to 8eff63f Compare September 21, 2017 11:09

laumann force-pushed the suggest-misspelt-methods branch from 8eff63f to 5d72356 Compare September 21, 2017 14:25

laumann force-pushed the suggest-misspelt-methods branch 2 times, most recently from b2685dd to 10098e2 Compare September 21, 2017 20:47

goffrie reviewed Sep 24, 2017

View reviewed changes

src/test/ui/suggestions/suggest-methods.rs Outdated Show resolved Hide resolved

laumann force-pushed the suggest-misspelt-methods branch from 10098e2 to 4963394 Compare September 24, 2017 17:59

Change Levensthein-based method to a single suggestion

4963394

The convention for suggesting close matches is to provide at most one match (the closest one). Change the suggestions for misspelt method names to obey that.

bors merged commit 4963394 into rust-lang:master Sep 26, 2017

laumann deleted the suggest-misspelt-methods branch September 26, 2017 07:22

petrochenkov mentioned this pull request Sep 26, 2017

Add levenshtein distance based suggestions everywhere #30197

Open

Add suggestions for misspelled method names #44297

Add suggestions for misspelled method names #44297

Conversation

laumann commented Sep 3, 2017

rust-highfive commented Sep 3, 2017

laumann commented Sep 3, 2017

eddyb commented Sep 4, 2017

arielb1 commented Sep 4, 2017

laumann commented Sep 5, 2017

nikomatsakis commented Sep 5, 2017

aidanhs commented Sep 13, 2017

laumann commented Sep 14, 2017

arielb1 commented Sep 14, 2017

arielb1 commented Sep 14, 2017

laumann commented Sep 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arielb1 Sep 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laumann commented Sep 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laumann commented Sep 20, 2017

arielb1 commented Sep 21, 2017

arielb1 commented Sep 21, 2017

bors commented Sep 21, 2017

bors commented Sep 21, 2017

bors commented Sep 21, 2017

arielb1 commented Sep 21, 2017

arielb1 commented Sep 21, 2017

arielb1 commented Sep 24, 2017

bors commented Sep 24, 2017

laumann commented Sep 24, 2017

arielb1 commented Sep 25, 2017

bors commented Sep 25, 2017

bors commented Sep 25, 2017

bors commented Sep 26, 2017

arielb1 Sep 20, 2017 •

edited

Loading