diff options
author | Markus Roberts <Markus@reality.com> | 2010-10-21 17:24:09 -0700 |
---|---|---|
committer | Nick Lewis <nick@puppetlabs.com> | 2011-04-12 12:47:31 -0700 |
commit | 31f8e660c8f4c0ec01f140322cf7c585144a0888 (patch) | |
tree | 115f2b3d2b31c005b4b0de81b74d9195fa6ca495 /lib | |
parent | d5dc3036eda210f318160e2442f7b65e0995ee23 (diff) | |
download | puppet-31f8e660c8f4c0ec01f140322cf7c585144a0888.tar.gz puppet-31f8e660c8f4c0ec01f140322cf7c585144a0888.tar.xz puppet-31f8e660c8f4c0ec01f140322cf7c585144a0888.zip |
Refactor en route to #5027 -- remove usestring parameter from lookupvar
The usestring parameter to lookupvar was objectionable for several reasons;
first, it performed a function orthogonal to the main purpose of the method,
second its default was the least common value, and third it was causing other
code to work for reasons that were not obvious (extlookup).
This refactor breaks the value-transforming function out into a seperate
method which allows the user to specify the value to be used in lieu of :undef
and removes the parameter. The function, Scope#undef_as(default,exp) is
written so that it can be used in user code (templates, functions, etc.) if
needed.
This refactor will introduce a user-visible behaviour change in the case where
users were counting on lookupvar to return "" for undefined variables. The
best solution is to have them use undef_as, replacing:
lookupvar('myvar')
with
undef_as('',lookupvar('myvar'))
(with the option to specify another default value if desired). If this is too
objectionable, we could rename the existing lookupvar as raw_lookupvar and
define
def lookupvar(v)
undef_as('',raw_lookupvar(v))
end
to restore the present behaviour.
Diffstat (limited to 'lib')
-rw-r--r-- | lib/puppet/parser/ast/leaf.rb | 2 | ||||
-rw-r--r-- | lib/puppet/parser/functions/extlookup.rb | 4 | ||||
-rw-r--r-- | lib/puppet/parser/resource.rb | 2 | ||||
-rw-r--r-- | lib/puppet/parser/scope.rb | 42 | ||||
-rw-r--r-- | lib/puppet/parser/templatewrapper.rb | 10 |
5 files changed, 25 insertions, 35 deletions
diff --git a/lib/puppet/parser/ast/leaf.rb b/lib/puppet/parser/ast/leaf.rb index 77617e992..b61634d6c 100644 --- a/lib/puppet/parser/ast/leaf.rb +++ b/lib/puppet/parser/ast/leaf.rb @@ -124,7 +124,7 @@ class Puppet::Parser::AST # not include syntactical constructs, like '$' and '{}'). def evaluate(scope) parsewrap do - if (var = scope.lookupvar(@value, false)) == :undefined + if (var = scope.lookupvar(@value)) == :undefined var = :undef end var diff --git a/lib/puppet/parser/functions/extlookup.rb b/lib/puppet/parser/functions/extlookup.rb index b5688d644..5fbf26cec 100644 --- a/lib/puppet/parser/functions/extlookup.rb +++ b/lib/puppet/parser/functions/extlookup.rb @@ -91,9 +91,9 @@ This is for back compatibility to interpolate variables with %. % interpolation raise Puppet::ParseError, ("extlookup(): wrong number of arguments (#{args.length}; must be <= 3)") if args.length > 3 - extlookup_datadir = lookupvar('::extlookup_datadir') + extlookup_datadir = undef_as('',lookupvar('::extlookup_datadir')) - extlookup_precedence = lookupvar('::extlookup_precedence').collect { |var| var.gsub(/%\{(.+?)\}/) { lookupvar("::#{$1}") } } + extlookup_precedence = undef_as([],lookupvar('::extlookup_precedence')).collect { |var| var.gsub(/%\{(.+?)\}/) { lookupvar("::#{$1}") } } datafiles = Array.new diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb index c007d4dbe..ace01bb4b 100644 --- a/lib/puppet/parser/resource.rb +++ b/lib/puppet/parser/resource.rb @@ -241,7 +241,7 @@ class Puppet::Parser::Resource < Puppet::Resource def add_backward_compatible_relationship_param(name) # Skip metaparams for which we get no value. - return unless val = scope.lookupvar(name.to_s, false) and val != :undefined + return unless val = scope.lookupvar(name.to_s) and val != :undefined # The default case: just set the value set_parameter(name, val) and return unless @parameters[name] diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb index 07347552b..df307915e 100644 --- a/lib/puppet/parser/scope.rb +++ b/lib/puppet/parser/scope.rb @@ -210,45 +210,41 @@ class Puppet::Parser::Scope find_definition(name) || find_hostclass(name) end - def lookup_qualified_var(name, usestring) + def undef_as(x,v) + (v == :undefined) ? x : (v == :undef) ? x : v + end + + def lookup_qualified_var(name) parts = name.split(/::/) shortname = parts.pop klassname = parts.join("::") klass = find_hostclass(klassname) unless klass warning "Could not look up qualified variable '#{name}'; class #{klassname} could not be found" - return usestring ? "" : :undefined + return :undefined end unless kscope = class_scope(klass) warning "Could not look up qualified variable '#{name}'; class #{klassname} has not been evaluated" - return usestring ? "" : :undefined + return :undefined end - kscope.lookupvar(shortname, usestring) + kscope.lookupvar(shortname) end private :lookup_qualified_var - # Look up a variable. The simplest value search we do. Default to returning - # an empty string for missing values, but support returning a constant. - def lookupvar(name, usestring = true) + # Look up a variable. The simplest value search we do. + def lookupvar(name) table = ephemeral?(name) ? @ephemeral.last : @symtable # If the variable is qualified, then find the specified scope and look the variable up there instead. if name =~ /::/ - return lookup_qualified_var(name, usestring) - end - # We can't use "if table[name]" here because the value might be false - if ephemeral_include?(name) or table.include?(name) - if usestring and table[name] == :undef - return "" - else - return table[name] - end - elsif self.parent - return parent.lookupvar(name, usestring) - elsif usestring - return "" + lookup_qualified_var(name) + elsif ephemeral_include?(name) or table.include?(name) + # We can't use "if table[name]" here because the value might be false + table[name] + elsif parent + parent.lookupvar(name) else - return :undefined + :undefined end end @@ -333,7 +329,7 @@ class Puppet::Parser::Scope table[name] = value else # append case # lookup the value in the scope if it exists and insert the var - table[name] = lookupvar(name) + table[name] = undef_as('',lookupvar(name)) # concatenate if string, append if array, nothing for other types case value when Array @@ -363,7 +359,7 @@ class Puppet::Parser::Scope if var and var =~ /^[0-9]+$/ and not ephemeral_include?(var) next end - out << lookupvar(var).to_s || "" + out << undef_as('',lookupvar(var)).to_s end elsif ss.scan(/^\\(.)/) # Puppet.debug("Got escape: pos:%d; m:%s" % [ss.pos, ss.matched]) diff --git a/lib/puppet/parser/templatewrapper.rb b/lib/puppet/parser/templatewrapper.rb index 6864aa1a9..73fcb8aac 100644 --- a/lib/puppet/parser/templatewrapper.rb +++ b/lib/puppet/parser/templatewrapper.rb @@ -20,11 +20,7 @@ class Puppet::Parser::TemplateWrapper # Should return true if a variable is defined, false if it is not def has_variable?(name) - if scope.lookupvar(name.to_s, false) != :undefined - true - else - false - end + scope.lookupvar(name.to_s) != :undefined end # Allow templates to access the defined classes @@ -55,9 +51,7 @@ class Puppet::Parser::TemplateWrapper # the missing_method definition here until we declare the syntax finally # dead. def method_missing(name, *args) - # We have to tell lookupvar to return :undefined to us when - # appropriate; otherwise it converts to "". - value = scope.lookupvar(name.to_s, false) + value = scope.lookupvar(name.to_s) if value != :undefined return value else |