From cbcb221de6990cb212698b695cf548ebc1050446 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Sun, 3 Mar 2024 15:32:30 -0800 Subject: [PATCH 01/16] Create AbstractCommand class --- Library/Homebrew/abstract_command.rb | 42 +++ Library/Homebrew/brew.rb | 8 +- Library/Homebrew/cmd/list.rb | 412 +++++++++++++------------ Library/Homebrew/test/cmd/list_spec.rb | 6 +- 4 files changed, 261 insertions(+), 207 deletions(-) create mode 100644 Library/Homebrew/abstract_command.rb diff --git a/Library/Homebrew/abstract_command.rb b/Library/Homebrew/abstract_command.rb new file mode 100644 index 0000000000..82fcdf4994 --- /dev/null +++ b/Library/Homebrew/abstract_command.rb @@ -0,0 +1,42 @@ +# typed: strong +# frozen_string_literal: true + +module Homebrew + class AbstractCommand + extend T::Helpers + + abstract! + + class << self + # registers subclasses for lookup by command name + sig { params(subclass: T.class_of(AbstractCommand)).void } + def inherited(subclass) + super + @cmds ||= T.let({}, T.nilable(T::Hash[String, T.class_of(AbstractCommand)])) + @cmds[subclass.command_name] = subclass + end + + sig { params(name: String).returns(T.nilable(T.class_of(AbstractCommand))) } + def command(name) = @cmds&.[](name) + + sig { returns(String) } + def command_name = T.must(name).split("::").fetch(-1).downcase + end + + # @note because `Args` makes use `OpenStruct`, subclasses may need to use a tapioca compiler, + # hash accessors, args.rbi, or other means to make this work with legacy commands: + sig { returns(Homebrew::CLI::Args) } + attr_reader :args + + sig { void } + def initialize + @args = T.let(raw_args.parse, Homebrew::CLI::Args) + end + + sig { abstract.returns(CLI::Parser) } + def raw_args; end + + sig { abstract.void } + def run; end + end +end diff --git a/Library/Homebrew/brew.rb b/Library/Homebrew/brew.rb index c50bf61ab5..2e785889bc 100644 --- a/Library/Homebrew/brew.rb +++ b/Library/Homebrew/brew.rb @@ -59,6 +59,7 @@ begin ENV["PATH"] = path.to_s + require "abstract_command" require "commands" require "settings" @@ -83,7 +84,12 @@ begin end if internal_cmd || Commands.external_ruby_v2_cmd_path(cmd) - Homebrew.send Commands.method_name(cmd) + cmd_class = Homebrew::AbstractCommand.command(T.must(cmd)) + if cmd_class + cmd_class.new.run + else + Homebrew.public_send Commands.method_name(cmd) + end elsif (path = Commands.external_ruby_cmd_path(cmd)) require?(path) exit Homebrew.failed? ? 1 : 0 diff --git a/Library/Homebrew/cmd/list.rb b/Library/Homebrew/cmd/list.rb index 1b69e2596e..47a05b8d62 100644 --- a/Library/Homebrew/cmd/list.rb +++ b/Library/Homebrew/cmd/list.rb @@ -1,6 +1,7 @@ # typed: true # frozen_string_literal: true +require "abstract_command" require "metafiles" require "formula" require "cli/parser" @@ -8,226 +9,233 @@ require "cask/list" require "system_command" module Homebrew - extend SystemCommand::Mixin + module Cmd + class List < AbstractCommand + include SystemCommand::Mixin - sig { returns(CLI::Parser) } - def self.list_args - Homebrew::CLI::Parser.new do - description <<~EOS - List all installed formulae and casks. - If is provided, summarise the paths within its current keg. - If is provided, list its artifacts. - EOS - switch "--formula", "--formulae", - description: "List only formulae, or treat all named arguments as formulae." - switch "--cask", "--casks", - description: "List only casks, or treat all named arguments as casks." - switch "--full-name", - description: "Print formulae with fully-qualified names. Unless `--full-name`, `--versions` " \ - "or `--pinned` are passed, other options (i.e. `-1`, `-l`, `-r` and `-t`) are " \ - "passed to `ls`(1) which produces the actual output." - switch "--versions", - description: "Show the version number for installed formulae, or only the specified " \ - "formulae if are provided." - switch "--multiple", - depends_on: "--versions", - description: "Only show formulae with multiple versions installed." - switch "--pinned", - description: "List only pinned formulae, or only the specified (pinned) " \ - "formulae if are provided. See also `pin`, `unpin`." - # passed through to ls - switch "-1", - description: "Force output to be one entry per line. " \ - "This is the default when output is not to a terminal." - switch "-l", - description: "List formulae and/or casks in long format. " \ - "Has no effect when a formula or cask name is passed as an argument." - switch "-r", - description: "Reverse the order of the formulae and/or casks sort to list the oldest entries first. " \ - "Has no effect when a formula or cask name is passed as an argument." - switch "-t", - description: "Sort formulae and/or casks by time modified, listing most recently modified first. " \ - "Has no effect when a formula or cask name is passed as an argument." + sig { override.returns(CLI::Parser) } + def raw_args + Homebrew::CLI::Parser.new do + description <<~EOS + List all installed formulae and casks. + If is provided, summarise the paths within its current keg. + If is provided, list its artifacts. + EOS + switch "--formula", "--formulae", + description: "List only formulae, or treat all named arguments as formulae." + switch "--cask", "--casks", + description: "List only casks, or treat all named arguments as casks." + switch "--full-name", + description: "Print formulae with fully-qualified names. Unless `--full-name`, `--versions` " \ + "or `--pinned` are passed, other options (i.e. `-1`, `-l`, `-r` and `-t`) are " \ + "passed to `ls`(1) which produces the actual output." + switch "--versions", + description: "Show the version number for installed formulae, or only the specified " \ + "formulae if are provided." + switch "--multiple", + depends_on: "--versions", + description: "Only show formulae with multiple versions installed." + switch "--pinned", + description: "List only pinned formulae, or only the specified (pinned) " \ + "formulae if are provided. See also `pin`, `unpin`." + # passed through to ls + switch "-1", + description: "Force output to be one entry per line. " \ + "This is the default when output is not to a terminal." + switch "-l", + description: "List formulae and/or casks in long format. " \ + "Has no effect when a formula or cask name is passed as an argument." + switch "-r", + description: "Reverse the order of the formulae and/or casks sort to list the oldest entries " \ + "first. Has no effect when a formula or cask name is passed as an argument." + switch "-t", + description: "Sort formulae and/or casks by time modified, listing most recently modified first. " \ + "Has no effect when a formula or cask name is passed as an argument." - conflicts "--formula", "--cask" - conflicts "--pinned", "--cask" - conflicts "--multiple", "--cask" - conflicts "--pinned", "--multiple" - ["-1", "-l", "-r", "-t"].each do |flag| - conflicts "--versions", flag - conflicts "--pinned", flag - end - ["--versions", "--pinned", "-l", "-r", "-t"].each do |flag| - conflicts "--full-name", flag + conflicts "--formula", "--cask" + conflicts "--pinned", "--cask" + conflicts "--multiple", "--cask" + conflicts "--pinned", "--multiple" + ["-1", "-l", "-r", "-t"].each do |flag| + conflicts "--versions", flag + conflicts "--pinned", flag + end + ["--versions", "--pinned", "-l", "-r", "-t"].each do |flag| + conflicts "--full-name", flag + end + + named_args [:installed_formula, :installed_cask] + end end - named_args [:installed_formula, :installed_cask] - end - end + sig { override.void } + def run + if args.full_name? + unless args.cask? + formula_names = args.no_named? ? Formula.installed : args.named.to_resolved_formulae + full_formula_names = formula_names.map(&:full_name).sort(&tap_and_name_comparison) + full_formula_names = Formatter.columns(full_formula_names) unless args.public_send(:"1?") + puts full_formula_names if full_formula_names.present? + end + if args.cask? || (!args.formula? && args.no_named?) + cask_names = if args.no_named? + Cask::Caskroom.casks + else + args.named.to_formulae_and_casks(only: :cask, method: :resolve) + end + # The cast is because `Keg`` does not define `full_name` + full_cask_names = T.cast(cask_names, T::Array[T.any(Formula, Cask::Cask)]) + .map(&:full_name).sort(&tap_and_name_comparison) + full_cask_names = Formatter.columns(full_cask_names) unless args.public_send(:"1?") + puts full_cask_names if full_cask_names.present? + end + elsif args[:pinned?] + filtered_list(args:) + elsif args[:versions?] + filtered_list(args:) unless args.cask? + list_casks(args:) if args.cask? || (!args.formula? && !args[:multiple?] && args.no_named?) + elsif args.no_named? + ENV["CLICOLOR"] = nil - def self.list - args = list_args.parse + ls_args = [] + ls_args << "-1" if args[:"1?"] + ls_args << "-l" if args[:l?] + ls_args << "-r" if args[:r?] + ls_args << "-t" if args[:t?] - if args.full_name? - unless args.cask? - formula_names = args.no_named? ? Formula.installed : args.named.to_resolved_formulae - full_formula_names = formula_names.map(&:full_name).sort(&tap_and_name_comparison) - full_formula_names = Formatter.columns(full_formula_names) unless args.public_send(:"1?") - puts full_formula_names if full_formula_names.present? + if !args.cask? && HOMEBREW_CELLAR.exist? && HOMEBREW_CELLAR.children.any? + ohai "Formulae" if $stdout.tty? && !args.formula? + safe_system "ls", *ls_args, HOMEBREW_CELLAR + puts if $stdout.tty? && !args.formula? + end + if !args.formula? && Cask::Caskroom.any_casks_installed? + ohai "Casks" if $stdout.tty? && !args.cask? + safe_system "ls", *ls_args, Cask::Caskroom.path + end + else + kegs, casks = args.named.to_kegs_to_casks + + if args.verbose? || !$stdout.tty? + find_args = %w[-not -type d -not -name .DS_Store -print] + system_command! "find", args: kegs.map(&:to_s) + find_args, print_stdout: true if kegs.present? + system_command! "find", args: casks.map(&:caskroom_path) + find_args, print_stdout: true if casks.present? + else + kegs.each { |keg| PrettyListing.new keg } if kegs.present? + list_casks(args:) if casks.present? + end + end end - if args.cask? || (!args.formula? && args.no_named?) - cask_names = if args.no_named? + + private + + def filtered_list(args:) + names = if args.no_named? + Formula.racks + else + racks = args.named.map { |n| Formulary.to_rack(n) } + racks.select do |rack| + Homebrew.failed = true unless rack.exist? + rack.exist? + end + end + if args.pinned? + pinned_versions = {} + names.sort.each do |d| + keg_pin = (HOMEBREW_PINNED_KEGS/d.basename.to_s) + pinned_versions[d] = keg_pin.readlink.basename.to_s if keg_pin.exist? || keg_pin.symlink? + end + pinned_versions.each do |d, version| + puts d.basename.to_s.concat(args.versions? ? " #{version}" : "") + end + else # --versions without --pinned + names.sort.each do |d| + versions = d.subdirs.map { |pn| pn.basename.to_s } + next if args.multiple? && versions.length < 2 + + puts "#{d.basename} #{versions * " "}" + end + end + end + + def list_casks(args:) + casks = if args.no_named? Cask::Caskroom.casks else - args.named.to_formulae_and_casks(only: :cask, method: :resolve) + args.named.dup.delete_if do |n| + Homebrew.failed = true unless Cask::Caskroom.path.join(n).exist? + !Cask::Caskroom.path.join(n).exist? + end.to_formulae_and_casks(only: :cask) end - full_cask_names = cask_names.map(&:full_name).sort(&tap_and_name_comparison) - full_cask_names = Formatter.columns(full_cask_names) unless args.public_send(:"1?") - puts full_cask_names if full_cask_names.present? - end - elsif args.pinned? - filtered_list(args:) - elsif args.versions? - filtered_list(args:) unless args.cask? - list_casks(args:) if args.cask? || (!args.formula? && !args.multiple? && args.no_named?) - elsif args.no_named? - ENV["CLICOLOR"] = nil + return if casks.blank? - ls_args = [] - ls_args << "-1" if args.public_send(:"1?") - ls_args << "-l" if args.l? - ls_args << "-r" if args.r? - ls_args << "-t" if args.t? - - if !args.cask? && HOMEBREW_CELLAR.exist? && HOMEBREW_CELLAR.children.any? - ohai "Formulae" if $stdout.tty? && !args.formula? - safe_system "ls", *ls_args, HOMEBREW_CELLAR - puts if $stdout.tty? && !args.formula? - end - if !args.formula? && Cask::Caskroom.any_casks_installed? - ohai "Casks" if $stdout.tty? && !args.cask? - safe_system "ls", *ls_args, Cask::Caskroom.path - end - else - kegs, casks = args.named.to_kegs_to_casks - - if args.verbose? || !$stdout.tty? - find_args = %w[-not -type d -not -name .DS_Store -print] - system_command! "find", args: kegs.map(&:to_s) + find_args, print_stdout: true if kegs.present? - system_command! "find", args: casks.map(&:caskroom_path) + find_args, print_stdout: true if casks.present? - else - kegs.each { |keg| PrettyListing.new keg } if kegs.present? - list_casks(args:) if casks.present? + Cask::List.list_casks( + *casks, + one: args.public_send(:"1?"), + full_name: args.full_name?, + versions: args.versions?, + ) end end - end - def self.filtered_list(args:) - names = if args.no_named? - Formula.racks - else - racks = args.named.map { |n| Formulary.to_rack(n) } - racks.select do |rack| - Homebrew.failed = true unless rack.exist? - rack.exist? - end - end - if args.pinned? - pinned_versions = {} - names.sort.each do |d| - keg_pin = (HOMEBREW_PINNED_KEGS/d.basename.to_s) - pinned_versions[d] = keg_pin.readlink.basename.to_s if keg_pin.exist? || keg_pin.symlink? - end - pinned_versions.each do |d, version| - puts d.basename.to_s.concat(args.versions? ? " #{version}" : "") - end - else # --versions without --pinned - names.sort.each do |d| - versions = d.subdirs.map { |pn| pn.basename.to_s } - next if args.multiple? && versions.length < 2 - - puts "#{d.basename} #{versions * " "}" - end - end - end - - def self.list_casks(args:) - casks = if args.no_named? - Cask::Caskroom.casks - else - args.named.dup.delete_if do |n| - Homebrew.failed = true unless Cask::Caskroom.path.join(n).exist? - !Cask::Caskroom.path.join(n).exist? - end.to_formulae_and_casks(only: :cask) - end - return if casks.blank? - - Cask::List.list_casks( - *casks, - one: args.public_send(:"1?"), - full_name: args.full_name?, - versions: args.versions?, - ) - end -end - -class PrettyListing - def initialize(path) - Pathname.new(path).children.sort_by { |p| p.to_s.downcase }.each do |pn| - case pn.basename.to_s - when "bin", "sbin" - pn.find { |pnn| puts pnn unless pnn.directory? } - when "lib" - print_dir pn do |pnn| - # dylibs have multiple symlinks and we don't care about them - (pnn.extname == ".dylib" || pnn.extname == ".pc") && !pnn.symlink? - end - when ".brew" - next # Ignore .brew - else - if pn.directory? - if pn.symlink? - puts "#{pn} -> #{pn.readlink}" + class PrettyListing + def initialize(path) + Pathname.new(path).children.sort_by { |p| p.to_s.downcase }.each do |pn| + case pn.basename.to_s + when "bin", "sbin" + pn.find { |pnn| puts pnn unless pnn.directory? } + when "lib" + print_dir pn do |pnn| + # dylibs have multiple symlinks and we don't care about them + (pnn.extname == ".dylib" || pnn.extname == ".pc") && !pnn.symlink? + end + when ".brew" + next # Ignore .brew else - print_dir pn + if pn.directory? + if pn.symlink? + puts "#{pn} -> #{pn.readlink}" + else + print_dir pn + end + elsif Metafiles.list?(pn.basename.to_s) + puts pn + end end - elsif Metafiles.list?(pn.basename.to_s) - puts pn + end + end + + def print_dir(root) + dirs = [] + remaining_root_files = [] + other = "" + + root.children.sort.each do |pn| + if pn.directory? + dirs << pn + elsif block_given? && yield(pn) + puts pn + other = "other " + elsif pn.basename.to_s != ".DS_Store" + remaining_root_files << pn + end + end + + dirs.each do |d| + files = [] + d.find { |pn| files << pn unless pn.directory? } + print_remaining_files files, d + end + + print_remaining_files remaining_root_files, root, other + end + + def print_remaining_files(files, root, other = "") + if files.length == 1 + puts files + elsif files.length > 1 + puts "#{root}/ (#{files.length} #{other}files)" end end end end - - def print_dir(root) - dirs = [] - remaining_root_files = [] - other = "" - - root.children.sort.each do |pn| - if pn.directory? - dirs << pn - elsif block_given? && yield(pn) - puts pn - other = "other " - elsif pn.basename.to_s != ".DS_Store" - remaining_root_files << pn - end - end - - dirs.each do |d| - files = [] - d.find { |pn| files << pn unless pn.directory? } - print_remaining_files files, d - end - - print_remaining_files remaining_root_files, root, other - end - - def print_remaining_files(files, root, other = "") - if files.length == 1 - puts files - elsif files.length > 1 - puts "#{root}/ (#{files.length} #{other}files)" - end - end end diff --git a/Library/Homebrew/test/cmd/list_spec.rb b/Library/Homebrew/test/cmd/list_spec.rb index 414862251c..6a9e34c9e7 100644 --- a/Library/Homebrew/test/cmd/list_spec.rb +++ b/Library/Homebrew/test/cmd/list_spec.rb @@ -1,12 +1,10 @@ # frozen_string_literal: true -require "cmd/shared_examples/args_parse" +require "cmd/list" -RSpec.describe "brew list" do +RSpec.describe Homebrew::Cmd::List do let(:formulae) { %w[bar foo qux] } - it_behaves_like "parseable arguments" - it "prints all installed Formulae", :integration_test do formulae.each do |f| (HOMEBREW_CELLAR/f/"1.0/somedir").mkpath From 5364b6ef039875e139d1ad8a5d37c976d6b303ee Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Mon, 4 Mar 2024 07:51:43 -0800 Subject: [PATCH 02/16] Use a command registry --- Library/Homebrew/abstract_command.rb | 8 +++----- Library/Homebrew/brew.rb | 2 +- Library/Homebrew/command_registry.rb | 19 +++++++++++++++++++ 3 files changed, 23 insertions(+), 6 deletions(-) create mode 100644 Library/Homebrew/command_registry.rb diff --git a/Library/Homebrew/abstract_command.rb b/Library/Homebrew/abstract_command.rb index 82fcdf4994..28d364bec8 100644 --- a/Library/Homebrew/abstract_command.rb +++ b/Library/Homebrew/abstract_command.rb @@ -1,6 +1,8 @@ # typed: strong # frozen_string_literal: true +require "command_registry" + module Homebrew class AbstractCommand extend T::Helpers @@ -12,13 +14,9 @@ module Homebrew sig { params(subclass: T.class_of(AbstractCommand)).void } def inherited(subclass) super - @cmds ||= T.let({}, T.nilable(T::Hash[String, T.class_of(AbstractCommand)])) - @cmds[subclass.command_name] = subclass + CommandRegistry.register(subclass) end - sig { params(name: String).returns(T.nilable(T.class_of(AbstractCommand))) } - def command(name) = @cmds&.[](name) - sig { returns(String) } def command_name = T.must(name).split("::").fetch(-1).downcase end diff --git a/Library/Homebrew/brew.rb b/Library/Homebrew/brew.rb index 2e785889bc..04495f6f6e 100644 --- a/Library/Homebrew/brew.rb +++ b/Library/Homebrew/brew.rb @@ -84,7 +84,7 @@ begin end if internal_cmd || Commands.external_ruby_v2_cmd_path(cmd) - cmd_class = Homebrew::AbstractCommand.command(T.must(cmd)) + cmd_class = Homebrew::CommandRegistry.command(T.must(cmd)) if cmd_class cmd_class.new.run else diff --git a/Library/Homebrew/command_registry.rb b/Library/Homebrew/command_registry.rb new file mode 100644 index 0000000000..59779f7b58 --- /dev/null +++ b/Library/Homebrew/command_registry.rb @@ -0,0 +1,19 @@ +# typed: strong +# frozen_string_literal: true + +module Homebrew + module CommandRegistry + extend T::Helpers + + Cmd = T.type_alias { T.class_of(AbstractCommand) } # rubocop:disable Style/MutableConstant + + sig { params(subclass: Cmd).void } + def self.register(subclass) + @cmds ||= T.let({}, T.nilable(T::Hash[String, Cmd])) + @cmds[subclass.command_name] = subclass + end + + sig { params(name: String).returns(T.nilable(Cmd)) } + def self.command(name) = @cmds&.[](name) + end +end From 2dceb65b427171d23077f3662d300102386210e4 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Mon, 4 Mar 2024 10:29:47 -0800 Subject: [PATCH 03/16] Port prof to use AbstractCommand --- Library/Homebrew/dev-cmd/prof.rb | 112 +++++++++++---------- Library/Homebrew/test/dev-cmd/prof_spec.rb | 6 +- 2 files changed, 59 insertions(+), 59 deletions(-) diff --git a/Library/Homebrew/dev-cmd/prof.rb b/Library/Homebrew/dev-cmd/prof.rb index b33bc82bfb..7a928c0dfc 100644 --- a/Library/Homebrew/dev-cmd/prof.rb +++ b/Library/Homebrew/dev-cmd/prof.rb @@ -1,65 +1,67 @@ -# typed: true +# typed: strict # frozen_string_literal: true +require "abstract_command" require "cli/parser" module Homebrew - module_function + module DevCmd + class Prof < AbstractCommand + sig { override.returns(CLI::Parser) } + def raw_args + Homebrew::CLI::Parser.new do + description <<~EOS + Run Homebrew with a Ruby profiler. For example, `brew prof readall`. + EOS + switch "--stackprof", + description: "Use `stackprof` instead of `ruby-prof` (the default)." - sig { returns(CLI::Parser) } - def prof_args - Homebrew::CLI::Parser.new do - description <<~EOS - Run Homebrew with a Ruby profiler. For example, `brew prof readall`. - EOS - switch "--stackprof", - description: "Use `stackprof` instead of `ruby-prof` (the default)." - - named_args :command, min: 1 - end - end - - def prof - args = prof_args.parse - - Homebrew.install_bundler_gems!(groups: ["prof"], setup_path: false) - - brew_rb = (HOMEBREW_LIBRARY_PATH/"brew.rb").resolved_path - FileUtils.mkdir_p "prof" - cmd = args.named.first - - case Commands.path(cmd)&.extname - when ".rb" - # expected file extension so we do nothing - when ".sh" - raise UsageError, <<~EOS - `#{cmd}` is a Bash command! - Try `hyperfine` for benchmarking instead. - EOS - else - raise UsageError, "`#{cmd}` is an unknown command!" - end - - Homebrew.setup_gem_environment! - - if args.stackprof? - with_env HOMEBREW_STACKPROF: "1" do - system(*HOMEBREW_RUBY_EXEC_ARGS, brew_rb, *args.named) + named_args :command, min: 1 + end + end + + sig { override.void } + def run + Homebrew.install_bundler_gems!(groups: ["prof"], setup_path: false) + + brew_rb = (HOMEBREW_LIBRARY_PATH/"brew.rb").resolved_path + FileUtils.mkdir_p "prof" + cmd = args.named.first + + case Commands.path(cmd)&.extname + when ".rb" + # expected file extension so we do nothing + when ".sh" + raise UsageError, <<~EOS + `#{cmd}` is a Bash command! + Try `hyperfine` for benchmarking instead. + EOS + else + raise UsageError, "`#{cmd}` is an unknown command!" + end + + Homebrew.setup_gem_environment! + + if args[:stackprof?] + with_env HOMEBREW_STACKPROF: "1" do + system(*HOMEBREW_RUBY_EXEC_ARGS, brew_rb, *args.named) + end + output_filename = "prof/d3-flamegraph.html" + safe_system "stackprof --d3-flamegraph prof/stackprof.dump > #{output_filename}" + else + output_filename = "prof/call_stack.html" + safe_system "ruby-prof", "--printer=call_stack", "--file=#{output_filename}", brew_rb, "--", *args.named + end + + exec_browser output_filename + rescue OptionParser::InvalidOption => e + ofail e + + # The invalid option could have been meant for the subcommand. + # Suggest `brew prof list -r` -> `brew prof -- list -r` + args = ARGV - ["--"] + puts "Try `brew prof -- #{args.join(" ")}` instead." end - output_filename = "prof/d3-flamegraph.html" - safe_system "stackprof --d3-flamegraph prof/stackprof.dump > #{output_filename}" - else - output_filename = "prof/call_stack.html" - safe_system "ruby-prof", "--printer=call_stack", "--file=#{output_filename}", brew_rb, "--", *args.named end - - exec_browser output_filename - rescue OptionParser::InvalidOption => e - ofail e - - # The invalid option could have been meant for the subcommand. - # Suggest `brew prof list -r` -> `brew prof -- list -r` - args = ARGV - ["--"] - puts "Try `brew prof -- #{args.join(" ")}` instead." end end diff --git a/Library/Homebrew/test/dev-cmd/prof_spec.rb b/Library/Homebrew/test/dev-cmd/prof_spec.rb index 25565776d1..44dba51e14 100644 --- a/Library/Homebrew/test/dev-cmd/prof_spec.rb +++ b/Library/Homebrew/test/dev-cmd/prof_spec.rb @@ -1,10 +1,8 @@ # frozen_string_literal: true -require "cmd/shared_examples/args_parse" - -RSpec.describe "brew prof" do - it_behaves_like "parseable arguments" +require "dev-cmd/prof" +RSpec.describe Homebrew::DevCmd::Prof do describe "integration tests", :integration_test, :needs_network do after do FileUtils.rm_rf HOMEBREW_LIBRARY_PATH/"prof" From fd652148fa4e790ef8c60619da38c11020626c3e Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Tue, 5 Mar 2024 10:40:02 -0800 Subject: [PATCH 04/16] Implement cmd_args block --- Library/Homebrew/abstract_command.rb | 29 +++++--- Library/Homebrew/cmd/list.rb | 99 ++++++++++++++-------------- Library/Homebrew/dev-cmd/prof.rb | 17 ++--- 3 files changed, 76 insertions(+), 69 deletions(-) diff --git a/Library/Homebrew/abstract_command.rb b/Library/Homebrew/abstract_command.rb index 28d364bec8..c0d4acdaaa 100644 --- a/Library/Homebrew/abstract_command.rb +++ b/Library/Homebrew/abstract_command.rb @@ -4,36 +4,49 @@ require "command_registry" module Homebrew + # Subclass this to implement a `brew` command. This is preferred to declaring a named function in the `Homebrew` + # module, because: + # - Each Command lives in an isolated namespace. + # - Each Command implements a defined interface. + # + # To subclass, implement a `run` method and provide a `cmd_args` block to document the command and its allowed args. class AbstractCommand extend T::Helpers abstract! class << self + sig { returns(T.nilable(T.proc.void)) } + attr_reader :parser_block + + sig { returns(String) } + def command_name = T.must(name).split("::").fetch(-1).downcase + + private + + sig { params(block: T.nilable(T.proc.bind(CLI::Parser).void)).void } + def cmd_args(&block) + @parser_block = T.let(block, T.nilable(T.proc.void)) + end + # registers subclasses for lookup by command name sig { params(subclass: T.class_of(AbstractCommand)).void } def inherited(subclass) super CommandRegistry.register(subclass) end - - sig { returns(String) } - def command_name = T.must(name).split("::").fetch(-1).downcase end # @note because `Args` makes use `OpenStruct`, subclasses may need to use a tapioca compiler, # hash accessors, args.rbi, or other means to make this work with legacy commands: - sig { returns(Homebrew::CLI::Args) } + sig { returns(CLI::Args) } attr_reader :args sig { void } def initialize - @args = T.let(raw_args.parse, Homebrew::CLI::Args) + @args = T.let(CLI::Parser.new(&self.class.parser_block).parse, CLI::Args) end - sig { abstract.returns(CLI::Parser) } - def raw_args; end - sig { abstract.void } def run; end end diff --git a/Library/Homebrew/cmd/list.rb b/Library/Homebrew/cmd/list.rb index 47a05b8d62..8f8184ec22 100644 --- a/Library/Homebrew/cmd/list.rb +++ b/Library/Homebrew/cmd/list.rb @@ -13,59 +13,56 @@ module Homebrew class List < AbstractCommand include SystemCommand::Mixin - sig { override.returns(CLI::Parser) } - def raw_args - Homebrew::CLI::Parser.new do - description <<~EOS - List all installed formulae and casks. - If is provided, summarise the paths within its current keg. - If is provided, list its artifacts. - EOS - switch "--formula", "--formulae", - description: "List only formulae, or treat all named arguments as formulae." - switch "--cask", "--casks", - description: "List only casks, or treat all named arguments as casks." - switch "--full-name", - description: "Print formulae with fully-qualified names. Unless `--full-name`, `--versions` " \ - "or `--pinned` are passed, other options (i.e. `-1`, `-l`, `-r` and `-t`) are " \ - "passed to `ls`(1) which produces the actual output." - switch "--versions", - description: "Show the version number for installed formulae, or only the specified " \ - "formulae if are provided." - switch "--multiple", - depends_on: "--versions", - description: "Only show formulae with multiple versions installed." - switch "--pinned", - description: "List only pinned formulae, or only the specified (pinned) " \ - "formulae if are provided. See also `pin`, `unpin`." - # passed through to ls - switch "-1", - description: "Force output to be one entry per line. " \ - "This is the default when output is not to a terminal." - switch "-l", - description: "List formulae and/or casks in long format. " \ - "Has no effect when a formula or cask name is passed as an argument." - switch "-r", - description: "Reverse the order of the formulae and/or casks sort to list the oldest entries " \ - "first. Has no effect when a formula or cask name is passed as an argument." - switch "-t", - description: "Sort formulae and/or casks by time modified, listing most recently modified first. " \ - "Has no effect when a formula or cask name is passed as an argument." + cmd_args do + description <<~EOS + List all installed formulae and casks. + If is provided, summarise the paths within its current keg. + If is provided, list its artifacts. + EOS + switch "--formula", "--formulae", + description: "List only formulae, or treat all named arguments as formulae." + switch "--cask", "--casks", + description: "List only casks, or treat all named arguments as casks." + switch "--full-name", + description: "Print formulae with fully-qualified names. Unless `--full-name`, `--versions` " \ + "or `--pinned` are passed, other options (i.e. `-1`, `-l`, `-r` and `-t`) are " \ + "passed to `ls`(1) which produces the actual output." + switch "--versions", + description: "Show the version number for installed formulae, or only the specified " \ + "formulae if are provided." + switch "--multiple", + depends_on: "--versions", + description: "Only show formulae with multiple versions installed." + switch "--pinned", + description: "List only pinned formulae, or only the specified (pinned) " \ + "formulae if are provided. See also `pin`, `unpin`." + # passed through to ls + switch "-1", + description: "Force output to be one entry per line. " \ + "This is the default when output is not to a terminal." + switch "-l", + description: "List formulae and/or casks in long format. " \ + "Has no effect when a formula or cask name is passed as an argument." + switch "-r", + description: "Reverse the order of the formulae and/or casks sort to list the oldest entries first. " \ + "Has no effect when a formula or cask name is passed as an argument." + switch "-t", + description: "Sort formulae and/or casks by time modified, listing most recently modified first. " \ + "Has no effect when a formula or cask name is passed as an argument." - conflicts "--formula", "--cask" - conflicts "--pinned", "--cask" - conflicts "--multiple", "--cask" - conflicts "--pinned", "--multiple" - ["-1", "-l", "-r", "-t"].each do |flag| - conflicts "--versions", flag - conflicts "--pinned", flag - end - ["--versions", "--pinned", "-l", "-r", "-t"].each do |flag| - conflicts "--full-name", flag - end - - named_args [:installed_formula, :installed_cask] + conflicts "--formula", "--cask" + conflicts "--pinned", "--cask" + conflicts "--multiple", "--cask" + conflicts "--pinned", "--multiple" + ["-1", "-l", "-r", "-t"].each do |flag| + conflicts "--versions", flag + conflicts "--pinned", flag end + ["--versions", "--pinned", "-l", "-r", "-t"].each do |flag| + conflicts "--full-name", flag + end + + named_args [:installed_formula, :installed_cask] end sig { override.void } diff --git a/Library/Homebrew/dev-cmd/prof.rb b/Library/Homebrew/dev-cmd/prof.rb index 7a928c0dfc..040662094c 100644 --- a/Library/Homebrew/dev-cmd/prof.rb +++ b/Library/Homebrew/dev-cmd/prof.rb @@ -7,17 +7,14 @@ require "cli/parser" module Homebrew module DevCmd class Prof < AbstractCommand - sig { override.returns(CLI::Parser) } - def raw_args - Homebrew::CLI::Parser.new do - description <<~EOS - Run Homebrew with a Ruby profiler. For example, `brew prof readall`. - EOS - switch "--stackprof", - description: "Use `stackprof` instead of `ruby-prof` (the default)." + cmd_args do + description <<~EOS + Run Homebrew with a Ruby profiler. For example, `brew prof readall`. + EOS + switch "--stackprof", + description: "Use `stackprof` instead of `ruby-prof` (the default)." - named_args :command, min: 1 - end + named_args :command, min: 1 end sig { override.void } From 96fc8bf66be3858808c51502a5f638232be0ec22 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Tue, 5 Mar 2024 11:55:06 -0800 Subject: [PATCH 05/16] bracket access over public_send --- Library/Homebrew/cmd/list.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/cmd/list.rb b/Library/Homebrew/cmd/list.rb index 8f8184ec22..c6f7e6bc09 100644 --- a/Library/Homebrew/cmd/list.rb +++ b/Library/Homebrew/cmd/list.rb @@ -71,7 +71,7 @@ module Homebrew unless args.cask? formula_names = args.no_named? ? Formula.installed : args.named.to_resolved_formulae full_formula_names = formula_names.map(&:full_name).sort(&tap_and_name_comparison) - full_formula_names = Formatter.columns(full_formula_names) unless args.public_send(:"1?") + full_formula_names = Formatter.columns(full_formula_names) unless args[:"1?"] puts full_formula_names if full_formula_names.present? end if args.cask? || (!args.formula? && args.no_named?) @@ -83,7 +83,7 @@ module Homebrew # The cast is because `Keg`` does not define `full_name` full_cask_names = T.cast(cask_names, T::Array[T.any(Formula, Cask::Cask)]) .map(&:full_name).sort(&tap_and_name_comparison) - full_cask_names = Formatter.columns(full_cask_names) unless args.public_send(:"1?") + full_cask_names = Formatter.columns(full_cask_names) unless args[:"1?"] puts full_cask_names if full_cask_names.present? end elsif args[:pinned?] @@ -167,7 +167,7 @@ module Homebrew Cask::List.list_casks( *casks, - one: args.public_send(:"1?"), + one: args[:"1?"], full_name: args.full_name?, versions: args.versions?, ) From 7d34717ccd36c94a44fd014e105152cfad1db034 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Wed, 6 Mar 2024 14:49:13 -0800 Subject: [PATCH 06/16] Search subclasses instead of using registry --- Library/Homebrew/abstract_command.rb | 13 ++++--------- Library/Homebrew/brew.rb | 2 +- Library/Homebrew/command_registry.rb | 19 ------------------- 3 files changed, 5 insertions(+), 29 deletions(-) delete mode 100644 Library/Homebrew/command_registry.rb diff --git a/Library/Homebrew/abstract_command.rb b/Library/Homebrew/abstract_command.rb index c0d4acdaaa..e6932ccc96 100644 --- a/Library/Homebrew/abstract_command.rb +++ b/Library/Homebrew/abstract_command.rb @@ -1,8 +1,6 @@ # typed: strong # frozen_string_literal: true -require "command_registry" - module Homebrew # Subclass this to implement a `brew` command. This is preferred to declaring a named function in the `Homebrew` # module, because: @@ -22,19 +20,16 @@ module Homebrew sig { returns(String) } def command_name = T.must(name).split("::").fetch(-1).downcase + # @return the AbstractCommand subclass associated with the brew CLI command name. + sig { params(name: String).returns(T.nilable(T.class_of(AbstractCommand))) } + def command(name) = subclasses.find { _1.command_name == name } + private sig { params(block: T.nilable(T.proc.bind(CLI::Parser).void)).void } def cmd_args(&block) @parser_block = T.let(block, T.nilable(T.proc.void)) end - - # registers subclasses for lookup by command name - sig { params(subclass: T.class_of(AbstractCommand)).void } - def inherited(subclass) - super - CommandRegistry.register(subclass) - end end # @note because `Args` makes use `OpenStruct`, subclasses may need to use a tapioca compiler, diff --git a/Library/Homebrew/brew.rb b/Library/Homebrew/brew.rb index 04495f6f6e..2e785889bc 100644 --- a/Library/Homebrew/brew.rb +++ b/Library/Homebrew/brew.rb @@ -84,7 +84,7 @@ begin end if internal_cmd || Commands.external_ruby_v2_cmd_path(cmd) - cmd_class = Homebrew::CommandRegistry.command(T.must(cmd)) + cmd_class = Homebrew::AbstractCommand.command(T.must(cmd)) if cmd_class cmd_class.new.run else diff --git a/Library/Homebrew/command_registry.rb b/Library/Homebrew/command_registry.rb deleted file mode 100644 index 59779f7b58..0000000000 --- a/Library/Homebrew/command_registry.rb +++ /dev/null @@ -1,19 +0,0 @@ -# typed: strong -# frozen_string_literal: true - -module Homebrew - module CommandRegistry - extend T::Helpers - - Cmd = T.type_alias { T.class_of(AbstractCommand) } # rubocop:disable Style/MutableConstant - - sig { params(subclass: Cmd).void } - def self.register(subclass) - @cmds ||= T.let({}, T.nilable(T::Hash[String, Cmd])) - @cmds[subclass.command_name] = subclass - end - - sig { params(name: String).returns(T.nilable(Cmd)) } - def self.command(name) = @cmds&.[](name) - end -end From 6fc99d956958d7e3dfad1d53a33b91e38a56f25e Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Wed, 6 Mar 2024 21:12:17 -0800 Subject: [PATCH 07/16] Add tests --- Library/Homebrew/abstract_command.rb | 6 +- .../Homebrew/test/abstract_command_spec.rb | 56 +++++++++++++++++++ .../Homebrew/test/abstract_command_spec.rbi | 4 ++ 3 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 Library/Homebrew/test/abstract_command_spec.rb create mode 100644 Library/Homebrew/test/abstract_command_spec.rbi diff --git a/Library/Homebrew/abstract_command.rb b/Library/Homebrew/abstract_command.rb index e6932ccc96..3d633058c4 100644 --- a/Library/Homebrew/abstract_command.rb +++ b/Library/Homebrew/abstract_command.rb @@ -37,9 +37,9 @@ module Homebrew sig { returns(CLI::Args) } attr_reader :args - sig { void } - def initialize - @args = T.let(CLI::Parser.new(&self.class.parser_block).parse, CLI::Args) + sig { params(argv: T::Array[String]).void } + def initialize(argv = ARGV.freeze) + @args = T.let(CLI::Parser.new(&self.class.parser_block).parse(argv), CLI::Args) end sig { abstract.void } diff --git a/Library/Homebrew/test/abstract_command_spec.rb b/Library/Homebrew/test/abstract_command_spec.rb new file mode 100644 index 0000000000..0b3b5e05a4 --- /dev/null +++ b/Library/Homebrew/test/abstract_command_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require "abstract_command" + +RSpec.describe Homebrew::AbstractCommand do + describe "subclasses" do + before do + cat = Class.new(described_class) do + cmd_args do + switch "--foo" + flag "--bar=" + end + def run; end + end + stub_const("Cat", cat) + end + + describe "parsing args" do + it "parses valid args" do + expect { Cat.new(["--foo"]).run }.not_to raise_error + end + + it "allows access to args" do + expect(Cat.new(["--bar", "baz"]).args[:bar]).to eq("baz") + end + + it "raises on invalid args" do + expect { Cat.new(["--bat"]) }.to raise_error(OptionParser::InvalidOption) + end + end + + describe "command names" do + it "has a default command name" do + expect(Cat.command_name).to eq("cat") + end + + it "can lookup command" do + expect(described_class.command("cat")).to be(Cat) + end + + describe "when command name is overridden" do + before do + tac = Class.new(described_class) do + def self.command_name = "t-a-c" + def run; end + end + stub_const("Tac", tac) + end + + it "can be looked up by command name" do + expect(described_class.command("t-a-c")).to be(Tac) + end + end + end + end +end diff --git a/Library/Homebrew/test/abstract_command_spec.rbi b/Library/Homebrew/test/abstract_command_spec.rbi new file mode 100644 index 0000000000..f282e9a02f --- /dev/null +++ b/Library/Homebrew/test/abstract_command_spec.rbi @@ -0,0 +1,4 @@ +# typed: strict + +class Cat < Homebrew::AbstractCommand; end +class Tac < Homebrew::AbstractCommand; end From cda276150409679ceda39b4526b1188d5b852fd8 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Fri, 8 Mar 2024 13:35:56 -0800 Subject: [PATCH 08/16] Preseve args_parse test for new commands --- Library/Homebrew/test/cmd/list_spec.rb | 3 +++ .../test/cmd/shared_examples/args_parse.rb | 15 +++++++++------ Library/Homebrew/test/dev-cmd/prof_spec.rb | 3 +++ 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/test/cmd/list_spec.rb b/Library/Homebrew/test/cmd/list_spec.rb index 6a9e34c9e7..b5b4a52902 100644 --- a/Library/Homebrew/test/cmd/list_spec.rb +++ b/Library/Homebrew/test/cmd/list_spec.rb @@ -1,10 +1,13 @@ # frozen_string_literal: true require "cmd/list" +require "cmd/shared_examples/args_parse" RSpec.describe Homebrew::Cmd::List do let(:formulae) { %w[bar foo qux] } + it_behaves_like "parseable arguments" + it "prints all installed Formulae", :integration_test do formulae.each do |f| (HOMEBREW_CELLAR/f/"1.0/somedir").mkpath diff --git a/Library/Homebrew/test/cmd/shared_examples/args_parse.rb b/Library/Homebrew/test/cmd/shared_examples/args_parse.rb index efce6f3e96..b2a89271ae 100644 --- a/Library/Homebrew/test/cmd/shared_examples/args_parse.rb +++ b/Library/Homebrew/test/cmd/shared_examples/args_parse.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.shared_examples "parseable arguments" do +RSpec.shared_examples "parseable arguments" do |argv: []| subject(:method_name) { "#{command_name.tr("-", "_")}_args" } let(:command_name) do |example| @@ -8,10 +8,13 @@ RSpec.shared_examples "parseable arguments" do end it "can parse arguments" do - require "dev-cmd/#{command_name}" unless require? "cmd/#{command_name}" - - parser = Homebrew.public_send(method_name) - - expect(parser).to respond_to(:parse) + if described_class + cmd = described_class.new(argv) + expect(cmd.args).to be_a Homebrew::CLI::Args + else + require "dev-cmd/#{command_name}" unless require? "cmd/#{command_name}" + parser = Homebrew.public_send(method_name) + expect(parser).to respond_to(:parse) + end end end diff --git a/Library/Homebrew/test/dev-cmd/prof_spec.rb b/Library/Homebrew/test/dev-cmd/prof_spec.rb index 44dba51e14..7737daa100 100644 --- a/Library/Homebrew/test/dev-cmd/prof_spec.rb +++ b/Library/Homebrew/test/dev-cmd/prof_spec.rb @@ -1,8 +1,11 @@ # frozen_string_literal: true +require "cmd/shared_examples/args_parse" require "dev-cmd/prof" RSpec.describe Homebrew::DevCmd::Prof do + it_behaves_like "parseable arguments", argv: ["--", "help"] + describe "integration tests", :integration_test, :needs_network do after do FileUtils.rm_rf HOMEBREW_LIBRARY_PATH/"prof" From 7c7444c2a58829498ab490d435a7ef552783d3b1 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Sat, 9 Mar 2024 21:45:10 -0800 Subject: [PATCH 09/16] No longer need to thread args --- Library/Homebrew/cmd/list.rb | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/Library/Homebrew/cmd/list.rb b/Library/Homebrew/cmd/list.rb index c6f7e6bc09..238f63f53a 100644 --- a/Library/Homebrew/cmd/list.rb +++ b/Library/Homebrew/cmd/list.rb @@ -87,10 +87,10 @@ module Homebrew puts full_cask_names if full_cask_names.present? end elsif args[:pinned?] - filtered_list(args:) + filtered_list elsif args[:versions?] - filtered_list(args:) unless args.cask? - list_casks(args:) if args.cask? || (!args.formula? && !args[:multiple?] && args.no_named?) + filtered_list unless args.cask? + list_casks if args.cask? || (!args.formula? && !args[:multiple?] && args.no_named?) elsif args.no_named? ENV["CLICOLOR"] = nil @@ -118,14 +118,14 @@ module Homebrew system_command! "find", args: casks.map(&:caskroom_path) + find_args, print_stdout: true if casks.present? else kegs.each { |keg| PrettyListing.new keg } if kegs.present? - list_casks(args:) if casks.present? + list_casks if casks.present? end end end private - def filtered_list(args:) + def filtered_list names = if args.no_named? Formula.racks else @@ -135,33 +135,35 @@ module Homebrew rack.exist? end end - if args.pinned? + if args[:pinned?] pinned_versions = {} names.sort.each do |d| keg_pin = (HOMEBREW_PINNED_KEGS/d.basename.to_s) pinned_versions[d] = keg_pin.readlink.basename.to_s if keg_pin.exist? || keg_pin.symlink? end pinned_versions.each do |d, version| - puts d.basename.to_s.concat(args.versions? ? " #{version}" : "") + puts d.basename.to_s.concat(args[:versions?] ? " #{version}" : "") end else # --versions without --pinned names.sort.each do |d| versions = d.subdirs.map { |pn| pn.basename.to_s } - next if args.multiple? && versions.length < 2 + next if args[:multiple?] && versions.length < 2 puts "#{d.basename} #{versions * " "}" end end end - def list_casks(args:) + def list_casks casks = if args.no_named? Cask::Caskroom.casks else - args.named.dup.delete_if do |n| + filtered_args = args.named.dup.delete_if do |n| Homebrew.failed = true unless Cask::Caskroom.path.join(n).exist? !Cask::Caskroom.path.join(n).exist? - end.to_formulae_and_casks(only: :cask) + end + # NamedAargs subclasses array + T.cast(filtered_args, Homebrew::CLI::NamedArgs).to_formulae_and_casks(only: :cask) end return if casks.blank? @@ -169,7 +171,7 @@ module Homebrew *casks, one: args[:"1?"], full_name: args.full_name?, - versions: args.versions?, + versions: args[:versions?], ) end end @@ -201,6 +203,8 @@ module Homebrew end end + private + def print_dir(root) dirs = [] remaining_root_files = [] From aad08cd5a483a553df1c785baa37f6f63ccbd706 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Fri, 15 Mar 2024 12:51:03 -0700 Subject: [PATCH 10/16] Revert using hash accessors for args --- Library/Homebrew/abstract_command.rb | 1 + Library/Homebrew/cmd/list.rb | 28 ++++++++++++++-------------- Library/Homebrew/dev-cmd/prof.rb | 2 +- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/Library/Homebrew/abstract_command.rb b/Library/Homebrew/abstract_command.rb index 3d633058c4..359bfb3f66 100644 --- a/Library/Homebrew/abstract_command.rb +++ b/Library/Homebrew/abstract_command.rb @@ -8,6 +8,7 @@ module Homebrew # - Each Command implements a defined interface. # # 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`. class AbstractCommand extend T::Helpers diff --git a/Library/Homebrew/cmd/list.rb b/Library/Homebrew/cmd/list.rb index 238f63f53a..619bc49feb 100644 --- a/Library/Homebrew/cmd/list.rb +++ b/Library/Homebrew/cmd/list.rb @@ -71,7 +71,7 @@ module Homebrew unless args.cask? formula_names = args.no_named? ? Formula.installed : args.named.to_resolved_formulae full_formula_names = formula_names.map(&:full_name).sort(&tap_and_name_comparison) - full_formula_names = Formatter.columns(full_formula_names) unless args[:"1?"] + full_formula_names = Formatter.columns(full_formula_names) unless args.public_send(:"1?") puts full_formula_names if full_formula_names.present? end if args.cask? || (!args.formula? && args.no_named?) @@ -83,22 +83,22 @@ module Homebrew # The cast is because `Keg`` does not define `full_name` full_cask_names = T.cast(cask_names, T::Array[T.any(Formula, Cask::Cask)]) .map(&:full_name).sort(&tap_and_name_comparison) - full_cask_names = Formatter.columns(full_cask_names) unless args[:"1?"] + full_cask_names = Formatter.columns(full_cask_names) unless args.public_send(:"1?") puts full_cask_names if full_cask_names.present? end - elsif args[:pinned?] + elsif args.pinned? filtered_list - elsif args[:versions?] + elsif args.versions? filtered_list unless args.cask? - list_casks if args.cask? || (!args.formula? && !args[:multiple?] && args.no_named?) + list_casks if args.cask? || (!args.formula? && !args.multiple? && args.no_named?) elsif args.no_named? ENV["CLICOLOR"] = nil ls_args = [] - ls_args << "-1" if args[:"1?"] - ls_args << "-l" if args[:l?] - ls_args << "-r" if args[:r?] - ls_args << "-t" if args[:t?] + ls_args << "-1" if args.public_send(:"1?") + ls_args << "-l" if args.l? + ls_args << "-r" if args.r? + ls_args << "-t" if args.t? if !args.cask? && HOMEBREW_CELLAR.exist? && HOMEBREW_CELLAR.children.any? ohai "Formulae" if $stdout.tty? && !args.formula? @@ -135,19 +135,19 @@ module Homebrew rack.exist? end end - if args[:pinned?] + if args.pinned? pinned_versions = {} names.sort.each do |d| keg_pin = (HOMEBREW_PINNED_KEGS/d.basename.to_s) pinned_versions[d] = keg_pin.readlink.basename.to_s if keg_pin.exist? || keg_pin.symlink? end pinned_versions.each do |d, version| - puts d.basename.to_s.concat(args[:versions?] ? " #{version}" : "") + puts d.basename.to_s.concat(args.versions? ? " #{version}" : "") end else # --versions without --pinned names.sort.each do |d| versions = d.subdirs.map { |pn| pn.basename.to_s } - next if args[:multiple?] && versions.length < 2 + next if args.multiple? && versions.length < 2 puts "#{d.basename} #{versions * " "}" end @@ -169,9 +169,9 @@ module Homebrew Cask::List.list_casks( *casks, - one: args[:"1?"], + one: args.public_send(:"1?"), full_name: args.full_name?, - versions: args[:versions?], + versions: args.versions?, ) end end diff --git a/Library/Homebrew/dev-cmd/prof.rb b/Library/Homebrew/dev-cmd/prof.rb index 040662094c..1242b3eace 100644 --- a/Library/Homebrew/dev-cmd/prof.rb +++ b/Library/Homebrew/dev-cmd/prof.rb @@ -39,7 +39,7 @@ module Homebrew Homebrew.setup_gem_environment! - if args[:stackprof?] + if args.stackprof? with_env HOMEBREW_STACKPROF: "1" do system(*HOMEBREW_RUBY_EXEC_ARGS, brew_rb, *args.named) end From 23336aa316a1bd0290e17b4ceb68c675ee83c8f9 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Fri, 15 Mar 2024 12:58:59 -0700 Subject: [PATCH 11/16] Update tests --- Library/Homebrew/abstract_command.rb | 8 ++++---- .../Homebrew/test/sorbet/tapioca/compilers/args_spec.rb | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Library/Homebrew/abstract_command.rb b/Library/Homebrew/abstract_command.rb index 359bfb3f66..62af4697bf 100644 --- a/Library/Homebrew/abstract_command.rb +++ b/Library/Homebrew/abstract_command.rb @@ -15,8 +15,8 @@ module Homebrew abstract! class << self - sig { returns(T.nilable(T.proc.void)) } - attr_reader :parser_block + sig { returns(T.nilable(CLI::Parser)) } + attr_reader :parser sig { returns(String) } def command_name = T.must(name).split("::").fetch(-1).downcase @@ -29,7 +29,7 @@ module Homebrew sig { params(block: T.nilable(T.proc.bind(CLI::Parser).void)).void } def cmd_args(&block) - @parser_block = T.let(block, T.nilable(T.proc.void)) + @parser = T.let(CLI::Parser.new(&block), T.nilable(CLI::Parser)) end end @@ -40,7 +40,7 @@ module Homebrew sig { params(argv: T::Array[String]).void } def initialize(argv = ARGV.freeze) - @args = T.let(CLI::Parser.new(&self.class.parser_block).parse(argv), CLI::Args) + @args = T.let(T.must(self.class.parser).parse(argv), CLI::Args) end sig { abstract.void } diff --git a/Library/Homebrew/test/sorbet/tapioca/compilers/args_spec.rb b/Library/Homebrew/test/sorbet/tapioca/compilers/args_spec.rb index 631a80d2c8..321a32e08d 100644 --- a/Library/Homebrew/test/sorbet/tapioca/compilers/args_spec.rb +++ b/Library/Homebrew/test/sorbet/tapioca/compilers/args_spec.rb @@ -8,7 +8,7 @@ RSpec.describe Tapioca::Compilers::Args do let(:compiler) { described_class.new(Tapioca::Dsl::Pipeline.new(requested_constants: []), RBI::Tree.new, Homebrew) } let(:list_parser) do require "cmd/list" - Homebrew.list_args + Homebrew::Cmd::List.parser end # good testing candidate, bc it has multiple for each of switch, flag, and comma_array args: let(:update_python_resources_parser) do From dfa01a5a84b84d69c2f053585192e2a1fca014ba Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Fri, 15 Mar 2024 14:24:26 -0700 Subject: [PATCH 12/16] Update args compiler --- .../sorbet/rbi/dsl/homebrew/cli/args.rbi | 18 ------ .../sorbet/rbi/dsl/homebrew/cmd/list.rbi | 64 +++++++++++++++++++ .../sorbet/rbi/dsl/homebrew/dev_cmd/prof.rbi | 34 ++++++++++ .../Homebrew/sorbet/tapioca/compilers/args.rb | 40 ++++++++---- 4 files changed, 125 insertions(+), 31 deletions(-) create mode 100644 Library/Homebrew/sorbet/rbi/dsl/homebrew/cmd/list.rbi create mode 100644 Library/Homebrew/sorbet/rbi/dsl/homebrew/dev_cmd/prof.rbi diff --git a/Library/Homebrew/sorbet/rbi/dsl/homebrew/cli/args.rbi b/Library/Homebrew/sorbet/rbi/dsl/homebrew/cli/args.rbi index c88f445543..d31956ed1e 100644 --- a/Library/Homebrew/sorbet/rbi/dsl/homebrew/cli/args.rbi +++ b/Library/Homebrew/sorbet/rbi/dsl/homebrew/cli/args.rbi @@ -401,9 +401,6 @@ class Homebrew::CLI::Args sig { returns(T.nilable(String)) } def keyboard_layoutdir; end - sig { returns(T::Boolean) } - def l?; end - sig { returns(T.nilable(T::Array[String])) } def language; end @@ -461,9 +458,6 @@ class Homebrew::CLI::Args sig { returns(T::Boolean) } def missing?; end - sig { returns(T::Boolean) } - def multiple?; end - sig { returns(T.nilable(String)) } def n; end @@ -578,9 +572,6 @@ class Homebrew::CLI::Args sig { returns(T::Boolean) } def perl?; end - sig { returns(T::Boolean) } - def pinned?; end - sig { returns(T::Boolean) } def plain?; end @@ -752,9 +743,6 @@ class Homebrew::CLI::Args sig { returns(T::Boolean) } def skip_style?; end - sig { returns(T::Boolean) } - def stackprof?; end - sig { returns(T.nilable(String)) } def start_with; end @@ -773,9 +761,6 @@ class Homebrew::CLI::Args sig { returns(T::Boolean) } def syntax?; end - sig { returns(T::Boolean) } - def t?; end - sig { returns(T.nilable(String)) } def tag; end @@ -857,9 +842,6 @@ class Homebrew::CLI::Args sig { returns(T.nilable(String)) } def version_intel; end - sig { returns(T::Boolean) } - def versions?; end - sig { returns(T.nilable(String)) } def vst3_plugindir; end diff --git a/Library/Homebrew/sorbet/rbi/dsl/homebrew/cmd/list.rbi b/Library/Homebrew/sorbet/rbi/dsl/homebrew/cmd/list.rbi new file mode 100644 index 0000000000..d9fc1c041d --- /dev/null +++ b/Library/Homebrew/sorbet/rbi/dsl/homebrew/cmd/list.rbi @@ -0,0 +1,64 @@ +# typed: true + +# DO NOT EDIT MANUALLY +# This is an autogenerated file for dynamic methods in `Homebrew::Cmd::List`. +# Please instead update this file by running `bin/tapioca dsl Homebrew::Cmd::List`. + +class Homebrew::CLI::Args + sig { returns(T::Boolean) } + def cask?; end + + sig { returns(T::Boolean) } + def casks?; end + + sig { returns(T::Boolean) } + def d?; end + + sig { returns(T::Boolean) } + def debug?; end + + sig { returns(T::Boolean) } + def formula?; end + + sig { returns(T::Boolean) } + def formulae?; end + + sig { returns(T::Boolean) } + def full_name?; end + + sig { returns(T::Boolean) } + def h?; end + + sig { returns(T::Boolean) } + def help?; end + + sig { returns(T::Boolean) } + def l?; end + + sig { returns(T::Boolean) } + def multiple?; end + + sig { returns(T::Boolean) } + def pinned?; end + + sig { returns(T::Boolean) } + def q?; end + + sig { returns(T::Boolean) } + def quiet?; end + + sig { returns(T::Boolean) } + def r?; end + + sig { returns(T::Boolean) } + def t?; end + + sig { returns(T::Boolean) } + def v?; end + + sig { returns(T::Boolean) } + def verbose?; end + + sig { returns(T::Boolean) } + def versions?; end +end diff --git a/Library/Homebrew/sorbet/rbi/dsl/homebrew/dev_cmd/prof.rbi b/Library/Homebrew/sorbet/rbi/dsl/homebrew/dev_cmd/prof.rbi new file mode 100644 index 0000000000..9083195ef5 --- /dev/null +++ b/Library/Homebrew/sorbet/rbi/dsl/homebrew/dev_cmd/prof.rbi @@ -0,0 +1,34 @@ +# typed: true + +# DO NOT EDIT MANUALLY +# This is an autogenerated file for dynamic methods in `Homebrew::DevCmd::Prof`. +# Please instead update this file by running `bin/tapioca dsl Homebrew::DevCmd::Prof`. + +class Homebrew::CLI::Args + sig { returns(T::Boolean) } + def d?; end + + sig { returns(T::Boolean) } + def debug?; end + + sig { returns(T::Boolean) } + def h?; end + + sig { returns(T::Boolean) } + def help?; end + + sig { returns(T::Boolean) } + def q?; end + + sig { returns(T::Boolean) } + def quiet?; end + + sig { returns(T::Boolean) } + def stackprof?; end + + sig { returns(T::Boolean) } + def v?; end + + sig { returns(T::Boolean) } + def verbose?; end +end diff --git a/Library/Homebrew/sorbet/tapioca/compilers/args.rb b/Library/Homebrew/sorbet/tapioca/compilers/args.rb index 5a226f1eaa..df47ba41ee 100644 --- a/Library/Homebrew/sorbet/tapioca/compilers/args.rb +++ b/Library/Homebrew/sorbet/tapioca/compilers/args.rb @@ -16,34 +16,34 @@ module Tapioca # FIXME: Enable cop again when https://github.com/sorbet/sorbet/issues/3532 is fixed. # rubocop:disable Style/MutableConstant + Parsable = T.type_alias { T.any(T.class_of(Homebrew::CLI::Args), T.class_of(Homebrew::AbstractCommand)) } ConstantType = type_member { { fixed: T.class_of(Homebrew::CLI::Args) } } # rubocop:enable Style/MutableConstant - sig { override.returns(T::Enumerable[T.class_of(Homebrew::CLI::Args)]) } + sig { override.returns(T::Enumerable[Parsable]) } def self.gather_constants # require all the commands to ensure the _arg methods are defined ["cmd", "dev-cmd"].each do |dir| Dir[File.join(__dir__, "../../../#{dir}", "*.rb")].each { require(_1) } end - [Homebrew::CLI::Args] + [Homebrew::CLI::Args] + Homebrew::AbstractCommand.subclasses end sig { override.void } def decorate - root.create_path(Homebrew::CLI::Args) do |klass| - Homebrew.methods(false).select { _1.end_with?("_args") }.each do |args_method_name| - next if NON_PARSER_ARGS_METHODS.include?(args_method_name) + if constant == Homebrew::CLI::Args + root.create_path(Homebrew::CLI::Args) do |klass| + Homebrew.methods(false).select { _1.end_with?("_args") }.each do |args_method_name| + next if NON_PARSER_ARGS_METHODS.include?(args_method_name) - parser = Homebrew.method(args_method_name).call - comma_array_methods = comma_arrays(parser) - args_table(parser).each do |method_name, value| - # some args are used in multiple commands (this is ok as long as they have the same type) - next if klass.nodes.any? { T.cast(_1, RBI::Method).name.to_sym == method_name } - - return_type = get_return_type(method_name, value, comma_array_methods) - klass.create_method(method_name.to_s, return_type:) + parser = Homebrew.method(args_method_name).call + create_args_methods(klass, parser) end end + else + root.create_path(Homebrew::CLI::Args) do |klass| + create_args_methods(klass, constant.parser) + end end end @@ -69,6 +69,20 @@ module Tapioca "T.nilable(String)" end end + + private + + sig { params(klass: RBI::Scope, parser: Homebrew::CLI::Parser).void } + def create_args_methods(klass, parser) + comma_array_methods = comma_arrays(parser) + args_table(parser).each do |method_name, value| + # some args are used in multiple commands (this is ok as long as they have the same type) + next if klass.nodes.any? { T.cast(_1, RBI::Method).name == method_name } + + return_type = get_return_type(method_name, value, comma_array_methods) + klass.create_method(method_name, return_type:) + end + end end end end From 625206b0bd3687d7aff1273a9a6612acc7fd283f Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Fri, 15 Mar 2024 15:50:07 -0700 Subject: [PATCH 13/16] Avoid duplicating global options --- Library/Homebrew/cli/parser.rb | 4 +++- .../sorbet/rbi/dsl/homebrew/cmd/list.rbi | 24 ------------------- .../sorbet/rbi/dsl/homebrew/dev_cmd/prof.rbi | 24 ------------------- .../Homebrew/sorbet/tapioca/compilers/args.rb | 23 ++++++++++++------ 4 files changed, 19 insertions(+), 56 deletions(-) diff --git a/Library/Homebrew/cli/parser.rb b/Library/Homebrew/cli/parser.rb index b9dcb1a84f..30d0938843 100644 --- a/Library/Homebrew/cli/parser.rb +++ b/Library/Homebrew/cli/parser.rb @@ -248,12 +248,14 @@ module Homebrew @conflicts << options.map { |option| option_to_name(option) } end - def option_to_name(option) + def self.option_to_name(option) option.sub(/\A--?(\[no-\])?/, "") .tr("-", "_") .delete("=") end + def option_to_name(option) = self.class.option_to_name(option) + def name_to_option(name) if name.length == 1 "-#{name}" diff --git a/Library/Homebrew/sorbet/rbi/dsl/homebrew/cmd/list.rbi b/Library/Homebrew/sorbet/rbi/dsl/homebrew/cmd/list.rbi index d9fc1c041d..a5b8ec328e 100644 --- a/Library/Homebrew/sorbet/rbi/dsl/homebrew/cmd/list.rbi +++ b/Library/Homebrew/sorbet/rbi/dsl/homebrew/cmd/list.rbi @@ -11,12 +11,6 @@ class Homebrew::CLI::Args sig { returns(T::Boolean) } def casks?; end - sig { returns(T::Boolean) } - def d?; end - - sig { returns(T::Boolean) } - def debug?; end - sig { returns(T::Boolean) } def formula?; end @@ -26,12 +20,6 @@ class Homebrew::CLI::Args sig { returns(T::Boolean) } def full_name?; end - sig { returns(T::Boolean) } - def h?; end - - sig { returns(T::Boolean) } - def help?; end - sig { returns(T::Boolean) } def l?; end @@ -41,24 +29,12 @@ class Homebrew::CLI::Args sig { returns(T::Boolean) } def pinned?; end - sig { returns(T::Boolean) } - def q?; end - - sig { returns(T::Boolean) } - def quiet?; end - sig { returns(T::Boolean) } def r?; end sig { returns(T::Boolean) } def t?; end - sig { returns(T::Boolean) } - def v?; end - - sig { returns(T::Boolean) } - def verbose?; end - sig { returns(T::Boolean) } def versions?; end end diff --git a/Library/Homebrew/sorbet/rbi/dsl/homebrew/dev_cmd/prof.rbi b/Library/Homebrew/sorbet/rbi/dsl/homebrew/dev_cmd/prof.rbi index 9083195ef5..27f3a84c69 100644 --- a/Library/Homebrew/sorbet/rbi/dsl/homebrew/dev_cmd/prof.rbi +++ b/Library/Homebrew/sorbet/rbi/dsl/homebrew/dev_cmd/prof.rbi @@ -5,30 +5,6 @@ # Please instead update this file by running `bin/tapioca dsl Homebrew::DevCmd::Prof`. class Homebrew::CLI::Args - sig { returns(T::Boolean) } - def d?; end - - sig { returns(T::Boolean) } - def debug?; end - - sig { returns(T::Boolean) } - def h?; end - - sig { returns(T::Boolean) } - def help?; end - - sig { returns(T::Boolean) } - def q?; end - - sig { returns(T::Boolean) } - def quiet?; end - sig { returns(T::Boolean) } def stackprof?; end - - sig { returns(T::Boolean) } - def v?; end - - sig { returns(T::Boolean) } - def verbose?; end end diff --git a/Library/Homebrew/sorbet/tapioca/compilers/args.rb b/Library/Homebrew/sorbet/tapioca/compilers/args.rb index df47ba41ee..8679b4415f 100644 --- a/Library/Homebrew/sorbet/tapioca/compilers/args.rb +++ b/Library/Homebrew/sorbet/tapioca/compilers/args.rb @@ -2,10 +2,17 @@ # frozen_string_literal: true require_relative "../../../global" +require "cli/parser" module Tapioca module Compilers class Args < Tapioca::Dsl::Compiler + GLOBAL_OPTIONS = T.let( + Homebrew::CLI::Parser.global_options.map { _1.slice(0, 2) }.flatten + .map { "#{Homebrew::CLI::Parser.option_to_name(_1)}?" }.freeze, + T::Array[String], + ) + # This is ugly, but we're moving to a new interface that will use a consistent DSL # These are cmd/dev-cmd methods that end in `_args` but are not parsers NON_PARSER_ARGS_METHODS = T.let([ @@ -17,7 +24,7 @@ module Tapioca # FIXME: Enable cop again when https://github.com/sorbet/sorbet/issues/3532 is fixed. # rubocop:disable Style/MutableConstant Parsable = T.type_alias { T.any(T.class_of(Homebrew::CLI::Args), T.class_of(Homebrew::AbstractCommand)) } - ConstantType = type_member { { fixed: T.class_of(Homebrew::CLI::Args) } } + ConstantType = type_member { { fixed: Parsable } } # rubocop:enable Style/MutableConstant sig { override.returns(T::Enumerable[Parsable]) } @@ -37,12 +44,12 @@ module Tapioca next if NON_PARSER_ARGS_METHODS.include?(args_method_name) parser = Homebrew.method(args_method_name).call - create_args_methods(klass, parser) + create_args_methods(klass, parser, include_global: true) end end else root.create_path(Homebrew::CLI::Args) do |klass| - create_args_methods(klass, constant.parser) + create_args_methods(klass, T.must(T.cast(constant, T.class_of(Homebrew::AbstractCommand)).parser)) end end end @@ -72,15 +79,17 @@ module Tapioca private - sig { params(klass: RBI::Scope, parser: Homebrew::CLI::Parser).void } - def create_args_methods(klass, parser) + sig { params(klass: RBI::Scope, parser: Homebrew::CLI::Parser, include_global: T::Boolean).void } + def create_args_methods(klass, parser, include_global: false) comma_array_methods = comma_arrays(parser) args_table(parser).each do |method_name, value| + method_name_str = method_name.to_s + next if GLOBAL_OPTIONS.include?(method_name_str) && !include_global # some args are used in multiple commands (this is ok as long as they have the same type) - next if klass.nodes.any? { T.cast(_1, RBI::Method).name == method_name } + next if klass.nodes.any? { T.cast(_1, RBI::Method).name == method_name_str } return_type = get_return_type(method_name, value, comma_array_methods) - klass.create_method(method_name, return_type:) + klass.create_method(method_name_str, return_type:) end end end From 131a5421a5639d4e6ee7ee8c9c879fadcd5c3cfa Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Fri, 15 Mar 2024 15:56:01 -0700 Subject: [PATCH 14/16] Make cmd_args block non-nilable --- Library/Homebrew/abstract_command.rb | 10 ++++++---- .../test/sorbet/tapioca/compilers/args_spec.rb | 3 +-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/abstract_command.rb b/Library/Homebrew/abstract_command.rb index 62af4697bf..0831c05702 100644 --- a/Library/Homebrew/abstract_command.rb +++ b/Library/Homebrew/abstract_command.rb @@ -6,6 +6,7 @@ module Homebrew # module, because: # - Each Command lives in an isolated namespace. # - Each Command implements a defined interface. + # - `args` is available as an ivar, and thus does not need to be passed as an argument to helper methods. # # 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`. @@ -27,20 +28,21 @@ module Homebrew private - sig { params(block: T.nilable(T.proc.bind(CLI::Parser).void)).void } + sig { params(block: T.proc.bind(CLI::Parser).void).void } def cmd_args(&block) @parser = T.let(CLI::Parser.new(&block), T.nilable(CLI::Parser)) end end - # @note because `Args` makes use `OpenStruct`, subclasses may need to use a tapioca compiler, - # hash accessors, args.rbi, or other means to make this work with legacy commands: sig { returns(CLI::Args) } attr_reader :args sig { params(argv: T::Array[String]).void } def initialize(argv = ARGV.freeze) - @args = T.let(T.must(self.class.parser).parse(argv), CLI::Args) + parser = self.class.parser + raise "Commands must include a `cmd_args` block" if parser.nil? + + @args = T.let(parser.parse(argv), CLI::Args) end sig { abstract.void } diff --git a/Library/Homebrew/test/sorbet/tapioca/compilers/args_spec.rb b/Library/Homebrew/test/sorbet/tapioca/compilers/args_spec.rb index 321a32e08d..7cc5318923 100644 --- a/Library/Homebrew/test/sorbet/tapioca/compilers/args_spec.rb +++ b/Library/Homebrew/test/sorbet/tapioca/compilers/args_spec.rb @@ -1,8 +1,7 @@ # frozen_string_literal: true -# require 'tapioca' require "tapioca/dsl" -require_relative "../../../../sorbet/tapioca/compilers/args" +require "sorbet/tapioca/compilers/args" RSpec.describe Tapioca::Compilers::Args do let(:compiler) { described_class.new(Tapioca::Dsl::Pipeline.new(requested_constants: []), RBI::Tree.new, Homebrew) } From 4b358fc713f8b97a344b23dc67f885b83b4d8fba Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Sat, 16 Mar 2024 08:35:02 -0700 Subject: [PATCH 15/16] Extract global args --- Library/Homebrew/cli/args.rbi | 19 +++++++++++++++ .../sorbet/rbi/dsl/homebrew/cli/args.rbi | 24 ------------------- .../Homebrew/sorbet/tapioca/compilers/args.rb | 8 +++---- 3 files changed, 23 insertions(+), 28 deletions(-) create mode 100644 Library/Homebrew/cli/args.rbi diff --git a/Library/Homebrew/cli/args.rbi b/Library/Homebrew/cli/args.rbi new file mode 100644 index 0000000000..7c161fd39b --- /dev/null +++ b/Library/Homebrew/cli/args.rbi @@ -0,0 +1,19 @@ +# typed: strict + +# This file contains global args as defined in `Homebrew::CLI::Parser.global_options` +# `Command`-specific args are defined in the commands themselves, with type signatures +# generated by the `Tapioca::Compilers::Args` compiler. + +class Homebrew::CLI::Args + sig { returns(T::Boolean) } + def debug?; end + + sig { returns(T::Boolean) } + def help?; end + + sig { returns(T::Boolean) } + def quiet?; end + + sig { returns(T::Boolean) } + def verbose?; end +end diff --git a/Library/Homebrew/sorbet/rbi/dsl/homebrew/cli/args.rbi b/Library/Homebrew/sorbet/rbi/dsl/homebrew/cli/args.rbi index d31956ed1e..06f3fb5b04 100644 --- a/Library/Homebrew/sorbet/rbi/dsl/homebrew/cli/args.rbi +++ b/Library/Homebrew/sorbet/rbi/dsl/homebrew/cli/args.rbi @@ -146,18 +146,12 @@ class Homebrew::CLI::Args sig { returns(T::Boolean) } def custom_remote?; end - sig { returns(T::Boolean) } - def d?; end - sig { returns(T.nilable(String)) } def days; end sig { returns(T::Boolean) } def debian?; end - sig { returns(T::Boolean) } - def debug?; end - sig { returns(T::Boolean) } def debug_symbols?; end @@ -317,12 +311,6 @@ class Homebrew::CLI::Args sig { returns(T.nilable(T::Array[String])) } def groups; end - sig { returns(T::Boolean) } - def h?; end - - sig { returns(T::Boolean) } - def help?; end - sig { returns(T.nilable(T::Array[String])) } def hide; end @@ -623,18 +611,12 @@ class Homebrew::CLI::Args sig { returns(T.nilable(String)) } def python_package_name; end - sig { returns(T::Boolean) } - def q?; end - sig { returns(T.nilable(String)) } def qlplugindir; end sig { returns(T::Boolean) } def quarantine?; end - sig { returns(T::Boolean) } - def quiet?; end - sig { returns(T.nilable(String)) } def r; end @@ -824,15 +806,9 @@ class Homebrew::CLI::Args sig { returns(T.nilable(T::Array[String])) } def user; end - sig { returns(T::Boolean) } - def v?; end - sig { returns(T::Boolean) } def variations?; end - sig { returns(T::Boolean) } - def verbose?; end - sig { returns(T.nilable(String)) } def version; end diff --git a/Library/Homebrew/sorbet/tapioca/compilers/args.rb b/Library/Homebrew/sorbet/tapioca/compilers/args.rb index 8679b4415f..215da283fd 100644 --- a/Library/Homebrew/sorbet/tapioca/compilers/args.rb +++ b/Library/Homebrew/sorbet/tapioca/compilers/args.rb @@ -44,7 +44,7 @@ module Tapioca next if NON_PARSER_ARGS_METHODS.include?(args_method_name) parser = Homebrew.method(args_method_name).call - create_args_methods(klass, parser, include_global: true) + create_args_methods(klass, parser) end end else @@ -79,12 +79,12 @@ module Tapioca private - sig { params(klass: RBI::Scope, parser: Homebrew::CLI::Parser, include_global: T::Boolean).void } - def create_args_methods(klass, parser, include_global: false) + sig { params(klass: RBI::Scope, parser: Homebrew::CLI::Parser).void } + def create_args_methods(klass, parser) comma_array_methods = comma_arrays(parser) args_table(parser).each do |method_name, value| method_name_str = method_name.to_s - next if GLOBAL_OPTIONS.include?(method_name_str) && !include_global + next if GLOBAL_OPTIONS.include?(method_name_str) # some args are used in multiple commands (this is ok as long as they have the same type) next if klass.nodes.any? { T.cast(_1, RBI::Method).name == method_name_str } From 133b9382f0545672540e6b0729370e48769e52c1 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Sun, 17 Mar 2024 09:29:40 -0700 Subject: [PATCH 16/16] Improve readability --- Library/Homebrew/sorbet/tapioca/compilers/args.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/sorbet/tapioca/compilers/args.rb b/Library/Homebrew/sorbet/tapioca/compilers/args.rb index 215da283fd..b79843eb8f 100644 --- a/Library/Homebrew/sorbet/tapioca/compilers/args.rb +++ b/Library/Homebrew/sorbet/tapioca/compilers/args.rb @@ -8,9 +8,9 @@ module Tapioca module Compilers class Args < Tapioca::Dsl::Compiler GLOBAL_OPTIONS = T.let( - Homebrew::CLI::Parser.global_options.map { _1.slice(0, 2) }.flatten - .map { "#{Homebrew::CLI::Parser.option_to_name(_1)}?" }.freeze, - T::Array[String], + Homebrew::CLI::Parser.global_options.map do |short_option, long_option, _| + [short_option, long_option].map { "#{Homebrew::CLI::Parser.option_to_name(_1)}?" } + end.flatten.freeze, T::Array[String] ) # This is ugly, but we're moving to a new interface that will use a consistent DSL