Check bundle formula link status

- Ensure `brew bundle check` catches formulae with wrong links
- Match implicit link expectations used by `brew bundle install`
- Cover `check`, `check --verbose` and `check --install`

Author: MikeMcQuaid

Library/Homebrew/bundle/brew.rb

@@ -73,6 +73,40 @@ def formula_installed_and_up_to_date?(formula, no_upgrade: false)
           !formula_upgradable?(formula)
         end
 
+        sig { params(formula: String, options: Homebrew::Bundle::EntryOptions).returns(T.nilable(T::Boolean)) }
+        def link_status_to_check(formula, options)
+          unless options.key?(:link)
+            return case formula_dump_link_status(formula)
+            when true
+              false
+            when false
+              true
+            end
+          end
+
+          expected_link_status?(options[:link], keg_only: Formula[formula].keg_only?)
+        end
+
+        sig { params(link: Homebrew::Bundle::EntryOption, keg_only: T::Boolean).returns(T::Boolean) }
+        def expected_link_status?(link, keg_only:)
+          case link
+          when :overwrite, true
+            true
+          when false
+            false
+          else
+            !keg_only
+          end
+        end
+
+        sig { params(formula: String).returns(T.nilable(T::Boolean)) }
+        def formula_dump_link_status(formula)
+          (
+            @formulae_by_full_name&.[](formula) ||
+            @formulae_by_name&.[](Utils.name_from_full_name(formula))
+          )&.fetch(:link?)
+        end
+
         sig { params(no_upgrade: T::Boolean, formula_name: String).returns(T::Boolean) }
         def no_upgrade_with_args?(no_upgrade, formula_name)
           no_upgrade && Bundle.upgrade_formulae.exclude?(formula_name)
@@ -406,9 +440,45 @@ def initialize(name = "", options = {})
 
       sig { override.params(formula: Object, no_upgrade: T::Boolean).returns(T::Boolean) }
       def installed_and_up_to_date?(formula, no_upgrade: false)
-        raise "formula must be a String, got #{formula.class}: #{formula}" unless formula.is_a?(String)
+        name = formula_name(formula)
+        return false unless self.class.formula_installed_and_up_to_date?(name, no_upgrade:)
+
+        options = formula_options(formula)
+        link_status = self.class.link_status_to_check(name, options)
+        return true if link_status.nil?
+
+        (options.key?(:link) ? Formula[name].linked? : self.class.formula_dump_link_status(name)) == link_status
+      end
+
+      sig { override.params(name: Object, no_upgrade: T::Boolean).returns(String) }
+      def failure_reason(name, no_upgrade:)
+        formula = formula_name(name)
+        options = formula_options(name)
+        return super(formula, no_upgrade:) unless self.class.formula_installed_and_up_to_date?(formula, no_upgrade:)
 
-        self.class.formula_installed_and_up_to_date?(formula, no_upgrade:)
+        link_status = self.class.link_status_to_check(formula, options)
+        return super(formula, no_upgrade:) if link_status.nil?
+
+        link_status = link_status ? "linked" : "unlinked"
+        "Formula #{formula} needs to be #{link_status}."
+      end
+
+      sig {
+        override.params(
+          entries:             T::Array[Object],
+          exit_on_first_error: T::Boolean,
+          no_upgrade:          T::Boolean,
+          verbose:             T::Boolean,
+        ).returns(T::Array[Object])
+      }
+      def find_actionable(entries, exit_on_first_error: false, no_upgrade: false, verbose: false)
+        requested = instance_of?(Homebrew::Bundle::Brew) ? checkable_entries(entries) : format_checkable(entries)
+
+        if exit_on_first_error
+          exit_early_check(requested, no_upgrade:)
+        else
+          full_check(requested, no_upgrade:)
+        end
       end
 
       sig { params(no_upgrade: T::Boolean, verbose: T::Boolean).returns(T::Boolean) }
@@ -524,22 +594,13 @@ def service_change_state!(verbose:)
       sig { params(verbose: T::Boolean).returns(T::Boolean) }
       def link_change_state!(verbose: false)
         link_args = []
-        link_args << "--force" if unlinked_and_keg_only?
-
-        cmd = case @link
-        when :overwrite
-          link_args << "--overwrite"
+        link_status = self.class.expected_link_status?(@link, keg_only: keg_only?)
+        cmd = if link_status
+          link_args << "--force" if unlinked_and_keg_only?
+          link_args << "--overwrite" if @link == :overwrite
           "link" unless linked?
-        when true
-          "link" unless linked?
-        when false
-          "unlink" if linked?
-        when nil
-          if keg_only?
-            "unlink" if linked?
-          else
-            "link" unless linked?
-          end
+        elsif linked?
+          "unlink"
         end
 
         if cmd.present?
@@ -563,6 +624,21 @@ def postinstall_change_state!(verbose:)
 
       private
 
+      sig { params(formula: Object).returns(String) }
+      def formula_name(formula)
+        return formula.name if formula.is_a?(Dsl::Entry)
+        return formula if formula.is_a?(String)
+
+        raise "formula must be a String or Dsl::Entry, got #{formula.class}: #{formula}"
+      end
+
+      sig { params(formula: Object).returns(Homebrew::Bundle::EntryOptions) }
+      def formula_options(formula)
+        return formula.options if formula.is_a?(Dsl::Entry)
+
+        {}
+      end
+
       sig { returns(T::Boolean) }
       def installed?
         self.class.formula_installed?(@name)

Library/Homebrew/test/cmd/bundle/check_subcommand_spec.rb

@@ -81,6 +81,65 @@
     end
   end
 
+  context "when formulae have the wrong link status" do
+    before do
+      allow(Homebrew::Bundle::Cask).to receive(:casks).and_return([])
+      allow(Homebrew::Bundle::Brew).to receive_messages(upgradable_formulae: [], installed_formulae: ["abc"])
+      stub_formula_loader formula("abc") { url "abc-1.0" }
+    end
+
+    it "raises an error" do
+      allow_any_instance_of(Pathname).to receive(:read).and_return("brew 'abc', link: true")
+      allow(Formula["abc"]).to receive_messages(linked?: false, keg_only?: false)
+
+      expect { do_check }.to raise_error(SystemExit).and \
+        output(/Run `brew bundle check --verbose` to list unmet dependencies\./).to_stdout
+    end
+
+    it "raises an error for an implicitly unlinked non-keg-only formula" do
+      Homebrew::Bundle::Brew.instance_variable_set(:@formulae_by_name, { "abc" => { link?: false } })
+      allow_any_instance_of(Pathname).to receive(:read).and_return("brew 'abc'")
+
+      expect { do_check }.to raise_error(SystemExit).and \
+        output(/Run `brew bundle check --verbose` to list unmet dependencies\./).to_stdout
+    end
+
+    context "with verbose mode enabled" do
+      let(:verbose) { true }
+
+      it "outputs the link status error" do
+        allow_any_instance_of(Pathname).to receive(:read).and_return("brew 'abc', link: false")
+        allow(Formula["abc"]).to receive_messages(linked?: true, keg_only?: false)
+
+        expect { do_check }.to raise_error(SystemExit).and \
+          output(/Formula abc needs to be unlinked\./).to_stdout
+      end
+
+      it "outputs the implicit link status error" do
+        Homebrew::Bundle::Brew.instance_variable_set(:@formulae_by_name, { "abc" => { link?: true } })
+        allow_any_instance_of(Pathname).to receive(:read).and_return("brew 'abc'")
+
+        expect { do_check }.to raise_error(SystemExit).and \
+          output(/Formula abc needs to be unlinked\./).to_stdout
+      end
+    end
+
+    context "with install mode enabled" do
+      it "raises an error after install leaves a formula with the wrong link status" do
+        args = args_for_subcommand(:check, install?: true, global?: false, verbose?: false, upgrade_formulae: nil,
+                                           jobs: nil, file: nil)
+        allow(Homebrew::Cmd::Bundle).to receive(:redirect_stdout).and_yield
+        allow(Homebrew::Bundle::Brew).to receive(:install!).and_return(true)
+        allow_any_instance_of(Pathname).to receive(:read).and_return("brew 'abc', link: true")
+        allow(Formula["abc"]).to receive_messages(linked?: false, keg_only?: false)
+
+        expect { Homebrew::Cmd::Bundle.dispatch(args, extensions: Homebrew::Bundle.extensions) }
+          .to raise_error(SystemExit).and \
+            output(/Run `brew bundle check --verbose` to list unmet dependencies\./).to_stdout
+      end
+    end
+  end
+
   context "when taps are not tapped" do
     it "raises an error" do
       allow(Homebrew::Bundle::Cask).to receive(:casks).and_return([])