Fixing scopes and AST so that definitions and classes are looked for in the scopes, instead of in a global list

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@950 980ebf18-57e1-0310-9a29-db15c13687c0

7 files changed, 196 insertions, 97 deletions

diff --git a/lib/puppet/parser/ast.rb b/lib/puppet/parser/ast.rb
index 5eb4ebaa2..d690ff1a8 100644
--- a/lib/puppet/parser/ast.rb
+++ b/lib/puppet/parser/ast.rb
@@ -15,7 +15,7 @@ module Puppet
                 [:typecheck, true, "Whether to validate types during parsing."],
                 [:paramcheck, true, "Whether to validate parameters during parsing."]
             )
-            attr_accessor :line, :file, :parent
+            attr_accessor :line, :file, :parent, :scope
 
             # Just used for 'tree', which is only used in debugging.
             @@pink = ""
diff --git a/lib/puppet/parser/ast/compdef.rb b/lib/puppet/parser/ast/compdef.rb
index ef099eff8..07eda339e 100644
--- a/lib/puppet/parser/ast/compdef.rb
+++ b/lib/puppet/parser/ast/compdef.rb
@@ -49,13 +49,6 @@ class Puppet::Parser::AST
             super
 
             #Puppet.debug "Defining type %s" % @name.value
-
-            # we need to both mark that a given argument is valid,
-            # and we need to also store any provided default arguments
-            # FIXME This creates a global list of types and their
-            # acceptable arguments.  This should really be scoped
-            # instead.
-            @@settypes[@name.value] = self
         end
 
         def tree(indent = 0)
@@ -70,42 +63,7 @@ class Puppet::Parser::AST
         def to_s
             return "define %s(%s) {\n%s }" % [@name, @args, @code]
         end
-
-        # Check whether a given argument is valid.  Searches up through
-        # any parent classes that might exist.
-        def validarg?(param)
-            found = false
-            if @args.is_a?(AST::ASTArray)
-                found = @args.detect { |arg|
-                    if arg.is_a?(AST::ASTArray)
-                        arg[0].value == param
-                    else
-                        arg.value == param
-                    end
-                }
-            else
-                found = @args.value == param
-                #Puppet.warning "got arg %s" % @args.inspect
-                #hash[@args.value] += 1
-            end
-
-            if found
-                return true
-            # a nil parentclass is an empty astarray
-            # stupid but true
-            elsif @parentclass
-                parent = @@settypes[@parentclass.value]
-                if parent and parent != []
-                    return parent.validarg?(param)
-                else
-                    raise Puppet::Error, "Could not find parent class %s" %
-                        @parentclass.value
-                end
-            else
-                return false
-            end
-
-        end
     end
-
 end
+
+# $Id$
diff --git a/lib/puppet/parser/ast/component.rb b/lib/puppet/parser/ast/component.rb
index f5105c44b..858ef86f6 100644
--- a/lib/puppet/parser/ast/component.rb
+++ b/lib/puppet/parser/ast/component.rb
@@ -13,8 +13,8 @@ class Puppet::Parser::AST
         attr_accessor :name, :args, :code, :scope, :autoname, :keyword
 
         def evaluate(scope,hash,objtype,objname)
-
             scope = scope.newscope
+            @scope = scope
 
             # The type is the component or class name
             scope.type = objtype
@@ -100,6 +100,41 @@ class Puppet::Parser::AST
             # under ours.  This allows them to find our definitions.
             return scope
         end
-    end
 
+        # Check whether a given argument is valid.  Searches up through
+        # any parent classes that might exist.
+        def validarg?(param)
+            found = false
+            unless @args.is_a? Array
+                @args = [@args]
+            end
+
+            found = @args.detect { |arg|
+                if arg.is_a? Array
+                    arg[0] == param
+                else
+                    arg == param
+                end
+            }
+
+            if found
+                # It's a valid arg for us
+                return true
+            elsif @parentclass
+                # Else, check any existing parent
+                parent = @scope.lookuptype(@parentclass)
+                if parent and parent != []
+                    return parent.validarg?(param)
+                else
+                    raise Puppet::Error, "Could not find parent class %s" %
+                        @parentclass
+                end
+            else
+                # Or just return false
+                return false
+            end
+        end
+    end
 end
+
+# $Id$
diff --git a/lib/puppet/parser/ast/objectdef.rb b/lib/puppet/parser/ast/objectdef.rb
index cf1525540..e75b07ccb 100644
--- a/lib/puppet/parser/ast/objectdef.rb
+++ b/lib/puppet/parser/ast/objectdef.rb
@@ -41,6 +41,7 @@ class Puppet::Parser::AST
         # Does not actually return an object; instead sets an object
         # in the current scope.
         def evaluate(scope)
+            @scope = scope
             hash = {}
 
             # Get our type and name.
@@ -52,7 +53,8 @@ class Puppet::Parser::AST
                 self.typecheck(objtype)
             end
 
-            # See if our object type was defined
+            # See if our object type was defined.  If not, we know it's
+            # builtin because we already typechecked.
             begin
                 object = scope.lookuptype(objtype)
             rescue Puppet::ParseError => except
@@ -67,20 +69,6 @@ class Puppet::Parser::AST
                 raise error
             end
 
-            unless object
-                # If not, verify that it's a builtin type
-                object = Puppet::Type.type(objtype)
-
-                # Type.type returns nil on object types that aren't found
-                unless object
-                    error = Puppet::ParseError.new("Invalid type %s" % objtype)
-                    error.line = self.line
-                    error.file = self.file
-                    raise error
-                end
-            end
-
-
             autonamed = false
             # Autogenerate the name if one was not passed.
             if defined? @name
@@ -110,7 +98,7 @@ class Puppet::Parser::AST
             objnames.collect { |objname|
                 # If the object is a class, that means it's a builtin type, so
                 # we just store it in the scope
-                if object.is_a?(Class)
+                unless object
                     begin
                         #Puppet.debug(
                         #    ("Setting object '%s' " +
@@ -184,15 +172,7 @@ class Puppet::Parser::AST
             @checked = false
             super
 
-            if @type.is_a?(Variable)
-                Puppet.debug "Delaying typecheck"
-                return
-            else
-                self.typecheck(@type.value)
-
-                objtype = @type.value
-            end
-
+            self.typecheck(@type.value)
         end
 
         # Verify that all passed parameters are valid
@@ -209,6 +189,9 @@ class Puppet::Parser::AST
                     self.paramdefinedcheck(objtype, param)
                 end
             }
+
+            # Mark that we've made it all the way through.
+            @checked = true
         end
 
         def parambuiltincheck(type, param)
@@ -234,7 +217,8 @@ class Puppet::Parser::AST
         end
 
         def paramdefinedcheck(objtype, param)
-            # FIXME we might need to do more here eventually...
+            # FIXME We might need to do more here eventually.  Metaparams
+            # behave strangely on containers.
             if Puppet::Type.metaparam?(param.param.value.intern)
                 return
             end
@@ -245,7 +229,9 @@ class Puppet::Parser::AST
                 raise Puppet::DevError, detail.to_s
             end
 
-            unless @@settypes[objtype].validarg?(pname)
+            # FIXME This should look through the scope tree, not in a global
+            # hash
+            unless objtype.validarg?(pname)
                 error = Puppet::ParseError.new(
                     "Invalid parameter '%s' for type '%s'" %
                         [pname,objtype]
@@ -306,22 +292,29 @@ class Puppet::Parser::AST
                 # nothing; we've already set builtin to false
             end
 
-            unless builtin or @@settypes.include?(objtype) 
-                error = Puppet::ParseError.new(
-                    "Unknown type '%s'" % objtype
-                )
-                error.line = self.line
-                error.file = self.file
-                raise error
-            end
-
-            #unless builtin
-            #    Puppet.debug "%s is a defined type" % objtype
-            #end
-
-            self.paramcheck(builtin, objtype)
+            typeobj = nil
+            if builtin
+                self.paramcheck(builtin, objtype)
+            else
+                # If there's no set scope, then we're in initialize, not
+                # evaluate, so we can't test defined types.
+                return true unless defined? @scope and @scope
+
+                # Unless we can look up the type, throw an error
+                unless objtype = @scope.lookuptype(objtype)
+                    error = Puppet::ParseError.new(
+                        "Unknown type '%s'" % objtype
+                    )
+                    error.line = self.line
+                    error.file = self.file
+                    raise error
+                end
 
-            @checked = true
+                # Now that we have the type, verify all of the parameters.
+                # Note that we're now passing an AST Class object or whatever
+                # as the type, not a simple string.
+                self.paramcheck(builtin, objtype)
+            end
         end
 
         def to_s
@@ -332,5 +325,4 @@ class Puppet::Parser::AST
             ]
         end
     end
-
 end
diff --git a/test/language/ast.rb b/test/language/ast.rb
index 0dfb0d56c..c4fe67975 100755
--- a/test/language/ast.rb
+++ b/test/language/ast.rb
@@ -413,4 +413,107 @@ class TestAST < Test::Unit::TestCase
             }
         }
     end
+
+    def test_typechecking
+        object = nil
+        children = []
+        type = "deftype"
+        assert_nothing_raised("Could not add AST nodes for calling") {
+            object = AST::ObjectDef.new(
+                :type => nameobj(type),
+                :name => nameobj("yayness"),
+                :params => astarray()
+            )
+        }
+
+        assert_nothing_raised("Typecheck failed") {
+            object.typecheck(type)
+        }
+
+        # Add a scope, which makes it think it's evaluating
+        assert_nothing_raised {
+            scope = Puppet::Parser::Scope.new()
+            object.scope = scope
+        }
+
+        # Verify an error is thrown when it can't find the type
+        assert_raise(Puppet::ParseError) {
+            object.typecheck(type)
+        }
+
+        # Create child class one
+        children << classobj(type)
+        children << object
+
+        top = nil
+        assert_nothing_raised("Could not create top object") {
+            top = AST::ASTArray.new(
+                :children => children
+            )
+        }
+
+        scope = nil
+        assert_nothing_raised("Could not evaluate") {
+            scope = Puppet::Parser::Scope.new()
+            objects = top.evaluate(scope)
+        }
+    end
+
+    def disabled_test_paramcheck
+        object = nil
+        children = []
+        type = "deftype"
+        params = %w{param1 param2}
+
+        comp = compobj(type, {
+            :args => astarray(
+                argobj("param1", "yay"),
+                argobj("param2", "rah")
+            ),
+            :code => AST::ASTArray.new(
+                :children => [
+                    varobj("%svar" % name, "%svalue" % name),
+                    fileobj("/%s" % name)
+                ]
+            )
+        })
+        assert_nothing_raised("Could not add AST nodes for calling") {
+            object = AST::ObjectDef.new(
+                :type => nameobj(type),
+                :name => nameobj("yayness"),
+                :params => astarray(
+                    astarray(stringobj("param1"), stringobj("value1")),
+                    astarray(stringobj("param2"), stringobj("value2"))
+                )
+            )
+        }
+
+        # Add a scope, which makes it think it's evaluating
+        assert_nothing_raised {
+            scope = Puppet::Parser::Scope.new()
+            object.scope = scope
+        }
+
+        # Verify an error is thrown when it can't find the type
+        assert_raise(Puppet::ParseError) {
+            object.paramcheck(false, comp)
+        }
+
+        # Create child class one
+        children << classobj(type)
+        children << object
+
+        top = nil
+        assert_nothing_raised("Could not create top object") {
+            top = AST::ASTArray.new(
+                :children => children
+            )
+        }
+
+        scope = nil
+        assert_nothing_raised("Could not evaluate") {
+            scope = Puppet::Parser::Scope.new()
+            objects = top.evaluate(scope)
+        }
+    end
 end
diff --git a/test/language/scope.rb b/test/language/scope.rb
index 707372d1a..20c07d742 100755
--- a/test/language/scope.rb
+++ b/test/language/scope.rb
@@ -305,7 +305,7 @@ class TestScope < Test::Unit::TestCase
 
     # Verify that two statements about a file within the same scope tree
     # will cause a conflict.
-    def test_znoconflicts
+    def test_noconflicts
         filename = tempfile()
         children = []
 
@@ -411,4 +411,7 @@ class TestScope < Test::Unit::TestCase
             assert_equal("bin", file["owner"], "Value did not override correctly")
         }
     end
+
+    def test_classscopes
+    end
 end
diff --git a/test/puppettest.rb b/test/puppettest.rb
index 4d44332bb..01e7a2f46 100644
--- a/test/puppettest.rb
+++ b/test/puppettest.rb
@@ -695,9 +695,9 @@ module ParserTesting
     include TestPuppet
     AST = Puppet::Parser::AST
 
-    def astarray
+    def astarray(*args)
         AST::ASTArray.new(
-            :children => []
+            :children => args
         )
     end
 
@@ -721,10 +721,10 @@ module ParserTesting
     end
 
     def compobj(name, args = {})
-        args[:file] = tempfile()
-        args[:line] = rand(100)
-        args[:name] = nameobj(name)
-        args[:code] = AST::ASTArray.new(
+        args[:file] ||= tempfile()
+        args[:line] ||= rand(100)
+        args[:name] ||= nameobj(name)
+        args[:code] ||= AST::ASTArray.new(
             :file => tempfile(),
             :line => rand(100),
             :children => [
@@ -817,6 +817,14 @@ module ParserTesting
             )
         }
     end
+
+    def argobj(name, value)
+        assert_nothing_raised("Could not create %s compargument" % name) {
+            return AST::CompArgument.new(
+                :children => [nameobj(name), stringobj(value)]
+            )
+        }
+    end
 end
 
 class PuppetTestSuite