Skip to content

Commit

Permalink
Have correctors accept nodes in addition to ranges.
Browse files Browse the repository at this point in the history
Provide better errors in case of wrong argument
Before: "undefined method `source_buffer' for 1..3:Range"
After: "Expected a Parser::Source::Range or Rubocop::AST::Node, got Range"
  • Loading branch information
marcandre authored and bbatsov committed Apr 10, 2020
1 parent 00e8170 commit 82eb350
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 31 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### New features

* [#7863](https://github.com/rubocop-hq/rubocop/issues/7863): Corrector now accepts nodes in addition to ranges. ([@marcandre][])
* [#7862](https://github.com/rubocop-hq/rubocop/issues/7862): Corrector now has a `wrap` method. ([@marcandre][])
* [#7850](https://github.com/rubocop-hq/rubocop/issues/7850): Make it possible to enable/disable pending cops. ([@koic][])
* [#7861](https://github.com/rubocop-hq/rubocop/issues/7861): Make it to allow `Style/CaseEquality` when the receiver is a constant. ([@rafaelfranca][])
Expand Down
68 changes: 41 additions & 27 deletions lib/rubocop/cop/corrector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,18 +72,18 @@ def rewrite

# Removes the source range.
#
# @param [Parser::Source::Range] range
def remove(range)
validate_range range
# @param [Parser::Source::Range, Rubocop::AST::Node] range or node
def remove(node_or_range)
range = to_range(node_or_range)
@source_rewriter.remove(range)
end

# Inserts new code before the given source range.
#
# @param [Parser::Source::Range] range
# @param [Parser::Source::Range, Rubocop::AST::Node] range or node
# @param [String] content
def insert_before(range, content)
validate_range range
def insert_before(node_or_range, content)
range = to_range(node_or_range)
# TODO: Fix Cops using bad ranges instead
if range.end_pos > @source_buffer.source.size
range = range.with(end_pos: @source_buffer.source.size)
Expand All @@ -94,38 +94,38 @@ def insert_before(range, content)

# Inserts new code after the given source range.
#
# @param [Parser::Source::Range] range
# @param [Parser::Source::Range, Rubocop::AST::Node] range or node
# @param [String] content
def insert_after(range, content)
validate_range range
def insert_after(node_or_range, content)
range = to_range(node_or_range)
@source_rewriter.insert_after(range, content)
end

# Wraps the given source range with the given before and after texts
#
# @param [Parser::Source::Range] range
# @param [Parser::Source::Range, Rubocop::AST::Node] range or node
# @param [String] before
# @param [String] after
def wrap(range, before, after)
validate_range range
def wrap(node_or_range, before, after)
range = to_range(node_or_range)
@source_rewriter.wrap(range, before, after)
end

# Replaces the code of the source range `range` with `content`.
#
# @param [Parser::Source::Range] range
# @param [Parser::Source::Range, Rubocop::AST::Node] range or node
# @param [String] content
def replace(range, content)
validate_range range
def replace(node_or_range, content)
range = to_range(node_or_range)
@source_rewriter.replace(range, content)
end

# Removes `size` characters prior to the source range.
#
# @param [Parser::Source::Range] range
# @param [Parser::Source::Range, Rubocop::AST::Node] range or node
# @param [Integer] size
def remove_preceding(range, size)
validate_range range
def remove_preceding(node_or_range, size)
range = to_range(node_or_range)
to_remove = Parser::Source::Range.new(range.source_buffer,
range.begin_pos - size,
range.begin_pos)
Expand All @@ -136,10 +136,10 @@ def remove_preceding(range, size)
# If `size` is greater than the size of `range`, the removed region can
# overrun the end of `range`.
#
# @param [Parser::Source::Range] range
# @param [Parser::Source::Range, Rubocop::AST::Node] range or node
# @param [Integer] size
def remove_leading(range, size)
validate_range range
def remove_leading(node_or_range, size)
range = to_range(node_or_range)
to_remove = Parser::Source::Range.new(range.source_buffer,
range.begin_pos,
range.begin_pos + size)
Expand All @@ -150,10 +150,10 @@ def remove_leading(range, size)
# If `size` is greater than the size of `range`, the removed region can
# overrun the beginning of `range`.
#
# @param [Parser::Source::Range] range
# @param [Parser::Source::Range, Rubocop::AST::Node] range or node
# @param [Integer] size
def remove_trailing(range, size)
validate_range range
def remove_trailing(node_or_range, size)
range = to_range(node_or_range)
to_remove = Parser::Source::Range.new(range.source_buffer,
range.end_pos - size,
range.end_pos)
Expand All @@ -163,11 +163,25 @@ def remove_trailing(range, size)
private

# :nodoc:
def validate_range(range)
buffer = range.source_buffer
def to_range(node_or_range)
range = case node_or_range
when ::RuboCop::AST::Node, ::Parser::Source::Comment
node_or_range.loc.expression
when ::Parser::Source::Range
node_or_range
else
raise TypeError,
'Expected a Parser::Source::Range, Comment or ' \
"Rubocop::AST::Node, got #{node_or_range.class}"
end
validate_buffer(range.source_buffer)
range
end

def validate_buffer(buffer)
return if buffer == @source_buffer

unless buffer.is_a?(Parser::Source::Buffer)
unless buffer.is_a?(::Parser::Source::Buffer)
# actually this should be enforced by parser gem
raise 'Corrector expected range source buffer to be a ' \
"Parser::Source::Buffer, but got #{buffer.class}"
Expand Down
8 changes: 4 additions & 4 deletions manual/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -256,15 +256,15 @@ And then define the `autocorrect` method on the cop side:
def autocorrect(node)
lambda do |corrector|
internal_expression = node.children[0].children[0].source
corrector.replace(node.loc.expression, "#{internal_expression}.any?")
corrector.replace(node, "#{internal_expression}.any?")
end
end
```

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

The range can be determined on `node.location` where it brings specific
Range can be determined on `node.location` where it brings specific
ranges for expression or other internal information that the node holds.

### Configuration
Expand All @@ -290,7 +290,7 @@ def autocorrect(node)
internal_expression = node.children[0].children[0].source
replacement = cop_config['ReplaceAnyWith'] || "any?"
new_expression = "#{internal_expression}.#{replacement}"
corrector.replace(node.loc.expression, new_expression)
corrector.replace(node, new_expression)
end
end
```
Expand Down
19 changes: 19 additions & 0 deletions spec/rubocop/cop/corrector_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,25 @@ def do_rewrite(corrections = nil, &block)
end.to rewrite_to 'true a false'
end

it 'accepts a node instead of a range' do
expect do |corrector|
corrector.replace(node.rhs, 'maybe')
end.to rewrite_to 'true and maybe'
end

it 'raises a useful error if not given a node or a range' do
# rubocop:disable Style/MultilineBlockChain
expect do
do_rewrite { |corr| corr.replace(1..3, 'oops') }
end.to raise_error(RuboCop::ErrorWithAnalyzedFileLocation) do |e|
expect(e.cause.message).to eq(
'Expected a Parser::Source::Range, Comment or Rubocop::AST::Node, ' \
'got Range'
)
end
# rubocop:enable Style/MultilineBlockChain
end

context 'when range is from incorrect source' do
let(:other_source) { parse_source(source) }
let(:op_other) do
Expand Down

0 comments on commit 82eb350

Please sign in to comment.