Merge pull request #3091 from GauthamGoli/audit_line_rubocop_part_3

audit: Port line_problems to rubocop and add tests part 3

3 files changed, 87 insertions, 17 deletions

diff --git a/Library/Homebrew/rubocops/conflicts_cop.rb b/Library/Homebrew/rubocops/conflicts_cop.rb
index c1b801559..6f05d0567 100644
--- a/Library/Homebrew/rubocops/conflicts_cop.rb
+++ b/Library/Homebrew/rubocops/conflicts_cop.rb
@@ -18,7 +18,7 @@ module RuboCop
 
         def audit_formula(_node, _class_node, _parent_class_node, body)
           return unless versioned_formula?
-          problem MSG if !formula_file_name.start_with?(*WHITELIST) &&
+          problem MSG if !@formula_name.start_with?(*WHITELIST) &&
                          method_called_ever?(body, :conflicts_with)
         end
       end
diff --git a/Library/Homebrew/rubocops/extend/formula_cop.rb b/Library/Homebrew/rubocops/extend/formula_cop.rb
index 59ad1aafb..47dd2d803 100644
--- a/Library/Homebrew/rubocops/extend/formula_cop.rb
+++ b/Library/Homebrew/rubocops/extend/formula_cop.rb
@@ -4,16 +4,17 @@ require_relative "../../extend/string"
 module RuboCop
   module Cop
     class FormulaCop < Cop
+      attr_accessor :file_path
       @registry = Cop.registry
 
       # This method is called by RuboCop and is the main entry point
       def on_class(node)
-        file_path = processed_source.buffer.name
-        return unless file_path_allowed?(file_path)
+        @file_path = processed_source.buffer.name
+        return unless file_path_allowed?
         return unless formula_class?(node)
         return unless respond_to?(:audit_formula)
         class_node, parent_class_node, @body = *node
-        @formula_name = class_name(class_node)
+        @formula_name = Pathname.new(@file_path).basename(".rb").to_s
         audit_formula(node, class_node, parent_class_node, @body)
       end
 
@@ -100,19 +101,22 @@ module RuboCop
       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
+          yield method if parameters_passed?(method, *args)
         end
       end
 
       # Matches a method with a receiver,
       # EX: to match `Formula.factory(name)`
       # call `find_instance_method_call(node, "Formula", :factory)`
+      # EX: to match `build.head?`
+      # call `find_instance_method_call(node, :build, :head?)`
       # 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 && method.receiver.const_name == instance
+          next if method.receiver.nil?
+          next if method.receiver.const_name != instance &&
+                  method.receiver.method_name != instance
           @offense_source_range = method.source_range
           @offensive_node = method
           yield method
@@ -400,12 +404,7 @@ module RuboCop
 
       # Returns true if the formula is versioned
       def versioned_formula?
-        formula_file_name.include?("@") || @formula_name.match(/AT\d+/)
-      end
-
-      # Returns filename of the formula without the extension
-      def formula_file_name
-        File.basename(processed_source.buffer.name, ".rb")
+        @formula_name.include?("@")
       end
 
       # Returns printable component name
@@ -414,6 +413,12 @@ module RuboCop
         method_name(component_node) if component_node.def_type?
       end
 
+      # Returns the formula tap
+      def formula_tap
+        return unless match_obj = @file_path.match(%r{/(homebrew-\w+)/})
+        match_obj[1]
+      end
+
       def problem(msg)
         add_offense(@offensive_node, @offense_source_range, msg)
       end
@@ -432,11 +437,11 @@ module RuboCop
         class_node && class_names.include?(string_content(class_node))
       end
 
-      def file_path_allowed?(file_path)
+      def file_path_allowed?
         paths_to_exclude = [%r{/Library/Homebrew/compat/},
                             %r{/Library/Homebrew/test/}]
-        return true if file_path.nil? # file_path is nil when source is directly passed to the cop eg., in specs
-        file_path !~ Regexp.union(paths_to_exclude)
+        return true if @file_path.nil? # file_path is nil when source is directly passed to the cop eg., in specs
+        @file_path !~ Regexp.union(paths_to_exclude)
       end
     end
   end
diff --git a/Library/Homebrew/rubocops/lines_cop.rb b/Library/Homebrew/rubocops/lines_cop.rb
index ed50ba49c..01b13585c 100644
--- a/Library/Homebrew/rubocops/lines_cop.rb
+++ b/Library/Homebrew/rubocops/lines_cop.rb
@@ -21,7 +21,7 @@ module RuboCop
           begin_pos = start_column(parent_class_node)
           end_pos = end_column(class_node)
           return unless begin_pos-end_pos != 3
-          problem "Use a space in class inheritance: class #{@formula_name} < #{class_name(parent_class_node)}"
+          problem "Use a space in class inheritance: class #{class_name(class_node)} < #{class_name(parent_class_node)}"
         end
       end
 
@@ -67,7 +67,72 @@ module RuboCop
             next unless block_arg.source.size>1
             problem "\"inreplace <filenames> do |s|\" is preferred over \"|#{block_arg.source}|\"."
           end
+
+          [:rebuild, :version_scheme].each do |method_name|
+            find_method_with_args(body_node, method_name, 0) do
+              problem "'#{method_name} 0' should be removed"
+            end
+          end
+
+          [:mac?, :linux?].each do |method_name|
+            next unless formula_tap == "homebrew-core"
+            find_instance_method_call(body_node, "OS", method_name) do |check|
+              problem "Don't use #{check.source}; Homebrew/core only supports macOS"
+            end
+          end
+
+          find_method_with_args(body_node, :fails_with, :llvm) do
+            problem "'fails_with :llvm' is now a no-op so should be removed"
+          end
+
+          find_method_with_args(body_node, :system, /^(otool|install_name_tool|lipo)$/) do
+            next if @formula_name == "cctools"
+            problem "Use ruby-macho instead of calling #{@offensive_node.source}"
+          end
+
+          find_every_method_call_by_name(body_node, :system).each do |method_node|
+            # Skip Kibana: npm cache edge (see formula for more details)
+            next if @formula_name =~ /^kibana(\@\d+(\.\d+)?)?$/
+            first_param, second_param = parameters(method_node)
+            next if !node_equals?(first_param, "npm") ||
+                    !node_equals?(second_param, "install")
+            offending_node(method_node)
+            problem "Use Language::Node for npm install args" unless languageNodeModule?(method_node)
+          end
+
+          if find_method_def(body_node, :test)
+            problem "Use new-style test definitions (test do)"
+          end
+
+          if find_method_def(body_node, :options)
+            problem "Use new-style option definitions"
+          end
+
+          find_method_with_args(body_node, :skip_clean, :all) do
+            problem <<-EOS.undent.chomp
+              `skip_clean :all` is deprecated; brew no longer strips symbols
+                      Pass explicit paths to prevent Homebrew from removing empty folders.
+            EOS
+          end
+
+          find_instance_method_call(body_node, :build, :universal?) do
+            next if @formula_name == "wine"
+            problem "macOS has been 64-bit only since 10.6 so build.universal? is deprecated."
+          end
+
+          find_instance_method_call(body_node, "ENV", :universal_binary) do
+            problem "macOS has been 64-bit only since 10.6 so ENV.universal_binary is deprecated."
+          end
+
+          find_instance_method_call(body_node, "ENV", :x11) do
+            problem 'Use "depends_on :x11" instead of "ENV.x11"'
+          end
         end
+
+        # Node Pattern search for Language::Node
+        def_node_search :languageNodeModule?, <<-EOS.undent
+          (const (const nil :Language) :Node)
+        EOS
       end
     end
   end