Merge pull request #3146 from reitermarkus/cask-fail-early

Make Cask commands fail early if a Cask is not found.

21 files changed, 72 insertions, 117 deletions

diff --git a/Library/Homebrew/cask/lib/hbc/cask_loader.rb b/Library/Homebrew/cask/lib/hbc/cask_loader.rb
index dd9c61089..532d9f3c3 100644
--- a/Library/Homebrew/cask/lib/hbc/cask_loader.rb
+++ b/Library/Homebrew/cask/lib/hbc/cask_loader.rb
@@ -116,6 +116,22 @@ module Hbc
       end
     end
 
+    class FromInstanceLoader
+      attr_reader :cask
+
+      def self.can_load?(ref)
+        ref.is_a?(Cask)
+      end
+
+      def initialize(cask)
+        @cask = cask
+      end
+
+      def load
+        cask
+      end
+    end
+
     class NullLoader < FromPathLoader
       def self.can_load?(*)
         true
@@ -149,6 +165,7 @@ module Hbc
 
     def self.for(ref)
       [
+        FromInstanceLoader,
         FromURILoader,
         FromTapLoader,
         FromTapPathLoader,
diff --git a/Library/Homebrew/cask/lib/hbc/cli/abstract_command.rb b/Library/Homebrew/cask/lib/hbc/cli/abstract_command.rb
index 77f85301e..001a9623b 100644
--- a/Library/Homebrew/cask/lib/hbc/cli/abstract_command.rb
+++ b/Library/Homebrew/cask/lib/hbc/cli/abstract_command.rb
@@ -42,41 +42,32 @@ module Hbc
         @args = process_arguments(*args)
       end
 
-      def self.warn_unavailable_with_suggestion(cask_token, e)
-        exact_match, partial_matches = Search.search(cask_token)
-        error_message = e.message
-        if exact_match
-          error_message.concat(" Did you mean:\n#{exact_match}")
-        elsif !partial_matches.empty?
-          error_message.concat(" Did you mean one of:\n")
-                       .concat(Formatter.columns(partial_matches.take(20)))
-        end
-        onoe error_message
-      end
-
       private
 
       def casks(alternative: -> { [] })
-        return to_enum(:casks, alternative: alternative) unless block_given?
-
-        count = 0
-
+        return @casks if defined?(@casks)
         casks = args.empty? ? alternative.call : args
+        @casks = casks.map { |cask| CaskLoader.load(cask) }
+      rescue CaskUnavailableError => e
+        reason = [e.reason, suggestion_message(e.token)].join(" ")
+        raise e.class.new(e.token, reason)
+      end
+
+      def suggestion_message(cask_token)
+        exact_match, partial_matches = Search.search(cask_token)
 
-        casks.each do |cask_or_token|
-          begin
-            yield cask_or_token.respond_to?(:token) ? cask_or_token : CaskLoader.load(cask_or_token)
-            count += 1
-          rescue CaskUnavailableError => e
-            cask_token = cask_or_token
-            self.class.warn_unavailable_with_suggestion cask_token, e
-          rescue CaskError => e
-            onoe e.message
-          end
+        if exact_match.nil? && partial_matches.count == 1
+          exact_match = partial_matches.first
         end
 
-        return :empty if casks.length.zero?
-        (count == casks.length) ? :complete : :incomplete
+        if exact_match
+          "Did you mean “#{exact_match}”?"
+        elsif !partial_matches.empty?
+          "Did you mean one of these?\n"
+            .concat(Formatter.columns(partial_matches.take(20)))
+        else
+          ""
+        end
       end
     end
   end
diff --git a/Library/Homebrew/cask/lib/hbc/cli/cat.rb b/Library/Homebrew/cask/lib/hbc/cli/cat.rb
index d08c87bea..043080556 100644
--- a/Library/Homebrew/cask/lib/hbc/cli/cat.rb
+++ b/Library/Homebrew/cask/lib/hbc/cli/cat.rb
@@ -7,10 +7,6 @@ module Hbc
       end
 
       def run
-        raise CaskError, "Cat incomplete." if cat_casks == :incomplete
-      end
-
-      def cat_casks
         casks.each do |cask|
           puts File.open(cask.sourcefile_path, &:read)
         end
diff --git a/Library/Homebrew/cask/lib/hbc/cli/edit.rb b/Library/Homebrew/cask/lib/hbc/cli/edit.rb
index b9485886c..8bce81c52 100644
--- a/Library/Homebrew/cask/lib/hbc/cli/edit.rb
+++ b/Library/Homebrew/cask/lib/hbc/cli/edit.rb
@@ -4,21 +4,18 @@ module Hbc
       def initialize(*)
         super
         raise CaskUnspecifiedError if args.empty?
-        raise ArgumentError, "Only one Cask can be created at a time." if args.count > 1
+        raise ArgumentError, "Only one Cask can be edited at a time." if args.count > 1
       end
 
       def run
-        cask_token = args.first
-        cask_path = begin
-          CaskLoader.load(cask_token).sourcefile_path
-        rescue CaskUnavailableError => e
-          reason = e.reason.empty? ? "" : "#{e.reason} "
-          reason.concat("Run #{Formatter.identifier("brew cask create #{e.token}")} to create a new Cask.")
-          raise e.class.new(e.token, reason)
-        end
-
-        odebug "Opening editor for Cask #{cask_token}"
+        cask = casks.first
+        cask_path = cask.sourcefile_path
+        odebug "Opening editor for Cask #{cask.token}"
         exec_editor cask_path
+      rescue CaskUnavailableError => e
+        reason = e.reason.empty? ? "" : "#{e.reason} "
+        reason.concat("Run #{Formatter.identifier("brew cask create #{e.token}")} to create a new Cask.")
+        raise e.class.new(e.token, reason)
       end
 
       def self.help
diff --git a/Library/Homebrew/cask/lib/hbc/cli/fetch.rb b/Library/Homebrew/cask/lib/hbc/cli/fetch.rb
index e31b1a17c..12c794f5f 100644
--- a/Library/Homebrew/cask/lib/hbc/cli/fetch.rb
+++ b/Library/Homebrew/cask/lib/hbc/cli/fetch.rb
@@ -9,10 +9,6 @@ module Hbc
       end
 
       def run
-        raise CaskError, "Fetch incomplete." if fetch_casks == :incomplete
-      end
-
-      def fetch_casks
         casks.each do |cask|
           ohai "Downloading external files for Cask #{cask}"
           downloaded_path = Download.new(cask, force: force?).perform
diff --git a/Library/Homebrew/cask/lib/hbc/cli/install.rb b/Library/Homebrew/cask/lib/hbc/cli/install.rb
index 0f1a5dd34..9a2116e6a 100644
--- a/Library/Homebrew/cask/lib/hbc/cli/install.rb
+++ b/Library/Homebrew/cask/lib/hbc/cli/install.rb
@@ -10,10 +10,6 @@ module Hbc
       end
 
       def run
-        raise CaskError, "Install incomplete." if install_casks == :incomplete
-      end
-
-      def install_casks
         casks.each do |cask|
           begin
             Installer.new(cask, binaries:       binaries?,
diff --git a/Library/Homebrew/cask/lib/hbc/cli/internal_appcast_checkpoint.rb b/Library/Homebrew/cask/lib/hbc/cli/internal_appcast_checkpoint.rb
index cd2679782..56f6d3c73 100644
--- a/Library/Homebrew/cask/lib/hbc/cli/internal_appcast_checkpoint.rb
+++ b/Library/Homebrew/cask/lib/hbc/cli/internal_appcast_checkpoint.rb
@@ -12,7 +12,7 @@ module Hbc
         if args.all? { |t| t =~ %r{^https?://} && t !~ /\.rb$/ }
           self.class.appcask_checkpoint_for_url(args)
         else
-          self.class.appcask_checkpoint(args, calculate?)
+          self.class.appcask_checkpoint(load_casks, calculate?)
         end
       end
 
@@ -23,33 +23,27 @@ module Hbc
         end
       end
 
-      def self.appcask_checkpoint(cask_tokens, calculate)
-        count = 0
-
-        cask_tokens.each do |cask_token|
-          cask = CaskLoader.load(cask_token)
-
+      def self.appcask_checkpoint(casks, calculate)
+        casks.each do |cask|
           if cask.appcast.nil?
             opoo "Cask '#{cask}' is missing an `appcast` stanza."
           else
-            if calculate
+            checkpoint = if calculate
               result = cask.appcast.calculate_checkpoint
-
-              checkpoint = result[:checkpoint]
+              result[:checkpoint]
             else
-              checkpoint = cask.appcast.checkpoint
+              cask.appcast.checkpoint
             end
 
-            if checkpoint.nil?
+            if calculate && checkpoint.nil?
               onoe "Could not retrieve `appcast` checkpoint for cask '#{cask}': #{result[:command_result].stderr}"
+            elsif casks.count > 1
+              puts "#{checkpoint}  #{cask}"
             else
-              puts((cask_tokens.count > 1) ? "#{checkpoint}  #{cask}" : checkpoint)
-              count += 1
+              puts checkpoint
             end
           end
         end
-
-        count == cask_tokens.count
       end
 
       def self.help
diff --git a/Library/Homebrew/cask/lib/hbc/cli/internal_dump.rb b/Library/Homebrew/cask/lib/hbc/cli/internal_dump.rb
index 46273cbe3..8a38ce1be 100644
--- a/Library/Homebrew/cask/lib/hbc/cli/internal_dump.rb
+++ b/Library/Homebrew/cask/lib/hbc/cli/internal_dump.rb
@@ -7,10 +7,6 @@ module Hbc
       end
 
       def run
-        raise CaskError, "Dump incomplete." if dump_casks == :incomplete
-      end
-
-      def dump_casks
         casks.each(&:dumpcask)
       end
 
diff --git a/Library/Homebrew/cask/lib/hbc/cli/internal_stanza.rb b/Library/Homebrew/cask/lib/hbc/cli/internal_stanza.rb
index 018b4e9ca..c04619798 100644
--- a/Library/Homebrew/cask/lib/hbc/cli/internal_stanza.rb
+++ b/Library/Homebrew/cask/lib/hbc/cli/internal_stanza.rb
@@ -46,12 +46,6 @@ module Hbc
       end
 
       def run
-        return unless print_stanzas == :incomplete
-        exit 1 if quiet?
-        raise CaskError, "Print incomplete."
-      end
-
-      def print_stanzas
         if ARTIFACTS.include?(stanza)
           artifact_name = stanza
           @stanza = :artifacts
diff --git a/Library/Homebrew/cask/lib/hbc/cli/list.rb b/Library/Homebrew/cask/lib/hbc/cli/list.rb
index 4b5fcd873..faa1160a1 100644
--- a/Library/Homebrew/cask/lib/hbc/cli/list.rb
+++ b/Library/Homebrew/cask/lib/hbc/cli/list.rb
@@ -10,8 +10,7 @@ module Hbc
       end)
 
       def run
-        retval = args.any? ? list : list_installed
-        raise CaskError, "Listing incomplete." if retval == :incomplete
+        args.any? ? list : list_installed
       end
 
       def list
@@ -46,8 +45,6 @@ module Hbc
         elsif !installed_casks.empty?
           puts Formatter.columns(installed_casks.map(&:to_s))
         end
-
-        installed_casks.empty? ? :empty : :complete
       end
 
       def self.format_versioned(cask)
diff --git a/Library/Homebrew/cask/lib/hbc/cli/reinstall.rb b/Library/Homebrew/cask/lib/hbc/cli/reinstall.rb
index 337a2eb9d..408be134d 100644
--- a/Library/Homebrew/cask/lib/hbc/cli/reinstall.rb
+++ b/Library/Homebrew/cask/lib/hbc/cli/reinstall.rb
@@ -1,7 +1,7 @@
 module Hbc
   class CLI
     class Reinstall < Install
-      def install_casks
+      def run
         casks.each do |cask|
           Installer.new(cask, binaries:       binaries?,
                               verbose:        verbose?,
diff --git a/Library/Homebrew/cask/lib/hbc/cli/uninstall.rb b/Library/Homebrew/cask/lib/hbc/cli/uninstall.rb
index c0697c808..7e55db5f1 100644
--- a/Library/Homebrew/cask/lib/hbc/cli/uninstall.rb
+++ b/Library/Homebrew/cask/lib/hbc/cli/uninstall.rb
@@ -9,10 +9,6 @@ module Hbc
       end
 
       def run
-        raise CaskError, "Uninstall incomplete." if uninstall_casks == :incomplete
-      end
-
-      def uninstall_casks
         casks.each do |cask|
           odebug "Uninstalling Cask #{cask}"
 
diff --git a/Library/Homebrew/cask/lib/hbc/cli/zap.rb b/Library/Homebrew/cask/lib/hbc/cli/zap.rb
index e709f4191..7f5e6785d 100644
--- a/Library/Homebrew/cask/lib/hbc/cli/zap.rb
+++ b/Library/Homebrew/cask/lib/hbc/cli/zap.rb
@@ -9,10 +9,6 @@ module Hbc
       end
 
       def run
-        raise CaskError, "Zap incomplete." if zap_casks == :incomplete
-      end
-
-      def zap_casks
         casks.each do |cask|
           odebug "Zapping Cask #{cask}"
           Installer.new(cask, verbose: verbose?, force: force?).zap
diff --git a/Library/Homebrew/test/cask/cli/audit_spec.rb b/Library/Homebrew/test/cask/cli/audit_spec.rb
index 01f506c8c..30ab437cb 100644
--- a/Library/Homebrew/test/cask/cli/audit_spec.rb
+++ b/Library/Homebrew/test/cask/cli/audit_spec.rb
@@ -1,8 +1,10 @@
 describe Hbc::CLI::Audit, :cask do
-  let(:cask) { double("cask", token: nil) }
+  let(:cask) { Hbc::Cask.new(nil) }
 
   describe "selection of Casks to audit" do
     it "audits all Casks if no tokens are given" do
+      expect(cask).to be_a Hbc::Cask
+
       allow(Hbc).to receive(:all).and_return([cask, cask])
 
       expect(Hbc::Auditor).to receive(:audit).twice.and_return(true)
diff --git a/Library/Homebrew/test/cask/cli/cat_spec.rb b/Library/Homebrew/test/cask/cli/cat_spec.rb
index b726a0b36..5a4b29c6f 100644
--- a/Library/Homebrew/test/cask/cli/cat_spec.rb
+++ b/Library/Homebrew/test/cask/cli/cat_spec.rb
@@ -35,8 +35,7 @@ describe Hbc::CLI::Cat, :cask do
 
   it "raises an exception when the Cask does not exist" do
     expect { Hbc::CLI::Cat.run("notacask") }
-      .to output(/is unavailable/).to_stderr
-      .and raise_error(Hbc::CaskError, "Cat incomplete.")
+      .to raise_error(Hbc::CaskUnavailableError, /is unavailable/)
   end
 
   describe "when no Cask is specified" do
diff --git a/Library/Homebrew/test/cask/cli/create_spec.rb b/Library/Homebrew/test/cask/cli/create_spec.rb
index d77b0a2aa..17d426f78 100644
--- a/Library/Homebrew/test/cask/cli/create_spec.rb
+++ b/Library/Homebrew/test/cask/cli/create_spec.rb
@@ -39,7 +39,7 @@ describe Hbc::CLI::Create, :cask do
   it "raises an exception when more than one Cask is given" do
     expect {
       described_class.run("additional-cask", "another-cask")
-    }.to raise_error(/Only one Cask can be created at a time./)
+    }.to raise_error(/Only one Cask can be created at a time\./)
   end
 
   it "raises an exception when the Cask already exists" do
diff --git a/Library/Homebrew/test/cask/cli/edit_spec.rb b/Library/Homebrew/test/cask/cli/edit_spec.rb
index 5d5cbf4b9..51542807f 100644
--- a/Library/Homebrew/test/cask/cli/edit_spec.rb
+++ b/Library/Homebrew/test/cask/cli/edit_spec.rb
@@ -12,7 +12,7 @@ describe Hbc::CLI::Edit, :cask do
   it "raises an error when given more than one argument" do
     expect {
       described_class.new("local-caffeine", "local-transmission")
-    }.to raise_error(/Only one Cask can be created at a time./)
+    }.to raise_error(/Only one Cask can be edited at a time\./)
   end
 
   it "raises an exception when the Cask doesnt exist" do
diff --git a/Library/Homebrew/test/cask/cli/fetch_spec.rb b/Library/Homebrew/test/cask/cli/fetch_spec.rb
index f71c23fb6..faaa69b35 100644
--- a/Library/Homebrew/test/cask/cli/fetch_spec.rb
+++ b/Library/Homebrew/test/cask/cli/fetch_spec.rb
@@ -42,7 +42,7 @@ describe Hbc::CLI::Fetch, :cask do
   it "properly handles Casks that are not present" do
     expect {
       Hbc::CLI::Fetch.run("notacask")
-    }.to raise_error(Hbc::CaskError, "Fetch incomplete.")
+    }.to raise_error(Hbc::CaskUnavailableError)
   end
 
   describe "when no Cask is specified" do
diff --git a/Library/Homebrew/test/cask/cli/install_spec.rb b/Library/Homebrew/test/cask/cli/install_spec.rb
index 64feacce9..e30489789 100644
--- a/Library/Homebrew/test/cask/cli/install_spec.rb
+++ b/Library/Homebrew/test/cask/cli/install_spec.rb
@@ -56,27 +56,19 @@ describe Hbc::CLI::Install, :cask do
   it "properly handles Casks that are not present" do
     expect {
       Hbc::CLI::Install.run("notacask")
-    }.to raise_error(Hbc::CaskError, "Install incomplete.")
+    }.to raise_error(Hbc::CaskUnavailableError)
   end
 
   it "returns a suggestion for a misspelled Cask" do
     expect {
-      begin
-        Hbc::CLI::Install.run("localcaffeine")
-      rescue Hbc::CaskError
-        nil
-      end
-    }.to output(/Cask 'localcaffeine' is unavailable: No Cask with this name exists\. Did you mean:\nlocal-caffeine/).to_stderr
+      Hbc::CLI::Install.run("localcaffeine")
+    }.to raise_error(Hbc::CaskUnavailableError, /Cask 'localcaffeine' is unavailable: No Cask with this name exists\. Did you mean “local-caffeine”?/)
   end
 
   it "returns multiple suggestions for a Cask fragment" do
     expect {
-      begin
-        Hbc::CLI::Install.run("local-caf")
-      rescue Hbc::CaskError
-        nil
-      end
-    }.to output(/Cask 'local-caf' is unavailable: No Cask with this name exists\. Did you mean one of:\nlocal-caffeine/).to_stderr
+      Hbc::CLI::Install.run("local")
+    }.to raise_error(Hbc::CaskUnavailableError, /Cask 'local' is unavailable: No Cask with this name exists\. Did you mean one of these\?\nlocal-caffeine\nlocal-transmission/)
   end
 
   describe "when no Cask is specified" do
diff --git a/Library/Homebrew/test/cask/cli/uninstall_spec.rb b/Library/Homebrew/test/cask/cli/uninstall_spec.rb
index 1a1c57e88..2ec506839 100644
--- a/Library/Homebrew/test/cask/cli/uninstall_spec.rb
+++ b/Library/Homebrew/test/cask/cli/uninstall_spec.rb
@@ -16,14 +16,12 @@ describe Hbc::CLI::Uninstall, :cask do
 
   it "shows an error when a bad Cask is provided" do
     expect { Hbc::CLI::Uninstall.run("notacask") }
-      .to output(/is unavailable/).to_stderr
-      .and raise_error(Hbc::CaskError, "Uninstall incomplete.")
+      .to raise_error(Hbc::CaskUnavailableError, /is unavailable/)
   end
 
   it "shows an error when a Cask is provided that's not installed" do
     expect { Hbc::CLI::Uninstall.run("local-caffeine") }
-    .to output(/is not installed/).to_stderr
-    .and raise_error(Hbc::CaskError, "Uninstall incomplete.")
+    .to raise_error(Hbc::CaskNotInstalledError, /is not installed/)
   end
 
   it "tries anyway on a non-present Cask when --force is given" do
@@ -76,8 +74,7 @@ describe Hbc::CLI::Uninstall, :cask do
     Hbc.appdir.join("MyFancyApp.app").rmtree
 
     expect { Hbc::CLI::Uninstall.run("with-uninstall-script-app") }
-    .to output(/does not exist/).to_stderr
-    .and raise_error(Hbc::CaskError, "Uninstall incomplete.")
+    .to raise_error(Hbc::CaskError, /uninstall script .* does not exist/)
 
     expect(cask).to be_installed
 
diff --git a/Library/Homebrew/test/cask/cli/zap_spec.rb b/Library/Homebrew/test/cask/cli/zap_spec.rb
index fdc5b4125..502bf8e69 100644
--- a/Library/Homebrew/test/cask/cli/zap_spec.rb
+++ b/Library/Homebrew/test/cask/cli/zap_spec.rb
@@ -1,8 +1,7 @@
 describe Hbc::CLI::Zap, :cask do
   it "shows an error when a bad Cask is provided" do
     expect { Hbc::CLI::Zap.run("notacask") }
-      .to output(/is unavailable/).to_stderr
-      .and raise_error(Hbc::CaskError, "Zap incomplete.")
+      .to raise_error(Hbc::CaskUnavailableError, /is unavailable/)
   end
 
   it "can zap and unlink multiple Casks at once" do