From cebe1f10a2a1af3c1b2c019db4b393ede063a7f5 Mon Sep 17 00:00:00 2001 From: Yauheni Dakuka Date: Sun, 19 Jan 2025 20:11:51 +0400 Subject: [PATCH 1/2] [Fix rubocop#1053] Fix an error occurred for Rails/FilePath cop when nested File.join --- ...ils_file_path_cop_when_nested_file_join.md | 1 + lib/rubocop/cop/rails/file_path.rb | 86 ++++++- spec/rubocop/cop/rails/file_path_spec.rb | 234 ++++++++++++++++++ 3 files changed, 318 insertions(+), 3 deletions(-) create mode 100644 changelog/fix_an_error_occurred_for_rails_file_path_cop_when_nested_file_join.md diff --git a/changelog/fix_an_error_occurred_for_rails_file_path_cop_when_nested_file_join.md b/changelog/fix_an_error_occurred_for_rails_file_path_cop_when_nested_file_join.md new file mode 100644 index 0000000000..54599cbd5c --- /dev/null +++ b/changelog/fix_an_error_occurred_for_rails_file_path_cop_when_nested_file_join.md @@ -0,0 +1 @@ +* [#1053](https://github.com/rubocop/rubocop-rails/issues/1053): Fix an error occurred for `Rails/FilePath` cop when nested `File.join`. ([@ydakuka][]) diff --git a/lib/rubocop/cop/rails/file_path.rb b/lib/rubocop/cop/rails/file_path.rb index b9438dbc40..62200045ce 100644 --- a/lib/rubocop/cop/rails/file_path.rb +++ b/lib/rubocop/cop/rails/file_path.rb @@ -51,6 +51,14 @@ class FilePath < Base (send (const {nil? cbase} :File) :join ...) PATTERN + def_node_matcher :file_join_with_rails_root_nodes?, <<~PATTERN + (send (const {nil? cbase} :File) :join (_ $_)* {(send #rails_root_nodes? :to_s) #rails_root_nodes?} ...) + PATTERN + + def_node_matcher :file_join_with_rails_root_join_nodes?, <<~PATTERN + (send (const {nil? cbase} :File) :join (_ $_)* {(send #rails_root_join_nodes? :to_s) #rails_root_join_nodes?} ...) + PATTERN + def_node_search :rails_root_nodes?, <<~PATTERN (send (const {nil? cbase} :Rails) :root) PATTERN @@ -68,8 +76,10 @@ def on_dstr(node) end def on_send(node) + check_for_file_join_with_rails_root_join(node) check_for_file_join_with_rails_root(node) return unless node.receiver + return if node.ancestors.any? { |ancestor| file_join_nodes?(ancestor) } check_for_rails_root_join_with_slash_separated_path(node) check_for_rails_root_join_with_string_arguments(node) @@ -99,11 +109,20 @@ def check_for_extension_after_rails_root_join_in_dstr(node) end def check_for_file_join_with_rails_root(node) - return unless file_join_nodes?(node) + return unless file_join_with_rails_root_nodes?(node) return unless valid_arguments_for_file_join_with_rails_root?(node.arguments) + return if node.descendants.any? { |descendant| file_join_nodes?(descendant) } + + register_offense(node, require_to_s: true) do |corrector| + autocorrect_file_join_with_rails_root(corrector, node) unless node.first_argument.array_type? + end + end + + def check_for_file_join_with_rails_root_join(node) + return unless file_join_with_rails_root_join_nodes?(node) register_offense(node, require_to_s: true) do |corrector| - autocorrect_file_join(corrector, node) unless node.first_argument.array_type? + autocorrect_file_join_with_rails_root_join(corrector, node) end end @@ -201,7 +220,7 @@ def autocorrect_extension_after_rails_root_join_in_dstr(corrector, node, rails_r corrector.remove(extension_node) end - def autocorrect_file_join(corrector, node) + def autocorrect_file_join_with_rails_root(corrector, node) replace_receiver_with_rails_root(corrector, node) remove_first_argument_with_comma(corrector, node) process_arguments(corrector, node.arguments) @@ -238,6 +257,67 @@ def append_to_string_conversion(corrector, node) corrector.insert_after(node, '.to_s') end + def autocorrect_file_join_with_rails_root_join(corrector, node) + rails_root_join_node = find_rails_root_join_node(node) + return unless rails_root_join_node + + combined_arguments = combine_arguments(rails_root_join_node, node) + corrector.replace(node, "Rails.root.join(#{combined_arguments}).to_s") + end + + def find_rails_root_join_node(node) + node.arguments.find { |argument| rails_root_join_typical?(argument) } + end + + def rails_root_join_typical?(node) + rails_root_join_nodes?(node) || (node.send_type? && rails_root_join_nodes?(node.receiver)) + end + + def extract_rails_root_call(rails_root_join_node) + if rails_root_join_node.send_type? && rails_root_join_node.method?(:to_s) + rails_root_join_node.receiver + else + rails_root_join_node + end + end + + def combine_arguments(rails_root_join_node, node) + rails_root_call = extract_rails_root_call(rails_root_join_node) + rails_root_arguments = rails_root_call.arguments + other_arguments = node.arguments.reject { |argument| argument == rails_root_join_node } + + case style + when :arguments + combine_as_arguments(rails_root_arguments, other_arguments) + when :slashes + combine_as_slashes(rails_root_arguments, other_arguments) + end + end + + def combine_as_arguments(rails_root_arguments, other_arguments) + first_argument = other_arguments.first + + other_arguments_values = + if first_argument.dstr_type? + first_argument.children.map do |child| + "'#{child.value.delete_prefix('/')}'" + end + else + ["'#{first_argument.value.delete_prefix('/')}'"] + end + + (rails_root_arguments.map(&:source) + other_arguments_values).join(', ') + end + + def combine_as_slashes(rails_root_arguments, other_arguments) + return "'#{rails_root_arguments.map(&:value).join}'" if other_arguments.empty? + + first_value = other_arguments.first.value + separator = first_value.start_with?(File::SEPARATOR) ? '' : '/' + + "'#{(rails_root_arguments + other_arguments).map(&:value).join(separator)}'" + end + def autocorrect_rails_root_join_with_string_arguments(corrector, node) corrector.replace(node.first_argument, %("#{node.arguments.map(&:value).join('/')}")) node.arguments[1..].each do |argument| diff --git a/spec/rubocop/cop/rails/file_path_spec.rb b/spec/rubocop/cop/rails/file_path_spec.rb index 8f6d6ca825..6f26d1df44 100644 --- a/spec/rubocop/cop/rails/file_path_spec.rb +++ b/spec/rubocop/cop/rails/file_path_spec.rb @@ -444,6 +444,123 @@ RUBY end end + + context 'when File.join wraps Rails.root.join with string arguments' do + it 'registers an offense once' do + expect_offense(<<~RUBY) + File.join(Rails.root.join('app', 'models'), 'user.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to').to_s`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.join('app/models/user.rb').to_s + RUBY + end + end + + context 'when File.join wraps Rails.root.join with non-string arguments' do + it 'registers an offense once' do + expect_offense(<<~RUBY) + File.join(Rails.root.join('app/models'), 'user.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to').to_s`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.join('app/models/user.rb').to_s + RUBY + end + end + + context 'when File.join wraps Rails.root.join with non-string arguments and path starting with `/`' do + it 'registers an offense once' do + expect_offense(<<~RUBY) + File.join(Rails.root.join('app/models'), '/vehicle' '/car.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to').to_s`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.join('app/models/vehicle/car.rb').to_s + RUBY + end + end + + context 'when File.join wraps Rails.root.join with string arguments and .to_s' do + it 'registers an offense once' do + expect_offense(<<~RUBY) + File.join(Rails.root.join('app', 'models').to_s, 'user.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to').to_s`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.join('app/models/user.rb').to_s + RUBY + end + end + + context 'when File.join wraps Rails.root.join with non-string arguments and .to_s' do + it 'registers an offense once' do + expect_offense(<<~RUBY) + File.join(Rails.root.join('app/models').to_s, 'user.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to').to_s`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.join('app/models/user.rb').to_s + RUBY + end + end + + context 'when File.join wraps Rails.root.join with non-string arguments and path starting with `/` and .to_s' do + it 'registers an offense once' do + expect_offense(<<~RUBY) + File.join(Rails.root.join('app/models').to_s, '/vehicle' '/car.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to').to_s`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.join('app/models/vehicle/car.rb').to_s + RUBY + end + end + + context 'when using nested File.join with Rails.root with string arguments' do + it 'registers an offense once' do + expect_offense(<<~RUBY) + File.join(File.join(Rails.root, 'app', 'models'), 'user.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to').to_s`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.join('app/models/user.rb').to_s + RUBY + end + end + + context 'when using nested File.join with Rails.root with non-string arguments' do + it 'registers an offense once' do + expect_offense(<<~RUBY) + File.join(File.join(Rails.root, 'app/models'), 'user.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to').to_s`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.join('app/models/user.rb').to_s + RUBY + end + end + + context 'when using nested File.join with Rails.root with non-string arguments and path starting with `/`' do + it 'registers an offense once' do + expect_offense(<<~RUBY) + File.join(File.join(Rails.root, '/app/models'), '/user.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to').to_s`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.join('app/models/user.rb').to_s + RUBY + end + end end context 'when EnforcedStyle is `arguments`' do @@ -754,5 +871,122 @@ RUBY end end + + context 'when File.join wraps Rails.root.join with string arguments' do + it 'registers an offense once' do + expect_offense(<<~RUBY) + File.join(Rails.root.join('app', 'models'), 'user.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to').to_s`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.join('app', 'models', 'user.rb').to_s + RUBY + end + end + + context 'when File.join wraps Rails.root.join with non-string arguments' do + it 'registers an offense once' do + expect_offense(<<~RUBY) + File.join(Rails.root.join('app/models'), 'user.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to').to_s`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.join('app', "models", 'user.rb').to_s + RUBY + end + end + + context 'when File.join wraps Rails.root.join with non-string arguments and path starting with `/`' do + it 'registers an offense once' do + expect_offense(<<~RUBY) + File.join(Rails.root.join('app/models'), '/vehicle' '/car.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to').to_s`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.join('app', "models", 'vehicle', 'car.rb').to_s + RUBY + end + end + + context 'when File.join wraps Rails.root.join with string arguments and .to_s' do + it 'registers an offense once' do + expect_offense(<<~RUBY) + File.join(Rails.root.join('app', 'models').to_s, 'user.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to').to_s`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.join('app', 'models', 'user.rb').to_s + RUBY + end + end + + context 'when File.join wraps Rails.root.join with non-string arguments and .to_s' do + it 'registers an offense once' do + expect_offense(<<~RUBY) + File.join(Rails.root.join('app/models').to_s, 'user.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to').to_s`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.join('app', "models", 'user.rb').to_s + RUBY + end + end + + context 'when File.join wraps Rails.root.join with non-string arguments and path starting with `/` and .to_s' do + it 'registers an offense once' do + expect_offense(<<~RUBY) + File.join(Rails.root.join('app/models').to_s, '/vehicle' '/car.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to').to_s`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.join('app', "models", 'vehicle', 'car.rb').to_s + RUBY + end + end + + context 'when using nested File.join with Rails.root with string arguments' do + it 'registers an offense once' do + expect_offense(<<~RUBY) + File.join(File.join(Rails.root, 'app', 'models'), 'user.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to').to_s`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.join("app", "models", 'user.rb').to_s + RUBY + end + end + + context 'when using nested File.join with Rails.root with non-string arguments' do + it 'registers an offense once' do + expect_offense(<<~RUBY) + File.join(File.join(Rails.root, 'app/models'), 'user.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to').to_s`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.join("app", "models", 'user.rb').to_s + RUBY + end + end + + context 'when using nested File.join with Rails.root with non-string arguments and path starting with `/`' do + it 'registers an offense once' do + expect_offense(<<~RUBY) + File.join(File.join(Rails.root, '/app/models'), '/user.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to').to_s`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.join("app", "models", 'user.rb').to_s + RUBY + end + end end end From 16807047aa6272506a7b58df9ba7475db5ac4975 Mon Sep 17 00:00:00 2001 From: Yauheni Dakuka Date: Sun, 26 Jan 2025 20:48:04 +0400 Subject: [PATCH 2/2] Add more tests for Rails/FilePath --- spec/rubocop/cop/rails/file_path_spec.rb | 257 +++++++++++++++++++++-- 1 file changed, 239 insertions(+), 18 deletions(-) diff --git a/spec/rubocop/cop/rails/file_path_spec.rb b/spec/rubocop/cop/rails/file_path_spec.rb index 6f26d1df44..7b9b362f23 100644 --- a/spec/rubocop/cop/rails/file_path_spec.rb +++ b/spec/rubocop/cop/rails/file_path_spec.rb @@ -4,41 +4,66 @@ context 'when EnforcedStyle is `slashes`' do let(:cop_config) { { 'EnforcedStyle' => 'slashes' } } - context 'when using Rails.root.join with some path strings' do + context 'when using Rails.root.parent' do it 'registers an offense' do expect_offense(<<~RUBY) - Rails.root.join('app', 'models', 'user.rb') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to')`. + Rails.root.parent.join("app", "models") + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to')`. RUBY expect_correction(<<~RUBY) - Rails.root.join("app/models/user.rb") + Rails.root.parent.join("app/models") RUBY end end - context 'when using File.join with Rails.root and path starting with `/`' do + context 'when using Rails.root.dirname' do it 'registers an offense' do expect_offense(<<~RUBY) - File.join(Rails.root, '/app/models', '/user.rb') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to').to_s`. + Rails.root.dirname.join("config", "initializers") + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to')`. RUBY expect_correction(<<~RUBY) - Rails.root.join("app/models/user.rb").to_s + Rails.root.dirname.join("config/initializers") RUBY end end - context 'when using ::Rails.root.join with some path strings' do + context 'when using Rails.root.basename' do it 'registers an offense' do expect_offense(<<~RUBY) - ::Rails.root.join('app', 'models', 'user.rb') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to')`. + Rails.root.basename.join("config", "initializers") + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to')`. RUBY expect_correction(<<~RUBY) - ::Rails.root.join("app/models/user.rb") + Rails.root.basename.join("config/initializers") + RUBY + end + end + + context 'when using Rails.application.config.root' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + File.join(Rails.application.config.root, "app", "models") + RUBY + + expect_no_offenses(<<~RUBY) + File.join(Rails.application.config.root, "app/models") + RUBY + end + end + + context 'when using Rails.root.join with some path strings' do + it 'registers an offense' do + expect_offense(<<~RUBY) + Rails.root.join('app', 'models', 'user.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to')`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.join("app/models/user.rb") RUBY end end @@ -56,6 +81,32 @@ end end + context 'when using File.join with Rails.root and path starting with `/`' do + it 'registers an offense' do + expect_offense(<<~RUBY) + File.join(Rails.root, '/app/models', '/user.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to').to_s`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.join("app/models/user.rb").to_s + RUBY + end + end + + context 'when using ::Rails.root.join with some path strings' do + it 'registers an offense' do + expect_offense(<<~RUBY) + ::Rails.root.join('app', 'models', 'user.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path/to')`. + RUBY + + expect_correction(<<~RUBY) + ::Rails.root.join("app/models/user.rb") + RUBY + end + end + context 'when using Rails.root.join in string interpolation with nothing after it' do it 'does not register an offense' do expect_no_offenses(<<~'RUBY') @@ -136,7 +187,9 @@ context 'when using Rails.root.join with slash separated path string' do it 'does not register an offense' do - expect_no_offenses("Rails.root.join('app/models/goober')") + expect_no_offenses(<<~RUBY) + Rails.root.join('app/models/goober') + RUBY end end @@ -237,6 +290,32 @@ end end + context 'when interpolation with `Rails.root` contains other operations' do + it 'does not register an offense for boolean method' do + expect_no_offenses(<<~'RUBY') + "#{Rails.root || '.'}/config" + RUBY + end + + it 'does not register an offense for `rescue`' do + expect_no_offenses(<<~'RUBY') + "#{Rails.root rescue '.'}/config" + RUBY + end + + it 'does not register an offense for if condition' do + expect_no_offenses(<<~'RUBY') + "#{Rails.root if flag}/app/models" + RUBY + end + + it 'does not register an offense for a ternary operator' do + expect_no_offenses(<<~'RUBY') + "#{some_condition ? Rails.root : '/tmp'}/app/models" + RUBY + end + end + context 'with `join` method with implicit receiver' do it 'does not register an offense' do expect_no_offenses(<<~RUBY) @@ -566,16 +645,92 @@ context 'when EnforcedStyle is `arguments`' do let(:cop_config) { { 'EnforcedStyle' => 'arguments' } } - context 'when using Rails.root.join with some path strings' do + context 'when using Rails.root.parent' do + it 'registers an offense' do + expect_offense(<<~RUBY) + Rails.root.parent.join("app/models") + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to')`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.parent.join("app", "models") + RUBY + end + end + + context 'when using Rails.root.dirname' do + it 'registers an offense' do + expect_offense(<<~RUBY) + Rails.root.dirname.join("config/initializers") + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to')`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.dirname.join("config", "initializers") + RUBY + end + end + + context 'when using Rails.root.basename' do + it 'registers an offense' do + expect_offense(<<~RUBY) + Rails.root.basename.join("config/initializers") + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to')`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.basename.join("config", "initializers") + RUBY + end + end + + context 'when using Rails.application.config.root' do it 'does not register an offense' do - expect_no_offenses("Rails.root.join('app', 'models', 'user.rb')") + expect_no_offenses(<<~RUBY) + File.join(Rails.application.config.root, "app", "models") + RUBY + + expect_no_offenses(<<~RUBY) + File.join(Rails.application.config.root, "app/models") + RUBY + end + end + + context 'when using Rails.root.join with some path strings' do + it 'registers an offense' do + expect_offense(<<~RUBY) + Rails.root.join('app/models/user.rb') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to')`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.join('app', "models", "user.rb") + RUBY end end context 'when using Rails.root.join in string interpolation of argument' do - it 'does not register an offense' do - expect_no_offenses(<<~'RUBY') - 'system "rm -rf #{Rails.root.join(\'a\', \'b.png\')}"' + it 'registers an offense' do + expect_offense(<<~'RUBY') + system "rm -rf #{Rails.root.join("a/b.png")}" + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to')`. + RUBY + + expect_correction(<<~'RUBY') + system "rm -rf #{Rails.root.join("a", "b.png")}" + RUBY + end + end + + context 'when using ::Rails.root.join with some path strings' do + it 'registers an offense' do + expect_offense(<<~RUBY) + ::Rails.root.join("app/models/user.rb") + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to')`. + RUBY + + expect_correction(<<~RUBY) + ::Rails.root.join("app", "models", "user.rb") RUBY end end @@ -588,6 +743,14 @@ end end + context 'when string interpolated `Rails.root` is followed by a message starting with `.`' do + it 'does not register an offense' do + expect_no_offenses(<<~'RUBY') + "#{Rails.root}. a message" + RUBY + end + end + context 'when using string interpolation without Rails.root' do it 'does not register an offense' do expect_no_offenses(<<~'RUBY') @@ -630,6 +793,39 @@ end end + context 'when using ::File.join with Rails.root' do + it 'registers an offense' 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 'when using File.join with an array' do + it 'registers an offense' do + expect_offense(<<~RUBY) + File.join([Rails.root, 'foo']) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to').to_s`. + RUBY + + expect_no_corrections + end + + it 'registers an offense for nested arrays' do + expect_offense(<<~RUBY) + File.join([Rails.root, 'foo', ['bar']]) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to').to_s`. + RUBY + + expect_no_corrections + end + end + context 'when using Rails.root.join with slash separated path string' do it 'registers an offense' do expect_offense(<<~RUBY) @@ -656,6 +852,19 @@ end end + context 'when using Rails.root called by double quoted string that ends with string interpolation' do + it 'registers an offense' do + expect_offense(<<~'RUBY') + "#{Rails.root}/a/#{b}" + ^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to')`. + RUBY + + expect_correction(<<~'RUBY') + "#{Rails.root.join("a/#{b}")}" + RUBY + end + end + context 'when concat Rails.root and file separator using string interpolation' do it 'registers an offense' do expect_offense(<<~'RUBY') @@ -744,6 +953,18 @@ "#{Rails.root rescue '.'}/config" RUBY end + + it 'does not register an offense for if condition' do + expect_no_offenses(<<~'RUBY') + "#{Rails.root if flag}/app/models" + RUBY + end + + it 'does not register an offense for a ternary operator' do + expect_no_offenses(<<~'RUBY') + "#{some_condition ? Rails.root : '/tmp'}/app/models" + RUBY + end end context 'with `join` method with implicit receiver' do