Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unfriendly error message for bad syntax #128

Closed
copumpkin opened this issue Mar 13, 2016 · 5 comments
Closed

Unfriendly error message for bad syntax #128

copumpkin opened this issue Mar 13, 2016 · 5 comments

Comments

@copumpkin
Copy link

I accidentally wrote something of this shape earlier today:

local id(x) = x;
local obj = {
  x: whoa, id(self.x)
};
obj

And was met with the following error message (v0.8.5):

STATIC ERROR: templates/util.jsonnet:3:15-20: Not an identifier: 0x7f838ac04c90

The whoa in there is clearly wrong, but somehow this is getting treated as something other than a parse error.

@sparkprime
Copy link
Member

In HEAD this is already a bit better, as that hex error was already spotted and fixed.

STATIC ERROR: test.jsonnet:3:15-20: Expected an identifier but got a complex expression.

What's happening is it's parsed up to:

local id(x) = x;
local obj = {
  x: whoa, id(self.x)

And interpreting it as an attempt to write something like:

local id(x) = x;
local obj = {
  x: whoa,
  id(self.x): 3,
}

Except you can't put self.x as a function param, only identifiers are allowed, so that's the error it gave you. In the following case, it parses a bit further and gives a different error:

local id(x) = x;
local obj = {
  x: whoa, id(x)
};
obj

STATIC ERROR: test.jsonnet:4:1: Expected token OPERATOR but got "}"

The OPERATOR it was looking for should have been one of : :: ::: +: +:: +::: but the error message unfortunately does not say this.

@copumpkin
Copy link
Author

Oh, so the error message is coming from the parser? I assumed by the text that it had parsed successfully and that the issue came from a later stage.

If it's the parser, I'm still a little confused: how is it taking id(self.x) as the beginning of a KV mapping production when there is no colon?

@copumpkin
Copy link
Author

STATIC ERROR: test.jsonnet:4:1: Expected token OPERATOR but got "}"

That new error looks far more like what I'd expect to see here (ideally with the meaning of OPERATOR as you say). Closing, but still curious what changed in the parser if you feel like writing it out.

@sparkprime
Copy link
Member

When the parser hits id(self.x) it knows it is in object scope, so it will try to parse the "key" which can be of the form id(e, e, e, ...) where e are arbitrary expressions, and then it looks for the colons. But it checks the e's are actually just identifiers. Why not just parse id(x, y, z) -- this is just an artifact of the implementaiton -- re-using some code that parses lists of expressions separated by commas, which are used in arrays as well. So the error could still be better if that code was rewritten instead of re-used. Then you'd get an error like "expected identifier but got self" or similar.

The change in the parser was to stop printing out the 'e' in the case that e was not an identifier. Because there is no ostream<< operator for ASTs, it just printed the pointer.

@copumpkin
Copy link
Author

Thanks, makes sense!
On Mon, Mar 21, 2016 at 19:44 Dave Cunningham notifications@github.com
wrote:

When the parser hits id(self.x) it knows it is in object scope, so it will
try to parse the "key" which can be of the form id(e, e, e, ...) where e
are arbitrary expressions, and then it looks for the colons. But it checks
the e's are actually just identifiers. Why not just parse id(x, y, z) --
this is just an artifact of the implementaiton -- re-using some code that
parses lists of expressions separated by commas, which are used in arrays
as well. So the error could still be better if that code was rewritten
instead of re-used. Then you'd get an error like "expected identifier but
got self" or similar.

The change in the parser was to stop printing out the 'e' in the case that
e was not an identifier. Because there is no ostream<< operator for ASTs,
it just printed the pointer.


You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub
#128 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants