Merge pull request #7983 from Rylan12/rubocop-refactor

style: use direct method matching instead of regex

Library/Homebrew/rubocops/lines.rb

@@ -114,20 +114,9 @@ module RuboCop
             return
           end
 
-          # Matches `depends_on "foo" if build.with?("bar")` or depends_on "foo" if build.without?("bar")`
+          depends_on_build_with(body_node) do |build_with_node|
-          depends_on_build_regex = /depends_on .+ (if build\.with(out)?\?\(["']\w+["']\))/
+            offending_node(build_with_node)
-
+            problem "Use `:optional` or `:recommended` instead of `if #{build_with_node.source}`"
-          find_instance_method_call(body_node, :build, :with?) do |n|
-            # TODO: this should be refactored to a direct method match
-            next unless match = n.parent.source.match(depends_on_build_regex)
-
-            problem "Use `:optional` or `:recommended` instead of `#{match[1]}`"
-          end
-
-          find_instance_method_call(body_node, :build, :without?) do |n|
-            next unless match = n.parent.source.match(depends_on_build_regex)
-
-            problem "Use `:optional` or `:recommended` instead of `#{match[1]}`"
           end
 
           find_instance_method_call(body_node, :build, :without?) do |method|
@@ -181,6 +170,12 @@ module RuboCop
 
           node.modifier_form? && node.unless?
         end
+
+        # Finds `depends_on "foo" if build.with?("bar")` or `depends_on "foo" if build.without?("bar")`
+        def_node_search :depends_on_build_with, <<~EOS
+          (if $(send (send nil? :build) {:with? :without?} str)
+            (send nil? :depends_on str) nil?)
+        EOS
       end
 
       class MpiCheck < FormulaCop

Library/Homebrew/rubocops/text.rb

@@ -106,20 +106,18 @@ module RuboCop
             problem "Use `depends_on :java` to set JAVA_HOME"
           end
 
-          find_strings(body_node).each do |n|
+          prefix_path(body_node) do |prefix_node, path|
-            # Skip strings that don't start with one of the keywords
+            next unless match = path.match(%r{^(bin|include|libexec|lib|sbin|share|Frameworks)(?:/| |$)})
-            next unless regex_match_group(n, %r{^(bin|include|libexec|lib|sbin|share|Frameworks)/?})
 
-            parent = n.parent
+            offending_node(prefix_node)
-            # Only look at keywords that have `prefix` before them
+            problem "Use `#{match[1].downcase}` instead of `prefix + \"#{match[1]}\"`"
-            # TODO: this should be refactored to a direct method match
-            prefix_keyword_regex = %r{(prefix\s*\+\s*["'](bin|include|libexec|lib|sbin|share|Frameworks))["'/]}
-            if match = parent.source.match(prefix_keyword_regex)
-              offending_node(parent)
-              problem "Use `#{match[2].downcase}` instead of `#{match[1]}\"`"
-            end
           end
         end
+
+        # Find: prefix + "foo"
+        def_node_search :prefix_path, <<~EOS
+          $(send (send nil? :prefix) :+ (str $_))
+        EOS
       end
     end
 
@@ -134,18 +132,14 @@ module RuboCop
             problem "`env :userpaths` in homebrew/core formulae is deprecated"
           end
 
-          body_node.each_descendant(:dstr) do |dstr_node|
+          share_path_starts_with(body_node, @formula_name) do |share_node|
-            next unless match = dstr_node.source.match(%r{(\#{share}/#{Regexp.escape(@formula_name)})[ /"]})
+            offending_node(share_node)
-
+            problem "Use `pkgshare` instead of `share/\"#{@formula_name}\"`"
-            offending_node(dstr_node)
-            problem "Use `\#{pkgshare}` instead of `#{match[1]}`"
           end
 
-          find_every_method_call_by_name(body_node, :share).each do |share_node|
+          interpolated_share_path_starts_with(body_node, "/#{@formula_name}") do |share_node|
-            if match = share_node.parent.source.match(%r{(share\s*[/+]\s*"#{Regexp.escape(@formula_name)})[/"]})
+            offending_node(share_node)
-              offending_node(share_node.parent)
+            problem "Use `\#{pkgshare}` instead of `\#{share}/#{@formula_name}`"
-              problem "Use `pkgshare` instead of `#{match[1]}\"`"
-            end
           end
 
           return unless formula_tap == "homebrew-core"
@@ -154,6 +148,21 @@ module RuboCop
             problem "`env :std` in homebrew/core formulae is deprecated"
           end
         end
+
+        # Check whether value starts with the formula name and then a "/", " " or EOS
+        def path_starts_with?(path, starts_with)
+          path.match?(%r{^#{Regexp.escape(starts_with)}(/| |$)})
+        end
+
+        # Find "#{share}/foo"
+        def_node_search :interpolated_share_path_starts_with, <<~EOS
+          $(dstr (begin (send nil? :share)) (str #path_starts_with?(%1)))
+        EOS
+
+        # Find share/"foo"
+        def_node_search :share_path_starts_with, <<~EOS
+          $(send (send nil? :share) :/ (str #path_starts_with?(%1)))
+        EOS
       end
     end
   end

Library/Homebrew/test/rubocops/text_spec.rb

@@ -449,5 +449,25 @@ describe RuboCop::Cop::FormulaAuditStrict::Text do
         end
       RUBY
     end
+
+    it "when formula name appears afer `share/\"bar\"`" do
+      expect_no_offenses(<<~RUBY, "/homebrew-core/Formula/foo.rb")
+        class Foo < Formula
+          def install
+            ohai share/"bar/foo"
+          end
+        end
+      RUBY
+    end
+
+    it "when formula name appears afer `\"\#{share}/bar\"`" do
+      expect_no_offenses(<<~RUBY, "/homebrew-core/Formula/foo.rb")
+        class Foo < Formula
+          def install
+            ohai "\#{share}/bar/foo"
+          end
+        end
+      RUBY
+    end
   end
 end