Merge pull request #1652 from Homebrew/remove-aliases-oldnames

remover: match against formula aliases and old names when called with `--formula`

Author: carlocab

cmd/bundle.rb

@@ -59,7 +59,7 @@ class BundleCmd < AbstractCommand
           Add entries to your `Brewfile`. Adds formulae by default. Use `--cask`, `--tap`, `--whalebrew` or `--vscode` to add the corresponding entry instead.
 
           `brew bundle remove` <name> [...]:
-          Remove entries that match `name` from your `Brewfile`. Use `--formula`, `--cask`, `--tap`, `--mas`, `--whalebrew` or `--vscode` to remove only entries of the corresponding type.
+          Remove entries that match `name` from your `Brewfile`. Use `--formula`, `--cask`, `--tap`, `--mas`, `--whalebrew` or `--vscode` to remove only entries of the corresponding type. Passing `--formula` also removes matches against formula aliases and old formula names.
 
           `brew bundle exec` <command>:
           Run an external command in an isolated build environment based on the `Brewfile` dependencies.

lib/bundle/remover.rb

@@ -7,14 +7,39 @@ module Remover
 
     def remove(*args, type:, global:, file:)
       brewfile = Brewfile.read(global:, file:)
-      content = brewfile.input.split("\n")
+      content = brewfile.input
       entry_type = type.to_s if type != :none
-      escaped_args = args.map { |arg| Regexp.escape(arg) }
-      content = content.grep_v(/#{entry_type}(\s+|\(\s*)"(#{escaped_args.join("|")})"/)
-                       .join("\n") << "\n"
+      escaped_args = args.flat_map do |arg|
+        names = if type == :brew
+          possible_names(arg)
+        else
+          [arg]
+        end
+
+        names.uniq.map { |a| Regexp.escape(a) }
+      end
+
+      new_content = content.split("\n")
+                           .grep_v(/#{entry_type}(\s+|\(\s*)"(#{escaped_args.join("|")})"/)
+                           .join("\n") << "\n"
+
+      if content.chomp == new_content.chomp &&
+         type == :none &&
+         args.any? { |arg| possible_names(arg, raise_error: false).count > 1 }
+        opoo "No matching entries found in Brewfile. Try again with `--formula` to match formula " \
+             "aliases and old formula names."
+        return
+      end
+
       path = Dumper.brewfile_path(global:, file:)
+      Dumper.write_file path, new_content
+    end
 
-      Dumper.write_file path, content
+    def possible_names(formula_name, raise_error: true)
+      formula = Formulary.factory(formula_name)
+      [formula_name, formula.name, formula.full_name, *formula.aliases, *formula.oldnames].compact.uniq
+    rescue FormulaUnavailableError
+      raise if raise_error
     end
   end
 end

lib/bundle/remover.rbi

@@ -0,0 +1,7 @@
+# typed: true
+
+module Bundle
+  module Remover
+    include Kernel
+  end
+end

spec/bundle/commands/remove_command_spec.rb

@@ -7,7 +7,7 @@
     described_class.run(*args, type:, global:, file:)
   end
 
-  before { File.write(file, "brew \"hello\"\n") }
+  before { File.write(file, content) }
   after { FileUtils.rm_f file }
 
   let(:global) { false }
@@ -16,10 +16,54 @@
     let(:args) { ["hello"] }
     let(:type) { :brew }
     let(:file) { "/tmp/some_random_brewfile#{Random.rand(2 ** 16)}" }
+    let(:content) do
+      <<~BREWFILE
+        brew "hello"
+      BREWFILE
+    end
 
     it "removes entries from the given Brewfile" do
       expect { remove }.not_to raise_error
       expect(File.read(file)).not_to include("#{type} \"#{args.first}\"")
     end
   end
+
+  context "when called with no type" do
+    let(:args) { ["foo"] }
+    let(:type) { :none }
+    let(:file) { "/tmp/some_random_brewfile#{Random.rand(2 ** 16)}" }
+    let(:content) do
+      <<~BREWFILE
+        tap "someone/tap"
+        brew "foo"
+        cask "foo"
+      BREWFILE
+    end
+
+    it "removes all matching entries from the given Brewfile" do
+      expect { remove }.not_to raise_error
+      expect(File.read(file)).not_to include(args.first)
+    end
+
+    context "with arguments that match entries only when considering formula aliases" do
+      let(:foo) do
+        instance_double(
+          Formula,
+          name:      "foo",
+          full_name: "qux/quuz/foo",
+          oldnames:  ["oldfoo"],
+          aliases:   ["foobar"],
+        )
+      end
+      let(:args) { ["foobar"] }
+
+      it "suggests using `--formula` to match against formula aliases" do
+        expect(Formulary).to receive(:factory).with("foobar").and_return(foo)
+        expect { remove }.not_to raise_error
+        expect(File.read(file)).to eq(content)
+        # FIXME: Why doesn't this work?
+        # expect { remove }.to output("--formula").to_stderr
+      end
+    end
+  end
 end

spec/bundle/remover_spec.rb

@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+describe Bundle::Remover do
+  subject(:remover) { described_class }
+
+  let(:name) { "foo" }
+
+  before { allow(Formulary).to receive(:factory).with(name).and_raise(FormulaUnavailableError) }
+
+  it "raises no errors when requested" do
+    expect { remover.possible_names(name, raise_error: false) }.not_to raise_error
+  end
+end