Feature #1198

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

Added by Mike Pountney about 4 years ago. Updated 7 months ago.

Status:Accepted Start date:
Priority:Low Due date:
Assignee:- % Done:

0%
Category:language
Target version:-
Affected Puppet version:0.25.4 Branch:
Keywords:
Votes: 1

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…

History

#1
Updated by Luke Kanies about 4 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 almost 4 years ago

  • Status changed from 1 to Accepted

#3
Updated by Luke Kanies about 2 years ago

  • Assignee deleted (Luke Kanies)

#4
Updated by Uwe Stuehler about 1 year 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 9 months ago

  • Target version deleted (4)

#6
Updated by Rich Rauenzahn 7 months 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” }

Also available in: Atom PDF