cli/named_args: raise priority of core casks

Library/Homebrew/cli/named_args.rb

@@ -138,9 +138,9 @@ def load_formula_or_cask(name, only: nil, method: nil, warn: nil)
         Homebrew.with_no_api_env_if_needed(@without_api) do
           unreadable_error = nil
 
-          if only != :cask
+          formula_or_kegs = if only != :cask
             begin
-              formula = case method
+              case method
               when nil, :factory
                 options = { warn:, force_bottle: @force_bottle, flags: @flags }.compact
                 Formulary.factory(name, *@override_spec, **options)
@@ -156,27 +156,32 @@ def load_formula_or_cask(name, only: nil, method: nil, warn: nil)
               else
                 raise
               end
-
-              warn_if_cask_conflicts(name, "formula") if only != :formula
-              return formula
             rescue FormulaUnreadableError, FormulaClassUnavailableError,
                    TapFormulaUnreadableError, TapFormulaClassUnavailableError,
                    FormulaSpecificationError => e
               # Need to rescue before `FormulaUnavailableError` (superclass of this)
               # The formula was found, but there's a problem with its implementation
               unreadable_error ||= e
+              nil
             rescue NoSuchKegError, FormulaUnavailableError => e
               raise e if only == :formula
+
+              nil
             end
           end
 
-          if only != :formula
+          if only == :formula
+            return formula_or_kegs if formula_or_kegs
+          elsif formula_or_kegs && (!formula_or_kegs.is_a?(Formula) || formula_or_kegs.tap&.core_tap?)
+            warn_if_cask_conflicts(name, "formula")
+            return formula_or_kegs
+          else
             want_keg_like_cask = [:latest_kegs, :default_kegs, :kegs].include?(method)
 
-            begin
+            cask = begin
               config = Cask::Config.from_args(@parent) if @cask_options
               options = { warn: }.compact
-              cask = Cask::CaskLoader.load(name, config:, **options)
+              candidate_cask = Cask::CaskLoader.load(name, config:, **options)
 
               if unreadable_error.present?
                 onoe <<~EOS
@@ -188,27 +193,40 @@ def load_formula_or_cask(name, only: nil, method: nil, warn: nil)
 
               # If we're trying to get a keg-like Cask, do our best to use the same cask
               # file that was used for installation, if possible.
-              if want_keg_like_cask && (installed_caskfile = cask.installed_caskfile) && installed_caskfile.exist?
-                cask = Cask::CaskLoader.load(installed_caskfile)
+              if want_keg_like_cask &&
+                 (installed_caskfile = candidate_cask.installed_caskfile) &&
+                 installed_caskfile.exist?
+                Cask::CaskLoader.load(installed_caskfile)
+              else
+                candidate_cask
               end
-
-              return cask
             rescue Cask::CaskUnreadableError, Cask::CaskInvalidError => e
               # If we're trying to get a keg-like Cask, do our best to handle it
               # not being readable and return something that can be used.
               if want_keg_like_cask
                 cask_version = Cask::Cask.new(name, config:).installed_version
-                cask = Cask::Cask.new(name, config:) do
+                Cask::Cask.new(name, config:) do
                   version cask_version if cask_version
                 end
-                return cask
+              else
+                # Need to rescue before `CaskUnavailableError` (superclass of this)
+                # The cask was found, but there's a problem with its implementation
+                unreadable_error ||= e
+                nil
               end
-
-              # Need to rescue before `CaskUnavailableError` (superclass of this)
-              # The cask was found, but there's a problem with its implementation
-              unreadable_error ||= e
             rescue Cask::CaskUnavailableError => e
               raise e if only == :cask
+
+              nil
+            end
+
+            # Prioritise formulae unless it's a core tap cask (we already prioritised core tap formulae above)
+            if formula_or_kegs && !cask&.tap&.core_cask_tap?
+              opoo package_conflicts_message(name, "formula", cask) if cask
+              return formula_or_kegs
+            elsif cask
+              opoo package_conflicts_message(name, "cask", formula_or_kegs) if formula_or_kegs
+              return cask
             end
           end
 
@@ -438,25 +456,45 @@ def resolve_default_keg(name)
         end
       end
 
-      def warn_if_cask_conflicts(ref, loaded_type)
+      def package_conflicts_message(ref, loaded_type, package)
         message = "Treating #{ref} as a #{loaded_type}."
-        begin
-          cask = Cask::CaskLoader.load(ref, warn: false)
+        case package
+        when Formula, Keg, Array
+          message += " For the formula, "
+          if package.is_a?(Formula) && (tap = package.tap)
+            message += "use #{tap.name}/#{package.name} or "
+          end
+          message += "specify the `--formula` flag."
+        when Cask::Cask
           message += " For the cask, "
-          message += "use #{cask.tap.name}/#{cask.token} or " if cask.tap
+          if (tap = package.tap)
+            message += "use #{tap.name}/#{package.token} or "
+          end
           message += "specify the `--cask` flag."
+        end
+        message.freeze
+      end
+
+      def warn_if_cask_conflicts(ref, loaded_type)
+        available = true
+        cask = begin
+          Cask::CaskLoader.load(ref, warn: false)
         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
+          nil
         rescue Cask::CaskUnavailableError
           # No ref conflict with a cask, do nothing
-          return
+          available = false
+          nil
         end
-        opoo message.freeze
+        return unless available
+
+        opoo package_conflicts_message(ref, loaded_type, cask)
       end
     end
   end

Library/Homebrew/test/cli/named_args_spec.rb

@@ -31,15 +31,15 @@ def setup_unredable_cask(name)
   end
 
   let(:baz) do
-    Cask::CaskLoader.load(+<<~RUBY)
+    Cask::CaskLoader::FromContentLoader.new(+<<~RUBY, tap: CoreCaskTap.instance).load(config: nil)
       cask "baz" do
         version "1.0"
       end
     RUBY
   end
 
   let(:foo_cask) do
-    Cask::CaskLoader.load(+<<~RUBY)
+    Cask::CaskLoader::FromContentLoader.new(+<<~RUBY, tap: CoreCaskTap.instance).load(config: nil)
       cask "foo" do
         version "1.0"
       end
@@ -90,6 +90,32 @@ def setup_unredable_cask(name)
       end
     end
 
+    context "when a non-core formula and a core cask are present" do
+      let(:non_core_formula) do
+        formula "foo", tap: Tap.fetch("some/tap") do
+          url "https://brew.sh"
+          version "1.0"
+        end
+      end
+
+      before do
+        stub_formula_loader non_core_formula, "foo"
+        stub_cask_loader foo_cask
+      end
+
+      it "returns the cask by default" do
+        expect(described_class.new("foo").to_formulae_and_casks).to eq [foo_cask]
+      end
+
+      it "returns formula if loading formula only" do
+        expect(described_class.new("foo").to_formulae_and_casks(only: :formula)).to eq [non_core_formula]
+      end
+
+      it "returns cask if loading cask only" do
+        expect(described_class.new("foo").to_formulae_and_casks(only: :cask)).to eq [foo_cask]
+      end
+    end
+
     context "when both formula and cask are unreadable" do
       before do
         setup_unredable_formula "foo"

Library/Homebrew/test/support/helper/formula.rb

@@ -5,8 +5,8 @@
 module Test
   module Helper
     module Formula
-      def formula(name = "formula_name", path: Formulary.core_path(name), spec: :stable, alias_path: nil, tap: nil,
-                  &block)
+      def formula(name = "formula_name", path: nil, spec: :stable, alias_path: nil, tap: nil, &block)
+        path ||= Formulary.find_formula_in_tap(name, tap || CoreTap.instance)
         Class.new(::Formula, &block).new(name, path, spec, alias_path:, tap:)
       end