Skip to content

Commit

Permalink
[Fix #948] Avoid auto-correction changing code meaning in Blocks
Browse files Browse the repository at this point in the history
Re-parse and check that the AST hasn't changed when doing auto-
correct in Blocks cop.
  • Loading branch information
jonas054 committed Apr 5, 2014
1 parent 2ba0c08 commit 89bb2fb
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* [#952](https://github.com/bbatsov/rubocop/pull/952): Handle implicit receiver in `StringConversionInInterpolation`. ([@bbatsov][])
* [#956](https://github.com/bbatsov/rubocop/pull/956): Apply `ClassMethods` check only on `class`/`module` bodies. ([@bbatsov][])
* [#945](https://github.com/bbatsov/rubocop/issues/945): Fix SpaceBeforeFirstArg cop for multiline argument and exclude assignments. ([@cschramm][])
* [#948](https://github.com/bbatsov/rubocop/issues/948): `Blocks` cop avoids auto-correction if it would introduce a semantic change. ([@jonas054][])

## 0.20.0 (02/04/2014)

Expand Down
20 changes: 17 additions & 3 deletions lib/rubocop/cop/style/blocks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,23 @@ def on_block(node)
end

def autocorrect(node)
@corrections << lambda do |corrector|
c = correction(node)
new_source = rewrite_node(node, c)

# Make the correction only if it doesn't change the AST.
@corrections << c if node == SourceParser.parse(new_source).ast
end

private

def rewrite_node(node, correction)
processed_source = SourceParser.parse(node.loc.expression.source)
c = correction(processed_source.ast)
Corrector.new(processed_source.buffer, [c]).rewrite
end

def correction(node)
lambda do |corrector|
b, e = node.loc.begin, node.loc.end
if b.is?('{')
# If the left brace is immediately preceded by a word character,
Expand All @@ -53,8 +69,6 @@ def autocorrect(node)
end
end

private

def get_block(node)
case node.type
when :block
Expand Down
6 changes: 6 additions & 0 deletions spec/rubocop/cop/style/blocks_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@
expect(new_source).to eq('block { |x| }')
end

it 'does not auto-correct do-end if {} would change the meaning' do
src = "s.subspec 'Subspec' do |sp| end"
new_source = autocorrect_source(cop, src)
expect(new_source).to eq(src)
end

context 'when there are braces around a multi-line block' do
it 'registers an offense in the simple case' do
inspect_source(cop, ['each { |x|',
Expand Down

0 comments on commit 89bb2fb

Please sign in to comment.