AST Scope variable names must be strings.

The Parser AST scope had an embedded assumption that variable names were
always strings, but didn't actually enforce that.  A consequence of this is
that a couple of tests violated that agreement.

This adds an internal error report when someone tries the wrong thing, and
cleans up the handful of tests that failed.

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>

Author: Daniel Pittman

lib/puppet/parser/compiler.rb

@@ -436,7 +436,7 @@ def initvars
   # Set the node's parameters into the top-scope as variables.
   def set_node_parameters
     node.parameters.each do |param, value|
-      @topscope[param] = value
+      @topscope[param.to_s] = value
     end
 
     # These might be nil.

lib/puppet/parser/scope.rb

@@ -236,6 +236,10 @@ def undef_as(x,v)
   end
 
   def lookupvar(name, options = {})
+    unless name.is_a? String
+      raise Puppet::DevError, "Scope variable name is a #{name.class}, not a string"
+    end
+
     # Save the originating scope for the request
     options[:origin] = self unless options[:origin]
     table = ephemeral?(name) ? @ephemeral.last : @symtable
@@ -327,6 +331,10 @@ def define_settings(type, params)
   #   It's preferred that you use self[]= instead of this; only use this
   # when you need to set options.
   def setvar(name, value, options = {})
+    unless name.is_a? String
+      raise Puppet::DevError, "Scope variable name is a #{name.class}, not a string"
+    end
+
     table = options[:ephemeral] ? @ephemeral.last : @symtable
     if table.include?(name)
       if options[:append]

spec/unit/parser/ast/vardef_spec.rb

@@ -11,11 +11,11 @@
   describe "when evaluating" do
 
     it "should evaluate arguments" do
-      name = mock 'name'
-      value = mock 'value'
+      name  = Puppet::Parser::AST::String.new :value => 'name'
+      value = Puppet::Parser::AST::String.new :value => 'value'
 
-      name.expects(:safeevaluate).with(@scope)
-      value.expects(:safeevaluate).with(@scope)
+      name.expects(:safeevaluate).with(@scope).returns('name')
+      value.expects(:safeevaluate).with(@scope).returns('value')
 
       vardef = Puppet::Parser::AST::VarDef.new :name => name, :value => value, :file => nil, :line => nil
       vardef.evaluate(@scope)

spec/unit/parser/functions/fqdn_rand_spec.rb

@@ -10,7 +10,7 @@
     node     = Puppet::Node.new('localhost')
     compiler = Puppet::Parser::Compiler.new(node)
     @scope   = Puppet::Parser::Scope.new(compiler)
-    @scope[:fqdn] = "127.0.0.1"
+    @scope["fqdn"] = "127.0.0.1"
   end
 
   it "should exist" do

spec/unit/parser/scope_spec.rb

@@ -129,6 +129,11 @@
       @scope.lookupvar("var").should == "yep"
     end
 
+    it "should fail if invoked with a non-string name" do
+      expect { @scope[:foo] }.to raise_error Puppet::DevError
+      expect { @scope[:foo] = 12 }.to raise_error Puppet::DevError
+    end
+
     it "should return nil for unset variables" do
       @scope["var"].should be_nil
     end