Merge pull request #1415 from ydakuka/fix/typeerror_in_the_file_path_cop

[Fix #1389] Handle `TypeError` caused by passing array literals as arguments to `File` methods in `Rails/FilePath` cop

Author: koic

.rubocop_todo.yml

@@ -25,7 +25,7 @@ InternalAffairs/NodeDestructuring:
 # Offense count: 10
 # Configuration parameters: CountComments, CountAsOne.
 Metrics/ClassLength:
-  Max: 163
+  Max: 179
 
 # Offense count: 41
 # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns.

changelog/fix_rails_file_path_to_handle_type_error_caused_by_an_array.md

@@ -0,0 +1 @@
+* [#1389](https://github.com/rubocop/rubocop-rails/issues/1389): Handle `TypeError` caused by passing array literals as arguments to `File` methods in `Rails/FilePath` cop. ([@ydakuka][])

lib/rubocop/cop/rails/file_path.rb

@@ -173,7 +173,17 @@ def autocorrect_extension_after_rails_root_join_in_dstr(corrector, node, rails_r
         end
 
         def autocorrect_file_join(corrector, node)
+          replace_receiver_with_rails_root(corrector, node)
+          remove_first_argument_with_comma(corrector, node)
+          process_arguments(corrector, node.arguments)
+          append_to_string_conversion(corrector, node)
+        end
+
+        def replace_receiver_with_rails_root(corrector, node)
           corrector.replace(node.receiver, 'Rails.root')
+        end
+
+        def remove_first_argument_with_comma(corrector, node)
           corrector.remove(
             range_with_surrounding_space(
               range_with_surrounding_comma(
@@ -183,9 +193,19 @@ def autocorrect_file_join(corrector, node)
               side: :right
             )
           )
-          node.arguments.filter(&:str_type?).each do |argument|
-            corrector.replace(argument, argument.value.delete_prefix('/').inspect)
+        end
+
+        def process_arguments(corrector, arguments)
+          arguments.each do |argument|
+            if argument.str_type?
+              corrector.replace(argument, argument.value.delete_prefix('/').inspect)
+            elsif argument.array_type?
+              corrector.replace(argument, "*#{argument.source}")
+            end
           end
+        end
+
+        def append_to_string_conversion(corrector, node)
           corrector.insert_after(node, '.to_s')
         end
 

spec/rubocop/cop/rails/file_path_spec.rb

@@ -244,6 +244,84 @@
         RUBY
       end
     end
+
+    context 'when using only [] syntax' do
+      it 'registers an offense once' do
+        expect_offense(<<~RUBY)
+          File.join(Rails.root, ['app', 'models'])
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to').to_s`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          Rails.root.join(*['app', 'models']).to_s
+        RUBY
+      end
+    end
+
+    context 'with a leading string and an array using [] syntax' do
+      it 'registers an offense once' do
+        expect_offense(<<~RUBY)
+          File.join(Rails.root, "app", ["models", "goober"])
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to').to_s`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          Rails.root.join("app", *["models", "goober"]).to_s
+        RUBY
+      end
+    end
+
+    context 'with an array using [] syntax and a trailing string' do
+      it 'registers an offense once' do
+        expect_offense(<<~RUBY)
+          File.join(Rails.root, ["app", "models"], "goober")
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to').to_s`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          Rails.root.join(*["app", "models"], "goober").to_s
+        RUBY
+      end
+    end
+
+    context 'when using only %w[] syntax' do
+      it 'registers an offense once' do
+        expect_offense(<<~RUBY)
+          File.join(Rails.root, %w[app models])
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to').to_s`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          Rails.root.join(*%w[app models]).to_s
+        RUBY
+      end
+    end
+
+    context 'with a leading string and an array using %w[] syntax' do
+      it 'registers an offense once' do
+        expect_offense(<<~RUBY)
+          File.join(Rails.root, "app", %w[models goober])
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to').to_s`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          Rails.root.join("app", *%w[models goober]).to_s
+        RUBY
+      end
+    end
+
+    context 'with an array using %w[] syntax and a trailing string' do
+      it 'registers an offense once' do
+        expect_offense(<<~RUBY)
+          File.join(Rails.root, %w[app models], "goober")
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to').to_s`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          Rails.root.join(*%w[app models], "goober").to_s
+        RUBY
+      end
+    end
   end
 
   context 'when EnforcedStyle is `arguments`' do
@@ -436,5 +514,83 @@
         RUBY
       end
     end
+
+    context 'when using only [] syntax' do
+      it 'registers an offense once' do
+        expect_offense(<<~RUBY)
+          File.join(Rails.root, ['app', 'models'])
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to').to_s`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          Rails.root.join(*['app', 'models']).to_s
+        RUBY
+      end
+    end
+
+    context 'with a leading string and an array using [] syntax' do
+      it 'registers an offense once' do
+        expect_offense(<<~RUBY)
+          File.join(Rails.root, "app", ["models", "goober"])
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to').to_s`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          Rails.root.join("app", *["models", "goober"]).to_s
+        RUBY
+      end
+    end
+
+    context 'with an array using [] syntax and a trailing string' do
+      it 'registers an offense once' do
+        expect_offense(<<~RUBY)
+          File.join(Rails.root, ["app", "models"], "goober")
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to').to_s`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          Rails.root.join(*["app", "models"], "goober").to_s
+        RUBY
+      end
+    end
+
+    context 'when using only %w[] syntax' do
+      it 'registers an offense once' do
+        expect_offense(<<~RUBY)
+          File.join(Rails.root, %w[app models])
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to').to_s`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          Rails.root.join(*%w[app models]).to_s
+        RUBY
+      end
+    end
+
+    context 'with a leading string and an array using %w[] syntax' do
+      it 'registers an offense once' do
+        expect_offense(<<~RUBY)
+          File.join(Rails.root, "app", %w[models goober])
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to').to_s`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          Rails.root.join("app", *%w[models goober]).to_s
+        RUBY
+      end
+    end
+
+    context 'with an array using %w[] syntax and a trailing string' do
+      it 'registers an offense once' do
+        expect_offense(<<~RUBY)
+          File.join(Rails.root, %w[app models], "goober")
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to').to_s`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          Rails.root.join(*%w[app models], "goober").to_s
+        RUBY
+      end
+    end
   end
 end