(PUP-5937) Detect duplicate keys in hashes

Before this commit, duplicate keys in hashes would go undetected
This commit changes this so that a duplicate key results in a warning:

"The key 'the_key' is declared more than once"

Author: hlindberg

lib/puppet/pops.rb

@@ -77,6 +77,7 @@ module Config
     end
 
     module Evaluator
+      require 'puppet/pops/evaluator/literal_evaluator'
       require 'puppet/pops/evaluator/callable_signature'
       require 'puppet/pops/evaluator/runtime3_converter'
       require 'puppet/pops/evaluator/runtime3_support'

lib/puppet/pops/evaluator/literal_evaluator.rb

@@ -1,5 +1,5 @@
-require 'rgen/ecore/ecore'
-
+module Puppet::Pops
+module Evaluator
 # Literal values for
 # String (not containing interpolation)
 # Numbers
@@ -14,14 +14,13 @@
 # Not considered literal
 # QualifiedReference  # i.e. File, FooBar
 #
-class Puppet::Pops::Evaluator::LiteralEvaluator
-  #include Puppet::Pops::Utils
+class LiteralEvaluator
 
   EMPTY_STRING = ''.freeze
   COMMA_SEPARATOR = ', '.freeze
 
   def initialize
-    @@literal_visitor ||= Puppet::Pops::Visitor.new(self, "literal", 0, 0)
+    @@literal_visitor ||= Visitor.new(self, "literal", 0, 0)
   end
 
   def literal(ast)
@@ -70,7 +69,7 @@ def literal_LiteralRegularExpression(o)
 
   def literal_ConcatenatedString(o)
     # use double quoted string value if there is no interpolation
-    throw :not_literal unless o.segments.size == 1 && o.segments[0].is_a?(Puppet::Pops::Model::LiteralString)
+    throw :not_literal unless o.segments.size == 1 && o.segments[0].is_a?(Model::LiteralString)
     o.segments[0].value
   end
 
@@ -85,3 +84,5 @@ def literal_LiteralHash(o)
     end
   end
 end
+end
+end

lib/puppet/pops/issues.rb

@@ -594,6 +594,10 @@ def self.hard_issue(issue_code, *args, &block)
     "The parameter '#{param_name}' is declared more than once in the parameter list"
   end
 
+  DUPLICATE_KEY = issue :DUPLICATE_KEY, :key do
+    "The key '#{key}' is declared more than once"
+  end
+
   RESERVED_PARAMETER = hard_issue :RESERVED_PARAMETER, :container, :param_name do
     "The parameter $#{param_name} redefines a built in parameter in #{label.the(container)}"
   end

lib/puppet/pops/validation/checker4_0.rb

@@ -11,14 +11,15 @@ module Validation
 #       Model objects via their metamodel. (I.e an extra call to multiplicity check in polymorph check).
 #       This is however mostly valuable when validating model to model transformations, and is therefore T.B.D
 #
-class Checker4_0
+class Checker4_0 < Evaluator::LiteralEvaluator
   attr_reader :acceptor
   attr_reader :migration_checker
 
   # Initializes the validator with a diagnostics producer. This object must respond to
   # `:will_accept?` and `:accept`.
   #
   def initialize(diagnostics_producer)
+    super()
     @@check_visitor       ||= Visitor.new(nil, "check", 0, 0)
     @@rvalue_visitor      ||= Visitor.new(nil, "rvalue", 0, 0)
     @@hostname_visitor    ||= Visitor.new(nil, "hostname", 1, 2)
@@ -467,6 +468,17 @@ def check_LiteralList(o)
     o.values.each {|v| rvalue(v) }
   end
 
+  def check_LiteralHash(o)
+    # the keys of a literal hash may be non-literal expressions. They cannot be checked.
+    unique = Set.new
+    o.entries.each do |entry|
+      catch(:not_literal) do
+        literal_key = literal(entry.key)
+        acceptor.accept(Issues::DUPLICATE_KEY, entry, {:key => literal_key}) if unique.add?(literal_key).nil?
+      end
+    end
+  end
+
   def check_NodeDefinition(o)
     # Check that hostnames are valid hostnames (or regular expressions)
     hostname(o.host_matches, o)

lib/puppet/pops/validation/validator_factory_4_0.rb

@@ -27,6 +27,7 @@ def severity_producer
 
     p[Issues::FUTURE_RESERVED_WORD]          = :deprecation
 
+    p[Issues::DUPLICATE_KEY]                 = :warning
     p[Issues::NAME_WITH_HYPHEN]              = :error
     p[Issues::EMPTY_RESOURCE_SPECIALIZATION] = :ignore
     p

spec/unit/pops/validator/validator_spec.rb

@@ -34,6 +34,13 @@ def validate(model)
     expect(validate(fqn('::_aa').var())).to_not have_issue(Puppet::Pops::Issues::ILLEGAL_VAR_NAME)
   end
 
+  it 'produces a warning for duplicate keyes in a literal hash' do
+    acceptor = validate(parse('{ a => 1, a => 2 }'))
+    expect(acceptor.warning_count).to eql(1)
+    expect(acceptor.error_count).to eql(0)
+    expect(acceptor).to have_issue(Puppet::Pops::Issues::DUPLICATE_KEY)
+  end
+
   context 'for non productive expressions' do
     [ '1',
       '3.14',