From 3c09861999cb856135266f050e42f2de98724766 Mon Sep 17 00:00:00 2001 From: Yauheni Dakuka Date: Sat, 18 Jan 2025 17:04:16 +0400 Subject: [PATCH] [Fix rubocop#1389] Handle TypeError caused by an array in Rails/FilePath cop --- .rubocop_todo.yml | 6 +- ...to_handle_type_error_caused_by_an_array.md | 1 + lib/rubocop/cop/rails/file_path.rb | 24 ++- spec/rubocop/cop/rails/file_path_spec.rb | 156 ++++++++++++++++++ 4 files changed, 184 insertions(+), 3 deletions(-) create mode 100644 changelog/fix_rails_file_path_to_handle_type_error_caused_by_an_array.md diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index acbcd95faa..908e36a600 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -25,7 +25,11 @@ InternalAffairs/NodeDestructuring: # Offense count: 10 # Configuration parameters: CountComments, CountAsOne. Metrics/ClassLength: - Max: 163 + Max: 139 + Exclude: + - 'lib/rubocop/cop/rails/time_zone.rb' + - 'lib/rubocop/cop/rails/save_bang.rb' + - 'lib/rubocop/cop/rails/file_path.rb' # Offense count: 41 # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns. diff --git a/changelog/fix_rails_file_path_to_handle_type_error_caused_by_an_array.md b/changelog/fix_rails_file_path_to_handle_type_error_caused_by_an_array.md new file mode 100644 index 0000000000..7dc5abb3ca --- /dev/null +++ b/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 an array in `Rails/FilePath` cop. ([@ydakuka][]) diff --git a/lib/rubocop/cop/rails/file_path.rb b/lib/rubocop/cop/rails/file_path.rb index 9c19c5e525..375f43e7b0 100644 --- a/lib/rubocop/cop/rails/file_path.rb +++ b/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 diff --git a/spec/rubocop/cop/rails/file_path_spec.rb b/spec/rubocop/cop/rails/file_path_spec.rb index 04d4cfd4e9..6bcdd88dea 100644 --- a/spec/rubocop/cop/rails/file_path_spec.rb +++ b/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