Skip to content

Commit

Permalink
Add AllowPercentLiteralArrayArgument option to `Lint/RedundantSplat…
Browse files Browse the repository at this point in the history
…Expansion`

Resolves standardrb/standard#223.

This PR adds `AllowPercentLiteralArrayArgument` option to `Lint/RedundantSplatExpansion.

I think it makes sense to allow the following splat usage.

```ruby
expect(data).to include(*%w[
  id type firstName lastName displayName photo
  phoneNumber publicPhone primaryMobile publicMobile publicEmail
  social primaryFacility facilities
])
```

Therefore this PR defaults to `AllowPercentLiteralArrayArgument: true`.
User can set `AllowPercentLiteralArrayArgument: false` if prefer
RuboCop 1.6 or lower behavior.
  • Loading branch information
koic authored and bbatsov committed Dec 25, 2020
1 parent bee3bb5 commit 7d1dece
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 51 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#9285](https://github.com/rubocop-hq/rubocop/pull/9285): Add `AllowPercentLiteralArrayArgument` option for `Lint/RedundantSplatExpansion` to enable the option by default. ([@koic][])
2 changes: 2 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1858,6 +1858,8 @@ Lint/RedundantSplatExpansion:
Description: 'Checks for splat unnecessarily being called on literals.'
Enabled: true
VersionAdded: '0.76'
VersionChanged: <<next>>
AllowPercentLiteralArrayArgument: true

Lint/RedundantStringCoercion:
Description: 'Checks for Object#to_s usage in string interpolation.'
Expand Down
67 changes: 50 additions & 17 deletions lib/rubocop/cop/lint/redundant_splat_expansion.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,47 +8,66 @@ module Lint
# @example
#
# # bad
#
# a = *[1, 2, 3]
# a = *'a'
# a = *1
#
# begin
# foo
# rescue *[StandardError, ApplicationError]
# bar
# end
#
# case foo
# when *[1, 2, 3]
# bar
# else
# baz
# end
#
# @example
# ['a', 'b', *%w(c d e), 'f', 'g']
#
# # good
#
# c = [1, 2, 3]
# a = *c
# a, b = *c
# a, *b = *c
# a = *1..10
# a = ['a']
# ['a', 'b', 'c', 'd', 'e', 'f', 'g']
#
# # bad
# do_something(*['foo', 'bar', 'baz'])
#
# # good
# do_something('foo', 'bar', 'baz')
#
# # bad
# begin
# foo
# rescue *[StandardError, ApplicationError]
# bar
# end
#
# # good
# begin
# foo
# rescue StandardError, ApplicationError
# bar
# end
#
# # bad
# case foo
# when *[1, 2, 3]
# bar
# else
# baz
# end
#
# # good
# case foo
# when 1, 2, 3
# bar
# else
# baz
# end
#
# @example AllowPercentLiteralArrayArgument: true (default)
#
# # good
# do_something(*%w[foo bar baz])
#
# @example AllowPercentLiteralArrayArgument: false
#
# # bad
# do_something(*%w[foo bar baz])
#
class RedundantSplatExpansion < Base
extend AutoCorrector

Expand All @@ -75,6 +94,9 @@ def on_splat(node)
redundant_splat_expansion(node) do
if array_splat?(node) &&
(method_argument?(node) || part_of_an_array?(node))
return if allow_percent_literal_array_argument? &&
use_percent_literal_array_argument?(node)

add_offense(node, message: ARRAY_PARAM_MSG) do |corrector|
autocorrect(corrector, node)
end
Expand Down Expand Up @@ -170,6 +192,17 @@ def remove_brackets(array)
elements.join(', ')
end
end

def use_percent_literal_array_argument?(node)
argument = node.children.first

node.parent.send_type? &&
(argument.percent_literal?(:string) || argument.percent_literal?(:symbol))
end

def allow_percent_literal_array_argument?
cop_config.fetch('AllowPercentLiteralArrayArgument', true)
end
end
end
end
Expand Down
101 changes: 67 additions & 34 deletions spec/rubocop/cop/lint/redundant_splat_expansion_spec.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Lint::RedundantSplatExpansion do
subject(:cop) { described_class.new }

RSpec.describe RuboCop::Cop::Lint::RedundantSplatExpansion, :config do
it 'allows assigning to a splat' do
expect_no_offenses('*, rhs = *node')
end
Expand Down Expand Up @@ -77,10 +75,6 @@
end

it_behaves_like 'array splat expansion', '[1, 2, 3]', as_args: '1, 2, 3'
it_behaves_like 'array splat expansion', '%w(one two three)',
as_args: "'one', 'two', 'three'"
it_behaves_like 'array splat expansion', '%W(one #{two} three)',
as_args: '"one", "#{two}", "three"'
it_behaves_like 'splat expansion', "'a'", as_array: "['a']"
it_behaves_like 'splat expansion', '"#{a}"', as_array: '["#{a}"]'
it_behaves_like 'splat expansion', '1', as_array: '[1]'
Expand Down Expand Up @@ -333,56 +327,95 @@
end
end

it_behaves_like 'array splat expansion', '%i(first second)',
as_args: ':first, :second'
it_behaves_like 'array splat expansion', '%I(first second #{third})',
as_args: ':"first", :"second", :"#{third}"'

context 'arrays being expanded with %i variants using splat expansion' do
context 'splat expansion of method parameters' do
it 'registers an offense and corrects an array literal %i' do
context 'splat expansion inside of an array' do
it 'registers an offense and corrects %i to a list of symbols' do
expect_offense(<<~RUBY)
array.push(*%i(first second))
^^^^^^^^^^^^^^^^^ Pass array contents as separate arguments.
[:a, :b, *%i(c d), :e]
^^^^^^^^ Pass array contents as separate arguments.
RUBY

expect_correction(<<~RUBY)
array.push(:first, :second)
[:a, :b, :c, :d, :e]
RUBY
end

it 'registers an offense and corrects an array literal %I' do
expect_offense(<<~RUBY)
array.push(*%I(\#{first} second))
^^^^^^^^^^^^^^^^^^^^ Pass array contents as separate arguments.
it 'registers an offense and changes %I to a list of symbols' do
expect_offense(<<~'RUBY')
[:a, :b, *%I(#{one} two), :e]
^^^^^^^^^^^^^^^ Pass array contents as separate arguments.
RUBY

expect_correction(<<~RUBY)
array.push(:"\#{first}", :"second")
expect_correction(<<~'RUBY')
[:a, :b, :"#{one}", :"two", :e]
RUBY
end
end
end

context 'when `AllowPercentLiteralArrayArgument: true`' do
let(:cop_config) do
{ 'AllowPercentLiteralArrayArgument' => true }
end

context 'splat expansion inside of an array' do
it 'registers an offense and corrects %i to a list of symbols' do
it 'does not register an offense when using percent string literal array' do
expect_no_offenses(<<~'RUBY')
do_something(*%w[foo bar baz])
RUBY
end

it 'does not register an offense when using percent symbol literal array' do
expect_no_offenses(<<~'RUBY')
do_something(*%i[foo bar baz])
RUBY
end
end

context 'when `AllowPercentLiteralArrayArgument: false`' do
let(:cop_config) do
{ 'AllowPercentLiteralArrayArgument' => false }
end

it_behaves_like 'array splat expansion', '%w(one two three)', as_args: "'one', 'two', 'three'"
it_behaves_like 'array splat expansion', '%W(one #{two} three)', as_args: '"one", "#{two}", "three"'

it 'registers an offense when using percent literal array' do
expect_offense(<<~'RUBY')
do_something(*%w[foo bar baz])
^^^^^^^^^^^^^^^^ Pass array contents as separate arguments.
RUBY
end

it_behaves_like 'array splat expansion', '%i(first second)', as_args: ':first, :second'
it_behaves_like 'array splat expansion', '%I(first second #{third})', as_args: ':"first", :"second", :"#{third}"'

it 'registers an offense when using percent symbol literal array' do
expect_offense(<<~'RUBY')
do_something(*%i[foo bar baz])
^^^^^^^^^^^^^^^^ Pass array contents as separate arguments.
RUBY
end

context 'splat expansion of method parameters' do
it 'registers an offense and corrects an array literal %i' do
expect_offense(<<~RUBY)
[:a, :b, *%i(c d), :e]
^^^^^^^^ Pass array contents as separate arguments.
array.push(*%i(first second))
^^^^^^^^^^^^^^^^^ Pass array contents as separate arguments.
RUBY

expect_correction(<<~RUBY)
[:a, :b, :c, :d, :e]
array.push(:first, :second)
RUBY
end

it 'registers an offense and changes %I to a list of symbols' do
expect_offense(<<~'RUBY')
[:a, :b, *%I(#{one} two), :e]
^^^^^^^^^^^^^^^ Pass array contents as separate arguments.
it 'registers an offense and corrects an array literal %I' do
expect_offense(<<~RUBY)
array.push(*%I(\#{first} second))
^^^^^^^^^^^^^^^^^^^^ Pass array contents as separate arguments.
RUBY

expect_correction(<<~'RUBY')
[:a, :b, :"#{one}", :"two", :e]
expect_correction(<<~RUBY)
array.push(:"\#{first}", :"second")
RUBY
end
end
Expand Down

0 comments on commit 7d1dece

Please sign in to comment.