The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com

Feature #1198

alter parser to throw an error on use of an undefined, unquoted, variable.

Added by Mike Pountney over 6 years ago. Updated 3 months ago.

Status:AcceptedStart date:
Priority:NormalDue date:
Assignee:eric sorenson% Done:

0%

Category:language
Target version:3.x
Affected Puppet version:0.25.4 Branch:
Keywords:customer

We've Moved!

Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com

This ticket is now tracked at: https://tickets.puppetlabs.com/browse/PUP-2837


Description

Currently, the parser treats the following cases as equivalent:


file {"/tmp/testfile":
    content => $content
}

file {"/tmp/testfile":
    content => "$content"
}

I think it would be beneficial to throw a compile error in the first case if $content is not defined, eg fail(‘Attempt to use unquoted, undefined variable $content’). This would sensibly catch many cases where I have:

  • typo’d on variable names,
  • misjudged scoping,
  • forgotten to define a variable that I am using in a defined type,
  • broken a facter fact.

The second form provides a means for people to continue with the current logic of ‘all variables are an empty string if undefined’ for such things as ‘if “$myvar”’ statements.

Anyway, please feel free to tell me that this ain’t gonna happen, just wanted to throw it out there…


Related issues

Related to Puppet - Feature #4408: A 'strict' variable mode should be added Duplicate 07/30/2010
Related to Puppet - Feature #2566: Please provide a method to consider empty variables an error Duplicate 08/24/2009
Related to Puppet - Feature #4987: Support for 'strict' mode when evaluating puppet variables Closed 10/12/2010
Related to Puppet - Bug #11911: Variables with {} should always work Duplicate 01/12/2012

History

#1 Updated by Luke Kanies over 6 years ago

I guess I’m okay with this change, but it will need to be scaled in over time. We’d start with a warning, and then maybe in a later release start throwing an exception.

#2 Updated by Redmine Admin over 6 years ago

  • Status changed from 1 to Accepted

#3 Updated by Luke Kanies over 4 years ago

  • Assignee deleted (Luke Kanies)

#4 Updated by Uwe Stuehler over 3 years ago

Mike Pountney wrote:

Currently, the parser treats the following cases as equivalent:

[…]

I think it would be beneficial to throw a compile error in the first case if $content is not defined, eg fail(‘Attempt to use unquoted, undefined variable $content’). This would sensibly catch many cases where I have:

I would intuitively expect a warning in both cases, not just in the unquoted case.

I’ve seen many people decorating variables even whey they are expected to contain a string with gratuitous quotes that would suppress the warning unintentionally. Also, considering resource names and if I understand the original suggestion correctly, this would not even produce a warning when I would like to have it fail:

file { "${vasedir}/subdir/file":
    ...
}

Is this related to if not a duplicated by #4408?

#5 Updated by James Turnbull about 3 years ago

  • Target version deleted (4)

#6 Updated by Rich Rauenzahn almost 3 years ago

Perhaps we could just start with a command line option or runtime declaration/pragma, like Perl (-w, use strict) did to enforce these.

…and I’d rather see “$foo” and $foo treated the same way. If you want to allow undefined, then do some kind of modifier, like shell does, “${foo:default value}”. Since it would be a command line switch, legacy code could still be supported.

defined() should also be expanded to allow…

if !defined($foo) { $foo = “blah” }

#7 Updated by Henrik Lindberg almost 2 years ago

Issue #4408 closed as a duplicate. It adds the idea that a ‘strict’ mode should be added to trigger validation of undefined variables.

#8 Updated by Robin Bowes over 1 year ago

What’s blocking the progress of this 5-year old feature request?

I’ve got to say that 90% of the time I waste debugging unexpected manifest behaviour is down to unset variables, typos in hiera keys and/or hash keys, etc. If puppet would throw an error if a variable was used when not explicitly set then I’d be a much happier bunny.

R.

#9 Updated by Larry Fast over 1 year ago

Just wanted to add my +1 to this issue. I’m currently trying to track down the source of this:

Error: Failed to apply catalog: Parameter require failed on File[C:/apache-flume-1.3.0-bin/]: No title provided and “File[]” is not a valid resource reference

Any chance of at least raising this from Low to Medium?

#10 Updated by eric sorenson over 1 year ago

  • Assignee set to eric sorenson
  • Priority changed from Low to Normal
  • Target version set to 3.x
  • Keywords set to customer

Lots of requests for ‘strict’ mode aggregated here, which should be easier with the Puppet 3.2 expression parser.

#11 Updated by Henrik Lindberg over 1 year ago

With eparser it is certainly possible to validate differently based on settings (i.e. “ignore”, “warning”, “error”) for certain types of potential problems. At the same time it is important that there is interoperability between newer and older code (needs to be determined at module level). There are also semantic issues where missing values have semantic meaning – e.g. a fact is not delivered at all and there is no trace of the variable, the name is still valid and the result of evaluting it should :undef as oppose to an error. Pair that with the (still) odd scoping rules to end up with a situation that is problematic to solve.

We are on a track where we will be able to solve this (cleanup part by part of the logic), but I am at this point uncertain what restrictions/inconveniences that may need to be enforced to be authoritative about if a variable exists or not.

#12 Updated by Tomas Doran 3 months ago

Redmine Issue #1198 has been migrated to JIRA:

https://tickets.puppetlabs.com/browse/PUP-2837

Also available in: Atom PDF