From b7b624c9bfb5c6a28776ffed8f1811fd82e5b022 Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Sat, 23 Jan 2021 15:06:44 -0500 Subject: [PATCH] parser: clarify errors for invalid number of args --- Library/Homebrew/cli/parser.rb | 43 +++++++++++--------- Library/Homebrew/test/cli/parser_spec.rb | 50 ++++++++++++++++++++++-- 2 files changed, 70 insertions(+), 23 deletions(-) diff --git a/Library/Homebrew/cli/parser.rb b/Library/Homebrew/cli/parser.rb index c687ea688b..9b4f41e6e4 100644 --- a/Library/Homebrew/cli/parser.rb +++ b/Library/Homebrew/cli/parser.rb @@ -599,21 +599,16 @@ module Homebrew end def check_named_args(args) + types = Array(@named_args_type).map do |type| + next type if type.is_a? Symbol + + :subcommand + end.compact.uniq + exception = if @min_named_args && args.size < @min_named_args - if @named_args_type.present? - types = Array(@named_args_type) - if types.any? { |arg| arg.is_a? String } - MinNamedArgumentsError.new(@min_named_args) - else - list = types.map { |type| type.to_s.tr("_", " ") } - list = list.to_sentence two_words_connector: " or ", last_word_connector: " or " - UsageError.new("this command requires a #{list} argument") - end - else - MinNamedArgumentsError.new(@min_named_args) - end + MinNamedArgumentsError.new(@min_named_args, types: types, exact: @min_named_args == @max_named_args) elsif @max_named_args && args.size > @max_named_args - MaxNamedArgumentsError.new(@max_named_args) + MaxNamedArgumentsError.new(@max_named_args, types: types) end raise exception if exception @@ -688,13 +683,17 @@ module Homebrew class MaxNamedArgumentsError < UsageError extend T::Sig - sig { params(maximum: Integer).void } - def initialize(maximum) + sig { params(maximum: Integer, types: T::Array[Symbol]).void } + def initialize(maximum, types: []) super case maximum when 0 "This command does not take named arguments." else - "This command does not take more than #{maximum} named #{"argument".pluralize(maximum)}" + types << :named if types.empty? + arg_types = types.map { |type| type.to_s.tr("_", " ") } + .to_sentence two_words_connector: " or ", last_word_connector: " or " + + "This command does not take more than #{maximum} #{arg_types} #{"argument".pluralize(maximum)}." end end end @@ -702,9 +701,15 @@ module Homebrew class MinNamedArgumentsError < UsageError extend T::Sig - sig { params(minimum: Integer).void } - def initialize(minimum) - super "This command requires at least #{minimum} named #{"argument".pluralize(minimum)}." + sig { params(minimum: Integer, types: T::Array[Symbol], exact: T::Boolean).void } + def initialize(minimum, types: [], exact: false) + number_phrase = exact ? "exactly" : "at least" + + types << :named if types.empty? + arg_types = types.map { |type| type.to_s.tr("_", " ") } + .to_sentence two_words_connector: " or ", last_word_connector: " or " + + super "This command requires #{number_phrase} #{minimum} #{arg_types} #{"argument".pluralize(minimum)}." end end end diff --git a/Library/Homebrew/test/cli/parser_spec.rb b/Library/Homebrew/test/cli/parser_spec.rb index 4b1ace9663..45f31c5223 100644 --- a/Library/Homebrew/test/cli/parser_spec.rb +++ b/Library/Homebrew/test/cli/parser_spec.rb @@ -475,18 +475,58 @@ describe Homebrew::CLI::Parser do expect { parser_none.parse([]) }.not_to raise_error end + it "displays the correct error message with no arg types and min" do + parser = described_class.new do + named_args min: 2 + end + expect { parser.parse([]) }.to raise_error( + Homebrew::CLI::MinNamedArgumentsError, /This command requires at least 2 named arguments/ + ) + end + + it "displays the correct error message with no arg types and number" do + parser = described_class.new do + named_args number: 2 + end + expect { parser.parse([]) }.to raise_error( + Homebrew::CLI::MinNamedArgumentsError, /This command requires exactly 2 named arguments/ + ) + end + + it "displays the correct error message with no arg types and max" do + parser = described_class.new do + named_args max: 1 + end + expect { parser.parse(%w[foo bar]) }.to raise_error( + Homebrew::CLI::MaxNamedArgumentsError, /This command does not take more than 1 named argument/ + ) + end + it "displays the correct error message with an array of strings" do parser = described_class.new do - named_args %w[on off], min: 1 + named_args %w[on off], number: 1 end - expect { parser.parse([]) }.to raise_error(Homebrew::CLI::MinNamedArgumentsError) + expect { parser.parse([]) }.to raise_error( + Homebrew::CLI::MinNamedArgumentsError, /This command requires exactly 1 subcommand/ + ) end it "displays the correct error message with an array of symbols" do parser = described_class.new do named_args [:formula, :cask], min: 1 end - expect { parser.parse([]) }.to raise_error(UsageError, /this command requires a formula or cask argument/) + expect { parser.parse([]) }.to raise_error( + Homebrew::CLI::MinNamedArgumentsError, /This command requires at least 1 formula or cask argument/ + ) + end + + it "displays the correct error message with an array of symbols and max" do + parser = described_class.new do + named_args [:formula, :cask], max: 1 + end + expect { parser.parse(%w[foo bar]) }.to raise_error( + Homebrew::CLI::MaxNamedArgumentsError, /This command does not take more than 1 formula or cask argument/ + ) end end @@ -509,7 +549,9 @@ describe Homebrew::CLI::Parser do formula_parser = described_class.new do named :formula end - expect { formula_parser.parse([]) }.to raise_error(UsageError, /this command requires a formula argument/) + expect { formula_parser.parse([]) }.to raise_error( + Homebrew::CLI::MinNamedArgumentsError, /This command requires exactly 1 formula argument/ + ) end it "doesn't allow more than the specified number of arguments" do