Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Display correction candidate if an incorrect cop name is given #5300

Merged
merged 1 commit into from
Dec 24, 2017
Merged

Display correction candidate if an incorrect cop name is given #5300

merged 1 commit into from
Dec 24, 2017

Conversation

yhirano55
Copy link
Contributor

@yhirano55 yhirano55 commented Dec 22, 2017 •
edited

Maybe we cannot memorize correctly cop names.

if an incorrect cop name is given with --only or --except options on CLI option,
it just only raises exception. I think it's not friendly.

So I've added its error message to correction candidate.

Example:

$ bundle exec rubocop --only Lint/Li
Inspecting 1081 files

0 files inspected, no offenses detected
Unrecognized cop or department: Lint/Li.
Did you mean? Lint/LiteralAsCondition, Lint/LiteralInInterpolation
...

Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.
  • Run rake default or rake parallel. It executes all tests and RuboCop for itself, and generates the documentation.

garettarrowood
garettarrowood approved these changes Dec 22, 2017
View reviewed changes
Copy link
Contributor

@garettarrowood garettarrowood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice improvement!

@yhirano55
Copy link
Contributor Author

...Mmm, It seems travis's bundler is a bad mood.

@garettarrowood
Copy link
Contributor

You should be good with a rebase.

@yhirano55
Display correction candidate if an incorrect cop name is given
aaa815d 
Maybe we cannot memorize correctly cop names. if an incorrect
cop name is given with `--only` or `--except` options on CLI option,
it just only raises exception. I think it's not friendly.

So I've added its error message to correction candidate.

Example:

```
$ bundle exec rubocop --only Lint/Li
Inspecting 1081 files

0 files inspected, no offenses detected
Unrecognized cop or department: Lint/Li.
Did you mean? Lint/LiteralAsCondition, Lint/LiteralInInterpolation
...
```
pocke
pocke approved these changes Dec 23, 2017
View reviewed changes
Copy link
Collaborator

@pocke pocke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇

@pocke
Copy link
Collaborator

pocke commented Dec 23, 2017

BTW, the message should also include candidates that are found with some similarity algorithm (e.g. Levenshtein distance).

@bbatsov bbatsov merged commit 0ea61a1 into rubocop:master Dec 24, 2017
@bbatsov
Copy link
Collaborator

bbatsov commented Dec 24, 2017

Yeah, that could be a nice future improvement.

@yhirano55 yhirano55 deleted the display_correction_candidate branch December 24, 2017 10:37
@koic koic mentioned this pull request Dec 25, 2017
Merged
8 tasks
koic added a commit to koic/rubocop that referenced this pull request Dec 25, 2017
@koic
Suppress backtrace after correct candidate cop name
d3740e7 
Follow up of rubocop#5300.

This commit suppresses backtrace when the following case.

## Before

```console
% rubocop --only Lint/RedundantWithI
Inspecting 1081 files

0 files inspected, no offenses detected
Unrecognized cop or department: Lint/RedundantWithI.
Did you mean? Lint/RedundantWithIndex
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/options.rb:200:in `block in validate_cop_list'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/options.rb:195:in `each'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/options.rb:195:in `validate_cop_list'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:275:in `block in mobilized_cop_classes'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:274:in `each'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:274:in `mobilized_cop_classes'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:257:in `inspect_file'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:205:in `block in do_inspection_loop'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:237:in `block in iterate_until_no_changes'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:230:in `loop'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:230:in `iterate_until_no_changes'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:201:in `do_inspection_loop'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:111:in `block in file_offenses'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:121:in `file_offense_cache'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:109:in `file_offenses'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:100:in `process_file'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:78:in `block in each_inspected_file'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:75:in `each'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:75:in `reduce'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:75:in `each_inspected_file'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:67:in `inspect_files'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:39:in `run'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/cli.rb:143:in `execute_runner'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/cli.rb:71:in `execute_runners'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/cli.rb:35:in `run'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/bin/rubocop:13:in `block in <top (required)>'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/2.4.0/benchmark.rb:308:in `realtime'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/bin/rubocop:12:in `<top (required)>'
/Users/koic/.rbenv/versions/2.4.3/bin/rubocop:23:in `load'
/Users/koic/.rbenv/versions/2.4.3/bin/rubocop:23:in `<main>'
```

## After

```console
% rubocop --only Lint/RedundantWithI
Inspecting 1081 files

0 files inspected, no offenses detected
Unrecognized cop or department: Lint/RedundantWithI.
Did you mean? Lint/RedundantWithIndex
```

As a general RuboCop users, it is undesirable to have the correct
candidate cop name buried in the backtrace.
bbatsov pushed a commit that referenced this pull request Dec 25, 2017
@koic @bbatsov
Suppress backtrace after correct candidate cop name
3e064e9 
Follow up of #5300.

This commit suppresses backtrace when the following case.

## Before

```console
% rubocop --only Lint/RedundantWithI
Inspecting 1081 files

0 files inspected, no offenses detected
Unrecognized cop or department: Lint/RedundantWithI.
Did you mean? Lint/RedundantWithIndex
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/options.rb:200:in `block in validate_cop_list'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/options.rb:195:in `each'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/options.rb:195:in `validate_cop_list'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:275:in `block in mobilized_cop_classes'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:274:in `each'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:274:in `mobilized_cop_classes'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:257:in `inspect_file'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:205:in `block in do_inspection_loop'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:237:in `block in iterate_until_no_changes'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:230:in `loop'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:230:in `iterate_until_no_changes'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:201:in `do_inspection_loop'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:111:in `block in file_offenses'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:121:in `file_offense_cache'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:109:in `file_offenses'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:100:in `process_file'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:78:in `block in each_inspected_file'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:75:in `each'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:75:in `reduce'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:75:in `each_inspected_file'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:67:in `inspect_files'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/runner.rb:39:in `run'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/cli.rb:143:in `execute_runner'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/cli.rb:71:in `execute_runners'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/lib/rubocop/cli.rb:35:in `run'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/bin/rubocop:13:in `block in <top (required)>'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/2.4.0/benchmark.rb:308:in `realtime'
/Users/koic/.rbenv/versions/2.4.3/lib/ruby/gems/2.4.0/gems/rubocop-0.52.0/bin/rubocop:12:in `<top (required)>'
/Users/koic/.rbenv/versions/2.4.3/bin/rubocop:23:in `load'
/Users/koic/.rbenv/versions/2.4.3/bin/rubocop:23:in `<main>'
```

## After

```console
% rubocop --only Lint/RedundantWithI
Inspecting 1081 files

0 files inspected, no offenses detected
Unrecognized cop or department: Lint/RedundantWithI.
Did you mean? Lint/RedundantWithIndex
```

As a general RuboCop users, it is undesirable to have the correct
candidate cop name buried in the backtrace.
koic added a commit to koic/rubocop that referenced this pull request Dec 27, 2017
@koic
Improve searching for correct cop candidates
32152ea 
Follow up of rubocop#5300 (comment).

This commit changed the search approach of the correct cop candidates
from prefix-match by regular expression to `StringUtil.similarity`.

## Before

When there is no prefix-match, candidates are not displayed.

```console
% rubocop --only Lint/RedundanWithI
Inspecting 1083 files

0 files inspected, no offenses detected
Unrecognized cop or department: Lint/RedundanWithI.
```

## After

When there is no prefix-match, candidate is displayed.

```console
% rubocop --only Lint/RedundanWithI
Inspecting 1083 files

0 files inspected, no offenses detected
Unrecognized cop or department: Lint/RedundanWithI.
Did you mean? Lint/RedundantWithIndex
```

Cop candidates different from user's expectations may be displayed,
however I think that there is no problem in cop names of similarity to
typing miss.

```console
% rubocop --only Lint/RedundanWithO
Inspecting 1083 files

0 files inspected, no offenses detected
Unrecognized cop or department: Lint/RedundanWithO.
Did you mean? Lint/RandOne, Lint/RedundantWithObject
```

## Other Information

Initially I thought about using the module of did_you_mean which is a
standard library of Ruby 2.3 or higher. However, since the following
code does not work unless it is Ruby 2.4 or higher, I decided to use
`StringUtil.similarity`.

```ruby
DidYouMean::SpellChecker.new(dictionary: cop_names).correct(name)
```

Also, since it displays multiple cop candidates, this commit did not
use `NameSimilarity#find_similar_name`.
@koic koic mentioned this pull request Dec 27, 2017
Merged
8 tasks
bbatsov pushed a commit that referenced this pull request Dec 27, 2017
@koic @bbatsov
Improve searching for correct cop candidates
ebf6e01 
Follow up of #5300 (comment).

This commit changed the search approach of the correct cop candidates
from prefix-match by regular expression to `StringUtil.similarity`.

## Before

When there is no prefix-match, candidates are not displayed.

```console
% rubocop --only Lint/RedundanWithI
Inspecting 1083 files

0 files inspected, no offenses detected
Unrecognized cop or department: Lint/RedundanWithI.
```

## After

When there is no prefix-match, candidate is displayed.

```console
% rubocop --only Lint/RedundanWithI
Inspecting 1083 files

0 files inspected, no offenses detected
Unrecognized cop or department: Lint/RedundanWithI.
Did you mean? Lint/RedundantWithIndex
```

Cop candidates different from user's expectations may be displayed,
however I think that there is no problem in cop names of similarity to
typing miss.

```console
% rubocop --only Lint/RedundanWithO
Inspecting 1083 files

0 files inspected, no offenses detected
Unrecognized cop or department: Lint/RedundanWithO.
Did you mean? Lint/RandOne, Lint/RedundantWithObject
```

## Other Information

Initially I thought about using the module of did_you_mean which is a
standard library of Ruby 2.3 or higher. However, since the following
code does not work unless it is Ruby 2.4 or higher, I decided to use
`StringUtil.similarity`.

```ruby
DidYouMean::SpellChecker.new(dictionary: cop_names).correct(name)
```

Also, since it displays multiple cop candidates, this commit did not
use `NameSimilarity#find_similar_name`.
This was referenced Dec 27, 2017
Open
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pocke pocke pocke approved these changes

@garettarrowood garettarrowood garettarrowood approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@yhirano55 @garettarrowood @pocke @bbatsov