Rubocop: Wrong Lint/UnneededSplatExpansion

[YukiJikumaru]

Expected behavior

$ cat hoge.rb
# frozen_string_literal: true
# nodoc
class Hello < ApplicationRecord
  OTHER_COLUMNS = %i(
    column_B
    column_C
  ).freeze

  validates(
    *[:column_A, *OTHER_COLUMNS],
    allow_nil: true,
    numericality: {
      only_integer: true,
      greater_than_or_equal_to: 0,
      less_than_or_equal_to: 10
    }
  )
end

$ rubocop hoge.rb

This should not generate Lint/UnneededSplatExpansion offenses.

Actual behavior

$ rubocop hoge.rb
Inspecting 1 file
W

Offenses:

hoge.rb:10:5: W: Unnecessary splat expansion.
    *[:column_A, *OTHER_COLUMNS],
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected

$ rubocop --rails hoge.rb
Inspecting 1 file
W

Offenses:

hoge.rb:10:5: W: Unnecessary splat expansion.
    *[:column_A, *OTHER_COLUMNS],
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected

If modify like below, offense stops.

  validates(
+    *[:column_A, *OTHER_COLUMNS],
-    *[:column_A, *OTHER_COLUMNS],
    allow_nil: true,

However this code produces NoMethodError.

NoMethodError: undefined method `to_sym' for #<Array:0x007efe849b0828>

Steps to reproduce the problem

$ mkdir work
$ cd work
$ rubocop -V
0.43.0 (using Parser 2.3.1.3, running on ruby 2.3.1 x86_64-linux)
$ wget https://raw.githubusercontent.com/bbatsov/rubocop/v0.43.0/config/default.yml
$ wget https://raw.githubusercontent.com/bbatsov/rubocop/v0.43.0/config/enabled.yml
$ wget https://raw.githubusercontent.com/bbatsov/rubocop/v0.43.0/config/disabled.yml
$ cat <<EOF > hoge.rb && rubocop --config default.yml hoge.rb
# frozen_string_literal: true
# nodoc
class Hello < ApplicationRecord
  OTHER_COLUMNS = %i(
    column_B
    column_C
  ).freeze

  validates(
    *[:column_A, *OTHER_COLUMNS],
    allow_nil: true,
    numericality: {
      only_integer: true,
      greater_than_or_equal_to: 0,
      less_than_or_equal_to: 10
    }
  )
end
EOF

RuboCop version

$ rubocop -V
0.43.0 (using Parser 2.3.1.3, running on ruby 2.3.1 x86_64-linux)

[rikas]

Same here!

In Rails

{'one' => 1, 'two' => 2, 'three' => 3}.slice(*%w(one three))
=> {"one"=>1, "three"=>3}

Rubocop tells me

W: Unnecessary splat expansion.
        {'one' => 1, 'two' => 2, 'three' => 3}.slice(*%w(one three))

Which is wrong, because the result is not the same.

{'one' => 1, 'two' => 2, 'three' => 3}.slice(%w(one three))
=> {}

$ rubocop -V
0.43.0 (using Parser 2.3.1.3, running on ruby 2.3.0 x86_64-darwin15)

[tejasbubane]

Rubocop is detecting and correcting the offense. This does not seem to be a bug:

⇒  cat rubocop_bug_test.rb
{ 'one' => 1, 'two' => 2, 'three' => 3 }.slice(*%w(one three))

⇒  rubocop -a rubocop_bug_test.rb
Inspecting 1 file
W

Offenses:

rubocop_bug_test.rb:1:48: W: [Corrected] Unnecessary splat expansion.
{ 'one' => 1, 'two' => 2, 'three' => 3 }.slice(*%w(one three))
                                               ^^^^^^^^^^^^^^

1 file inspected, 1 offense detected, 1 offense corrected

⇒  cat rubocop_bug_test.rb
{ 'one' => 1, 'two' => 2, 'three' => 3 }.slice('one', 'three')

[NobodysNightmare]

Also false positive for Pathname#join:

Rails.root.join(*['a', 'b', 'c'])

Please tell me if you would rather like a separate issue.

[backus]

@NobodysNightmare I'm not sure what is wrong with your example. It correctly autocorrects to Rails.root.join('a', 'b', 'c'). What is the problem?

[NobodysNightmare]

Okay I see... Apparently the error message was simply confusing to me.

"Unneccessary splat expansion" implies to me, that I do not need to expand the array (i.e. leave the splat operator out). However, what the cop wants to tell me is that I could just have written the parameter list out manually... Right?

So it was me misunderstanding the cop, sorry...

[rikas]

@tejasbubane you're right. I didn't get the obvious change. 😞

[backus]

:thumbsup: no problem @NobodysNightmare

[rikas]

I agree that the message can be misinterpreted. That was also my interpretation, hence my example above wihtout the splat operator but keeping the array.

Maybe changing the error copy would be a good thing to do.

[tejasbubane]

Regarding the original issue by @YukiJikumaru (with splat inside splat). Rubocop works as expected.

original code:

validates(
    *[:column_A, *OTHER_COLUMNS]
)

gets autocorrected to:

validates(
  :column_A, *OTHER_COLUMNS
)

Hence this is not a bug.

Maybe changing the error copy would be a good thing to do.

@rikas Sent a PR.

[YukiJikumaru]

@tejasbubane Thank you for your answer!

[samnang]

It seems false positive here:

Rails.root.join(*%w(db seeders users.rb))

Rubocop complaints on this:

Lint/UnneededSplatExpansion: Pass array contents as separate arguments.

I know I could get rid of this warning with:

Rails.root.join('db', 'seeders', 'users.rb')

%i, %w, and %W seem doesn't recognise here. If it's a bug, please let me know if you want a separate issue.

[tejasbubane]

@samnang That is the desired behaviour: https://github.com/bbatsov/rubocop/blob/master/spec/rubocop/cop/lint/unneeded_splat_expansion_spec.rb#L57-L64