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

[Fix #8347] Fix an incorrect auto-correct for Style/HashSyntax #8351

Merged
merged 1 commit into from Jul 18, 2020
Merged

[Fix #8347] Fix an incorrect auto-correct for Style/HashSyntax #8351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -9,6 +9,7 @@
* [#8323](https://github.com/rubocop-hq/rubocop/issues/8323): Fix a false positive for `Style/HashAsLastArrayItem` when hash is not a last array item. ([@fatkodima][])
* [#8299](https://github.com/rubocop-hq/rubocop/issues/8299): Fix an incorrect auto-correct for `Style/RedundantCondition` when using `raise`, `rescue`, or `and` without argument parentheses in `else`. ([@koic][])
* [#8335](https://github.com/rubocop-hq/rubocop/issues/8335): Fix incorrect character class detection for nested or POSIX bracket character classes in `Style/RedundantRegexpEscape`. ([@owst][])
* [#8347](https://github.com/rubocop-hq/rubocop/issues/8347): Fix an incorrect auto-correct for `EnforcedStyle: hash_rockets` of `Style/HashSyntax` with `Layout/HashAlignment`. ([@koic][])

### Changes

Expand Down
3 changes: 2 additions & 1 deletion lib/rubocop/cop/style/hash_syntax.rb
View file
Open in desktop
Expand Up @@ -199,7 +199,8 @@ def argument_without_space?(node)
def autocorrect_hash_rockets(corrector, pair_node)
op = pair_node.loc.operator

corrector.wrap(pair_node.key, ':', pair_node.inverse_delimiter(true))
key_with_hash_rocket = ":#{pair_node.key.source}#{pair_node.inverse_delimiter(true)}"
corrector.replace(pair_node.key, key_with_hash_rocket)
corrector.remove(range_with_surrounding_space(range: op))
end

Expand Down
22 changes: 22 additions & 0 deletions spec/rubocop/cli/cli_autocorrect_spec.rb
View file
Open in desktop
Expand Up @@ -174,6 +174,28 @@ def batch
RUBY
end

it 'corrects `EnforcedStyle: hash_rockets` of `Style/HashSyntax` with `Layout/HashAlignment`' do
create_file('.rubocop.yml', <<~YAML)
Style/HashSyntax:
EnforcedStyle: hash_rockets
YAML
source = <<~RUBY
some_method(a: 'abc', b: 'abc',
c: 'abc', d: 'abc'
)
RUBY
create_file('example.rb', source)
expect(cli.run([
'--auto-correct-all',
'--only', 'Style/HashSyntax,Style/HashAlignment'
])).to eq(0)
expect(IO.read('example.rb')).to eq(<<~RUBY)
some_method(:a => 'abc', :b => 'abc',
:c => 'abc', :d => 'abc'
)
RUBY
end

Copy link

Choose a reason for hiding this comment

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

Amazing - I just spent about four hours trying to get this to work via way overcomplicating the expect_correction method (basically by adding a let(:run_first) variable). This is a million times cleaner. :) Going to miss having a PR to Rubocop but I guess I'll have other opportunities!

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious: what was the difficulty with expect_correction? This test with the CLI is much slower...

Copy link

Choose a reason for hiding this comment

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

The issue is that you can only run one cop at a time. There's no easy way to pass it multiple cops to run one after another. This bug only happens when one cop makes a change and the next one makes a second change - the two insertions end up in an invalid state.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was under the impression that let(:other_cops) { { 'Other/Cop' => ... } } was the way to go, but indeed looks like it's not the case. I opened #8352 as it should be easily doable to specify additional cops.

describe 'trailing comma cops' do
let(:source) do
<<~RUBY
Expand Down