Removing Resource::Reference classes

This commit is hopefully less messy than it
first appears, but it's certainly cross-cutting.

The reason for all of this is that we previously only
looked up builtin resource types from outside the parser,
but now that the defined resource types are available globally
via environments, we can push that lookup code to Resource.

Once we do that, however, we have to have environment and
namespace information in every resource.

Here I remove the Resource::Reference classes (except
the AST class), and use Resource instances instead. I
did this because the shared code between the two classes
got incredibly complicated, such that they should have had
a hierarchical relationship disallowed by their constants.
This complexity convinced me just to get rid of References
entirely.

I also make Puppet::Parser::Resource a subclass
of Puppet::Resource.

There are still broken tests in test/, but this was a big
enough commit I wanted to get it in.

Signed-off-by: Luke Kanies <luke@reductivelabs.com>

11 files changed, 84 insertions, 296 deletions

diff --git a/lib/puppet/parser/ast/resource.rb b/lib/puppet/parser/ast/resource.rb
index d222893b3..5da40b32f 100644
--- a/lib/puppet/parser/ast/resource.rb
+++ b/lib/puppet/parser/ast/resource.rb
@@ -14,7 +14,7 @@ class Resource < AST::ResourceReference
     # in the current scope.
     def evaluate(scope)
         # Evaluate all of the specified params.
-        paramobjects = @params.collect { |param|
+        paramobjects = params.collect { |param|
             param.safeevaluate(scope)
         }
 
@@ -25,8 +25,6 @@ class Resource < AST::ResourceReference
             resource_titles = [resource_titles]
         end
 
-        resource_type = qualified_type(scope)
-
         # We want virtual to be true if exported is true.  We can't
         # just set :virtual => self.virtual in the initialization,
         # because sometimes the :virtual attribute is set *after*
@@ -39,9 +37,7 @@ class Resource < AST::ResourceReference
         # many times.
         resource_titles.flatten.collect { |resource_title|
             exceptwrap :type => Puppet::ParseError do
-                resource = Puppet::Parser::Resource.new(
-                    :type => resource_type,
-                    :title => resource_title,
+                resource = Puppet::Parser::Resource.new(type, resource_title,
                     :params => paramobjects,
                     :file => self.file,
                     :line => self.line,
diff --git a/lib/puppet/parser/ast/resource_defaults.rb b/lib/puppet/parser/ast/resource_defaults.rb
index 3fde7ade2..f0746ecf1 100644
--- a/lib/puppet/parser/ast/resource_defaults.rb
+++ b/lib/puppet/parser/ast/resource_defaults.rb
@@ -12,7 +12,7 @@ class Puppet::Parser::AST
         # object type.
         def evaluate(scope)
             # Use a resource reference to canonize the type
-            ref = Puppet::Resource::Reference.new(@type, "whatever")
+            ref = Puppet::Resource.new(@type, "whatever")
             type = ref.type
             params = @params.safeevaluate(scope)
 
diff --git a/lib/puppet/parser/ast/resource_override.rb b/lib/puppet/parser/ast/resource_override.rb
index 5eac50982..2d4f7a871 100644
--- a/lib/puppet/parser/ast/resource_override.rb
+++ b/lib/puppet/parser/ast/resource_override.rb
@@ -36,9 +36,7 @@ class Puppet::Parser::AST
             resource = [resource] unless resource.is_a?(Array)
 
             resource = resource.collect do |r|
-                res = Puppet::Parser::Resource.new(
-                    :type => r.type,
-                    :title => r.title,
+                res = Puppet::Parser::Resource.new(r.type, r.title,
                     :params => params,
                     :file => file,
                     :line => line,
diff --git a/lib/puppet/parser/ast/resource_reference.rb b/lib/puppet/parser/ast/resource_reference.rb
index 794e505b8..8e09a8e3b 100644
--- a/lib/puppet/parser/ast/resource_reference.rb
+++ b/lib/puppet/parser/ast/resource_reference.rb
@@ -1,76 +1,23 @@
+require 'puppet/parser/ast'
 require 'puppet/parser/ast/branch'
+require 'puppet/resource'
 
-class Puppet::Parser::AST
-    # A reference to an object.  Only valid as an rvalue.
-    class ResourceReference < AST::Branch
-        attr_accessor :title, :type
-        # Is the type a builtin type?
-        def builtintype?(type)
-            if typeklass = Puppet::Type.type(type)
-                return typeklass
-            else
-                return false
-            end
-        end
-
-        def each
-            [@type,@title].flatten.each { |param|
-                #Puppet.debug("yielding param %s" % param)
-                yield param
-            }
-        end
+class Puppet::Parser::AST::ResourceReference < Puppet::Parser::AST::Branch
+    attr_accessor :title, :type
 
-        # Evaluate our object, but just return a simple array of the type
-        # and name.
-        def evaluate(scope)
-            title = @title.safeevaluate(scope)
-            title = [title] unless title.is_a?(Array)
-
-            if @type.to_s.downcase == "class"
-                resource_type = "class"
-                title = title.collect { |t| qualified_class(scope, t) }
-            else
-                resource_type = qualified_type(scope)
-            end
-
-            title = title.collect { |t| Puppet::Parser::Resource::Reference.new(
-                :type => resource_type, :title => t
-            ) }
-            return title.pop if title.length == 1
-            return title
-        end
-
-        # Look up a fully qualified class name.
-        def qualified_class(scope, title)
-            # Look up the full path to the class
-            if classobj = scope.find_hostclass(title)
-                title = classobj.name
-            else
-                raise Puppet::ParseError, "Could not find class %s" % title
-            end
-        end
-
-        # Look up a fully-qualified type.  This method is
-        # also used in AST::Resource.
-        def qualified_type(scope, title = nil)
-            # We want a lower-case type.  For some reason.
-            objtype = @type.downcase
-            unless builtintype?(objtype)
-                if dtype = scope.find_definition(objtype)
-                    objtype = dtype.name
-                else
-                    raise Puppet::ParseError, "Could not find resource type %s" % objtype
-                end
-            end
-            return objtype
-        end
+    # Evaluate our object, but just return a simple array of the type
+    # and name.
+    def evaluate(scope)
+        titles = Array(title.safeevaluate(scope)).collect { |t| Puppet::Resource.new(type, t, :namespaces => scope.namespaces) }
+        return titles.pop if titles.length == 1
+        return titles
+    end
 
-        def to_s
-            if title.is_a?(ASTArray)
-                "#{type.to_s.capitalize}#{title}"
-            else
-                "#{type.to_s.capitalize}[#{title}]"
-            end
+    def to_s
+        if title.is_a?(Puppet::Parser::AST::ASTArray)
+            "#{type.to_s.capitalize}#{title}"
+        else
+            "#{type.to_s.capitalize}[#{title}]"
         end
     end
 end
diff --git a/lib/puppet/parser/collector.rb b/lib/puppet/parser/collector.rb
index a6763c419..044518793 100644
--- a/lib/puppet/parser/collector.rb
+++ b/lib/puppet/parser/collector.rb
@@ -75,7 +75,7 @@ class Puppet::Parser::Collector
         @collected = {}
 
         # Canonize the type
-        @type = Puppet::Resource::Reference.new(type, "whatever").type
+        @type = Puppet::Resource.new(type, "whatever").type
         @equery = equery
         @vquery = vquery
 
diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb
index c669076bb..8e84f5a5b 100644
--- a/lib/puppet/parser/compiler.rb
+++ b/lib/puppet/parser/compiler.rb
@@ -275,9 +275,9 @@ class Puppet::Parser::Compiler
 
     # Find and evaluate our main object, if possible.
     def evaluate_main
-        @main = known_resource_types.find_hostclass("", "") || known_resource_types.add(Puppet::Resource::Type.new(:hostclass, ""))
+        @main = known_resource_types.find_hostclass([""], "") || known_resource_types.add(Puppet::Resource::Type.new(:hostclass, ""))
         @topscope.source = @main
-        @main_resource = Puppet::Parser::Resource.new(:type => "class", :title => :main, :scope => @topscope, :source => @main)
+        @main_resource = Puppet::Parser::Resource.new("class", :main, :scope => @topscope, :source => @main)
         @topscope.resource = @main_resource
 
         @resources << @main_resource
diff --git a/lib/puppet/parser/functions/defined.rb b/lib/puppet/parser/functions/defined.rb
index 5ad74a79e..4d1d8c6fd 100644
--- a/lib/puppet/parser/functions/defined.rb
+++ b/lib/puppet/parser/functions/defined.rb
@@ -6,21 +6,21 @@ Puppet::Parser::Functions::newfunction(:defined, :type => :rvalue, :doc => "Dete
     (e.g., ``if defined(File['/tmp/myfile']) { ... }``).  This function is unfortunately
     dependent on the parse order of the configuration when testing whether a resource is defined.") do |vals|
         result = false
+        vals = [vals] unless vals.is_a?(Array)
         vals.each do |val|
             case val
             when String
-                # For some reason, it doesn't want me to return from here.
                 if Puppet::Type.type(val) or find_definition(val) or find_hostclass(val)
                     result = true
                     break
                 end
-            when Puppet::Parser::Resource::Reference
+            when Puppet::Resource
                 if findresource(val.to_s)
                     result = true
                     break
                 end
             else
-                raise ArgumentError, "Invalid argument of type %s to 'defined'" % val.class
+                raise ArgumentError, "Invalid argument of type '#{val.class}' to 'defined'"
             end
         end
         result
diff --git a/lib/puppet/parser/functions/require.rb b/lib/puppet/parser/functions/require.rb
index 66d60b9ce..294484a2b 100644
--- a/lib/puppet/parser/functions/require.rb
+++ b/lib/puppet/parser/functions/require.rb
@@ -49,7 +49,7 @@ fail if used with earlier clients.
             # The 'obvious' way is just to add an edge in the catalog,
             # but that is considered a containment edge, not a dependency
             # edge, so it usually gets lost on the client.
-            ref = Puppet::Parser::Resource::Reference.new(:type => :class, :title => klass)
+            ref = Puppet::Resource.new(:class, klass)
             resource.set_parameter(:require, [resource[:require]].flatten.compact << ref)
         end
     end
diff --git a/lib/puppet/parser/parser_support.rb b/lib/puppet/parser/parser_support.rb
index e6bc46b86..d43b0d37d 100644
--- a/lib/puppet/parser/parser_support.rb
+++ b/lib/puppet/parser/parser_support.rb
@@ -388,15 +388,4 @@ class Puppet::Parser::Parser
     def watch_file(filename)
         known_resource_types.watch_file(filename)
     end
-
-    private
-
-    def check_and_add_to_watched_files(filename)
-        unless @files.include?(filename)
-            @files[filename] = Puppet::Util::LoadedFile.new(filename)
-            return true
-        else
-            return false
-        end
-    end
 end
diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb
index 428b9df50..36f1fbe93 100644
--- a/lib/puppet/parser/resource.rb
+++ b/lib/puppet/parser/resource.rb
@@ -1,8 +1,11 @@
-# A resource that we're managing.  This handles making sure that only subclasses
-# can set parameters.
-class Puppet::Parser::Resource
+require 'puppet/resource'
+
+# The primary difference between this class and its
+# parent is that this class has rules on who can set
+# parameters
+class Puppet::Parser::Resource < Puppet::Resource
     require 'puppet/parser/resource/param'
-    require 'puppet/parser/resource/reference'
+    require 'puppet/resource'
     require 'puppet/util/tagging'
     require 'puppet/file_collection/lookup'
     require 'puppet/parser/yaml_trimmer'
@@ -29,15 +32,8 @@ class Puppet::Parser::Resource
         @relationship_names.include?(name)
     end
 
-    # Proxy a few methods to our @ref object.
-    [:builtin?, :type, :title].each do |method|
-        define_method(method) do
-            @ref.send(method)
-        end
-    end
-
     # Set up some boolean test methods
-    [:exported, :translated, :override, :virtual, :evaluated].each do |method|
+    [:translated, :override, :evaluated].each do |method|
         newmeth = (method.to_s + "?").intern
         define_method(newmeth) do
             self.send(method)
@@ -60,25 +56,25 @@ class Puppet::Parser::Resource
         set_parameter(param, value)
     end
 
-    def builtin=(bool)
-        @ref.builtin = bool
-    end
-
     def eachparam
         @params.each do |name, param|
             yield param
         end
     end
 
+    def environment
+        scope.environment
+    end
+
     # Retrieve the associated definition and evaluate it.
     def evaluate
-        if klass = @ref.definedtype
+        if klass = resource_type and ! builtin_type?
             finish()
             return klass.evaluate_code(self)
         elsif builtin?
-            devfail "Cannot evaluate a builtin type"
+            devfail "Cannot evaluate a builtin type (#{type})"
         else
-            self.fail "Cannot find definition %s" % self.type
+            self.fail "Cannot find definition #{type}"
         end
     ensure
         @evaluated = true
@@ -111,61 +107,41 @@ class Puppet::Parser::Resource
         defined?(@finished) and @finished
     end
 
-    def initialize(options)
+    def initialize(type, title, options)
+        self.type = type
+        self.title = title
+
+        @params = {}
+        # Define all of the parameters
+        if params = options[:params]
+            extract_parameters(params)
+            options.delete(:params)
+        end
+
         # Set all of the options we can.
         options.each do |option, value|
             if respond_to?(option.to_s + "=")
                 send(option.to_s + "=", value)
                 options.delete(option)
+            else
+                raise ArgumentError, "Resources do not accept #{option}"
             end
         end
 
         unless self.scope
             raise ArgumentError, "Resources require a scope"
         end
-        @source ||= scope.source
-
-        options = symbolize_options(options)
-
-        # Set up our reference.
-        if type = options[:type] and title = options[:title]
-            options.delete(:type)
-            options.delete(:title)
-        else
-            raise ArgumentError, "Resources require a type and title"
-        end
-
-        @ref = Reference.new(:type => type, :title => title, :scope => self.scope)
-
-        @params = {}
-
-        # Define all of the parameters
-        if params = options[:params]
-            options.delete(:params)
-            params.each do |param|
-                # Don't set the same parameter twice
-                if @params[param.name]
-                    self.fail Puppet::ParseError, "Duplicate parameter '%s' for on %s" %
-                        [param.name, self.to_s]
-                end
-
-                set_parameter(param)
-            end
-        end
 
-        # Throw an exception if we've got any arguments left to set.
-        unless options.empty?
-            raise ArgumentError, "Resources do not accept %s" % options.keys.collect { |k| k.to_s }.join(", ")
-        end
+        @source ||= scope.source
 
-        tag(@ref.type)
-        tag(@ref.title) if valid_tag?(@ref.title.to_s)
+        tag(self.type)
+        tag(self.title) if valid_tag?(self.title.to_s)
     end
 
     # Is this resource modeling an isomorphic resource type?
     def isomorphic?
-        if builtin?
-            return @ref.builtintype.isomorphic?
+        if builtin_type?
+            return resource_type.isomorphic?
         else
             return true
         end
@@ -199,12 +175,8 @@ class Puppet::Parser::Resource
         @name
     end
 
-    # This *significantly* reduces the number of calls to Puppet.[].
-    def paramcheck?
-        unless defined? @@paramcheck
-            @@paramcheck = Puppet[:paramcheck]
-        end
-        @@paramcheck
+    def namespaces
+        scope.namespaces
     end
 
     # A temporary occasion, until I get paths in the scopes figured out.
@@ -212,11 +184,6 @@ class Puppet::Parser::Resource
         to_s
     end
 
-    # Return the short version of our name.
-    def ref
-        @ref.to_s
-    end
-
     # Define a parameter in our resource.
     # if we ever receive a parameter named 'tag', set
     # the resource tags with its value.
@@ -254,16 +221,16 @@ class Puppet::Parser::Resource
         result = Puppet::Resource.new(type, title)
 
         to_hash.each do |p, v|
-            if v.is_a?(Puppet::Parser::Resource::Reference)
-                v = Puppet::Resource::Reference.new(v.type, v.title)
+            if v.is_a?(Puppet::Resource)
+                v = Puppet::Resource.new(v.type, v.title)
             elsif v.is_a?(Array)
                 # flatten resource references arrays
-                if v.flatten.find { |av| av.is_a?(Puppet::Parser::Resource::Reference) }
+                if v.flatten.find { |av| av.is_a?(Puppet::Resource) }
                     v = v.flatten
                 end
                 v = v.collect do |av|
-                    if av.is_a?(Puppet::Parser::Resource::Reference)
-                        av = Puppet::Resource::Reference.new(av.type, av.title)
+                    if av.is_a?(Puppet::Resource)
+                        av = Puppet::Resource.new(av.type, av.title)
                     end
                     av
                 end
@@ -289,10 +256,6 @@ class Puppet::Parser::Resource
         return result
     end
 
-    def to_s
-        self.ref
-    end
-
     # Translate our object to a transportable object.
     def to_trans
         return nil if virtual?
@@ -389,28 +352,26 @@ class Puppet::Parser::Resource
         set_parameter(param)
     end
 
-    # Verify that all passed parameters are valid.  This throws an error if
-    #  there's a problem, so we don't have to worry about the return value.
-    def paramcheck(param)
-        param = param.to_s
-        # Now make sure it's a valid argument to our class.  These checks
-        # are organized in order of commonhood -- most types, it's a valid
-        # argument and paramcheck is enabled.
-        if @ref.typeclass.valid_parameter?(param)
-            true
-        elsif %w{name title}.include?(param) # always allow these
-            true
-        elsif paramcheck?
-            self.fail Puppet::ParseError, "Invalid parameter '%s' for type '%s'" %
-                    [param, @ref.type]
-        end
-    end
-
     # Make sure the resource's parameters are all valid for the type.
     def validate
         @params.each do |name, param|
-            # Make sure it's a valid parameter.
-            paramcheck(name)
+            validate_parameter(name)
+        end
+    rescue => detail
+        fail Puppet::ParseError, detail.to_s
+    end
+
+    private
+
+    def extract_parameters(params)
+        params.each do |param|
+            # Don't set the same parameter twice
+            if @params[param.name]
+                self.fail Puppet::ParseError, "Duplicate parameter '%s' for on %s" %
+                    [param.name, self.to_s]
+            end
+
+            set_parameter(param)
         end
     end
 end
diff --git a/lib/puppet/parser/resource/reference.rb b/lib/puppet/parser/resource/reference.rb
deleted file mode 100644
index ac1c79aef..000000000
--- a/lib/puppet/parser/resource/reference.rb
+++ /dev/null
@@ -1,103 +0,0 @@
-# A reference to a resource.  Mostly just the type and title.
-require 'puppet/resource/reference'
-require 'puppet/file_collection/lookup'
-require 'puppet/parser/yaml_trimmer'
-
-require 'puppet/resource/type_collection_helper'
-
-# A reference to a resource.  Mostly just the type and title.
-class Puppet::Parser::Resource::Reference < Puppet::Resource::Reference
-    include Puppet::Parser::YamlTrimmer
-    include Puppet::FileCollection::Lookup
-    include Puppet::Util::MethodHelper
-    include Puppet::Util::Errors
-    include Puppet::Resource::TypeCollectionHelper
-
-    attr_accessor :builtin, :file, :line, :scope
-
-    # Are we a builtin type?
-    def builtin?
-        unless defined? @builtin
-            if builtintype()
-                @builtin = true
-            else
-                @builtin = false
-            end
-        end
-
-        @builtin
-    end
-
-    def builtintype
-        if t = Puppet::Type.type(self.type.downcase) and t.name != :component
-            t
-        else
-            nil
-        end
-    end
-
-    # Return the defined type for our obj.  This can return classes,
-    # definitions or nodes.
-    def definedtype
-        unless defined? @definedtype
-            case self.type
-            when "Class" # look for host classes
-                name = self.title == :main ? "" : self.title
-                unless tmp = known_resource_types.find_hostclass("", name)
-                    fail Puppet::ParseError, "Could not find '#{title}' class"
-                end
-            when "Node" # look for node definitions
-                unless tmp = known_resource_types.node(self.title)
-                    fail Puppet::ParseError, "Could not find node '%s'" % self.title
-                end
-            else # normal definitions
-                # The resource type is capitalized, so we have to downcase.  Really,
-                # we should have a better interface for finding these, but eh.
-                tmp = known_resource_types.definition(self.type.downcase)
-            end
-
-            if tmp
-                @definedtype = tmp
-            else
-                fail Puppet::ParseError, "Could not find resource type '%s'" % self.type
-            end
-        end
-
-        @definedtype
-    end
-
-    def environment
-        scope.environment
-    end
-
-    def initialize(hash)
-        set_options(hash)
-        requiredopts(:type, :title)
-    end
-
-    def skip_for_yaml
-        %w{@typeclass @definedtype}
-    end
-
-    def to_ref
-        # We have to return different cases to provide backward compatibility
-        # from 0.24.x to 0.23.x.
-        if builtin?
-            return [type.to_s.downcase, title.to_s]
-        else
-            return [type.to_s, title.to_s]
-        end
-    end
-
-    def typeclass
-        unless defined? @typeclass
-            if tmp = builtintype || definedtype
-                @typeclass = tmp
-            else
-                fail Puppet::ParseError, "Could not find type %s" % self.type
-            end
-        end
-
-        @typeclass
-    end
-end