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.

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

Improving auto correction's API #7868

Merged
merged 1 commit into from Jun 22, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -2,6 +2,10 @@

## master (unreleased)

### New features

* [#7868](https://github.com/rubocop-hq/rubocop/pull/7868): `Cop::Base` is the new recommended base class for cops. Extensive refactoring of `Team`, `Commissioner`. ([@marcandre][])

## 0.86.0 (2020-06-22)

### New features
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/nav.adoc
Expand Up @@ -18,6 +18,7 @@
** xref:cops_naming.adoc[Naming]
** xref:cops_security.adoc[Security]
** xref:cops_style.adoc[Style]
** xref:v1_upgrade_notes.adoc[Style]
* xref:extensions.adoc[Extensions]
* xref:integration_with_other_tools.adoc[Integration with Other Tools]
* xref:automated_code_review.adoc[Automated Code Review]
Expand Down
3 changes: 1 addition & 2 deletions docs/modules/ROOT/pages/cops_lint.adoc
Expand Up @@ -3274,8 +3274,7 @@ end
| -
|===

This is not actually a cop. It does not inspect anything. It just
provides methods to repack Parser's diagnostics/errors
This cop repacks Parser's diagnostics/errors
into RuboCop's offenses.

== Lint/ToJSON
Expand Down
61 changes: 37 additions & 24 deletions docs/modules/ROOT/pages/development.adoc
Expand Up @@ -186,10 +186,13 @@ the expression achieved previously:
[source,ruby]
----
def_node_matcher :not_empty_call?, <<~PATTERN
(send (send (...) :empty?) :!)
(send (send $(...) :empty?) :!)
PATTERN
----

Note that we added a `$` sign to capture the "expression" in `!<expression>.empty?`,
it will become useful later.

Get yourself familiar with the AST node hooks that
https://www.rubydoc.info/gems/parser/Parser/AST/Processor[`parser`]
and https://www.rubydoc.info/gems/rubocop-ast/RuboCop/AST/Traversal[`rubocop-ast`]
Expand Down Expand Up @@ -222,11 +225,11 @@ module RuboCop
#
# # good
# array.any?
class SimplifyNotEmptyWithAny < Cop
class SimplifyNotEmptyWithAny < Base
MSG = 'Use `.any?` and remove the negation part.'.freeze

def_node_matcher :not_empty_call?, <<~PATTERN
(send (send (...) :empty?) :!)
(send (send $(...) :empty?) :!)
PATTERN

def on_send(node)
Expand All @@ -244,10 +247,7 @@ Update the spec to cover the expected syntax:

[source,ruby]
----
describe RuboCop::Cop::Style::SimplifyNotEmptyWithAny do
let(:config) { RuboCop::Config.new }
subject(:cop) { described_class.new(config) }

describe RuboCop::Cop::Style::SimplifyNotEmptyWithAny, :config do
it 'registers an offense when using `!a.empty?`' do
expect_offense(<<~RUBY)
!array.empty?
Expand All @@ -267,33 +267,45 @@ end
=== Auto-correct

The auto-correct can help humans automatically fix offenses that have been detected.
It's necessary to define an `autocorrect` method that returns a lambda
https://github.com/whitequark/parser/blob/master/lib/parser/rewriter.rb[rewriter]
with the corrector where you can give instructions about what to do with the
It's necessary to `extend AutoCorrector`.
The method `add_offense` yields a corrector object that is a thin wrapper on

Choose a reason for hiding this comment

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

Nit: It might be nice to link to the corrector class here

https://www.rubydoc.info/gems/parser/Parser/Source/TreeRewriter[parser's TreeRewriter]
to which you can give instructions about what to do with the
offensive node.

Let's start with a simple spec to cover it:

[source,ruby]
----
it 'autocorrect `!a.empty?` to `a.any?` ' do
expect(autocorrect_source('!a.empty?')).to eq('a.any?')
it 'corrects `!a.empty?`' do
expect_offense(<<~RUBY)
!array.empty?
^^^^^^^^^^^^^ Use `.any?` and remove the negation part.
RUBY

expect_correction(<<~RUBY)
array.any?
RUBY
end
----

And then define the `autocorrect` method on the cop side:
And then add the autocorrecting block on the cop side:

[source,ruby]
----
def autocorrect(node)
lambda do |corrector|
internal_expression = node.children[0].children[0].source
corrector.replace(node, "#{internal_expression}.any?")
extend AutoCorrector

def on_send(node)
expression = not_empty_call?(node)
return unless expression

add_offense(node) do |corrector|
corrector.replace(node, "#{expression.source}.any?")
end
end
----

The corrector allows you to `insert_after` and `insert_before` or
The corrector allows you to `insert_after`, `insert_before`, `wrap` or
`replace` a specific node or in any specific range of the code.

Range can be determined on `node.location` where it brings specific
Expand All @@ -319,12 +331,13 @@ And then on the autocorrect method, you just need to use the `cop_config` it:

[source,ruby]
----
def autocorrect(node)
lambda do |corrector|
internal_expression = node.children[0].children[0].source
replacement = cop_config['ReplaceAnyWith'] || "any?"
new_expression = "#{internal_expression}.#{replacement}"
corrector.replace(node, new_expression)
def on_send(node)
expression = not_empty_call?(node)
return unless expression

add_offense(node) do |corrector|
replacement = cop_config['ReplaceAnyWith'] || 'any?'
corrector.replace(node, "#{expression.source}.#{replacement}")
end
end
----
Expand Down