Add notability checks for casks

Library/Homebrew/cask/audit.rb

@@ -6,6 +6,7 @@
 require "digest"
 require "utils/curl"
 require "utils/git"
+require "utils/notability"
 
 module Cask
   class Audit
@@ -14,22 +15,22 @@ class Audit
 
     attr_reader :cask, :commit_range, :download
 
-    attr_predicate :check_appcast?
+    attr_predicate :appcast?
 
-    def initialize(cask, check_appcast: false, download: false, check_token_conflicts: false,
-                   commit_range: nil, command: SystemCommand)
+    def initialize(cask, appcast: false, download: false,
+                   token_conflicts: false, online: false, strict: false,
+                   new_cask: false, commit_range: nil, command: SystemCommand)
       @cask = cask
-      @check_appcast = check_appcast
+      @appcast = appcast
       @download = download
+      @online = online
+      @strict = strict
+      @new_cask = new_cask
       @commit_range = commit_range
-      @check_token_conflicts = check_token_conflicts
+      @token_conflicts = token_conflicts
       @command = command
     end
 
-    def check_token_conflicts?
-      @check_token_conflicts
-    end
-
     def run!
       check_blacklist
       check_required_stanzas
@@ -48,6 +49,9 @@ def run!
       check_latest_with_auto_updates
       check_stanza_requires_uninstall
       check_appcast_contains_version
+      check_github_repository
+      check_gitlab_repository
+      check_bitbucket_repository
       self
     rescue => e
       odebug "#{e.message}\n#{e.backtrace.join("\n")}"
@@ -272,7 +276,7 @@ def check_generic_artifacts
     end
 
     def check_token_conflicts
-      return unless check_token_conflicts?
+      return unless @token_conflicts
       return unless core_formula_names.include?(cask.token)
 
       add_warning "possible duplicate, cask token conflicts with Homebrew core formula: #{core_formula_url}"
@@ -301,7 +305,7 @@ def check_download
     end
 
     def check_appcast_contains_version
-      return unless check_appcast?
+      return unless appcast?
       return if cask.appcast.to_s.empty?
       return if cask.appcast.configuration == :no_check
 
@@ -322,6 +326,50 @@ def check_appcast_contains_version
       add_error "appcast at URL '#{appcast_stanza}' offline or looping"
     end
 
+    def check_github_repository
+      user, repo = get_repo_data(%r{https?://github\.com/([^/]+)/([^/]+)/?.*})
+      return if user.nil?
+
+      odebug "Auditing GitHub repo"
+
+      error = SharedAudits.github(user, repo)
+      add_error error if error
+    end
+
+    def check_gitlab_repository
+      user, repo = get_repo_data(%r{https?://gitlab\.com/([^/]+)/([^/]+)/?.*})
+      return if user.nil?
+
+      odebug "Auditing GitLab repo"
+
+      error = SharedAudits.gitlab(user, repo)
+      add_error error if error
+    end
+
+    def check_bitbucket_repository
+      user, repo = get_repo_data(%r{https?://bitbucket\.org/([^/]+)/([^/]+)/?.*})
+      return if user.nil?
+
+      odebug "Auditing Bitbucket repo"
+
+      error = SharedAudits.bitbucket(user, repo)
+      add_error error if error
+    end
+
+    def get_repo_data(regex)
+      return unless @online
+      return unless @new_cask
+
+      _, user, repo = *regex.match(cask.url.to_s)
+      _, user, repo = *regex.match(cask.homepage) unless user
+      _, user, repo = *regex.match(cask.appcast.to_s) unless user
+      return if !user || !repo
+
+      repo.gsub!(/.git$/, "")
+
+      [user, repo]
+    end
+
     def check_blacklist
       return if cask.tap&.user != "Homebrew"
       return unless reason = Blacklist.blacklisted_reason(cask.token)

Library/Homebrew/cask/auditor.rb

@@ -8,34 +8,37 @@ class Auditor
     extend Predicable
 
     def self.audit(cask, audit_download: false, audit_appcast: false,
-                   check_token_conflicts: false, quarantine: true, commit_range: nil)
+                   audit_online: false, audit_strict: false,
+                   audit_token_conflicts: false, audit_new_cask: false,
+                   quarantine: true, commit_range: nil)
       new(cask, audit_download: audit_download,
                 audit_appcast: audit_appcast,
-                check_token_conflicts: check_token_conflicts,
+                audit_online: audit_online,
+                audit_new_cask: audit_new_cask,
+                audit_strict: audit_strict,
+                audit_token_conflicts: audit_token_conflicts,
                 quarantine: quarantine, commit_range: commit_range).audit
     end
 
     attr_reader :cask, :commit_range
 
     def initialize(cask, audit_download: false, audit_appcast: false,
-                   check_token_conflicts: false, quarantine: true, commit_range: nil)
+                   audit_online: false, audit_strict: false,
+                   audit_token_conflicts: false, audit_new_cask: false,
+                   quarantine: true, commit_range: nil)
       @cask = cask
       @audit_download = audit_download
       @audit_appcast = audit_appcast
+      @audit_online = audit_online
+      @audit_strict = audit_strict
+      @audit_new_cask = audit_new_cask
       @quarantine = quarantine
       @commit_range = commit_range
-      @check_token_conflicts = check_token_conflicts
+      @audit_token_conflicts = audit_token_conflicts
     end
 
-    def audit_download?
-      @audit_download
-    end
-
-    attr_predicate :audit_appcast?, :quarantine?
-
-    def check_token_conflicts?
-      @check_token_conflicts
-    end
+    attr_predicate :audit_appcast?, :audit_download?, :audit_online?,
+                   :audit_strict?, :audit_new_cask?, :audit_token_conflicts?, :quarantine?
 
     def audit
       if !Homebrew.args.value("language") && language_blocks
@@ -64,10 +67,13 @@ def audit_languages(languages)
 
     def audit_cask_instance(cask)
       download = audit_download? && Download.new(cask, quarantine: quarantine?)
-      audit = Audit.new(cask, check_appcast:         audit_appcast?,
-                              download:              download,
-                              check_token_conflicts: check_token_conflicts?,
-                              commit_range:          commit_range)
+      audit = Audit.new(cask, appcast:         audit_appcast?,
+                              online:          audit_online?,
+                              strict:          audit_strict?,
+                              new_cask:        audit_new_cask?,
+                              token_conflicts: audit_token_conflicts?,
+                              download:        download,
+                              commit_range:    commit_range)
       audit.run!
       puts audit.summary
       audit.success?

Library/Homebrew/cask/cmd/audit.rb

@@ -1,32 +1,66 @@
 # frozen_string_literal: true
 
+require "cli/parser"
+
 module Cask
   class Cmd
     class Audit < AbstractCommand
-      option "--download",        :download,        false
-      option "--appcast",         :appcast,         false
-      option "--token-conflicts", :token_conflicts, false
+      option "--download",        :download_arg,        false
+      option "--appcast",         :appcast_arg,         false
+      option "--token-conflicts", :token_conflicts_arg, false
+      option "--strict",          :strict_arg,          false
+      option "--online",          :online_arg,          false
+      option "--new-cask",        :new_cask_arg,        false
+
+      def self.usage
+        <<~EOS
+          `cask audit` [<options>] [<cask>]
+
+          --strict          - Run additional, stricter style checks.
+          --online          - Run additional, slower style checks that require a network connection.
+          --new-cask        - Run various additional style checks to determine if a new cask is eligible
+                              for Homebrew. This should be used when creating new casks and implies
+                              `--strict` and `--online`.
+          --download        - Audit the downloaded file
+          --appcast         - Audit the appcast
+          --token-conflicts - Audit for token conflicts
+
+          Check <cask> for Homebrew coding style violations. This should be run before
+          submitting a new cask. If no <casks> are provided, check all locally
+          available casks. Will exit with a non-zero status if any errors are
+          found, which can be useful for implementing pre-commit hooks.
+        EOS
+      end
 
       def self.help
         "verifies installability of Casks"
       end
 
       def run
+        Homebrew.auditing = true
+        strict = new_cask_arg? || strict_arg?
+        token_conflicts = strict || token_conflicts_arg?
+
+        online = new_cask_arg? || online_arg?
+        download = online || download_arg?
+        appcast = online || appcast_arg?
+
         failed_casks = casks(alternative: -> { Cask.to_a })
-                       .reject { |cask| audit(cask) }
+                       .reject do |cask|
+          odebug "Auditing Cask #{cask}"
+          Auditor.audit(cask, audit_download:        download,
+                              audit_appcast:         appcast,
+                              audit_online:          online,
+                              audit_strict:          strict,
+                              audit_new_cask:        new_cask_arg?,
+                              audit_token_conflicts: token_conflicts,
+                              quarantine:            quarantine?)
+        end
 
         return if failed_casks.empty?
 
         raise CaskError, "audit failed for casks: #{failed_casks.join(" ")}"
       end
-
-      def audit(cask)
-        odebug "Auditing Cask #{cask}"
-        Auditor.audit(cask, audit_download:        download?,
-                            audit_appcast:         appcast?,
-                            check_token_conflicts: token_conflicts?,
-                            quarantine:            quarantine?)
-      end
     end
   end
 end

Library/Homebrew/dev-cmd/audit.rb

@@ -3,6 +3,7 @@
 require "formula"
 require "formula_versions"
 require "utils/curl"
+require "utils/notability"
 require "extend/ENV"
 require "formula_cellar_checks"
 require "cmd/search"
@@ -510,79 +511,30 @@ def audit_github_repository
       user, repo = get_repo_data(%r{https?://github\.com/([^/]+)/([^/]+)/?.*})
       return if user.nil?
 
-      begin
-        metadata = GitHub.repository(user, repo)
-      rescue GitHub::HTTPNotFoundError
-        return
-      end
-
-      return if metadata.nil?
-
-      new_formula_problem "GitHub fork (not canonical repository)" if metadata["fork"]
-      if (metadata["forks_count"] < 30) && (metadata["subscribers_count"] < 30) &&
-         (metadata["stargazers_count"] < 75)
-        new_formula_problem "GitHub repository not notable enough (<30 forks, <30 watchers and <75 stars)"
-      end
+      warning = SharedAudits.github(user, repo)
+      return if warning.nil?
 
-      return if Date.parse(metadata["created_at"]) <= (Date.today - 30)
-
-      new_formula_problem "GitHub repository too new (<30 days old)"
+      new_formula_problem warning
     end
 
     def audit_gitlab_repository
       user, repo = get_repo_data(%r{https?://gitlab\.com/([^/]+)/([^/]+)/?.*})
       return if user.nil?
 
-      out, _, status= curl_output("--request", "GET", "https://gitlab.com/api/v4/projects/#{user}%2F#{repo}")
-      return unless status.success?
-
-      metadata = JSON.parse(out)
-      return if metadata.nil?
-
-      new_formula_problem "GitLab fork (not canonical repository)" if metadata["fork"]
-      if (metadata["forks_count"] < 30) && (metadata["star_count"] < 75)
-        new_formula_problem "GitLab repository not notable enough (<30 forks and <75 stars)"
-      end
-
-      return if Date.parse(metadata["created_at"]) <= (Date.today - 30)
+      warning = SharedAudits.gitlab(user, repo)
+      return if warning.nil?
 
-      new_formula_problem "GitLab repository too new (<30 days old)"
+      new_formula_problem warning
     end
 
     def audit_bitbucket_repository
       user, repo = get_repo_data(%r{https?://bitbucket\.org/([^/]+)/([^/]+)/?.*})
       return if user.nil?
 
-      api_url = "https://api.bitbucket.org/2.0/repositories/#{user}/#{repo}"
-      out, _, status= curl_output("--request", "GET", api_url)
-      return unless status.success?
-
-      metadata = JSON.parse(out)
-      return if metadata.nil?
-
-      new_formula_problem "Uses deprecated mercurial support in Bitbucket" if metadata["scm"] == "hg"
-
-      new_formula_problem "Bitbucket fork (not canonical repository)" unless metadata["parent"].nil?
-
-      if Date.parse(metadata["created_on"]) >= (Date.today - 30)
-        new_formula_problem "Bitbucket repository too new (<30 days old)"
-      end
-
-      forks_out, _, forks_status= curl_output("--request", "GET", "#{api_url}/forks")
-      return unless forks_status.success?
-
-      watcher_out, _, watcher_status= curl_output("--request", "GET", "#{api_url}/watchers")
-      return unless watcher_status.success?
-
-      forks_metadata = JSON.parse(forks_out)
-      return if forks_metadata.nil?
-
-      watcher_metadata = JSON.parse(watcher_out)
-      return if watcher_metadata.nil?
-
-      return if (forks_metadata["size"] < 30) && (watcher_metadata["size"] < 75)
+      warning = SharedAudits.bitbucket(user, repo)
+      return if warning.nil?
 
-      new_formula_problem "Bitbucket repository not notable enough (<30 forks and <75 watchers)"
+      new_formula_problem warning
     end
 
     def get_repo_data(regex)

Library/Homebrew/test/cask/audit_spec.rb

@@ -39,12 +39,12 @@ def include_msg?(messages, msg)
 
   let(:cask) { instance_double(Cask::Cask) }
   let(:download) { false }
-  let(:check_token_conflicts) { false }
+  let(:token_conflicts) { false }
   let(:fake_system_command) { class_double(SystemCommand) }
   let(:audit) {
-    described_class.new(cask, download:              download,
-                              check_token_conflicts: check_token_conflicts,
-                              command:               fake_system_command)
+    described_class.new(cask, download:        download,
+                              token_conflicts: token_conflicts,
+                              command:         fake_system_command)
   }
 
   describe "#result" do
@@ -517,7 +517,7 @@ def include_msg?(messages, msg)
 
     describe "token conflicts" do
       let(:cask_token) { "with-binary" }
-      let(:check_token_conflicts) { true }
+      let(:token_conflicts) { true }
 
       context "when cask token conflicts with a core formula" do
         let(:formula_names) { %w[with-binary other-formula] }