[Fix rubocop#3512] Change error message of `Lint/UnneededSplatExpansi…

…on` for array (rubocop#3526)

In case of array splat for method parameter, the default message was
ambiguous. `Unnecessary splat expansion.` made people just remove the
splat expansion and pass the array as parameter which is obviously not
the same as passing splat arguments. Change the message to
`Pass array contents as separate arguments.` to
the message in such cases to make the intension more clear.

CHANGELOG.md

@@ -7,6 +7,10 @@
 * [#3513](https://github.com/bbatsov/rubocop/pull/3513): Fix false positive in `Style/TernaryParentheses` for a ternary with ranges. ([@dreyks][])
 * [#3520](https://github.com/bbatsov/rubocop/issues/3520): Fix regression causing `Lint/AssignmentInCondition` false positive. ([@savef][])
 
+### Changes
+
+* [#3512](https://github.com/bbatsov/rubocop/issues/3512): Change error message of `Lint/UnneededSplatExpansion` for array in method parameters. ([@tejasbubane][])
+
 ## 0.43.0 (2016-09-19)
 
 ### New features

lib/rubocop/cop/lint/unneeded_splat_expansion.rb

@@ -47,6 +47,7 @@ module Lint
       #   end
       class UnneededSplatExpansion < Cop
         MSG = 'Unnecessary splat expansion.'.freeze
+        ARRAY_PARAM_MSG = 'Pass array contents as separate arguments.'.freeze
         PERCENT_W = '%w'.freeze
         PERCENT_CAPITAL_W = '%W'.freeze
         PERCENT_I = '%i'.freeze
@@ -63,7 +64,12 @@ def on_splat(node)
             if object.send_type?
               return unless ASSIGNMENT_TYPES.include?(node.parent.parent.type)
             end
-            add_offense(node, :expression)
+
+            if array_splat?(node) && method_argument?(node)
+              add_offense(node, :expression, ARRAY_PARAM_MSG)
+            else
+              add_offense(node, :expression)
+            end
           end
         end
 
@@ -85,6 +91,14 @@ def autocorrect(node)
           end
         end
 
+        def array_splat?(node)
+          node.children.first.array_type?
+        end
+
+        def method_argument?(node)
+          node.parent.send_type?
+        end
+
         def unneeded_brackets?(node)
           parent = node.parent
 

spec/rubocop/cop/lint/unneeded_splat_expansion_spec.rb

@@ -5,6 +5,8 @@
 
 describe RuboCop::Cop::Lint::UnneededSplatExpansion do
   subject(:cop) { described_class.new }
+  let(:message) { 'Unnecessary splat expansion.' }
+  let(:array_param_message) { 'Pass array contents as separate arguments.' }
 
   it 'allows assigning to a splat' do
     inspect_source(cop, '*, rhs = *node')
@@ -43,29 +45,44 @@
     expect(cop.offenses).to be_empty
   end
 
-  shared_examples 'splat expansion' do |literal|
-    context 'expansion of method parameters' do
-      it 'registers an offense for expansion of parameters' do
+  shared_examples 'splat literal assignment' do |literal|
+    it 'registers an offense for ' do
+      inspect_source(cop, "a = *#{literal}")
+
+      expect(cop.messages).to eq([message])
+      expect(cop.highlights).to eq(["*#{literal}"])
+    end
+  end
+
+  shared_examples 'array splat expansion' do |literal|
+    context 'method parameters' do
+      it 'registers an offense' do
         inspect_source(cop, "array.push(*#{literal})")
 
-        expect(cop.messages).to eq(['Unnecessary splat expansion.'])
+        expect(cop.messages).to eq([array_param_message])
         expect(cop.highlights).to eq(["*#{literal}"])
       end
     end
 
-    context 'assignment to splat expansion of literals' do
-      it 'registers an offense for ' do
-        inspect_source(cop, "a = *#{literal}")
+    it_behaves_like 'splat literal assignment', literal
+  end
 
-        expect(cop.messages).to eq(['Unnecessary splat expansion.'])
+  shared_examples 'splat expansion' do |literal|
+    context 'method parameters' do
+      it 'registers an offense' do
+        inspect_source(cop, "array.push(*#{literal})")
+
+        expect(cop.messages).to eq([message])
         expect(cop.highlights).to eq(["*#{literal}"])
       end
     end
+
+    it_behaves_like 'splat literal assignment', literal
   end
 
-  it_behaves_like 'splat expansion', '[1, 2, 3]'
-  it_behaves_like 'splat expansion', '%w(one two three)'
-  it_behaves_like 'splat expansion', '%W(one #{two} three)'
+  it_behaves_like 'array splat expansion', '[1, 2, 3]'
+  it_behaves_like 'array splat expansion', '%w(one two three)'
+  it_behaves_like 'array splat expansion', '%W(one #{two} three)'
   it_behaves_like 'splat expansion', "'a'"
   it_behaves_like 'splat expansion', '"#{a}"'
   it_behaves_like 'splat expansion', '1'
@@ -75,7 +92,7 @@
     it 'registers an offense for an array using a constructor' do
       inspect_source(cop, 'a = *Array.new(3) { 42 }')
 
-      expect(cop.messages).to eq(['Unnecessary splat expansion.'])
+      expect(cop.messages).to eq([message])
       expect(cop.highlights).to eq(['*Array.new(3) { 42 }'])
     end
   end
@@ -87,7 +104,7 @@
                            '  bar',
                            'end'])
 
-      expect(cop.messages).to eq(['Unnecessary splat expansion.'])
+      expect(cop.messages).to eq([message])
       expect(cop.highlights).to eq(['*[first, second]'])
     end
 
@@ -97,7 +114,7 @@
                            '  bar',
                            'end'])
 
-      expect(cop.messages).to eq(['Unnecessary splat expansion.'])
+      expect(cop.messages).to eq([message])
       expect(cop.highlights).to eq(['*%w(first second)'])
     end
 
@@ -107,7 +124,7 @@
                            '  bar',
                            'end'])
 
-      expect(cop.messages).to eq(['Unnecessary splat expansion.'])
+      expect(cop.messages).to eq([message])
       expect(cop.highlights).to eq(['*%W(#{first} second)'])
     end
 
@@ -138,7 +155,7 @@
                          '  bar',
                          'end'])
 
-    expect(cop.messages).to eq(['Unnecessary splat expansion.'])
+    expect(cop.messages).to eq([message])
     expect(cop.highlights).to eq(['*[First, Second]'])
   end
 
@@ -284,8 +301,8 @@
   end
 
   context 'ruby >= 2.0', :ruby20 do
-    it_behaves_like 'splat expansion', '%i(first second)'
-    it_behaves_like 'splat expansion', '%I(first second #{third})'
+    it_behaves_like 'array splat expansion', '%i(first second)'
+    it_behaves_like 'array splat expansion', '%I(first second #{third})'
 
     context 'arrays being expanded with %i variants using splat expansion' do
       it 'registers an offense for an array literal being expanded in a ' \
@@ -306,14 +323,14 @@
         it 'registers an offense for an array literal %i' do
           inspect_source(cop, 'array.push(*%i(first second))')
 
-          expect(cop.messages).to eq(['Unnecessary splat expansion.'])
+          expect(cop.messages).to eq([array_param_message])
           expect(cop.highlights).to eq(['*%i(first second)'])
         end
 
         it 'registers an offense for an array literal %I' do
           inspect_source(cop, 'array.push(*%I(#{first} second))')
 
-          expect(cop.messages).to eq(['Unnecessary splat expansion.'])
+          expect(cop.messages).to eq([array_param_message])
           expect(cop.highlights).to eq(['*%I(#{first} second)'])
         end
       end