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

This issue tracker is now in read-only archive mode and automatic ticket export has been disabled. Redmine users will need to create a new JIRA account to file tickets using https://tickets.puppetlabs.com. See the following page for information on filing tickets with JIRA:

Bug #21608

Syntax error at '}'; expected '}'

Added by Chris Wilson almost 3 years ago. Updated over 2 years ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Henrik Lindberg% Done:

0%

Category:language
Target version:-
Affected Puppet version:2.6.18 Branch:
Keywords:parser errors

We've Moved!

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


Description

From the following code:

  # http://stackoverflow.com/questions/6399922/are-there-iterators-and-loops-in-puppet
  define f {
    file { "/root/.ssh/${title}":
      ensure => present,
      source => 'puppet:///files/ssh_keys/${title}',
      owner   => 'root', group => 'root', mode => '0400',
    }
  }
  f { ["id_beep_down", "id_beep_down.pub", "id_beep_up", "id_beep_up.pub",] }

I think it’s missing a colon after the last ], but that’s not what the error message says.

[puppet server]$ rpm -q puppet
puppet-2.6.18-3.el6.noarch

History

#1 Updated by eric sorenson almost 3 years ago

  • Category set to language
  • Status changed from Unreviewed to Investigating
  • Keywords set to parser errors

The problem in that code is actually that you’re trying to do variable interpolation with single quotes in the source attribute. When I corrected that to use double-quotes I got a better error message, which indicates the line number correctly:

Error: Could not parse for environment production: Syntax error at '}' at line 8 on node glitch.local

This could probably be improved in the future parser available in Puppet 3.2 and up though; it also does not indicate that there’s a missing : on the resource declaration:

Error: Could not parse for environment production: Syntax error at '}' at line 8:77 on node glitch.local

Henrik what do you think?

#2 Updated by eric sorenson over 2 years ago

  • Status changed from Investigating to Needs Decision
  • Assignee set to Henrik Lindberg

Whups, I forgot to actually set this to your ownership, Henrik, when I asked a question.

#3 Updated by Henrik Lindberg over 2 years ago

  • Status changed from Needs Decision to Closed

I tried this in the future parser:

Syntax error at '}' at ./foo.pp:8:75

Indicating that the parser does not know what to do at the very final }.

It is unfortunately impossible to differentiate between the two cases: * a function call of function f with a hash as an argument (with an array as a key, missing a => and a value), which BTW is illegal since an array is not valid hash key * a resource expression creating an instance of a resource of type F with a title being an array, but missing a colon.

This is very difficult to anything about – the parser technology is not smart enough to report what would make it happy at that point. It only announce its unhappiness when seeing the final `}ยด.

In order to be able to give a better error message the parser would need to be able to accept an expression block – i.e.

'{' expression '}'

This is however not easily introduced because of the ambiguities it creates – we would need to relax the grammar substantially and move the complexity elsewhere. IMO, the distinction between a literal hash and a resource expression is one of the harder to solve ambiguities in the language and was one of the difficult things to handle when being able to use literal hashes everywhere a value is allowed.

It may still be solvable, effort is not known.

When giving the same logic to Geppetto and trying it with 3.2 and with future langauge versions – it gives a series of errors:

  • Not a top level expression (the entire expression
  • missing } at [
  • extranous input }, expecting EOF
  • Unknown function ‘f’

If the final comma in the title array is removed, it also gives this problem: * A resource reference must start with a [(deprecated) name, or] class reference. (i.e. this is given because the LHS before the [] is not a name (it is an error), it could have been f[‘foo’].

From that it is possible to figure out that the expression clearly is highly ambiguous (which we sort of knew already), but it tells us that ‘f’ is an unknown function – so clearly what follows is not recognized as a resource (body/bodies) expression. I.e. the function ‘f’ is the last piece it can make sense of.

This is pretty much the same as what happens in the puppet parser.

To summarize. The ‘:’ is a darned important clue to the parser what is actually being expressed, and it is almost impossible to make it suggest a missing ‘:’ as being the problem simply because it does not know that is the problem.

IMO, the detailed position information from future parser – or using Geppetto which tries more options is as good as it gets without either a very large effort (possible not being able to fix it), or changing the language itself.

I am therefore marking this issue as closed. There is no “works as best as it can in future version”– status to set.

There may be a greater chance in improving the error message in Geppetto (different parser technology), but I still think it is going to be hard because of the fundamental problem “is it a hash” or a “resource expression”.

#4 Updated by Chris Wilson over 2 years ago

OK, at least the error message is better than the original one, which we found very funny in the office.

But I still think that, at least in theory, during parsing there should be two options after the “]”: either a “:” or a “=>”. Anything else would be a parser error. (Assuming that the lexer has separated the input into tokens and thrown away any white space, then the parser doesn’t need to account for that possibility). If the parser was able to present a list of the valid options, that would have helped a lot.

I still find the value “syntax error” pretty useless. Surely there is more than one class of syntax error? How on earth is one supposed to resolve it, apart from commenting things out until it stops breaking?

Why does the parser even consider the possibility of a function call? I assume that parentheses areound the expression are absolutely requires, and there are no parentheses here. It was an option between “f” and “{”, but after seeing “{” it’s no longer an option.

#5 Updated by Henrik Lindberg over 2 years ago

Chris Wilson wrote:

OK, at least the error message is better than the original one, which we found very funny in the office.

Oh, the pleasure we get from the WATs :–)

But I still think that, at least in theory, during parsing there should be two options after the “]”: either a “:” or a “=>”. Anything else would be a parser error. (Assuming that the lexer has separated the input into tokens and thrown away any white space, then the parser doesn’t need to account for that possibility). If the parser was able to present a list of the valid options, that would have helped a lot.

As far as I know, Racc (the parser generator used) does not present the options that would have made it a non-syntax error. So, yeah, in theory.

I still find the value “syntax error” pretty useless. Surely there is more than one class of syntax error? How on earth is one supposed to resolve it, apart from commenting things out until it stops breaking?

You are right, a syntax error is the last resort. That happens when the grammar runs out of options. In the future parser the grammar has been substantially relaxed (it allows as much as possible as “understandable” and uses a validation step to flag sentences that are not valid in the puppet language). As you will find when using the future parser it does a much better job in general of reporting errors. It is however not possible to completely get rid of emitting syntax error (ultimately it requires intelligence and intuition, and frankly, a parser is quite dumb).

Why does the parser even consider the possibility of a function call? I assume that parentheses around the expression are absolutely requires, and there are no parentheses here. It was an option between “f” and “{”, but after seeing “{” it’s no longer an option.

The puppet language (like ruby) allows function calls to be made without the parentheses. In Puppet this is allowed when one argument is given to the function, and the expression is a “top level” construct (i.e. appears as a statement rather than part of a expression). Thus the following two expressions are equivalent:

f 1
f(1)

But you have to have the parentheses in cases like this:

10 + f(1)

Some things that may be perceived as statements in the language are actually functions; e.g. include and require.

It would be a good thing to get rid of this by making include and require keywords and separate statements and then require parentheses around the argument list at all times. That would increase the opportunity to provide better error messages as the number of possible options in the parser are then greatly reduced. However, that is a quite major change that makes introduction of new statement like functions impossible without language change.

Again, I like to point out that Geppetto uses a different parser technology and you may find that it is easier to find and figure out what is wrong given that it is able to give more information in many cases.

Also available in: Atom PDF