(#6749) detect duplicate aliases in a single option statement.

This ensures that an option declaration that shadows itself is found, and
reported to the user, rather than silently eating one of the two.

This could have actually lost, for example, the distinction between an
argument-requiring and an argument-missing variant of the same thing.

Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>

2 files changed, 26 insertions, 3 deletions

diff --git a/lib/puppet/string/option.rb b/lib/puppet/string/option.rb
index 26b769c2e..70d62a01f 100644
--- a/lib/puppet/string/option.rb
+++ b/lib/puppet/string/option.rb
@@ -1,5 +1,5 @@
 class Puppet::String::Option
-  attr_reader   :string
+  attr_reader   :parent
   attr_reader   :name
   attr_reader   :aliases
   attr_accessor :desc
@@ -11,17 +11,26 @@ class Puppet::String::Option
     !!@optional_argument
   end
 
-  def initialize(string, *declaration, &block)
-    @string   = string
+  def initialize(parent, *declaration, &block)
+    @parent   = parent
     @optparse = []
 
     # Collect and sort the arguments in the declaration.
+    dups = {}
     declaration.each do |item|
       if item.is_a? String and item.to_s =~ /^-/ then
         unless item =~ /^-[a-z]\b/ or item =~ /^--[^-]/ then
           raise ArgumentError, "#{item.inspect}: long options need two dashes (--)"
         end
         @optparse << item
+
+        # Duplicate checking...
+        name = optparse_to_name(item)
+        if dup = dups[name] then
+          raise ArgumentError, "#{item.inspect}: duplicates existing alias #{dup.inspect} in #{@parent}"
+        else
+          dups[name] = item
+        end
       else
         raise ArgumentError, "#{item.inspect} is not valid for an option argument"
       end
diff --git a/spec/shared_behaviours/things_that_declare_options.rb b/spec/shared_behaviours/things_that_declare_options.rb
index 6abce99e3..1b41c2279 100644
--- a/spec/shared_behaviours/things_that_declare_options.rb
+++ b/spec/shared_behaviours/things_that_declare_options.rb
@@ -53,6 +53,20 @@ shared_examples_for "things that declare options" do
     }.should raise_error ArgumentError, /Option f conflicts with existing option f/
   end
 
+  ["-f", "--foo"].each do |option|
+    ["", " FOO", "=FOO", " [FOO]", "=[FOO]"].each do |argument|
+      input = option + argument
+      it "should detect conflicts within a single option like #{input.inspect}" do
+        expect {
+          add_options_to do
+            option input, input
+          end
+        }.should raise_error ArgumentError, /duplicates existing alias/
+      end
+    end
+  end
+
+
   # Verify the range of interesting conflicts to check for ordering causing
   # the behaviour to change, or anything exciting like that.
   [ %w{--foo}, %w{-f}, %w{-f --foo}, %w{--baz -f},