Merge pull request #10440 from SeekingMeaning/named-args-unreadable

named_args: raise or print error if both or either formula/cask unreadable

Library/Homebrew/cli/named_args.rb

@@ -78,7 +78,7 @@ module Homebrew
       end
 
       def load_formula_or_cask(name, only: nil, method: nil)
-        unreadable_errors = []
+        unreadable_error = nil
 
         if only != :cask
           begin
@@ -102,7 +102,7 @@ module Homebrew
                  TapFormulaUnreadableError, TapFormulaClassUnavailableError => e
             # Need to rescue before `FormulaUnavailableError` (superclass of this)
             # The formula was found, but there's a problem with its implementation
-            unreadable_errors << e
+            unreadable_error ||= e
           rescue NoSuchKegError, FormulaUnavailableError => e
             raise e if only == :formula
           end
@@ -110,17 +110,27 @@ module Homebrew
 
         if only != :formula
           begin
-            return Cask::CaskLoader.load(name, config: Cask::Config.from_args(@parent))
+            cask = Cask::CaskLoader.load(name, config: Cask::Config.from_args(@parent))
+
+            if unreadable_error.present?
+              onoe <<~EOS
+                Failed to load formula: #{name}
+                #{unreadable_error}
+              EOS
+              opoo "Treating #{name} as a cask."
+            end
+
+            return cask
           rescue Cask::CaskUnreadableError => e
             # Need to rescue before `CaskUnavailableError` (superclass of this)
             # The cask was found, but there's a problem with its implementation
-            unreadable_errors << e
+            unreadable_error ||= e
           rescue Cask::CaskUnavailableError => e
             raise e if only == :cask
           end
         end
 
-        raise unreadable_errors.first if unreadable_errors.count == 1
+        raise unreadable_error if unreadable_error.present?
 
         raise FormulaOrCaskUnavailableError, name
       end
@@ -287,12 +297,22 @@ module Homebrew
       end
 
       def warn_if_cask_conflicts(ref, loaded_type)
-        cask = Cask::CaskLoader.load ref
         message = "Treating #{ref} as a #{loaded_type}."
+        begin
+          cask = Cask::CaskLoader.load ref
           message += " For the cask, use #{cask.tap.name}/#{cask.token}" if cask.tap.present?
-        opoo message.freeze
+        rescue Cask::CaskUnreadableError => e
+          # Need to rescue before `CaskUnavailableError` (superclass of this)
+          # The cask was found, but there's a problem with its implementation
+          onoe <<~EOS
+            Failed to load cask: #{ref}
+            #{e}
+          EOS
         rescue Cask::CaskUnavailableError
           # No ref conflict with a cask, do nothing
+          return
+        end
+        opoo message.freeze
       end
     end
   end

Library/Homebrew/test/cli/named_args_spec.rb

@@ -99,7 +99,7 @@ describe Homebrew::CLI::NamedArgs do
       end
 
       it "raises an error" do
-        expect { described_class.new("foo").to_formulae_and_casks }.to raise_error(FormulaOrCaskUnavailableError)
+        expect { described_class.new("foo").to_formulae_and_casks }.to raise_error(FormulaUnreadableError)
       end
 
       it "raises an error if loading formula only" do
@@ -122,6 +122,7 @@ describe Homebrew::CLI::NamedArgs do
       setup_unredable_cask "foo"
 
       expect(described_class.new("foo").to_formulae_and_casks).to eq [foo]
+      expect { described_class.new("foo").to_formulae_and_casks }.to output(/Failed to load cask: foo/).to_stderr
     end
 
     it "returns cask when formula is unreadable and cask is present" do
@@ -129,6 +130,7 @@ describe Homebrew::CLI::NamedArgs do
       stub_cask_loader foo_cask
 
       expect(described_class.new("foo").to_formulae_and_casks).to eq [foo_cask]
+      expect { described_class.new("foo").to_formulae_and_casks }.to output(/Failed to load formula: foo/).to_stderr
     end
 
     it "raises an error when formula is absent and cask is unreadable" do

Library/Homebrew/test/cmd/--cache_spec.rb

@@ -10,6 +10,10 @@ end
 describe "brew --cache", :integration_test do
   it "prints all cache files for a given Formula" do
     expect { brew "--cache", testball }
+      .to output(%r{#{HOMEBREW_CACHE}/downloads/[\da-f]{64}--testball-}o).to_stdout
+      .and output(/Treating #{Regexp.escape(testball)} as a formula/).to_stderr
+      .and be_a_success
+    expect { brew "--cache", "--formula", testball }
       .to output(%r{#{HOMEBREW_CACHE}/downloads/[\da-f]{64}--testball-}o).to_stdout
       .and not_to_output.to_stderr
       .and be_a_success
@@ -17,6 +21,10 @@ describe "brew --cache", :integration_test do
 
   it "prints the cache files for a given Cask" do
     expect { brew "--cache", cask_path("local-caffeine") }
+      .to output(%r{#{HOMEBREW_CACHE}/downloads/[\da-f]{64}--caffeine\.zip}o).to_stdout
+      .and output(/Treating #{Regexp.escape(cask_path("local-caffeine"))} as a cask/).to_stderr
+      .and be_a_success
+    expect { brew "--cache", "--cask", cask_path("local-caffeine") }
       .to output(%r{#{HOMEBREW_CACHE}/downloads/[\da-f]{64}--caffeine\.zip}o).to_stdout
       .and not_to_output.to_stderr
       .and be_a_success
@@ -30,7 +38,7 @@ describe "brew --cache", :integration_test do
           #{HOMEBREW_CACHE}/downloads/[\da-f]{64}--caffeine\.zip
         }xo,
       ).to_stdout
-      .and not_to_output.to_stderr
+      .and output(/(Treating .* as a formula).*(Treating .* as a cask)/m).to_stderr
       .and be_a_success
   end
 end

Library/Homebrew/test/cmd/home_spec.rb

@@ -39,6 +39,10 @@ describe "brew home", :integration_test do
 
   it "opens the homepage for a given Cask" do
     expect { brew "home", local_caffeine_path, "HOMEBREW_BROWSER" => "echo" }
+      .to output(/#{local_caffeine_homepage}/).to_stdout
+      .and output(/Treating #{Regexp.escape(local_caffeine_path)} as a cask/).to_stderr
+      .and be_a_success
+    expect { brew "home", "--cask", local_caffeine_path, "HOMEBREW_BROWSER" => "echo" }
       .to output(/#{local_caffeine_homepage}/).to_stdout
       .and not_to_output.to_stderr
       .and be_a_success
@@ -49,7 +53,7 @@ describe "brew home", :integration_test do
 
     expect { brew "home", "testballhome", local_caffeine_path, "HOMEBREW_BROWSER" => "echo" }
       .to output(/#{testballhome_homepage} #{local_caffeine_homepage}/).to_stdout
-      .and not_to_output.to_stderr
+      .and output(/Treating #{Regexp.escape(local_caffeine_path)} as a cask/).to_stderr
       .and be_a_success
   end
 end