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

More helpful syntax error messages #9

Open
anko opened this issue Sep 23, 2015 · 4 comments
Open

More helpful syntax error messages #9

anko opened this issue Sep 23, 2015 · 4 comments
Labels
Milestone

Comments

@anko
Copy link
Owner

anko commented Sep 23, 2015

Syntax errors look like this right now:

$ eslc <<< "("

/home/an/code/eslisp/node_modules/sexpr-plus/index.js:1054
      throw peg$buildException(null, peg$maxFailExpected, peg$maxFailPos);
      ^
SyntaxError: Expected ")", comment or whitespace but end of input found.

The SyntaxError-object sexpr-plus returns has line and column properties. We should perhaps use them.


Invalid AST errors look like this:

$ eslc <<< "(+ x %)"
{ [InvalidAstError: Identifier `name` member must be a valid IdentifierName]
  node: { type: 'Identifier', name: '%' },
  message: 'Identifier `name` member must be a valid IdentifierName' }
x + %;

That's already getting there, but it would still be best if exact positions in code were passed on to AST nodes, so the error reporter could point out the exact in-source position that an error happened.

@anko anko added the idea label Sep 23, 2015
@anko anko mentioned this issue Sep 25, 2015
anko added a commit that referenced this issue Sep 25, 2015
Moving to parse s-expressions to objects is necessary for eventually
enabling returning also source-location information along with the
parsed nodes, for more informative errors (#9) and source maps (#18).
anko added a commit that referenced this issue Sep 26, 2015
And for contexts where colour doesn't show up, show the line number and
offset also.

Significantly improves #9.
@anko
Copy link
Owner Author

anko commented Sep 26, 2015

Here's what we have as of bf7bb54, shown in the REPL:

example eslc session, showing off colours

An illegal identifier occurring in a built-in macro is highlighted in source. This should work with any error returned by esvalid, but I haven't tested it comprehensively.

As you can see in the last command, the invalid AST node originating from the user-defined macro nonsense isn't shown with highlighted source. This is because macros aren't necessarily even in the source: they may have been required from another module or themselves generated by another macro.

It would be good to at least display the macro's name ([ returned from macro 'nonsense' ]), but macro return values are not currently tagged with that information.

anko added a commit that referenced this issue Nov 25, 2015
This makes the clearer errors for #9 from bf7bb54 work again; with the
updated name for the location property and no longer needing the -1
offset-offset, thanks to the smarter parser.
@dead-claudia
Copy link
Contributor

@anko

Could this be implemented via an env::fail() method? That could also be potentially useful for macro writers as well, if they need to fail with a more descriptive error than simply "Oh, by the way, this will break if you're not careful", which is a common problem with macros, especially when you're dealing with DSLs.

I was just thinking of this when I was working on #41. That as a high-level method might be the best way to do it, and it would definitely be worth exposing.

@anko
Copy link
Owner Author

anko commented Jan 2, 2016

@isiahmeadows I've just had macros throw an error when user input is nonsensical.

However, the compilation env makes no attempt to catch errors from macros at the moment. Hence the stack traces don't tell you useful stuff e.g. which macro it was that errored. I think catching would be a better way than an env::fail, because something the macro-writer didn't think to check for might throw an error too.

@anko
Copy link
Owner Author

anko commented Jan 2, 2016

f19be37 now catches macro errors and adds diagnostic data before rethrowing. Thanks for raising that.

@anko anko changed the title Error messages are quite uninformative More helpful syntax error messages Jan 2, 2016
@anko anko added this to the v1 milestone Jul 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants