Merge pull request #8595 from reitermarkus/skip-appcast

Support `--no-appcast` for `brew cask audit`.

Library/Homebrew/cask/audit.rb

@@ -14,22 +14,32 @@ module Cask
   class Audit
     extend Predicable
 
-    attr_reader :cask, :commit_range, :download
+    attr_reader :cask, :download
 
-    attr_predicate :appcast?, :new_cask?, :strict?, :online?
+    attr_predicate :appcast?, :new_cask?, :strict?, :online?, :token_conflicts?
+
+    def initialize(cask, appcast: nil, download: nil, quarantine: nil,
+                   token_conflicts: nil, online: nil, strict: nil,
+                   new_cask: nil)
+
+      # `new_cask` implies `online` and `strict`
+      online = new_cask if online.nil?
+      strict = new_cask if strict.nil?
+
+      # `online` implies `appcast` and `download`
+      appcast = online if appcast.nil?
+      download = online if download.nil?
+
+      # `strict` implies `token_conflicts`
+      token_conflicts = strict if token_conflicts.nil?
 
-    def initialize(cask, appcast: false, download: false, quarantine: nil,
-                   token_conflicts: false, online: false, strict: false,
-                   new_cask: false, commit_range: nil, command: SystemCommand)
       @cask = cask
       @appcast = appcast
       @download = Download.new(cask, quarantine: quarantine) if download
       @online = online
       @strict = strict
       @new_cask = new_cask
-      @commit_range = commit_range
       @token_conflicts = token_conflicts
-      @command = command
     end
 
     def run!
@@ -345,7 +355,7 @@ module Cask
     end
 
     def check_token_conflicts
-      return unless @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}"

Library/Homebrew/cask/auditor.rb

@@ -7,42 +7,54 @@ module Cask
   #
   # @api private
   class Auditor
-    extend Predicable
-
-    def self.audit(cask, audit_download: false, audit_appcast: false,
-                   audit_online: false, audit_strict: false,
-                   audit_token_conflicts: false, audit_new_cask: false,
-                   quarantine: true, commit_range: nil, language: nil)
-      new(cask, audit_download: audit_download,
-                audit_appcast: audit_appcast,
-                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, language: language).audit
+    def self.audit(
+      cask,
+      audit_download: nil,
+      audit_appcast: nil,
+      audit_online: nil,
+      audit_new_cask: nil,
+      audit_strict: nil,
+      audit_token_conflicts: nil,
+      quarantine: nil,
+      language: nil
+    )
+      new(
+        cask,
+        audit_download:        audit_download,
+        audit_appcast:         audit_appcast,
+        audit_online:          audit_online,
+        audit_new_cask:        audit_new_cask,
+        audit_strict:          audit_strict,
+        audit_token_conflicts: audit_token_conflicts,
+        quarantine:            quarantine,
+        language:              language,
+      ).audit
     end
 
-    attr_reader :cask, :commit_range, :language
+    attr_reader :cask, :language
 
-    def initialize(cask, audit_download: false, audit_appcast: false,
-                   audit_online: false, audit_strict: false,
-                   audit_token_conflicts: false, audit_new_cask: false,
-                   quarantine: true, commit_range: nil, language: nil)
+    def initialize(
+      cask,
+      audit_download: nil,
+      audit_appcast: nil,
+      audit_online: nil,
+      audit_strict: nil,
+      audit_token_conflicts: nil,
+      audit_new_cask: nil,
+      quarantine: nil,
+      language: 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
+      @audit_strict = audit_strict
       @quarantine = quarantine
-      @commit_range = commit_range
       @audit_token_conflicts = audit_token_conflicts
       @language = language
     end
 
-    attr_predicate :audit_appcast?, :audit_download?, :audit_online?,
-                   :audit_strict?, :audit_new_cask?, :audit_token_conflicts?, :quarantine?
-
     def audit
       warnings = Set.new
       errors = Set.new
@@ -76,14 +88,16 @@ module Cask
     end
 
     def audit_cask_instance(cask)
-      audit = Audit.new(cask, appcast:         audit_appcast?,
-                              online:          audit_online?,
-                              strict:          audit_strict?,
-                              new_cask:        audit_new_cask?,
-                              token_conflicts: audit_token_conflicts?,
-                              download:        audit_download?,
-                              quarantine:      quarantine?,
-                              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:        @audit_download,
+        quarantine:      @quarantine,
+      )
       audit.run!
       audit
     end

Library/Homebrew/cask/cmd/audit.rb

@@ -19,7 +19,7 @@ module Cask
         super do
           switch "--download",
                  description: "Audit the downloaded file"
-          switch "--appcast",
+          switch "--[no-]appcast",
                  description: "Audit the appcast"
           switch "--token-conflicts",
                  description: "Audit for token conflicts"
@@ -38,16 +38,14 @@ module Cask
         require "cask/auditor"
 
         Homebrew.auditing = true
-        strict = args.new_cask? || args.strict?
-        online = args.new_cask? || args.online?
 
         options = {
-          audit_download:        online || args.download?,
-          audit_appcast:         online || args.appcast?,
-          audit_online:          online,
-          audit_strict:          strict,
+          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: strict || args.token_conflicts?,
+          audit_token_conflicts: args.token_conflicts?,
           quarantine:            args.quarantine?,
           language:              args.language,
         }.compact

Library/Homebrew/test/cask/audit_spec.rb

@@ -30,19 +30,52 @@ describe Cask::Audit, :cask do
   end
 
   let(:cask) { instance_double(Cask::Cask) }
-  let(:download) { false }
-  let(:token_conflicts) { false }
-  let(:strict) { false }
-  let(:new_cask) { false }
-  let(:fake_system_command) { class_double(SystemCommand) }
+  let(:new_cask) { nil }
+  let(:online) { nil }
+  let(:strict) { nil }
+  let(:token_conflicts) { nil }
   let(:audit) {
-    described_class.new(cask, download:        download,
-                              token_conflicts: token_conflicts,
-                              command:         fake_system_command,
+    described_class.new(cask, online:          online,
+
                               strict:          strict,
-                              new_cask:        new_cask)
+                              new_cask:        new_cask,
+                              token_conflicts: token_conflicts)
   }
 
+  describe "#new" do
+    context "when `new_cask` is specified" do
+      let(:new_cask) { true }
+
+      it "implies `online`" do
+        expect(audit).to be_online
+      end
+
+      it "implies `strict`" do
+        expect(audit).to be_strict
+      end
+    end
+
+    context "when `online` is specified" do
+      let(:online) { true }
+
+      it "implies `appcast`" do
+        expect(audit.appcast?).to be true
+      end
+
+      it "implies `download`" do
+        expect(audit.download).to be_truthy
+      end
+    end
+
+    context "when `strict` is specified" do
+      let(:strict) { true }
+
+      it "implies `token_conflicts`" do
+        expect(audit.token_conflicts?).to be true
+      end
+    end
+  end
+
   describe "#result" do
     subject { audit.result }
 

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

@@ -57,10 +57,10 @@ describe Cask::Cmd::Audit, :cask do
     described_class.run("casktoken", "--token-conflicts")
   end
 
-  it "passes `audit_strict` and `audit_token_conflicts` if the `--strict` flag is specified" do
+  it "passes `audit_strict` if the `--strict` flag is specified" do
     allow(Cask::CaskLoader).to receive(:load).and_return(cask)
     expect(Cask::Auditor).to receive(:audit)
-      .with(cask, audit_strict: true, audit_token_conflicts: true, quarantine: true)
+      .with(cask, audit_strict: true, quarantine: true)
       .and_return(result)
 
     described_class.run("casktoken", "--strict")
@@ -69,23 +69,16 @@ describe Cask::Cmd::Audit, :cask do
   it "passes `audit_online` if the `--online` flag is specified" do
     allow(Cask::CaskLoader).to receive(:load).and_return(cask)
     expect(Cask::Auditor).to receive(:audit)
-      .with(cask, audit_online: true, audit_appcast: true, audit_download: true, quarantine: true)
+      .with(cask, audit_online: true, quarantine: true)
       .and_return(result)
 
     described_class.run("casktoken", "--online")
   end
 
-  it "passes `audit_appcast`, `audit_download`, `audit_new_cask`, `audit_online`, `audit_strict` " \
-     "and `audit_token_conflicts` if the `--new-cask` flag is specified" do
+  it "passes `audit_new_cask` if the `--new-cask` flag is specified" do
     allow(Cask::CaskLoader).to receive(:load).and_return(cask)
     expect(Cask::Auditor).to receive(:audit)
-      .with(cask, audit_appcast:         true,
-                  audit_download:        true,
-                  audit_new_cask:        true,
-                  audit_online:          true,
-                  audit_strict:          true,
-                  audit_token_conflicts: true,
-                  quarantine:            true)
+      .with(cask, audit_new_cask: true, quarantine: true)
       .and_return(result)
 
     described_class.run("casktoken", "--new-cask")