Merge pull request #16875 from Homebrew/fix-untap-cmd-bugs

Fix untap cmd bugs

Library/Homebrew/cmd/untap.rb

@@ -2,12 +2,11 @@
 # frozen_string_literal: true
 
 require "cli/parser"
+require "untap"
 
 module Homebrew
-  module_function
-
   sig { returns(CLI::Parser) }
-  def untap_args
+  def self.untap_args
     Homebrew::CLI::Parser.new do
       description <<~EOS
         Remove a tapped formula repository.
@@ -20,46 +19,15 @@ module Homebrew
   end
 
   sig { void }
-  def untap
+  def self.untap
     args = untap_args.parse
 
     args.named.to_installed_taps.each do |tap|
       odie "Untapping #{tap} is not allowed" if tap.core_tap? && Homebrew::EnvConfig.no_install_from_api?
 
       if Homebrew::EnvConfig.no_install_from_api? || (!tap.core_tap? && !tap.core_cask_tap?)
-        installed_formula_names = T.let(nil, T.nilable(T::Set[String]))
+        installed_tap_formulae = Untap.installed_formulae_for(tap:)
-        installed_tap_formulae = tap.formula_names.filter_map do |formula_name|
+        installed_tap_casks = Untap.installed_casks_for(tap:)
-          # initialise lazily in case there's no formulae in this tap
-          installed_formula_names ||= Set.new(Formula.installed_formula_names)
-          next unless installed_formula_names.include?(formula_name)
-
-          formula = begin
-            Formulary.factory("#{tap.name}/#{formula_name}")
-          rescue
-            # Don't blow up because of a single unavailable formula.
-            next
-          end
-
-          # Can't use Formula#any_version_installed? because it doesn't consider
-          # taps correctly.
-          formula if formula.installed_kegs.any? { |keg| keg.tab.tap == tap }
-        end
-
-        installed_cask_tokens = T.let(nil, T.nilable(T::Set[String]))
-        installed_tap_casks = tap.cask_tokens.filter_map do |cask_token|
-          # initialise lazily in case there's no casks in this tap
-          installed_cask_tokens ||= Set.new(Cask::Caskroom.tokens)
-          next unless installed_cask_tokens.include?(cask_token)
-
-          cask = begin
-            Cask::CaskLoader.load("#{tap.name}/#{cask_token}")
-          rescue
-            # Don't blow up because of a single unavailable cask.
-            next
-          end
-
-          cask if cask.installed?
-        end
 
         if installed_tap_formulae.present? || installed_tap_casks.present?
           installed_names = (installed_tap_formulae + installed_tap_casks.map(&:token)).join("\n")

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

@@ -5,8 +5,9 @@ require "formulary"
 module Test
   module Helper
     module Formula
-      def formula(name = "formula_name", path: Formulary.core_path(name), spec: :stable, alias_path: nil, &block)
+      def formula(name = "formula_name", path: Formulary.core_path(name), spec: :stable, alias_path: nil, tap: nil,
-        Class.new(::Formula, &block).new(name, path, spec, alias_path:)
+                  &block)
+        Class.new(::Formula, &block).new(name, path, spec, alias_path:, tap:)
       end
 
       # Use a stubbed {Formulary::FormulaLoader} to make a given formula be found

Library/Homebrew/test/support/helper/spec/shared_context/integration_test.rb

@@ -129,7 +129,7 @@ RSpec.shared_context "integration test" do # rubocop:disable RSpec/ContextWordin
     end
   end
 
-  def setup_test_formula(name, content = nil, bottle_block: nil)
+  def setup_test_formula(name, content = nil, tap: CoreTap.instance, bottle_block: nil)
     case name
     when /^testball/
       tarball = if OS.linux?
@@ -174,14 +174,14 @@ RSpec.shared_context "integration test" do # rubocop:disable RSpec/ContextWordin
       RUBY
     end
 
-    Formulary.core_path(name).tap do |formula_path|
+    Formulary.find_formula_in_tap(name.downcase, tap).tap do |formula_path|
       formula_path.write <<~RUBY
         class #{Formulary.class_s(name)} < Formula
         #{content.gsub(/^(?!$)/, "  ")}
         end
       RUBY
 
-      CoreTap.instance.clear_cache
+      tap.clear_cache
     end
   end
 

Library/Homebrew/test/untap_spec.rb

@@ -0,0 +1,126 @@
+# frozen_string_literal: true
+
+require "untap"
+
+RSpec.describe Homebrew::Untap do
+  describe ".installed_formulae_for", :integration_test do
+    shared_examples "finds installed formulae in tap" do
+      def load_formula(name:, with_formula_file: false, mock_install: false)
+        formula = if with_formula_file
+          path = setup_test_formula(name, tap:)
+          Formulary.factory(path)
+        else
+          formula(name, tap:) do
+            url "https://brew.sh/#{name}-1.0.tgz"
+          end
+        end
+
+        if mock_install
+          keg_path = HOMEBREW_CELLAR/name/"1.2.3"
+          keg_path.mkpath
+
+          tab_path = keg_path/Tab::FILENAME
+          tab_path.write <<~JSON
+            {
+              "source": {
+                "tap": "#{tap}"
+              }
+            }
+          JSON
+        end
+
+        formula
+      end
+
+      let!(:currently_installed_formula) do
+        load_formula(name: "current_install", with_formula_file: true, mock_install: true)
+      end
+
+      before do
+        # Formula that is available from a tap but not installed.
+        load_formula(name: "no_install", with_formula_file: true)
+
+        # Formula that was installed from a tap but is no longer available from that tap.
+        load_formula(name: "legacy_install", mock_install: true)
+
+        tap.clear_cache
+      end
+
+      it "returns the expected formulae" do
+        expect(described_class.installed_formulae_for(tap:).map(&:full_name))
+          .to eq([currently_installed_formula.full_name])
+      end
+    end
+
+    context "with core tap" do
+      let(:tap) { CoreTap.instance }
+
+      include_examples "finds installed formulae in tap"
+    end
+
+    context "with non-core tap" do
+      let(:tap) { Tap.fetch("homebrew", "foo") }
+
+      before do
+        tap.formula_dir.mkpath
+      end
+
+      include_examples "finds installed formulae in tap"
+    end
+  end
+
+  describe ".installed_casks_for", :cask do
+    shared_examples "finds installed casks in tap" do
+      def load_cask(token:, with_cask_file: false, mock_install: false)
+        cask_loader = Cask::CaskLoader::FromContentLoader.new(<<~RUBY, tap:)
+          cask '#{token}' do
+            version "1.2.3"
+            sha256 :no_check
+
+            url 'https://brew.sh/'
+          end
+        RUBY
+
+        cask = cask_loader.load(config: nil)
+
+        if with_cask_file
+          cask_path = tap.cask_dir/"#{token}.rb"
+          cask_path.parent.mkpath
+          cask_path.write cask.source
+        end
+
+        InstallHelper.install_with_caskfile(cask) if mock_install
+
+        cask
+      end
+
+      let!(:currently_installed_cask) do
+        load_cask(token: "current_install", with_cask_file: true, mock_install: true)
+      end
+
+      before do
+        # Cask that is available from a tap but not installed.
+        load_cask(token: "no_install", with_cask_file: true)
+
+        # Cask that was installed from a tap but is no longer available from that tap.
+        load_cask(token: "legacy_install", mock_install: true)
+      end
+
+      it "returns the expected casks" do
+        expect(described_class.installed_casks_for(tap:)).to eq([currently_installed_cask])
+      end
+    end
+
+    context "with core cask tap" do
+      let(:tap) { CoreCaskTap.instance }
+
+      include_examples "finds installed casks in tap"
+    end
+
+    context "with non-core cask tap" do
+      let(:tap) { Tap.fetch("homebrew", "foo") }
+
+      include_examples "finds installed casks in tap"
+    end
+  end
+end

Library/Homebrew/untap.rb

@@ -0,0 +1,60 @@
+# typed: true
+# frozen_string_literal: true
+
+require "extend/cachable"
+
+module Homebrew
+  # Helpers for the `brew untap` command.
+  # @api private
+  module Untap
+    extend Cachable
+
+    # All installed formulae currently available in a tap by formula full name.
+    sig { params(tap: Tap).returns(T::Array[Formula]) }
+    def self.installed_formulae_for(tap:)
+      tap.formula_names.filter_map do |formula_name|
+        next unless installed_formulae_names.include?(T.must(formula_name.split("/").last))
+
+        formula = begin
+          Formulary.factory(formula_name)
+        rescue FormulaUnavailableError
+          # Don't blow up because of a single unavailable formula.
+          next
+        end
+
+        # Can't use Formula#any_version_installed? because it doesn't consider
+        # taps correctly.
+        formula if formula.installed_kegs.any? { |keg| keg.tab.tap == tap }
+      end
+    end
+
+    sig { returns(T::Set[String]) }
+    def self.installed_formulae_names
+      cache[:installed_formulae_names] ||= Formula.installed_formula_names.to_set.freeze
+    end
+    private_class_method :installed_formulae_names
+
+    # All installed casks currently available in a tap by cask full name.
+    sig { params(tap: Tap).returns(T::Array[Cask::Cask]) }
+    def self.installed_casks_for(tap:)
+      tap.cask_tokens.filter_map do |cask_token|
+        next unless installed_cask_tokens.include?(T.must(cask_token.split("/").last))
+
+        cask = begin
+          Cask::CaskLoader.load(cask_token)
+        rescue Cask::CaskUnavailableError
+          # Don't blow up because of a single unavailable cask.
+          next
+        end
+
+        cask if cask.installed?
+      end
+    end
+
+    sig { returns(T::Set[String]) }
+    def self.installed_cask_tokens
+      cache[:installed_cask_tokens] ||= Cask::Caskroom.tokens.to_set.freeze
+    end
+    private_class_method :installed_cask_tokens
+  end
+end