Merge pull request #768 from koic/fix_an_incorrect_autocorrect_for_ra…

…ils_root_pathname_methods

[Fix #763] Fix a false positive for `Rails/RootPathnameMethods`

changelog/fix_an_incorrect_autocorrect_for_rails_root_pathname_methods.md

@@ -0,0 +1 @@
+* [#763](https://github.com/rubocop/rubocop-rails/issues/763): Mark `Rails/RootPathnameMethods` as unsafe and fix an incorrect autocorrect when using `Dir.glob`. ([@koic][])

config/default.yml

@@ -845,6 +845,7 @@ Rails/RootJoinChain:
 Rails/RootPathnameMethods:
   Description: 'Use `Rails.root` IO methods instead of passing it to `File`.'
   Enabled: pending
+  SafeAutocorrect: false
   VersionAdded: '2.16'
 
 Rails/RootPublicPath:

lib/rubocop/cop/rails/root_pathname_methods.rb

@@ -11,6 +11,10 @@ module Rails
       # This cop works best when used together with
       # `Style/FileRead`, `Style/FileWrite` and `Rails/RootJoinChain`.
       #
+      # @safety
+      #   This cop is unsafe for autocorrection because `Dir`'s `children`, `each_child`, `entries`, and `glob`
+      #   methods return string element, but these methods of `Pathname` return `Pathname` element.
+      #
       # @example
       #   # bad
       #   File.open(Rails.root.join('db', 'schema.rb'))
@@ -30,6 +34,7 @@ module Rails
       #
       class RootPathnameMethods < Base
         extend AutoCorrector
+        include RangeHelp
 
         MSG = '`%<rails_root>s` is a `Pathname` so you can just append `#%<method>s`.'
 
@@ -138,6 +143,11 @@ class RootPathnameMethods < Base
           }
         PATTERN
 
+        def_node_matcher :dir_glob?, <<~PATTERN
+          (send
+            (const {cbase nil?} :Dir) :glob ...)
+        PATTERN
+
         def_node_matcher :rails_root_pathname?, <<~PATTERN
           {
             $#rails_root?
@@ -153,8 +163,12 @@ class RootPathnameMethods < Base
         def on_send(node)
           evidence(node) do |method, path, args, rails_root|
             add_offense(node, message: format(MSG, method: method, rails_root: rails_root.source)) do |corrector|
-              replacement = "#{path.source}.#{method}"
-              replacement += "(#{args.map(&:source).join(', ')})" unless args.empty?
+              if dir_glob?(node)
+                replacement = build_path_glob(path, method)
+              else
+                replacement = "#{path.source}.#{method}"
+                replacement += "(#{args.map(&:source).join(', ')})" unless args.empty?
+              end
 
               corrector.replace(node, replacement)
             end
@@ -169,6 +183,31 @@ def evidence(node)
 
           yield(method, path, args, rails_root)
         end
+
+        def build_path_glob(path, method)
+          receiver = range_between(path.loc.expression.begin_pos, path.children.first.loc.selector.end_pos).source
+
+          argument = if path.arguments.one?
+                       path.first_argument.source
+                     else
+                       join_arguments(path.arguments)
+                     end
+
+          "#{receiver}.#{method}(#{argument})"
+        end
+
+        def include_interpolation?(arguments)
+          arguments.any? do |argument|
+            argument.children.any? { |child| child.respond_to?(:begin_type?) && child.begin_type? }
+          end
+        end
+
+        def join_arguments(arguments)
+          quote = include_interpolation?(arguments) ? '"' : "'"
+          joined_arguments = arguments.map(&:value).join('/')
+
+          "#{quote}#{joined_arguments}#{quote}"
+        end
       end
     end
   end

spec/rubocop/cop/rails/root_pathname_methods_spec.rb

@@ -9,6 +9,8 @@
     IO: described_class::FILE_METHODS
   }.each do |receiver, methods|
     methods.each do |method|
+      next if method == :glob
+
       it "registers an offense when using `#{receiver}.#{method}(Rails.public_path)` (if arity exists)" do
         expect_offense(<<~RUBY, receiver: receiver, method: method)
           %{receiver}.%{method}(Rails.public_path)
@@ -44,6 +46,63 @@
     end
   end
 
+  context 'when using `Dir.glob`' do
+    it "registers an offense when using `Dir.glob(Rails.root.join('**/*.rb'))`" do
+      expect_offense(<<~RUBY)
+        Dir.glob(Rails.root.join('**/*.rb'))
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#glob`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        Rails.root.glob('**/*.rb')
+      RUBY
+    end
+
+    it "registers an offense when using `::Dir.glob(Rails.root.join('**/*.rb'))`" do
+      expect_offense(<<~RUBY)
+        ::Dir.glob(Rails.root.join('**/*.rb'))
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#glob`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        Rails.root.glob('**/*.rb')
+      RUBY
+    end
+
+    it "registers an offense when using `Dir.glob(Rails.root.join('**/\#{path}/*.rb'))`" do
+      expect_offense(<<~'RUBY')
+        Dir.glob(Rails.root.join("**/#{path}/*.rb"))
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#glob`.
+      RUBY
+
+      expect_correction(<<~'RUBY')
+        Rails.root.glob("**/#{path}/*.rb")
+      RUBY
+    end
+
+    it "registers an offense when using `Dir.glob(Rails.root.join('**', '*.rb'))`" do
+      expect_offense(<<~RUBY)
+        Dir.glob(Rails.root.join('**', '*.rb'))
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#glob`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        Rails.root.glob('**/*.rb')
+      RUBY
+    end
+
+    it "registers an offense when using `Dir.glob(Rails.root.join('**', \"\#{path}\", '*.rb'))`" do
+      expect_offense(<<~'RUBY')
+        Dir.glob(Rails.root.join('**', "#{path}", '*.rb'))
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#glob`.
+      RUBY
+
+      expect_correction(<<~'RUBY')
+        Rails.root.glob("**/#{path}/*.rb")
+      RUBY
+    end
+  end
+
   # This is handled by `Rails/RootJoinChain`
   it 'does not register an offense when using `File.read(Rails.root.join(...).join(...))`' do
     expect_no_offenses(<<~RUBY)