Merge pull request #9187 from reitermarkus/cask-audit

Preparations for merging `brew cask audit` into `brew audit`.

Library/Homebrew/cask/cmd/audit.rb

@@ -42,23 +42,6 @@ module Cask
 
       sig { void }
       def run
-        require "cask/auditor"
-
-        Homebrew.auditing = true
-
-        options = {
-          audit_download:        args.download?,
-          audit_appcast:         args.appcast?,
-          audit_online:          args.online?,
-          audit_strict:          args.strict?,
-          audit_new_cask:        args.new_cask?,
-          audit_token_conflicts: args.token_conflicts?,
-          quarantine:            args.quarantine?,
-          language:              args.language,
-        }.compact
-
-        options[:quarantine] = true if options[:quarantine].nil?
-
         casks = args.named.flat_map do |name|
           next name if File.exist?(name)
           next Tap.fetch(name).cask_files if name.count("/") == 1
@@ -68,6 +51,47 @@ module Cask
         casks = casks.map { |c| CaskLoader.load(c, config: Config.from_args(args)) }
         casks = Cask.to_a if casks.empty?
 
+        self.class.audit_casks(
+          *casks,
+          download:        args.download?,
+          appcast:         args.appcast?,
+          online:          args.online?,
+          strict:          args.strict?,
+          new_cask:        args.new_cask?,
+          token_conflicts: args.token_conflicts?,
+          quarantine:      args.quarantine?,
+          language:        args.language,
+        )
+      end
+
+      def self.audit_casks(
+        *casks,
+        download: nil,
+        appcast: nil,
+        online: nil,
+        strict: nil,
+        new_cask: nil,
+        token_conflicts: nil,
+        quarantine: nil,
+        language: nil
+      )
+        options = {
+          audit_download:        download,
+          audit_appcast:         appcast,
+          audit_online:          online,
+          audit_strict:          strict,
+          audit_new_cask:        new_cask,
+          audit_token_conflicts: token_conflicts,
+          quarantine:            quarantine,
+          language:              language,
+        }.compact
+
+        options[:quarantine] = true if options[:quarantine].nil?
+
+        Homebrew.auditing = true
+
+        require "cask/auditor"
+
         failed_casks = casks.reject do |cask|
           odebug "Auditing Cask #{cask}"
           result = Auditor.audit(cask, **options)

Library/Homebrew/formula_text_auditor.rb

+# typed: false
+# frozen_string_literal: true
+
+module Homebrew
+  # Auditor for checking common violations in {Formula} text content.
+  #
+  # @api private
+  class FormulaTextAuditor
+    def initialize(path)
+      @text = path.open("rb", &:read)
+      @lines = @text.lines.to_a
+    end
+
+    def without_patch
+      @text.split("\n__END__").first
+    end
+
+    def trailing_newline?
+      /\Z\n/ =~ @text
+    end
+
+    def =~(other)
+      other =~ @text
+    end
+
+    def include?(s)
+      @text.include? s
+    end
+
+    def line_number(regex, skip = 0)
+      index = @lines.drop(skip).index { |line| line =~ regex }
+      index ? index + 1 : nil
+    end
+
+    def reverse_line_number(regex)
+      index = @lines.reverse.index { |line| line =~ regex }
+      index ? @lines.count - index : nil
+    end
+  end
+end

Library/Homebrew/resource_auditor.rb

+# typed: false
+# frozen_string_literal: true
+
+module Homebrew
+  # Auditor for checking common violations in {Resource}s.
+  #
+  # @api private
+  class ResourceAuditor
+    attr_reader :name, :version, :checksum, :url, :mirrors, :using, :specs, :owner, :spec_name, :problems
+
+    def initialize(resource, spec_name, options = {})
+      @name     = resource.name
+      @version  = resource.version
+      @checksum = resource.checksum
+      @url      = resource.url
+      @mirrors  = resource.mirrors
+      @using    = resource.using
+      @specs    = resource.specs
+      @owner    = resource.owner
+      @spec_name = spec_name
+      @online    = options[:online]
+      @strict    = options[:strict]
+      @problems  = []
+    end
+
+    def audit
+      audit_version
+      audit_download_strategy
+      audit_urls
+      self
+    end
+
+    def audit_version
+      if version.nil?
+        problem "missing version"
+      elsif !version.detected_from_url?
+        version_text = version
+        version_url = Version.detect(url, **specs)
+        if version_url.to_s == version_text.to_s && version.instance_of?(Version)
+          problem "version #{version_text} is redundant with version scanned from URL"
+        end
+      end
+    end
+
+    def audit_download_strategy
+      url_strategy = DownloadStrategyDetector.detect(url)
+
+      if (using == :git || url_strategy == GitDownloadStrategy) && specs[:tag] && !specs[:revision]
+        problem "Git should specify :revision when a :tag is specified."
+      end
+
+      return unless using
+
+      if using == :cvs
+        mod = specs[:module]
+
+        problem "Redundant :module value in URL" if mod == name
+
+        if url.match?(%r{:[^/]+$})
+          mod = url.split(":").last
+
+          if mod == name
+            problem "Redundant CVS module appended to URL"
+          else
+            problem "Specify CVS module as `:module => \"#{mod}\"` instead of appending it to the URL"
+          end
+        end
+      end
+
+      return unless url_strategy == DownloadStrategyDetector.detect("", using)
+
+      problem "Redundant :using value in URL"
+    end
+
+    def self.curl_openssl_and_deps
+      @curl_openssl_and_deps ||= begin
+        formulae_names = ["curl", "openssl"]
+        formulae_names += formulae_names.flat_map do |f|
+          Formula[f].recursive_dependencies.map(&:name)
+        end
+        formulae_names.uniq
+      rescue FormulaUnavailableError
+        []
+      end
+    end
+
+    def audit_urls
+      return unless @online
+
+      urls = [url] + mirrors
+      urls.each do |url|
+        next if !@strict && mirrors.include?(url)
+
+        strategy = DownloadStrategyDetector.detect(url, using)
+        if strategy <= CurlDownloadStrategy && !url.start_with?("file")
+          # A `brew mirror`'ed URL is usually not yet reachable at the time of
+          # pull request.
+          next if url.match?(%r{^https://dl.bintray.com/homebrew/mirror/})
+
+          if http_content_problem = curl_check_http_content(url)
+            problem http_content_problem
+          end
+        elsif strategy <= GitDownloadStrategy
+          problem "The URL #{url} is not a valid git URL" unless Utils::Git.remote_exists? url
+        elsif strategy <= SubversionDownloadStrategy
+          next unless DevelopmentTools.subversion_handles_most_https_certificates?
+          next unless Utils::Svn.available?
+
+          problem "The URL #{url} is not a valid svn URL" unless Utils::Svn.remote_exists? url
+        end
+      end
+    end
+
+    def problem(text)
+      @problems << text
+    end
+  end
+end

Library/Homebrew/tap_auditor.rb

+# typed: true
+# frozen_string_literal: true
+
+module Homebrew
+  # Auditor for checking common violations in {Tap}s.
+  #
+  # @api private
+  class TapAuditor
+    extend T::Sig
+
+    attr_reader :name, :path, :tap_audit_exceptions, :problems
+
+    sig { params(tap: Tap, strict: T::Boolean).void }
+    def initialize(tap, strict:)
+      @name                 = tap.name
+      @path                 = tap.path
+      @tap_audit_exceptions = tap.audit_exceptions
+      @problems             = []
+    end
+
+    sig { void }
+    def audit
+      audit_json_files
+      audit_tap_audit_exceptions
+    end
+
+    sig { void }
+    def audit_json_files
+      json_patterns = Tap::HOMEBREW_TAP_JSON_FILES.map { |pattern| @path/pattern }
+      Pathname.glob(json_patterns).each do |file|
+        JSON.parse file.read
+      rescue JSON::ParserError
+        problem "#{file.to_s.delete_prefix("#{@path}/")} contains invalid JSON"
+      end
+    end
+
+    sig { void }
+    def audit_tap_audit_exceptions
+      @tap_audit_exceptions.each do |list_name, formula_names|
+        unless [Hash, Array].include? formula_names.class
+          problem <<~EOS
+            audit_exceptions/#{list_name}.json should contain a JSON array
+            of formula names or a JSON object mapping formula names to values
+          EOS
+          next
+        end
+
+        formula_names = formula_names.keys if formula_names.is_a? Hash
+
+        invalid_formulae = []
+        formula_names.each do |name|
+          invalid_formulae << name if Formula[name].tap != @name
+        rescue FormulaUnavailableError
+          invalid_formulae << name
+        end
+
+        next if invalid_formulae.empty?
+
+        problem <<~EOS
+          audit_exceptions/#{list_name}.json references
+          formulae that are not found in the #{@name} tap.
+          Invalid formulae: #{invalid_formulae.join(", ")}
+        EOS
+      end
+    end
+
+    sig { params(message: String).void }
+    def problem(message)
+      @problems << ({ message: message, location: nil })
+    end
+  end
+end

Library/Homebrew/test/dev-cmd/audit_spec.rb

@@ -18,7 +18,7 @@ module Count
 end
 
 module Homebrew
-  describe FormulaText do
+  describe FormulaTextAuditor do
     alias_matcher :have_data, :be_data
     alias_matcher :have_end, :be_end
     alias_matcher :have_trailing_newline, :be_trailing_newline