Add notibility checks for casks

Library/Homebrew/cask/audit.rb

@@ -6,6 +6,7 @@ require "cask/download"
 require "digest"
 require "utils/curl"
 require "utils/git"
+require "utils/notability"
 
 module Cask
   class Audit
@@ -14,22 +15,22 @@ module Cask
 
     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,
+    def initialize(cask, appcast: false, download: false,
-                   commit_range: nil, command: SystemCommand)
+                   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 @@ module Cask
       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 @@ module Cask
     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 @@ module Cask
     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 @@ module Cask
       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 @@ module Cask
     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?
+    attr_predicate :audit_appcast?, :audit_download?, :audit_online?,
-      @audit_download
+                   :audit_strict?, :audit_new_cask?, :audit_token_conflicts?, :quarantine?
-    end
-
-    attr_predicate :audit_appcast?, :quarantine?
-
-    def check_token_conflicts?
-      @check_token_conflicts
-    end
 
     def audit
       if !Homebrew.args.value("language") && language_blocks
@@ -64,10 +67,13 @@ module Cask
 
     def audit_cask_instance(cask)
       download = audit_download? && Download.new(cask, quarantine: quarantine?)
-      audit = Audit.new(cask, check_appcast:         audit_appcast?,
+      audit = Audit.new(cask, appcast:         audit_appcast?,
-                              download:              download,
+                              online:          audit_online?,
-                              check_token_conflicts: check_token_conflicts?,
+                              strict:          audit_strict?,
-                              commit_range:          commit_range)
+                              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 "--download",        :download_arg,        false
-      option "--appcast",         :appcast,         false
+      option "--appcast",         :appcast_arg,         false
-      option "--token-conflicts", :token_conflicts, 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 @@ module Homebrew
       user, repo = get_repo_data(%r{https?://github\.com/([^/]+)/([^/]+)/?.*})
       return if user.nil?
 
-      begin
+      warning = SharedAudits.github(user, repo)
-        metadata = GitHub.repository(user, repo)
+      return if warning.nil?
-      rescue GitHub::HTTPNotFoundError
-        return
-      end
 
-      return if metadata.nil?
+      new_formula_problem warning
-
-      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
-
-      return if Date.parse(metadata["created_at"]) <= (Date.today - 30)
-
-      new_formula_problem "GitHub repository too new (<30 days old)"
     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}")
+      warning = SharedAudits.gitlab(user, repo)
-      return unless status.success?
+      return if warning.nil?
 
-      metadata = JSON.parse(out)
+      new_formula_problem warning
-      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)
-
-      new_formula_problem "GitLab repository too new (<30 days old)"
     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}"
+      warning = SharedAudits.bitbucket(user, repo)
-      out, _, status= curl_output("--request", "GET", api_url)
+      return if warning.nil?
-      return unless status.success?
 
-      metadata = JSON.parse(out)
+      new_formula_problem warning
-      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)
-
-      new_formula_problem "Bitbucket repository not notable enough (<30 forks and <75 watchers)"
     end
 
     def get_repo_data(regex)

Library/Homebrew/test/cask/audit_spec.rb

@@ -39,12 +39,12 @@ describe Cask::Audit, :cask do
 
   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,
+    described_class.new(cask, download:        download,
-                              check_token_conflicts: check_token_conflicts,
+                              token_conflicts: token_conflicts,
-                              command:               fake_system_command)
+                              command:         fake_system_command)
   }
 
   describe "#result" do
@@ -517,7 +517,7 @@ describe Cask::Audit, :cask do
 
     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] }

Library/Homebrew/test/cask/cmd/audit_spec.rb

@@ -21,7 +21,13 @@ describe Cask::Cmd::Audit, :cask do
       expect(Cask::CaskLoader).to receive(:load).with(cask_token).and_return(cask)
 
       expect(Cask::Auditor).to receive(:audit)
-        .with(cask, audit_download: false, audit_appcast: false, check_token_conflicts: false, quarantine: true)
+        .with(cask, audit_download:        false,
+                    audit_appcast:         false,
+                    audit_token_conflicts: false,
+                    audit_new_cask:        false,
+                    audit_online:          false,
+                    audit_strict:          false,
+                    quarantine:            true)
         .and_return(true)
 
       described_class.run(cask_token)
@@ -32,7 +38,13 @@ describe Cask::Cmd::Audit, :cask do
     it "does not download the Cask per default" do
       allow(Cask::CaskLoader).to receive(:load).and_return(cask)
       expect(Cask::Auditor).to receive(:audit)
-        .with(cask, audit_download: false, audit_appcast: false, check_token_conflicts: false, quarantine: true)
+        .with(cask, audit_download:        false,
+                    audit_appcast:         false,
+                    audit_token_conflicts: false,
+                    audit_new_cask:        false,
+                    audit_online:          false,
+                    audit_strict:          false,
+                    quarantine:            true)
         .and_return(true)
 
       described_class.run("casktoken")
@@ -41,7 +53,13 @@ describe Cask::Cmd::Audit, :cask do
     it "download a Cask if --download flag is set" do
       allow(Cask::CaskLoader).to receive(:load).and_return(cask)
       expect(Cask::Auditor).to receive(:audit)
-        .with(cask, audit_download: true, audit_appcast: false, check_token_conflicts: false, quarantine: true)
+        .with(cask, audit_download:        true,
+                    audit_appcast:         false,
+                    audit_token_conflicts: false,
+                    audit_new_cask:        false,
+                    audit_online:          false,
+                    audit_strict:          false,
+                    quarantine:            true)
         .and_return(true)
 
       described_class.run("casktoken", "--download")
@@ -52,7 +70,13 @@ describe Cask::Cmd::Audit, :cask do
     it "does not check for token conflicts per default" do
       allow(Cask::CaskLoader).to receive(:load).and_return(cask)
       expect(Cask::Auditor).to receive(:audit)
-        .with(cask, audit_download: false, audit_appcast: false, check_token_conflicts: false, quarantine: true)
+        .with(cask, audit_download:        false,
+                    audit_appcast:         false,
+                    audit_token_conflicts: false,
+                    audit_new_cask:        false,
+                    audit_online:          false,
+                    audit_strict:          false,
+                    quarantine:            true)
         .and_return(true)
 
       described_class.run("casktoken")
@@ -61,10 +85,112 @@ describe Cask::Cmd::Audit, :cask do
     it "checks for token conflicts if --token-conflicts flag is set" do
       allow(Cask::CaskLoader).to receive(:load).and_return(cask)
       expect(Cask::Auditor).to receive(:audit)
-        .with(cask, audit_download: false, audit_appcast: false, check_token_conflicts: true, quarantine: true)
+        .with(cask, audit_download:        false,
+                    audit_appcast:         false,
+                    audit_token_conflicts: true,
+                    audit_new_cask:        false,
+                    audit_online:          false,
+                    audit_strict:          false,
+                    quarantine:            true)
         .and_return(true)
 
       described_class.run("casktoken", "--token-conflicts")
     end
   end
+
+  describe "rules for checking strictly" do
+    it "does not check strictly per default" do
+      allow(Cask::CaskLoader).to receive(:load).and_return(cask)
+      expect(Cask::Auditor).to receive(:audit)
+        .with(cask, audit_download:        false,
+                    audit_appcast:         false,
+                    audit_token_conflicts: false,
+                    audit_new_cask:        false,
+                    audit_online:          false,
+                    audit_strict:          false,
+                    quarantine:            true)
+        .and_return(true)
+
+      described_class.run("casktoken")
+    end
+
+    it "checks strictly if --strict flag is set" do
+      allow(Cask::CaskLoader).to receive(:load).and_return(cask)
+      expect(Cask::Auditor).to receive(:audit)
+        .with(cask, audit_download:        false,
+                    audit_appcast:         false,
+                    audit_token_conflicts: true,
+                    audit_new_cask:        false,
+                    audit_online:          false,
+                    audit_strict:          true,
+                    quarantine:            true)
+        .and_return(true)
+
+      described_class.run("casktoken", "--strict")
+    end
+  end
+
+  describe "rules for checking online" do
+    it "does not check online per default" do
+      allow(Cask::CaskLoader).to receive(:load).and_return(cask)
+      expect(Cask::Auditor).to receive(:audit)
+        .with(cask, audit_download:        false,
+                    audit_appcast:         false,
+                    audit_token_conflicts: false,
+                    audit_new_cask:        false,
+                    audit_online:          false,
+                    audit_strict:          false,
+                    quarantine:            true)
+        .and_return(true)
+
+      described_class.run("casktoken")
+    end
+
+    it "checks online if --online flag is set" do
+      allow(Cask::CaskLoader).to receive(:load).and_return(cask)
+      expect(Cask::Auditor).to receive(:audit)
+        .with(cask, audit_download:        true,
+                    audit_appcast:         true,
+                    audit_token_conflicts: false,
+                    audit_new_cask:        false,
+                    audit_online:          true,
+                    audit_strict:          false,
+                    quarantine:            true)
+        .and_return(true)
+
+      described_class.run("casktoken", "--online")
+    end
+  end
+
+  describe "rules for checking new casks" do
+    it "does not check new casks per default" do
+      allow(Cask::CaskLoader).to receive(:load).and_return(cask)
+      expect(Cask::Auditor).to receive(:audit)
+        .with(cask, audit_download:        false,
+                    audit_appcast:         false,
+                    audit_token_conflicts: false,
+                    audit_new_cask:        false,
+                    audit_online:          false,
+                    audit_strict:          false,
+                    quarantine:            true)
+        .and_return(true)
+
+      described_class.run("casktoken")
+    end
+
+    it "checks new casks if --new-cask flag is set" do
+      allow(Cask::CaskLoader).to receive(:load).and_return(cask)
+      expect(Cask::Auditor).to receive(:audit)
+        .with(cask, audit_download:        true,
+                    audit_appcast:         true,
+                    audit_token_conflicts: true,
+                    audit_new_cask:        true,
+                    audit_online:          true,
+                    audit_strict:          true,
+                    quarantine:            true)
+        .and_return(true)
+
+      described_class.run("casktoken", "--new-cask")
+    end
+  end
 end

Library/Homebrew/utils/notability.rb

@@ -0,0 +1,75 @@
+# frozen_string_literal: true
+
+require "utils/curl"
+
+module SharedAudits
+  module_function
+
+  def github(user, repo)
+    begin
+      metadata = GitHub.repository(user, repo)
+    rescue GitHub::HTTPNotFoundError
+      return
+    end
+
+    return if metadata.nil?
+
+    return "GitHub fork (not canonical repository)" if metadata["fork"]
+    if (metadata["forks_count"] < 30) && (metadata["subscribers_count"] < 30) &&
+       (metadata["stargazers_count"] < 75)
+      return "GitHub repository not notable enough (<30 forks, <30 watchers and <75 stars)"
+    end
+
+    return if Date.parse(metadata["created_at"]) <= (Date.today - 30)
+
+    "GitHub repository too new (<30 days old)"
+  end
+
+  def gitlab(user, repo)
+    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?
+
+    return "GitLab fork (not canonical repository)" if metadata["fork"]
+    if (metadata["forks_count"] < 30) && (metadata["star_count"] < 75)
+      return "GitLab repository not notable enough (<30 forks and <75 stars)"
+    end
+
+    return if Date.parse(metadata["created_at"]) <= (Date.today - 30)
+
+    "GitLab repository too new (<30 days old)"
+  end
+
+  def bitbucket(user, repo)
+    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?
+
+    return "Uses deprecated mercurial support in Bitbucket" if metadata["scm"] == "hg"
+
+    return "Bitbucket fork (not canonical repository)" unless metadata["parent"].nil?
+
+    return "Bitbucket repository too new (<30 days old)" if Date.parse(metadata["created_on"]) >= (Date.today - 30)
+
+    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)
+
+    "Bitbucket repository not notable enough (<30 forks and <75 watchers)"
+  end
+end