Skip to content

Commit

Permalink
(PUP-11693) Global OptionParser ignores partially matched invalid params
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
AriaXLi committed Apr 12, 2024
1 parent 9b3514e commit 8e15ed3
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 2 deletions.
16 changes: 14 additions & 2 deletions lib/puppet/util/command_line/trollop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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. Partial argument matching will be deprecated in Puppet 9.")
@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. Partial argument matching will be deprecated in Puppet 9.")
@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
Expand Down
70 changes: 70 additions & 0 deletions spec/unit/util/command_line_utils/puppet_option_parser_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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. 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. 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],
Expand All @@ -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. 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. Partial argument matching will be deprecated in Puppet 9./)
end

it "parses a 'short' option" do
parses(
:option => ["--angry", "-a", "Angry", :NONE],
Expand All @@ -55,6 +91,40 @@
)
end

it "resolves '-' to '_' with '--no-blah' syntax" do
parses(
:option => ["--[no-]an_gry", "Angry", :NONE],
:from_arguments => ["--no-an-gry"],
:expects => false
)
end

it "resolves '_' to '-' with '--no-blah' syntax" do
parses(
:option => ["--[no-]an-gry", "Angry", :NONE],
:from_arguments => ["--no-an_gry"],
:expects => false
)
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. 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. 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],
Expand Down

0 comments on commit 8e15ed3

Please sign in to comment.