Skip to content

Commit

Permalink
Merge pull request #9323 from AriaXLi/update_option_parser
Browse files Browse the repository at this point in the history
(PUP-11693) Update behavior when partial argument match detected
  • Loading branch information
joshcooper authored Apr 16, 2024
2 parents e9bccae + 9d30294 commit 8148824
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
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
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 8148824

Please sign in to comment.