Refactor and add more methods in formula_cop.rb

Library/Homebrew/rubocops/extend/formula_cop.rb

@@ -1,3 +1,5 @@
+require "parser/current"
+
 module RuboCop
   module Cop
     class FormulaCop < Cop
@@ -9,9 +11,9 @@ module RuboCop
         return unless file_path_allowed?(file_path)
         return unless formula_class?(node)
         return unless respond_to?(:audit_formula)
-        class_node, parent_class_node, body = *node
+        class_node, parent_class_node, @body = *node
         @formula_name = class_name(class_node)
-        audit_formula(node, class_node, parent_class_node, body)
+        audit_formula(node, class_node, parent_class_node, @body)
       end
 
       # Checks for regex match of pattern in the node and
@@ -64,11 +66,34 @@ module RuboCop
         node.each_descendant(:send).select { |method_node| method_name == method_node.method_name }
       end
 
+      # Given a method_name and arguments, yields to a block with
+      # matching method passed as a parameter to the block
+      def find_method_with_args(node, method_name, *args)
+        methods = find_every_method_call_by_name(node, method_name)
+        methods.each do |method|
+          next unless parameters_passed?(method, *args)
+          yield method
+        end
+      end
+
+      # Matches a method with a receiver,
+      # EX: to match `Formula.factory(name)`
+      # call `find_instance_method_call(node, "Formula", :factory)`
+      # yields to a block with matching method node
+      def find_instance_method_call(node, instance, method_name)
+        methods = find_every_method_call_by_name(node, method_name)
+        methods.each do |method|
+          next unless method.receiver.const_name == instance
+          @offense_source_range = method.source_range
+          @offensive_node = method
+          yield method
+        end
+      end
+
       # Returns nil if does not depend on dependency_name
-      # args: node - formula class' body node
-      #       dependency_name - dependency's name
-      def depends_on?(node, dependency_name)
-        dependency_nodes = find_every_method_call_by_name(node, :depends_on)
+      # args: node - dependency_name - dependency's name
+      def depends_on?(dependency_name)
+        dependency_nodes = find_every_method_call_by_name(@body, :depends_on)
         idx = dependency_nodes.index do |n|
           depends_on_name_type?(n, dependency_name, :required) ||
             depends_on_name_type?(n, dependency_name, :build) ||
@@ -76,29 +101,45 @@ module RuboCop
             depends_on_name_type?(n, dependency_name, :recommended) ||
             depends_on_name_type?(n, dependency_name, :run)
         end
-        return nil if idx.nil?
+        return if idx.nil?
         @offense_source_range = dependency_nodes[idx].source_range
         @offensive_node = dependency_nodes[idx]
       end
 
       # Returns true if given dependency name and dependency type exist in given dependency method call node
-      def depends_on_name_type?(node, dependency_name = nil, dependency_type = :required)
-        dependency_name_match = true if dependency_name.nil? # Match only by type
-        case dependency_type
-        when :required
-          dependency_type_match = !node.method_args.nil? && node.method_args.first.str_type?
-          dependency_name_match = (string_content(node.method_args.first) == dependency_name) if dependency_type_match
-        when :build || :optional || :recommended || :run
-          dependency_type_match = !node.method_args.nil? &&
-                                  node.method_args.first.hash_type? &&
-                                  node.method_args.first.values[0].children.first == dependency_type
-          dependency_name_match = (node.method_args.first.keys[0].children.first == dependency_name) if dependency_type_match
+      # TODO: Add case where key of hash is an array
+      def depends_on_name_type?(node, name = nil, type = :required)
+        if name
+          name_match = false
+        else
+          name_match = true # Match only by type when name is nil
         end
-        if dependency_type_match || dependency_name_match
+
+        case type
+        when :required
+          type_match = !node.method_args.nil? && node.method_args.first.str_type?
+          if type_match && !name_match
+            name_match = node_equals?(node.method_args.first, name)
+          end
+        when :build, :optional, :recommended, :run
+          type_match = !node.method_args.nil? &&
+                       node.method_args.first.hash_type? &&
+                       node.method_args.first.values.first.children.first == type
+          if type_match && !name_match
+            name_match = node_equals?(node.method_args.first.keys.first.children.first, name)
+          end
+        end
+
+        if type_match || name_match
           @offensive_node = node
           @offense_source_range = node.source_range
         end
-        dependency_type_match && dependency_name_match
+        type_match && name_match
+      end
+
+      # To compare node with appropriate Ruby variable
+      def node_equals?(node, var)
+        node == Parser::CurrentRuby.parse(var.inspect)
       end
 
       # Returns a block named block_name inside node
@@ -200,12 +241,19 @@ module RuboCop
 
       # Returns true if the given parameters are present in method call
       # and sets the method call as the offending node
+      # params can be string, symbol, array, hash, matching regex
       def parameters_passed?(method_node, *params)
         method_params = parameters(method_node)
         @offensive_node = method_node
         @offense_source_range = method_node.source_range
         params.all? do |given_param|
-          method_params.any? { |method_param| given_param == string_content(method_param) }
+          method_params.any? do |method_param|
+            if given_param.class == Regexp
+              regex_match_group(method_param, given_param)
+            else
+              node_equals?(method_param, given_param)
+            end
+          end
         end
       end