Skip to content

Commit

Permalink
(PUP-11693) Update behavior when partial argument match detected
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@AriaXLi
AriaXLi committed Apr 16, 2024
1 parent e9bccae commit 9d30294
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 12 deletions.
14 changes: 10 additions & 4 deletions lib/puppet/util/command_line/trollop.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 13 additions & 8 deletions spec/unit/util/command_line_utils/puppet_option_parser_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -45,6 +47,7 @@
:from_arguments => ["--later", "tomorrow", "--later", "morgen"],
:expects => "morgen"
)
expect(@logs).to be_empty
end
end

Expand All @@ -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
Expand All @@ -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
Expand All @@ -89,6 +92,7 @@
:from_arguments => ["--no-rage"],
:expects => false
)
expect(@logs).to be_empty
end

it "resolves '-' to '_' with '--no-blah' syntax" do
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -133,6 +137,7 @@
:from_arguments => ["--rage", "--no-rage"],
:expects => false
)
expect(@logs).to be_empty
end
end

Expand Down

0 comments on commit 9d30294

Please sign in to comment.