Step towards #5027 -- scopes should know if they are dynamic

The logic for distinguishing dynamic / static scopes was borrowed from Nick &
Paul's patch, the main differences here being 1) calling it "dynamic" (true/
false) rather than "parent_relationship" (:inherited/:dynamic) 2) aligning the
default so that it only needs to get set in one place (the one that will
eventually go away) and 3) setting it on createion rather than with a setter.

Setting it in one place, on creation, also makes it easier to see that anytime
we access a scope it will have the correct setting of Scope#dynamic and that
this does not change.

This commit also contains a minor refactor (removing Type#subscope) that is not
strictly tied to the main purpose but lies in the direction we are needing to
go and it simplified things to do it now.

4 files changed, 16 insertions, 43 deletions

diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb
index 77032992c..f7a0134fc 100644
--- a/lib/puppet/parser/scope.rb
+++ b/lib/puppet/parser/scope.rb
@@ -21,7 +21,7 @@ class Puppet::Parser::Scope
   attr_accessor :source, :resource
   attr_accessor :base, :keyword
   attr_accessor :top, :translated, :compiler
-  attr_accessor :parent
+  attr_accessor :parent, :dynamic
   attr_reader :namespaces
 
   # thin wrapper around an ephemeral
diff --git a/lib/puppet/resource/type.rb b/lib/puppet/resource/type.rb
index 48d8c1f48..f8d820b77 100644
--- a/lib/puppet/resource/type.rb
+++ b/lib/puppet/resource/type.rb
@@ -62,13 +62,11 @@ class Puppet::Resource::Type
 
   # Now evaluate the code associated with this class or definition.
   def evaluate_code(resource)
-    scope = resource.scope
 
-    if tmp = evaluate_parent_type(resource)
-      scope = tmp
-    end
+    static_parent = evaluate_parent_type(resource)
+    scope = static_parent || resource.scope
 
-    scope = subscope(scope, resource) unless resource.title == :main
+    scope = scope.newscope(:namespace => namespace, :source => self, :resource => resource, :dynamic => !static_parent) unless resource.title == :main
     scope.compiler.add_class(name) unless definition?
 
     set_resource_parameters(resource, scope)
@@ -263,11 +261,6 @@ class Puppet::Resource::Type
 
   end
 
-  # Create a new subscope in which to evaluate our code.
-  def subscope(scope, resource)
-    scope.newscope :resource => resource, :namespace => self.namespace, :source => self
-  end
-
   # Check whether a given argument is valid.
   def valid_parameter?(param)
     param = param.to_s
diff --git a/spec/unit/parser/scope_spec.rb b/spec/unit/parser/scope_spec.rb
index 17f485c03..8215535dd 100755
--- a/spec/unit/parser/scope_spec.rb
+++ b/spec/unit/parser/scope_spec.rb
@@ -73,6 +73,14 @@ describe Puppet::Parser::Scope do
 
       Puppet::Parser::Scope.new.singleton_class.ancestors.should be_include(mod)
     end
+
+    it "should remember if it is dynamic" do
+      (!!Puppet::Parser::Scope.new(:dynamic => true).dynamic).should == true
+    end
+
+    it "should assume it is not dynamic" do
+      (!Puppet::Parser::Scope.new.dynamic).should == true
+    end
   end
 
   describe "when looking up a variable" do
diff --git a/spec/unit/resource/type_spec.rb b/spec/unit/resource/type_spec.rb
index b6a5f6982..45c880440 100755
--- a/spec/unit/resource/type_spec.rb
+++ b/spec/unit/resource/type_spec.rb
@@ -237,35 +237,6 @@ describe Puppet::Resource::Type do
     end
   end
 
-  describe "when creating a subscope" do
-    before do
-      @scope = stub 'scope', :newscope => nil
-      @resource = stub 'resource'
-      @type = Puppet::Resource::Type.new(:hostclass, "foo")
-    end
-
-    it "should return a new scope created with the provided scope as the parent" do
-      @scope.expects(:newscope).returns "foo"
-      @type.subscope(@scope, @resource).should == "foo"
-    end
-
-    it "should set the source as itself" do
-      @scope.expects(:newscope).with { |args| args[:source] == @type }
-      @type.subscope(@scope, @resource)
-    end
-
-    it "should set the scope's namespace to its namespace" do
-      @type.expects(:namespace).returns "yayness"
-      @scope.expects(:newscope).with { |args| args[:namespace] == "yayness" }
-      @type.subscope(@scope, @resource)
-    end
-
-    it "should set the scope's resource to the provided resource" do
-      @scope.expects(:newscope).with { |args| args[:resource] == @resource }
-      @type.subscope(@scope, @resource)
-    end
-  end
-
   describe "when setting its parameters in the scope" do
     before do
       @scope = Puppet::Parser::Scope.new(:compiler => stub("compiler", :environment => Puppet::Node::Environment.new), :source => stub("source"))
@@ -465,7 +436,7 @@ describe Puppet::Resource::Type do
 
     it "should set all of its parameters in a subscope" do
       subscope = stub 'subscope', :compiler => @compiler
-      @type.expects(:subscope).with(@scope, @resource).returns subscope
+      @scope.expects(:newscope).with(:source => @type, :dynamic => true, :namespace => 'foo', :resource => @resource).returns subscope
       @type.expects(:set_resource_parameters).with(@resource, subscope)
 
       @type.evaluate_code(@resource)
@@ -493,8 +464,9 @@ describe Puppet::Resource::Type do
     it "should evaluate the AST code if any is provided" do
       code = stub 'code'
       @type.stubs(:code).returns code
-      @type.stubs(:subscope).returns stub_everything("subscope", :compiler => @compiler)
-      code.expects(:safeevaluate).with @type.subscope
+      subscope = stub_everything("subscope", :compiler => @compiler)
+      @scope.stubs(:newscope).returns subscope
+      code.expects(:safeevaluate).with subscope
 
       @type.evaluate_code(@resource)
     end