audit: Port rules from line_problems to rubocop part 3
This commit is contained in:
		
							parent
							
								
									751334a257
								
							
						
					
					
						commit
						267def28fa
					
				@ -892,10 +892,6 @@ class FormulaAuditor
 | 
			
		||||
 | 
			
		||||
    problem "Use spaces instead of tabs for indentation" if line =~ /^[ ]*\t/
 | 
			
		||||
 | 
			
		||||
    if line.include?("ENV.x11")
 | 
			
		||||
      problem "Use \"depends_on :x11\" instead of \"ENV.x11\""
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    if line.include?("ENV.java_cache")
 | 
			
		||||
      problem "In-formula ENV.java_cache usage has been deprecated & should be removed."
 | 
			
		||||
    end
 | 
			
		||||
@ -913,14 +909,6 @@ class FormulaAuditor
 | 
			
		||||
      problem "Use ENV instead of invoking '#{Regexp.last_match(1)}' to modify the environment"
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    if formula.name != "wine" && line =~ /ENV\.universal_binary/
 | 
			
		||||
      problem "macOS has been 64-bit only since 10.6 so ENV.universal_binary is deprecated."
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    if line =~ /build\.universal\?/
 | 
			
		||||
      problem "macOS has been 64-bit only so build.universal? is deprecated."
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    if line =~ /version == ['"]HEAD['"]/
 | 
			
		||||
      problem "Use 'build.head?' instead of inspecting 'version'"
 | 
			
		||||
    end
 | 
			
		||||
@ -961,12 +949,6 @@ class FormulaAuditor
 | 
			
		||||
      problem "Use build instead of ARGV to check options"
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    problem "Use new-style option definitions" if line.include?("def options")
 | 
			
		||||
 | 
			
		||||
    if line.end_with?("def test")
 | 
			
		||||
      problem "Use new-style test definitions (test do)"
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    if line.include?("MACOS_VERSION")
 | 
			
		||||
      problem "Use MacOS.version instead of MACOS_VERSION"
 | 
			
		||||
    end
 | 
			
		||||
@ -980,11 +962,6 @@ class FormulaAuditor
 | 
			
		||||
      problem "\"#{$&}\" is deprecated, use a comparison to MacOS.version instead"
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    if line =~ /skip_clean\s+:all/
 | 
			
		||||
      problem "`skip_clean :all` is deprecated; brew no longer strips symbols\n" \
 | 
			
		||||
              "\tPass explicit paths to prevent Homebrew from removing empty folders."
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    if line =~ /depends_on [A-Z][\w:]+\.new$/
 | 
			
		||||
      problem "`depends_on` can take requirement classes instead of instances"
 | 
			
		||||
    end
 | 
			
		||||
@ -1023,30 +1000,6 @@ class FormulaAuditor
 | 
			
		||||
      problem "Use `assert_match` instead of `assert ...include?`"
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    if line.include?('system "npm", "install"') && !line.include?("Language::Node") &&
 | 
			
		||||
       formula.name !~ /^kibana(\@\d+(\.\d+)?)?$/
 | 
			
		||||
      problem "Use Language::Node for npm install args"
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    if line.include?("fails_with :llvm")
 | 
			
		||||
      problem "'fails_with :llvm' is now a no-op so should be removed"
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    if line =~ /system\s+['"](otool|install_name_tool|lipo)/ && formula.name != "cctools"
 | 
			
		||||
      problem "Use ruby-macho instead of calling #{Regexp.last_match(1)}"
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    if formula.tap.to_s == "homebrew/core"
 | 
			
		||||
      ["OS.mac?", "OS.linux?"].each do |check|
 | 
			
		||||
        next unless line.include?(check)
 | 
			
		||||
        problem "Don't use #{check}; Homebrew/core only supports macOS"
 | 
			
		||||
      end
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    if line =~ /((revision|version_scheme)\s+0)/
 | 
			
		||||
      problem "'#{Regexp.last_match(1)}' should be removed"
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    return unless @strict
 | 
			
		||||
 | 
			
		||||
    problem "`#{Regexp.last_match(1)}` in formulae is deprecated" if line =~ /(env :(std|userpaths))/
 | 
			
		||||
 | 
			
		||||
@ -4,12 +4,13 @@ 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
 | 
			
		||||
@ -100,8 +101,7 @@ 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
 | 
			
		||||
 | 
			
		||||
@ -112,7 +112,9 @@ module RuboCop
 | 
			
		||||
      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
 | 
			
		||||
@ -414,6 +416,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
 | 
			
		||||
@ -425,11 +433,11 @@ module RuboCop
 | 
			
		||||
        class_node && string_content(class_node) == "Formula"
 | 
			
		||||
      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
 | 
			
		||||
 | 
			
		||||
@ -67,7 +67,71 @@ 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|
 | 
			
		||||
            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 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
 | 
			
		||||
 | 
			
		||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user