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

Allow chained Hash#[] when BlockDelimiters braces_for_chaining is true #6847

Merged
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -10,6 +10,7 @@
* [#6998](https://github.com/rubocop-hq/rubocop/pull/6998): Fix autocorrect of `Naming/RescuedExceptionsVariableName` to also rename all references to the variable. ([@Darhazer][])
* [#6992](https://github.com/rubocop-hq/rubocop/pull/6992): Fix unknown default configuration for `Layout/IndentFirstParameter` cop. ([@drenmi][])
* [#6972](https://github.com/rubocop-hq/rubocop/issues/6972): Fix a false positive for `Style/MixinUsage` when using inside block and `if` condition is after `include`. ([@koic][])
* [#6847](https://github.com/rubocop-hq/rubocop/pull/6847): Fix `Style/BlockDelimiters` to properly check if the node is chaned when `braces_for_chaining` is set. ([@att14][])

## 0.68.0 (2019-04-29)

Expand Down Expand Up @@ -3989,3 +3990,4 @@
[@andreaseger]: https://github.com/andreaseger
[@yakout]: https://github.com/yakout
[@RicardoTrindade]: https://github.com/RicardoTrindade
[@att14]: https://github.com/att14
8 changes: 2 additions & 6 deletions lib/rubocop/cop/style/block_delimiters.rb
Expand Up @@ -165,7 +165,7 @@ def semantic_message(node)

def braces_for_chaining_message(node)
if node.multiline?
if return_value_chaining?(node)
if node.chained?
'Prefer `{...}` over `do...end` for multi-line chained blocks.'
else
'Prefer `do...end` for multi-line blocks without chaining.'
Expand Down Expand Up @@ -267,7 +267,7 @@ def braces_for_chaining_style?(node)
block_begin = node.loc.begin.source

block_begin == if node.multiline?
(return_value_chaining?(node) ? '{' : 'do')
(node.chained? ? '{' : 'do')
else
'{'
end
Expand All @@ -277,10 +277,6 @@ def braces_style?(node)
node.loc.begin.source == '{'
end

def return_value_chaining?(node)
node.parent && node.parent.send_type? && node.parent.dot?
end

def correction_would_break_code?(node)
return unless node.keywords?

Expand Down
39 changes: 39 additions & 0 deletions spec/rubocop/cop/style/block_delimiters_spec.rb
Expand Up @@ -490,6 +490,45 @@
RUBY
end

it 'allows when :[] is chained' do
expect_no_offenses(<<-RUBY.strip_indent)
foo = [{foo: :bar}].find { |h|
h.key?(:foo)
}[:foo]
RUBY
end

it 'allows do/end inside Hash[]' do
expect_no_offenses(<<-RUBY.strip_indent)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This example doesn't match the it description. In general I'd have one it per example.

Copy link
Author

Choose a reason for hiding this comment

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

👌

Hash[
{foo: :bar}.map do |k, v|
[k, v]
end
]
RUBY
end

it 'allows chaining to } inside of Hash[]' do
expect_no_offenses(<<-RUBY.strip_indent)
Hash[
{foo: :bar}.map { |k, v|
[k, v]
}.uniq
]
RUBY
end

it 'disallows {} with no chain inside of Hash[]' do
expect_offense(<<-RUBY.strip_indent)
Hash[
{foo: :bar}.map { |k, v|
^ Prefer `do...end` for multi-line blocks without chaining.
[k, v]
}
]
RUBY
end

context 'when there are braces around a multi-line block' do
it 'registers an offense in the simple case' do
expect_offense(<<-RUBY.strip_indent)
Expand Down