From 9d30294767eeb7bb29f4577b5adcf98ccda86086 Mon Sep 17 00:00:00 2001 From: Aria Li Date: Tue, 16 Apr 2024 14:34:02 -0700 Subject: [PATCH] (PUP-11693) Update behavior when partial argument match detected This commit updates the Global OptionParser (Trollop) to only warn when the passed in argument partially matches a valid global option. Additionally, the warning is more descriptive now and includes the argument given and the correct global option. --- lib/puppet/util/command_line/trollop.rb | 14 +++++++++---- .../puppet_option_parser_spec.rb | 21 ++++++++++++------- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/lib/puppet/util/command_line/trollop.rb b/lib/puppet/util/command_line/trollop.rb index 0f7a6778d9d..ec5c210a5ae 100644 --- a/lib/puppet/util/command_line/trollop.rb +++ b/lib/puppet/util/command_line/trollop.rb @@ -337,16 +337,22 @@ def parse cmdline = ARGV when /^--no-([^-]\S*)$/ 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('_', '-')}"] + partial_match = @long["[no-]#{::Regexp.last_match(1).tr('-', '_')}"] || @long["[no-]#{::Regexp.last_match(1).tr('_', '-')}"] + if partial_match + Puppet.deprecation_warning _("Partial argument match detected: correct argument is %{partial_match}, got %{arg}. Partial argument matching is deprecated and will be removed in a future release.") % { arg: arg, partial_match: partial_match } + end + partial_match else possible_match end when /^--([^-]\S*)$/ 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('_', '-')}"] + partial_match = @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('_', '-')}"] + if partial_match + Puppet.deprecation_warning _("Partial argument match detected: correct argument is %{partial_match}, got %{arg}. Partial argument matching is deprecated and will be removed in a future release.") % { arg: arg, partial_match: partial_match } + end + partial_match else possible_match 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 aa64c1dd242..cf25004b673 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 @@ -11,6 +11,7 @@ :from_arguments => ["--angry", "foo"], :expects => "foo" ) + expect(@logs).to be_empty end it "parses a 'long' option with a value and converts '-' to '_' & warns" do @@ -19,7 +20,7 @@ :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./) + expect(@logs).to have_matching_log(/Partial argument match detected: correct argument is --an_gry, got --an-gry. Partial argument matching is deprecated and will be removed in a future release./) end it "parses a 'long' option with a value and converts '_' to '-' & warns" do @@ -28,7 +29,7 @@ :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./) + expect(@logs).to have_matching_log(/Partial argument match detected: correct argument is --an-gry, got --an_gry. Partial argument matching is deprecated and will be removed in a future release./) end it "parses a 'short' option with a value" do @@ -37,6 +38,7 @@ :from_arguments => ["-a", "foo"], :expects => "foo" ) + expect(@logs).to be_empty end it "overrides a previous argument with a later one" do @@ -45,6 +47,7 @@ :from_arguments => ["--later", "tomorrow", "--later", "morgen"], :expects => "morgen" ) + expect(@logs).to be_empty end end @@ -63,7 +66,7 @@ :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./) + expect(@logs).to have_matching_log(/Partial argument match detected: correct argument is --an-gry, got --an_gry. Partial argument matching is deprecated and will be removed in a future release./) end it "converts '-' to '_' with a 'long' option & warns" do @@ -72,7 +75,7 @@ :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./) + expect(@logs).to have_matching_log(/Partial argument match detected: correct argument is --an_gry, got --an-gry. Partial argument matching is deprecated and will be removed in a future release./) end it "parses a 'short' option" do @@ -89,6 +92,7 @@ :from_arguments => ["--no-rage"], :expects => false ) + expect(@logs).to be_empty end it "resolves '-' to '_' with '--no-blah' syntax" do @@ -97,7 +101,7 @@ :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./) + expect(@logs).to have_matching_log(/Partial argument match detected: correct argument is --\[no-\]an_gry, got --no-an-gry. Partial argument matching is deprecated and will be removed in a future release./) end it "resolves '_' to '-' with '--no-blah' syntax" do @@ -106,7 +110,7 @@ :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./) + expect(@logs).to have_matching_log(/Partial argument match detected: correct argument is --\[no-\]an-gry, got --no-an_gry. Partial argument matching is deprecated and will be removed in a future release./) end it "resolves '-' to '_' & warns when option is defined with '--no-blah syntax' but argument is given in '--option' syntax" do @@ -115,7 +119,7 @@ :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./) + expect(@logs).to have_matching_log(/Partial argument match detected: correct argument is --\[no-\]rag-e, got --rag_e. Partial argument matching is deprecated and will be removed in a future release./) end it "resolves '_' to '-' & warns when option is defined with '--no-blah syntax' but argument is given in '--option' syntax" do @@ -124,7 +128,7 @@ :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./) + expect(@logs).to have_matching_log(/Partial argument match detected: correct argument is --\[no-\]rag_e, got --rag-e. Partial argument matching is deprecated and will be removed in a future release./) end it "overrides a previous argument with a later one" do @@ -133,6 +137,7 @@ :from_arguments => ["--rage", "--no-rage"], :expects => false ) + expect(@logs).to be_empty end end