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

Bug #14704

Capitalised hash keys do not parse

Added by James Turnbull almost 2 years ago. Updated 10 months ago.

Status:ClosedStart date:05/25/2012
Priority:NormalDue date:
Assignee:Charlie Sharpsteen% Done:

0%

Category:references
Target version:-
Keywords:hash key capitalised parse error goalie_06_28_2012 customer Affected URL:https://github.com/puppetlabs/puppet-docs/pull/172
Branch:

We've Moved!

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

This issue is currently not available for export. If you are experiencing the issue described below, please file a new ticket in JIRA. Once a new ticket has been created, please add a link to it that points back to this Redmine ticket.


Description

Hash keys with leading capitals do not parse.

This does not parse:

$packages = {
Mysql-cluster => {version => '7.1.21-1.sles11'}
}

This parses:

$packages = {
mysql-cluster => {version => '7.1.21-1.sles11'}
}

Related issues

Related to Puppet - Feature #21117: Improve error message for illegal hash keys Needs Decision

History

#1 Updated by James Turnbull almost 2 years ago

  • Description updated (diff)

#2 Updated by Brice Figureau almost 2 years ago

Capitalized hash keys are parsed as resource references, because that’s the only legal keyword type in the DSL. The workaround is to quote those keys when they are capitalized.

#3 Updated by Josh Cooper over 1 year ago

  • Status changed from Needs Decision to Accepted
  • Priority changed from Urgent to Normal

Puppet’s grammar defines a hash key with the following production:

 key:      NAME { result = val[0][:value] }
   | quotedtext { result = val[0] }

where NAME is defined to be those strings matching %r{((::)?[a-z0-9][-\w]*)(::[a-z0-9][-\w]*)*} (excluding uppercase). Puppet is generating an error as Brice mentioned because CLASSREF is defined to be %r{((::){0,1}[A-Z][-\w]*)+}. Since quoting the key is a valid workaround, I’m reducing the priority of this bug.

I’ve also talked this over with Nick L. He suggested that we could change the grammar so that the key nonterminal be modified to accept any rvalue, and modify the rvalue to include all of puppet’s keywords, e.g. if, unless, etc. This would of course be a large change, e.g. 3.x.

#4 Updated by Josh Cooper over 1 year ago

  • Keywords changed from hash key capitalised parse error to hash key capitalised parse error goalie_06_28_2012

#5 Updated by Charlie Sharpsteen about 1 year ago

  • Keywords changed from hash key capitalised parse error goalie_06_28_2012 to hash key capitalised parse error goalie_06_28_2012 customer

#7 Updated by Daniel Pittman 11 months ago

  • Assignee deleted (Daniel Pittman)

#8 Updated by Charlie Sharpsteen 11 months ago

  • Assignee set to Charlie Sharpsteen

#9 Updated by Henrik Lindberg 11 months ago

Although possible to fix this in the future parser (I tried – it works) (not sure it is doable with a reasonable effort in the 3.2 regular parser) the question is if it is worth it. However – an upper case word will continue to be a qualified reference to a type. It can be made to work but only for existing types. (Compare to Ruby, where you can’t use a reference to a class / constant that does not exist and use that as a key). Thus if a user tries e.g. {Lollipop => 'candy' } it will result in an error that the type Lollipop is unknown, but it would work to do {File => 'used to get out of jail'} since File exists.

The “fix” for this is to give a better error message – “Illegal hash key – a QualifiedReference is not a valid hash key”. This can be done easily in the —parser future of 3.x.

I am reluctant to make hash-key a special case and turn the QualifiedReference into a string since other future work makes use of QualifiedReference to mean reference to existing type (type in the general sense, not just resource types).

In any case – this is not a bug, it was not intended to work.

Suggest either closing this issue as “won’t fix” and possibly open new issues for “allow any object as hash key”, and “improve error message for illegal hash keys”, or modify this issue to be about “better error message” and change it to a feature.

#10 Updated by Charlie Sharpsteen 11 months ago

  • Project changed from Puppet to Puppet Documentation
  • Category changed from parser to references

Thanks for the background info Henrik. I’m going to work on a documentation update so that the current behavior is clearly explained. I will also open a feature request for better error messages in the future parser.

#11 Updated by Charlie Sharpsteen 10 months ago

  • Status changed from Accepted to In Topic Branch Pending Review
  • Affected URL set to https://github.com/puppetlabs/puppet-docs/pull/172

Documentation update submitted.

#12 Updated by Charlie Sharpsteen 10 months ago

  • Status changed from In Topic Branch Pending Review to Closed

Updated docs released in 45fb95b.

Also available in: Atom PDF