[4638] Cleanup of plurals and inheritance relationships in AST

Changed the grammar so that the following "plural" constructs always
parse as an ASTArray:

- funcvalues
- rvalues
- resourceinstances
- anyparams
- params
- caseopts
- casevalues

And the following "singluar" construct never parses as an ASTArray:

- statement

The previous behavior was for these constructs to parse as a scalar
when they represented a single item and an ASTArray when they
contained zero or multiple items.  ("Statement" could sometimes
represent a single item because a single resource declaration could
represent multiple resources).  This complicated other grammar rules
and caused ambiguous handling of nested arrays.

Also made these changes to the AST class hierarchy:

- ResourceInstance no longer derives from ASTArray.  This relationship
  was not meaningful because a ResourceInstance is a (title,
  parameters) pair, not an array, and it produced complications when
  we wanted to represent an array of ResourceInstance objects.

- Resource no longer derives from ResourceReference.  No significant
  functionality was being inherited and the relationship doesn't make
  sense in an AST context.

- ResourceOverride no longer derives from Resource.  No significant
  functionality was being inherited and the relationship doesn't make
  sense in an AST context.

- Resource can now represent a compound resource instance such as
  "notify { foo: ; bar: }".  This saves the parser from having to
  use represent a statement as an array of objects.

- ASTArray's evaluate method never flattens out arrays of arrays.
This commit is contained in:
Paul Berry 2010-08-25 11:29:23 -07:00
Родитель 16f701edd8
Коммит df088c9ba1
14 изменённых файлов: 363 добавлений и 519 удалений

Просмотреть файл

@ -129,6 +129,7 @@ require 'puppet/parser/ast/not'
require 'puppet/parser/ast/relationship'
require 'puppet/parser/ast/resource'
require 'puppet/parser/ast/resource_defaults'
require 'puppet/parser/ast/resource_instance'
require 'puppet/parser/ast/resource_override'
require 'puppet/parser/ast/resource_reference'
require 'puppet/parser/ast/resourceparam'

Просмотреть файл

@ -21,22 +21,8 @@ class Puppet::Parser::AST
# Evaluate our children.
def evaluate(scope)
# Make a new array, so we don't have to deal with the details of
# flattening and such
items = []
# First clean out any AST::ASTArrays
@children.each { |child|
if child.instance_of?(AST::ASTArray)
child.each do |ac|
items << ac
end
else
items << child
end
}
rets = items.flatten.collect { |child|
result = []
@children.each do |child|
if child.respond_to? :instantiate
if is_a_namespace
# no problem, just don't evaluate it.
@ -48,10 +34,14 @@ class Puppet::Parser::AST
raise error
end
else
child.safeevaluate(scope)
item = child.safeevaluate(scope)
if !item.nil?
# nil values are implicitly removed.
result.push(item)
end
end
}
rets.reject { |o| o.nil? }
end
result
end
def push(*ary)
@ -69,10 +59,4 @@ class Puppet::Parser::AST
"[" + @children.collect { |c| c.to_s }.join(', ') + "]"
end
end
# A simple container class, containing the parameters for an object.
# Used for abstracting the grammar declarations. Basically unnecessary
# except that I kept finding bugs because I had too many arrays that
# meant completely different things.
class ResourceInstance < ASTArray; end
end

Просмотреть файл

@ -18,16 +18,12 @@ class Puppet::Parser::AST
# Cache the @default value.
return @default if defined?(@default)
if @value.is_a?(AST::ASTArray)
@value.each { |subval|
if subval.is_a?(AST::Default)
@default = true
break
end
}
else
@default = true if @value.is_a?(AST::Default)
end
@value.each { |subval|
if subval.is_a?(AST::Default)
@default = true
break
end
}
@default ||= false
@ -36,23 +32,15 @@ class Puppet::Parser::AST
# You can specify a list of values; return each in turn.
def eachvalue(scope)
if @value.is_a?(AST::ASTArray)
@value.each { |subval|
yield subval.safeevaluate(scope)
}
else
yield @value.safeevaluate(scope)
end
@value.each { |subval|
yield subval.safeevaluate(scope)
}
end
def eachopt
if @value.is_a?(AST::ASTArray)
@value.each { |subval|
yield subval
}
else
yield @value
end
@value.each { |subval|
yield subval
}
end
# Evaluate the actual statements; this only gets called if

Просмотреть файл

@ -3,26 +3,15 @@ require 'puppet/parser/ast/resource_reference'
# Any normal puppet resource declaration. Can point to a definition or a
# builtin type.
class Puppet::Parser::AST
class Resource < AST::ResourceReference
class Resource < AST::Branch
associates_doc
attr_accessor :title, :type, :exported, :virtual
attr_reader :parameters
attr_accessor :type, :instances, :exported, :virtual
# Does not actually return an object; instead sets an object
# in the current scope.
def evaluate(scope)
# Evaluate all of the specified params.
paramobjects = parameters.collect { |param|
param.safeevaluate(scope)
}
resource_titles = @title.safeevaluate(scope)
# it's easier to always use an array, even for only one name
resource_titles = [resource_titles] unless resource_titles.is_a?(Array)
# 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*
@ -30,49 +19,49 @@ class Resource < AST::ResourceReference
# is true. Argh, this was a very tough one to track down.
virt = self.virtual || self.exported
# This is where our implicit iteration takes place; if someone
# passed an array as the name, then we act just like the called us
# many times.
fully_qualified_type, resource_titles = scope.resolve_type_and_titles(type, resource_titles)
# First level of implicit iteration: build a resource for each
# instance. This handles things like:
# file { '/foo': owner => blah; '/bar': owner => blah }
@instances.collect { |instance|
resource_titles.flatten.collect { |resource_title|
exceptwrap :type => Puppet::ParseError do
resource = Puppet::Parser::Resource.new(
fully_qualified_type, resource_title,
:parameters => paramobjects,
:file => self.file,
:line => self.line,
:exported => self.exported,
:virtual => virt,
:source => scope.source,
:scope => scope,
:strict => true
)
# Evaluate all of the specified params.
paramobjects = instance.parameters.collect { |param|
param.safeevaluate(scope)
}
# And then store the resource in the compiler.
# At some point, we need to switch all of this to return
# resources instead of storing them like this.
scope.compiler.add_resource(scope, resource)
resource
end
}.reject { |resource| resource.nil? }
end
resource_titles = instance.title.safeevaluate(scope)
# Set the parameters for our object.
def parameters=(params)
if params.is_a?(AST::ASTArray)
@parameters = params
else
# it's easier to always use an array, even for only one name
resource_titles = [resource_titles] unless resource_titles.is_a?(Array)
@parameters = AST::ASTArray.new(
:line => params.line,
:file => params.file,
:children => [params]
)
end
fully_qualified_type, resource_titles = scope.resolve_type_and_titles(type, resource_titles)
# Second level of implicit iteration; build a resource for each
# title. This handles things like:
# file { ['/foo', '/bar']: owner => blah }
resource_titles.flatten.collect { |resource_title|
exceptwrap :type => Puppet::ParseError do
resource = Puppet::Parser::Resource.new(
fully_qualified_type, resource_title,
:parameters => paramobjects,
:file => self.file,
:line => self.line,
:exported => self.exported,
:virtual => virt,
:source => scope.source,
:scope => scope,
:strict => true
)
# And then store the resource in the compiler.
# At some point, we need to switch all of this to return
# resources instead of storing them like this.
scope.compiler.add_resource(scope, resource)
resource
end
}
}.flatten.reject { |resource| resource.nil? }
end
end
end

Просмотреть файл

@ -0,0 +1,9 @@
require 'puppet/parser/ast/branch'
class Puppet::Parser::AST
class ResourceInstance < Branch
# A simple container for a parameter for an object. Consists of a
# title and a set of parameters.
attr_accessor :title, :parameters
end
end

Просмотреть файл

@ -3,12 +3,11 @@ require 'puppet/parser/ast/resource'
class Puppet::Parser::AST
# Set a parameter on a resource specification created somewhere else in the
# configuration. The object is responsible for verifying that this is allowed.
class ResourceOverride < Resource
class ResourceOverride < AST::Branch
associates_doc
attr_accessor :object
attr_reader :parameters
attr_accessor :object, :parameters
# Iterate across all of our children.
def each

Просмотреть файл

@ -35,10 +35,12 @@ program: statements {
| nil
statements: statement {
result = ast AST::ASTArray, :children => [val[0]]
result = ast AST::ASTArray, :children => (val[0] ? [val[0]] : [])
}
| statements statement {
val[0].push(val[1])
if val[1]
val[0].push(val[1])
end
result = val[0]
}
@ -70,19 +72,17 @@ relationship_side: resource | resourceref | collection
edge: IN_EDGE | OUT_EDGE | IN_EDGE_SUB | OUT_EDGE_SUB
fstatement: NAME LPAREN funcvalues RPAREN {
args = aryfy(val[2])
result = ast AST::Function,
:name => val[0][:value],
:line => val[0][:line],
:arguments => args,
:arguments => val[2],
:ftype => :statement
}
| NAME LPAREN funcvalues COMMA RPAREN {
args = aryfy(val[2])
result = ast AST::Function,
:name => val[0][:value],
:line => val[0][:line],
:arguments => args,
:arguments => val[2],
:ftype => :statement
} | NAME LPAREN RPAREN {
result = ast AST::Function,
@ -92,29 +92,22 @@ fstatement: NAME LPAREN funcvalues RPAREN {
:ftype => :statement
}
| NAME funcvalues {
args = aryfy(val[1])
result = ast AST::Function,
:name => val[0][:value],
:line => val[0][:line],
:arguments => args,
:arguments => val[1],
:ftype => :statement
}
funcvalues: namestring
| resourceref
funcvalues: namestring { result = aryfy(val[0]) }
| resourceref { result = aryfy(val[0]) }
| funcvalues COMMA namestring {
result = aryfy(val[0], val[2])
result.line = @lexer.line
result.file = @lexer.file
val[0].push(val[2])
result = val[0]
}
| funcvalues COMMA resourceref {
unless val[0].is_a?(AST::ASTArray)
val[0] = aryfy(val[0])
end
val[0].push(val[2])
result = val[0]
val[0].push(val[2])
result = val[0]
}
# This is *almost* an rvalue, but I couldn't get a full
@ -133,23 +126,7 @@ namestring: name
resource: classname LBRACE resourceinstances endsemi RBRACE {
@lexer.commentpop
array = val[2]
array = [array] if array.instance_of?(AST::ResourceInstance)
result = ast AST::ASTArray
# this iterates across each specified resourceinstance
array.each { |instance|
raise Puppet::Dev, "Got something that isn't an instance" unless instance.instance_of?(AST::ResourceInstance)
# now, i need to somehow differentiate between those things with
# arrays in their names, and normal things
result.push ast(
AST::Resource,
:type => val[0],
:title => instance[0],
:parameters => instance[1])
}
result = ast(AST::Resource, :type => val[0], :instances => val[2])
} | classname LBRACE params endcomma RBRACE {
# This is a deprecated syntax.
error "All resource specifications require names"
@ -178,14 +155,8 @@ virtualresource: at resource {
method = type.to_s + "="
# Just mark our resources as exported and pass them through.
if val[1].instance_of?(AST::ASTArray)
val[1].each do |obj|
obj.send(method, true)
end
else
val[1].send(method, true)
end
# Just mark our resource as exported and pass it through.
val[1].send(method, true)
result = val[1]
}
@ -284,17 +255,13 @@ colllval: variable
| name
resourceinst: resourcename COLON params endcomma {
result = ast AST::ResourceInstance, :children => [val[0],val[2]]
result = ast AST::ResourceInstance, :title => val[0], :parameters => val[2]
}
resourceinstances: resourceinst
resourceinstances: resourceinst { result = aryfy(val[0]) }
| resourceinstances SEMIC resourceinst {
if val[0].instance_of?(AST::ResourceInstance)
result = ast AST::ASTArray, :children => [val[0],val[2]]
else
val[0].push val[2]
result = val[0]
end
}
endsemi: # nothing
@ -339,14 +306,10 @@ params: # nothing
{
result = ast AST::ASTArray
}
| param { result = val[0] }
| param { result = aryfy(val[0]) }
| params COMMA param {
if val[0].instance_of?(AST::ASTArray)
val[0].push(val[2])
result = val[0]
else
result = ast AST::ASTArray, :children => [val[0],val[2]]
end
}
param: NAME FARROW rvalue {
@ -365,24 +328,14 @@ anyparams: # nothing
{
result = ast AST::ASTArray
}
| anyparam { result = val[0] }
| anyparam { result = aryfy(val[0]) }
| anyparams COMMA anyparam {
if val[0].instance_of?(AST::ASTArray)
val[0].push(val[2])
result = val[0]
else
result = ast AST::ASTArray, :children => [val[0],val[2]]
end
}
rvalues: rvalue
| rvalues comma rvalue {
if val[0].instance_of?(AST::ASTArray)
result = val[0].push(val[2])
else
result = ast AST::ASTArray, :children => [val[0],val[2]]
end
}
rvalues: rvalue { result = aryfy(val[0]) }
| rvalues comma rvalue { result = val[0].push(val[2]) }
simplervalue: quotedtext
| name
@ -406,10 +359,9 @@ rvalue: quotedtext
# We currently require arguments in these functions.
funcrvalue: NAME LPAREN funcvalues RPAREN {
args = aryfy(val[2])
result = ast AST::Function,
:name => val[0][:value], :line => val[0][:line],
:arguments => args,
:arguments => val[2],
:ftype => :rvalue
} | NAME LPAREN RPAREN {
result = ast AST::Function,
@ -553,19 +505,13 @@ expression: rvalue
casestatement: CASE rvalue LBRACE caseopts RBRACE {
@lexer.commentpop
options = val[3]
options = ast AST::ASTArray, :children => [val[3]] unless options.instance_of?(AST::ASTArray)
result = ast AST::CaseStatement, :test => val[1], :options => options
result = ast AST::CaseStatement, :test => val[1], :options => val[3]
}
caseopts: caseopt
caseopts: caseopt { result = aryfy(val[0]) }
| caseopts caseopt {
if val[0].instance_of?(AST::ASTArray)
val[0].push val[1]
result = val[0]
else
result = ast AST::ASTArray, :children => [val[0], val[1]]
end
}
caseopt: casevalues COLON LBRACE statements RBRACE {
@ -582,14 +528,10 @@ caseopt: casevalues COLON LBRACE statements RBRACE {
)
}
casevalues: selectlhand
casevalues: selectlhand { result = aryfy(val[0]) }
| casevalues COMMA selectlhand {
if val[0].instance_of?(AST::ASTArray)
val[0].push(val[2])
result = val[0]
else
result = ast AST::ASTArray, :children => [val[0],val[2]]
end
}
selector: selectlhand QMARK svalues {
@ -638,7 +580,7 @@ import: IMPORT strings {
import(file)
end
result = AST::ASTArray.new(:children => [])
result = nil
}
# Disable definition inheritance for now. 8/27/06, luke
@ -764,22 +706,9 @@ variable: VARIABLE {
result = ast AST::Variable, :value => val[0][:value], :line => val[0][:line]
}
array: LBRACK rvalues RBRACK {
if val[1].instance_of?(AST::ASTArray)
result = val[1]
else
result = ast AST::ASTArray, :children => [val[1]]
end
}
| LBRACK rvalues COMMA RBRACK {
if val[1].instance_of?(AST::ASTArray)
result = val[1]
else
result = ast AST::ASTArray, :children => [val[1]]
end
} | LBRACK RBRACK {
result = ast AST::ASTArray
}
array: LBRACK rvalues RBRACK { result = val[1] }
| LBRACK rvalues COMMA RBRACK { result = val[1] }
| LBRACK RBRACK { result = ast AST::ASTArray }
comma: FARROW
| COMMA

Разница между файлами не показана из-за своего большого размера Загрузить разницу

Просмотреть файл

@ -29,18 +29,9 @@ class Puppet::Parser::Parser
message
end
# Create an AST array out of all of the args
def aryfy(*args)
if args[0].instance_of?(AST::ASTArray)
result = args.shift
args.each { |arg|
result.push arg
}
else
result = ast AST::ASTArray, :children => args
end
result
# Create an AST array containing a single element
def aryfy(arg)
ast AST::ASTArray, :children => [arg]
end
# Create an AST object, and automatically add the file and line information if

Просмотреть файл

@ -23,43 +23,26 @@ describe Puppet::Parser::AST::ASTArray do
operator.evaluate(@scope)
end
it "should evaluate childrens of type ASTArray" do
item1 = stub "item1", :is_a? => true
item2 = stub "item2"
item2.stubs(:is_a?).with(Puppet::Parser::AST).returns(true)
item2.stubs(:instance_of?).with(Puppet::Parser::AST::ASTArray).returns(true)
item2.stubs(:each).yields(item1)
item1.expects(:safeevaluate).with(@scope).returns(123)
operator = Puppet::Parser::AST::ASTArray.new :children => [item2]
operator.evaluate(@scope).should == [123]
end
it "should flatten children coming from children ASTArray" do
item1 = stub "item1", :is_a? => true
item2 = stub "item2"
item2.stubs(:is_a?).with(Puppet::Parser::AST).returns(true)
item2.stubs(:instance_of?).with(Puppet::Parser::AST::ASTArray).returns(true)
item2.stubs(:each).yields([item1])
item1.expects(:safeevaluate).with(@scope).returns(123)
operator = Puppet::Parser::AST::ASTArray.new :children => [item2]
operator.evaluate(@scope).should == [123]
it "should not flatten children coming from children ASTArray" do
item = Puppet::Parser::AST::String.new :value => 'foo'
inner_array = Puppet::Parser::AST::ASTArray.new :children => [item, item]
operator = Puppet::Parser::AST::ASTArray.new :children => [inner_array, inner_array]
operator.evaluate(@scope).should == [['foo', 'foo'], ['foo', 'foo']]
end
it "should not flatten the results of children evaluation" do
item1 = stub "item1", :is_a? => true
item2 = stub "item2"
item2.stubs(:is_a?).with(Puppet::Parser::AST).returns(true)
item2.stubs(:instance_of?).with(Puppet::Parser::AST::ASTArray).returns(true)
item2.stubs(:each).yields([item1])
item = Puppet::Parser::AST::String.new :value => 'foo'
item.stubs(:evaluate).returns(['foo'])
operator = Puppet::Parser::AST::ASTArray.new :children => [item, item]
operator.evaluate(@scope).should == [['foo'], ['foo']]
end
item1.expects(:safeevaluate).with(@scope).returns([123])
operator = Puppet::Parser::AST::ASTArray.new :children => [item2]
operator.evaluate(@scope).should == [[123]]
it "should discard nil results from children evaluation" do
item1 = Puppet::Parser::AST::String.new :value => 'foo'
item2 = Puppet::Parser::AST::String.new :value => 'foo'
item2.stubs(:evaluate).returns(nil)
operator = Puppet::Parser::AST::ASTArray.new :children => [item1, item2]
operator.evaluate(@scope).should == ['foo']
end
it "should return a valid string with to_s" do

Просмотреть файл

@ -10,14 +10,15 @@ describe Puppet::Parser::AST::Resource do
@compiler = Puppet::Parser::Compiler.new(Puppet::Node.new("mynode"))
@scope = Puppet::Parser::Scope.new(:compiler => @compiler)
@scope.stubs(:resource).returns(stub_everything)
@resource = ast::Resource.new(:title => @title, :type => "file", :parameters => ast::ASTArray.new(:children => []) )
@instance = ast::ResourceInstance.new(:title => @title, :parameters => ast::ASTArray.new(:children => []))
@resource = ast::Resource.new(:type => "file", :instances => ast::ASTArray.new(:children => [@instance]))
@resource.stubs(:qualified_type).returns("Resource")
end
it "should evaluate all its parameters" do
param = stub 'param'
param.expects(:safeevaluate).with(@scope).returns Puppet::Parser::Resource::Param.new(:name => "myparam", :value => "myvalue", :source => stub("source"))
@resource.stubs(:parameters).returns [param]
@instance.stubs(:parameters).returns [param]
@resource.evaluate(@scope)
end
@ -34,7 +35,7 @@ describe Puppet::Parser::AST::Resource do
array = Puppet::Parser::AST::ASTArray.new(:children => titles)
@resource.title = array
@instance.title = array
result = @resource.evaluate(@scope).collect { |r| r.title }
result.should be_include("one")
result.should be_include("two")
@ -48,12 +49,19 @@ describe Puppet::Parser::AST::Resource do
array = Puppet::Parser::AST::ASTArray.new(:children => titles)
@resource.title = array
@instance.title = array
result = @resource.evaluate(@scope).collect { |r| r.title }
result.should be_include("one")
result.should be_include("two")
end
it "should implicitly iterate over instances" do
new_title = Puppet::Parser::AST::String.new(:value => "other_title")
new_instance = ast::ResourceInstance.new(:title => new_title, :parameters => ast::ASTArray.new(:children => []))
@resource.instances.push(new_instance)
@resource.evaluate(@scope).collect { |r| r.title }.should == ["mytitle", "other_title"]
end
it "should handover resources to the compiler" do
titles = []
%w{one two}.each do |title|
@ -62,7 +70,7 @@ describe Puppet::Parser::AST::Resource do
array = Puppet::Parser::AST::ASTArray.new(:children => titles)
@resource.title = array
@instance.title = array
result = @resource.evaluate(@scope)
result.each do |res|
@ -98,7 +106,10 @@ describe Puppet::Parser::AST::Resource do
def resource(type, params = nil)
params ||= Puppet::Parser::AST::ASTArray.new(:children => [])
Puppet::Parser::AST::Resource.new(:type => type, :title => Puppet::Parser::AST::String.new(:value => "myresource"), :parameters => params)
instance = Puppet::Parser::AST::ResourceInstance.new(
:title => Puppet::Parser::AST::String.new(:value => "myresource"), :parameters => params)
Puppet::Parser::AST::Resource.new(:type => type,
:instances => Puppet::Parser::AST::ASTArray.new(:children => [instance]))
end
it "should be able to generate resources with fully qualified type information" do

Просмотреть файл

@ -143,7 +143,6 @@ describe Puppet::Parser do
end
it "should create an ast::ResourceReference" do
ast::Resource.stubs(:new)
ast::ResourceReference.expects(:new).with { |arg|
arg[:line]==1 and arg[:type]=="File" and arg[:title].is_a?(ast::ASTArray)
}
@ -386,11 +385,11 @@ describe Puppet::Parser do
end
it "should correctly mark exported resources as exported" do
@parser.parse("@@file { '/file': }").code[0][0].exported.should be_true
@parser.parse("@@file { '/file': }").code[0].exported.should be_true
end
it "should correctly mark virtual resources as virtual" do
@parser.parse("@file { '/file': }").code[0][0].virtual.should be_true
@parser.parse("@file { '/file': }").code[0].virtual.should be_true
end
end

Просмотреть файл

@ -477,9 +477,11 @@ file { "/tmp/yayness":
end
assert_instance_of(AST::ASTArray, ret.hostclass("").code)
resdef = ret.hostclass("").code[0][0]
resdef = ret.hostclass("").code[0]
assert_instance_of(AST::Resource, resdef)
assert_equal("/tmp/testing", resdef.title.value)
assert_instance_of(AST::ASTArray, resdef.instances)
assert_equal(1, resdef.instances.children.length)
assert_equal("/tmp/testing", resdef.instances[0].title.value)
# We always get an astarray back, so...
check.call(resdef, "simple resource")

Просмотреть файл

@ -94,16 +94,15 @@ module PuppetTest::ParserTesting
def resourcedef(type, title, params)
title = stringobj(title) unless title.is_a?(AST)
instance = AST::ResourceInstance.new(:title => title, :parameters => resourceparams(params))
assert_nothing_raised("Could not create #{type} #{title}") {
return AST::Resource.new(
:file => __FILE__,
:line => __LINE__,
:title => title,
:type => type,
:parameters => resourceinst(params)
:instances => AST::ASTArray.new(:children => [instance])
)
}
end
@ -122,9 +121,7 @@ module PuppetTest::ParserTesting
:file => __FILE__,
:line => __LINE__,
:object => resourceref(type, title),
:type => type,
:parameters => resourceinst(params)
:parameters => resourceparams(params)
)
}
end
@ -197,13 +194,13 @@ module PuppetTest::ParserTesting
}
end
def resourceinst(hash)
def resourceparams(hash)
assert_nothing_raised("Could not create resource instance") {
params = hash.collect { |param, value|
resourceparam(param, value)
}
return AST::ResourceInstance.new(
return AST::ASTArray.new(
:file => tempfile,