Merge pull request #18008 from Homebrew/deprecate-old-style-cmds

Library/Homebrew/.rubocop.yml

@@ -24,6 +24,7 @@ Style/Documentation:
   AllowedConstants:
     - Homebrew
   Include:
+    - abstract_command.rb
     - cask/cask.rb
     - cask/dsl.rb
     - cask/dsl/version.rb

Library/Homebrew/abstract_command.rb

@@ -14,6 +14,8 @@ module Homebrew
   #
   # To subclass, implement a `run` method and provide a `cmd_args` block to document the command and its allowed args.
   # To generate method signatures for command args, run `brew typecheck --update`.
+  #
+  # @api public
   class AbstractCommand
     extend T::Helpers
 
@@ -44,6 +46,9 @@ module Homebrew
 
       private
 
+      # The description and arguments of the command should be defined within this block.
+      #
+      # @api public
       sig { params(block: T.proc.bind(CLI::Parser).void).void }
       def cmd_args(&block)
         @parser_block = T.let(block, T.nilable(T.proc.void))
@@ -59,7 +64,15 @@ module Homebrew
       @args = T.let(self.class.parser.parse(argv), CLI::Args)
     end
 
+    # This method will be invoked when the command is run.
+    #
+    # @api public
     sig { abstract.void }
     def run; end
   end
+
+  module Cmd
+    # The command class for `brew` itself, allowing its args to be parsed.
+    class Brew < AbstractCommand; end
+  end
 end

Library/Homebrew/brew.rb

@@ -48,7 +48,7 @@ begin
   ARGV.delete_at(help_cmd_index) if help_cmd_index
 
   require "cli/parser"
-  args = Homebrew::CLI::Parser.new.parse(ARGV.dup.freeze, ignore_invalid_options: true)
+  args = Homebrew::CLI::Parser.new(Homebrew::Cmd::Brew).parse(ARGV.dup.freeze, ignore_invalid_options: true)
   Context.current = args.context
 
   path = PATH.new(ENV.fetch("PATH"))

Library/Homebrew/cli/parser.rb

@@ -167,6 +167,11 @@ module Homebrew
           @command_name = T.let(T.must(cmd_location.label).chomp("_args").tr("_", "-"), String)
           @is_dev_cmd = T.let(T.must(cmd_location.absolute_path).start_with?(Commands::HOMEBREW_DEV_CMD_PATH),
                               T::Boolean)
+          # odeprecated(
+          #   "`brew #{@command_name}'. This command needs to be refactored, as it is written in a style that",
+          #   "inheritance from `Homebrew::AbstractCommand' ( see https://docs.brew.sh/External-Commands )",
+          #   disable_for_developers: false,
+          # )
         end
 
         @constraints = T.let([], T::Array[[String, String]])

Library/Homebrew/extend/kernel.rb

@@ -172,11 +172,13 @@ module Kernel
 
     disable = true if disable_for_developers && Homebrew::EnvConfig.developer?
     if disable || Homebrew.raise_deprecation_exceptions?
+      require "utils/github/actions"
       GitHub::Actions.puts_annotation_if_env_set(:error, message, file:, line:)
       exception = MethodDeprecatedError.new(message)
       exception.set_backtrace(backtrace)
       raise exception
     elsif !Homebrew.auditing?
+      require "utils/github/actions"
       GitHub::Actions.puts_annotation_if_env_set(:warning, message, file:, line:)
       opoo message
     end

Library/Homebrew/test/cli/parser_spec.rb

@@ -3,9 +3,11 @@
 require_relative "../../cli/parser"
 
 RSpec.describe Homebrew::CLI::Parser do
+  before { stub_const("Cmd", Class.new(Homebrew::AbstractCommand)) }
+
   describe "test switch options" do
     subject(:parser) do
-      described_class.new do
+      described_class.new(Cmd) do
         switch "--more-verbose", description: "Flag for higher verbosity"
         switch "--pry", env: :pry
         switch "--hidden", hidden: true
@@ -18,7 +20,7 @@ RSpec.describe Homebrew::CLI::Parser do
 
     context "when using binary options" do
       subject(:parser) do
-        described_class.new do
+        described_class.new(Cmd) do
           switch "--[no-]positive"
         end
       end
@@ -46,7 +48,7 @@ RSpec.describe Homebrew::CLI::Parser do
 
     context "when using negative options" do
       subject(:parser) do
-        described_class.new do
+        described_class.new(Cmd) do
           switch "--no-positive"
         end
       end
@@ -116,7 +118,7 @@ RSpec.describe Homebrew::CLI::Parser do
 
   describe "test long flag options" do
     subject(:parser) do
-      described_class.new do
+      described_class.new(Cmd) do
         flag        "--filename=", description: "Name of the file"
         comma_array "--files",     description: "Comma-separated filenames"
         flag        "--hidden=",      hidden: true
@@ -147,7 +149,7 @@ RSpec.describe Homebrew::CLI::Parser do
 
   describe "test short flag options" do
     subject(:parser) do
-      described_class.new do
+      described_class.new(Cmd) do
         flag "-f", "--filename=", description: "Name of the file"
       end
     end
@@ -161,7 +163,7 @@ RSpec.describe Homebrew::CLI::Parser do
 
   describe "test constraints for flag options" do
     subject(:parser) do
-      described_class.new do
+      described_class.new(Cmd) do
         flag      "--flag1="
         flag      "--flag2=", depends_on: "--flag1="
         flag      "--flag3="
@@ -192,7 +194,7 @@ RSpec.describe Homebrew::CLI::Parser do
 
   describe "test invalid constraints" do
     subject(:parser) do
-      described_class.new do
+      described_class.new(Cmd) do
         flag      "--flag1="
         flag      "--flag2=", depends_on: "--flag1="
         conflicts "--flag1=", "--flag2="
@@ -206,7 +208,7 @@ RSpec.describe Homebrew::CLI::Parser do
 
   describe "test constraints for switch options" do
     subject(:parser) do
-      described_class.new do
+      described_class.new(Cmd) do
         switch      "-a", "--switch-a", env: "switch_a"
         switch      "-b", "--switch-b", env: "switch_b"
         switch      "--switch-c", depends_on: "--switch-a"
@@ -253,7 +255,7 @@ RSpec.describe Homebrew::CLI::Parser do
 
   describe "test immutability of args" do
     subject(:parser) do
-      described_class.new do
+      described_class.new(Cmd) do
         switch "-a", "--switch-a"
         switch "-b", "--switch-b"
       end
@@ -267,7 +269,7 @@ RSpec.describe Homebrew::CLI::Parser do
 
   describe "test inferrability of args" do
     subject(:parser) do
-      described_class.new do
+      described_class.new(Cmd) do
         switch "--switch-a"
         switch "--switch-b"
         switch "--foo-switch"
@@ -303,7 +305,7 @@ RSpec.describe Homebrew::CLI::Parser do
 
   describe "test argv extensions" do
     subject(:parser) do
-      described_class.new do
+      described_class.new(Cmd) do
         switch "--foo"
         flag   "--bar"
         switch "-s"
@@ -333,7 +335,7 @@ RSpec.describe Homebrew::CLI::Parser do
 
   describe "usage banner generation" do
     it "includes `[options]` if more than two non-global options are available" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         switch "--foo"
         switch "--baz"
         switch "--bar"
@@ -342,7 +344,7 @@ RSpec.describe Homebrew::CLI::Parser do
     end
 
     it "includes individual options if less than two non-global options are available" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         switch "--foo"
         switch "--bar"
       end
@@ -350,7 +352,7 @@ RSpec.describe Homebrew::CLI::Parser do
     end
 
     it "formats flags correctly when less than two non-global options are available" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         flag "--foo"
         flag "--bar="
       end
@@ -358,26 +360,26 @@ RSpec.describe Homebrew::CLI::Parser do
     end
 
     it "formats comma arrays correctly when less than two non-global options are available" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         comma_array "--foo"
       end
       expect(parser.generate_help_text).to match(/\[--foo=\]/)
     end
 
     it "does not include hidden options" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         switch "--foo", hidden: true
       end
       expect(parser.generate_help_text).not_to match(/\[--foo\]/)
     end
 
     it "doesn't include `[options]` if non non-global options are available" do
-      parser = described_class.new
+      parser = described_class.new(Cmd)
       expect(parser.generate_help_text).not_to match(/\[options\]/)
     end
 
     it "includes a description" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         description <<~EOS
           This command does something
         EOS
@@ -386,14 +388,14 @@ RSpec.describe Homebrew::CLI::Parser do
     end
 
     it "allows the usage banner to be overridden" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         usage_banner "`test` [foo] <bar>"
       end
       expect(parser.generate_help_text).to match(/test \[foo\] bar/)
     end
 
     it "allows a usage banner and a description to be overridden" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         usage_banner "`test` [foo] <bar>"
         description <<~EOS
           This command does something
@@ -404,42 +406,42 @@ RSpec.describe Homebrew::CLI::Parser do
     end
 
     it "shows the correct usage for no named argument" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         named_args :none
       end
       expect(parser.generate_help_text).to match(/^Usage: [^\[]+$/s)
     end
 
     it "shows the correct usage for a single typed argument" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         named_args :formula, number: 1
       end
       expect(parser.generate_help_text).to match(/^Usage: .* formula$/s)
     end
 
     it "shows the correct usage for a subcommand argument with a maximum" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         named_args %w[off on], max: 1
       end
       expect(parser.generate_help_text).to match(/^Usage: .* \[subcommand\]$/s)
     end
 
     it "shows the correct usage for multiple typed argument with no maximum or minimum" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         named_args [:tap, :command]
       end
       expect(parser.generate_help_text).to match(/^Usage: .* \[tap|command ...\]$/s)
     end
 
     it "shows the correct usage for a subcommand argument with a minimum of 1" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         named_args :installed_formula, min: 1
       end
       expect(parser.generate_help_text).to match(/^Usage: .* installed_formula \[...\]$/s)
     end
 
     it "shows the correct usage for a subcommand argument with a minimum greater than 1" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         named_args :installed_formula, min: 2
       end
       expect(parser.generate_help_text).to match(/^Usage: .* installed_formula ...$/s)
@@ -448,19 +450,19 @@ RSpec.describe Homebrew::CLI::Parser do
 
   describe "named_args" do
     let(:parser_none) do
-      described_class.new do
+      described_class.new(Cmd) do
         named_args :none
       end
     end
     let(:parser_number) do
-      described_class.new do
+      described_class.new(Cmd) do
         named_args number: 1
       end
     end
 
     it "doesn't allow :none passed with a number" do
       expect do
-        described_class.new do
+        described_class.new(Cmd) do
           named_args :none, number: 1
         end
       end.to raise_error(ArgumentError, /Do not specify both `number`, `min` or `max` with `named_args :none`/)
@@ -468,7 +470,7 @@ RSpec.describe Homebrew::CLI::Parser do
 
     it "doesn't allow number and min" do
       expect do
-        described_class.new do
+        described_class.new(Cmd) do
           named_args number: 1, min: 1
         end
       end.to raise_error(ArgumentError, /Do not specify both `number` and `min` or `max`/)
@@ -496,7 +498,7 @@ RSpec.describe Homebrew::CLI::Parser do
     end
 
     it "displays the correct error message with no arg types and min" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         named_args min: 2
       end
       expect { parser.parse([]) }.to raise_error(
@@ -505,7 +507,7 @@ RSpec.describe Homebrew::CLI::Parser do
     end
 
     it "displays the correct error message with no arg types and number" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         named_args number: 2
       end
       expect { parser.parse([]) }.to raise_error(
@@ -514,7 +516,7 @@ RSpec.describe Homebrew::CLI::Parser do
     end
 
     it "displays the correct error message with no arg types and max" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         named_args max: 1
       end
       expect { parser.parse(%w[foo bar]) }.to raise_error(
@@ -523,7 +525,7 @@ RSpec.describe Homebrew::CLI::Parser do
     end
 
     it "displays the correct error message with an array of strings" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         named_args %w[on off], number: 1
       end
       expect { parser.parse([]) }.to raise_error(
@@ -532,7 +534,7 @@ RSpec.describe Homebrew::CLI::Parser do
     end
 
     it "displays the correct error message with an array of symbols" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         named_args [:formula, :cask], min: 1
       end
       expect { parser.parse([]) }.to raise_error(
@@ -541,7 +543,7 @@ RSpec.describe Homebrew::CLI::Parser do
     end
 
     it "displays the correct error message with an array of symbols and max" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         named_args [:formula, :cask], max: 1
       end
       expect { parser.parse(%w[foo bar]) }.to raise_error(
@@ -550,14 +552,14 @@ RSpec.describe Homebrew::CLI::Parser do
     end
 
     it "accepts commands with :command" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         named_args :command
       end
       expect { parser.parse(["--prefix", "--version"]) }.not_to raise_error
     end
 
     it "doesn't accept invalid options with :command" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         named_args :command
       end
       expect { parser.parse(["--not-a-command"]) }.to raise_error(OptionParser::InvalidOption, /--not-a-command/)
@@ -567,7 +569,7 @@ RSpec.describe Homebrew::CLI::Parser do
   describe "--cask on linux", :needs_linux do
     context "without --formula switch" do
       subject(:parser) do
-        described_class.new do
+        described_class.new(Cmd) do
           switch "--cask"
         end
       end
@@ -590,7 +592,7 @@ RSpec.describe Homebrew::CLI::Parser do
 
     context "with conflicting --formula switch" do
       subject(:parser) do
-        described_class.new do
+        described_class.new(Cmd) do
           switch "--cask"
           switch "--formula"
           conflicts "--cask", "--formula"
@@ -615,13 +617,13 @@ RSpec.describe Homebrew::CLI::Parser do
 
   describe "--formula on linux", :needs_linux do
     it "doesn't set --formula when not defined" do
-      parser = described_class.new
+      parser = described_class.new(Cmd)
       args = parser.parse([])
       expect(args.respond_to?(:formula?)).to be(false)
     end
 
     it "sets --formula to true when defined" do
-      parser = described_class.new do
+      parser = described_class.new(Cmd) do
         switch "--formula"
       end
       args = parser.parse([])

Library/Homebrew/test/cli/parser_spec.rbi

@@ -0,0 +1,3 @@
+# typed: strict
+
+class Cmd < Homebrew::AbstractCommand; end

Library/Homebrew/test/support/helper/cmd/brew-verify-formula-undefined.rb

@@ -3,7 +3,14 @@
 
 require "cli/parser"
 
-parser = Homebrew::CLI::Parser.new do
+module Homebrew
+  module Cmd
+    class VerifyFormulaUndefined < AbstractCommand
+    end
+  end
+end
+
+parser = Homebrew::CLI::Parser.new(Homebrew::Cmd::VerifyFormulaUndefined) do
   usage_banner <<~EOS
     `verify-formula-undefined`