From c57978667261fb3f771a3f53886b99806e97363e Mon Sep 17 00:00:00 2001 From: Aria Li Date: Wed, 3 Apr 2024 19:34:04 -0700 Subject: [PATCH] (PUP-11693) Global OptionParser ignores partially matched invalid params This commit modifies how partial matches are handled when parsing global command line options. Before, Puppet would accept and do nothing with options global options that use '-' instead of '_'. For example, it would accept '--show-diff' and do nothing when the correct name is '--show_diff'. With this change, while handling any global options given in the command line options, if the command line option does not match existing global options, it will convert '-' to '_' and vice versa. Then, check if that matches a valid global option. Additionally, if the command line option needs to convert '-' to '_' or vice versa, Puppet will also warn to let the user know a partial match was detected and that this behavior will be deprecated in Puppet 9. --- lib/puppet/util/command_line/trollop.rb | 16 ++++- .../puppet_option_parser_spec.rb | 72 +++++++++++++++++++ 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/lib/puppet/util/command_line/trollop.rb b/lib/puppet/util/command_line/trollop.rb index 8b0fba147aa..0f7a6778d9d 100644 --- a/lib/puppet/util/command_line/trollop.rb +++ b/lib/puppet/util/command_line/trollop.rb @@ -335,9 +335,21 @@ def parse cmdline = ARGV when /^-([^-])$/ @short[::Regexp.last_match(1)] when /^--no-([^-]\S*)$/ - @long["[no-]#{::Regexp.last_match(1)}"] + possible_match = @long["[no-]#{::Regexp.last_match(1)}"] + if !possible_match + Puppet.warning _("Partial argument match detected: %{arg}. Partial argument matching will be deprecated in Puppet 9.") % { arg: arg } + @long["[no-]#{::Regexp.last_match(1).tr('-', '_')}"] || @long["[no-]#{::Regexp.last_match(1).tr('_', '-')}"] + else + possible_match + end when /^--([^-]\S*)$/ - @long[::Regexp.last_match(1)] || @long["[no-]#{::Regexp.last_match(1)}"] + possible_match = @long[::Regexp.last_match(1)] || @long["[no-]#{::Regexp.last_match(1)}"] + if !possible_match + Puppet.warning _("Partial argument match detected: %{arg}. Partial argument matching will be deprecated in Puppet 9.") % { arg: arg } + @long[::Regexp.last_match(1).tr('-', '_')] || @long[::Regexp.last_match(1).tr('_', '-')] || @long["[no-]#{::Regexp.last_match(1).tr('-', '_')}"] || @long["[no-]#{::Regexp.last_match(1).tr('_', '-')}"] + else + possible_match + end else raise CommandlineError, _("invalid argument syntax: '%{arg}'") % { arg: arg } end diff --git a/spec/unit/util/command_line_utils/puppet_option_parser_spec.rb b/spec/unit/util/command_line_utils/puppet_option_parser_spec.rb index c8365581600..aa64c1dd242 100644 --- a/spec/unit/util/command_line_utils/puppet_option_parser_spec.rb +++ b/spec/unit/util/command_line_utils/puppet_option_parser_spec.rb @@ -13,6 +13,24 @@ ) end + it "parses a 'long' option with a value and converts '-' to '_' & warns" do + parses( + :option => ["--an_gry", "Angry", :REQUIRED], + :from_arguments => ["--an-gry", "foo"], + :expects => "foo" + ) + expect(@logs).to have_matching_log(/Partial argument match detected: --an-gry. Partial argument matching will be deprecated in Puppet 9./) + end + + it "parses a 'long' option with a value and converts '_' to '-' & warns" do + parses( + :option => ["--an-gry", "Angry", :REQUIRED], + :from_arguments => ["--an_gry", "foo"], + :expects => "foo" + ) + expect(@logs).to have_matching_log(/Partial argument match detected: --an_gry. Partial argument matching will be deprecated in Puppet 9./) + end + it "parses a 'short' option with a value" do parses( :option => ["--angry", "-a", "Angry", :REQUIRED], @@ -39,6 +57,24 @@ ) end + it "converts '_' to '-' with a 'long' option & warns" do + parses( + :option => ["--an-gry", "Angry", :NONE], + :from_arguments => ["--an_gry"], + :expects => true + ) + expect(@logs).to have_matching_log(/Partial argument match detected: --an_gry. Partial argument matching will be deprecated in Puppet 9./) + end + + it "converts '-' to '_' with a 'long' option & warns" do + parses( + :option => ["--an_gry", "Angry", :NONE], + :from_arguments => ["--an-gry"], + :expects => true + ) + expect(@logs).to have_matching_log(/Partial argument match detected: --an-gry. Partial argument matching will be deprecated in Puppet 9./) + end + it "parses a 'short' option" do parses( :option => ["--angry", "-a", "Angry", :NONE], @@ -55,6 +91,42 @@ ) end + it "resolves '-' to '_' with '--no-blah' syntax" do + parses( + :option => ["--[no-]an_gry", "Angry", :NONE], + :from_arguments => ["--no-an-gry"], + :expects => false + ) + expect(@logs).to have_matching_log(/Partial argument match detected: --no-an-gry. Partial argument matching will be deprecated in Puppet 9./) + end + + it "resolves '_' to '-' with '--no-blah' syntax" do + parses( + :option => ["--[no-]an-gry", "Angry", :NONE], + :from_arguments => ["--no-an_gry"], + :expects => false + ) + expect(@logs).to have_matching_log(/Partial argument match detected: --no-an_gry. Partial argument matching will be deprecated in Puppet 9./) + end + + it "resolves '-' to '_' & warns when option is defined with '--no-blah syntax' but argument is given in '--option' syntax" do + parses( + :option => ["--[no-]rag-e", "Rage", :NONE], + :from_arguments => ["--rag_e"], + :expects => true + ) + expect(@logs).to have_matching_log(/Partial argument match detected: --rag_e. Partial argument matching will be deprecated in Puppet 9./) + end + + it "resolves '_' to '-' & warns when option is defined with '--no-blah syntax' but argument is given in '--option' syntax" do + parses( + :option => ["--[no-]rag_e", "Rage", :NONE], + :from_arguments => ["--rag-e"], + :expects => true + ) + expect(@logs).to have_matching_log(/Partial argument match detected: --rag-e. Partial argument matching will be deprecated in Puppet 9./) + end + it "overrides a previous argument with a later one" do parses( :option => ["--[no-]rage", "Rage", :NONE],