Re: [PATCH/puppet 1/1] Cleanup of plurals and inheritance relationships in AST (fixes [4633])

I was reviewing this when I realized it had been reposted under a
different ticket number #4638.  #4633 has been rejected in favor of
#4638

On Fri, Aug 27, 2010 at 8:27 AM, Paul Berry <pa### @puppetlabs.com>
wrote:
 Found a minor problem with this patch yesterday.  It turns out that
these
 changes make it unnecessary for ASTArray's evaluate method to flatten
out
 arrays, and there's no reasonable need for it from a user perspective.
 I'll
 rework the patch and post a revised version, without the
auto-flattening.
 Paul

 On Thu, Aug 26, 2010 at 11:19 AM, Paul Berry
<pau### @puppetlabs.com> wrote:
>
> 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 arrays in the grammar (which
> contributed confusion to issue 4633).
>
> 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 always flattens out arrays of arrays
to
>  arbitrary depth.  This fixes issue 4633.
>
> Signed-off-by: Paul Berry <pau### @puppetlabs.com>
> ---
>  lib/puppet/parser/ast.rb                   |    1 +
>  lib/puppet/parser/ast/astarray.rb          |   37 +--
>  lib/puppet/parser/ast/caseopt.rb           |   36 +--
>  lib/puppet/parser/ast/resource.rb          |   93 +++----
>  lib/puppet/parser/ast/resource_instance.rb |    9 +
>  lib/puppet/parser/ast/resource_override.rb |    5 +-
>  lib/puppet/parser/grammar.ra               |  131
++------
>  lib/puppet/parser/parser.rb                |  454
> +++++++++++++