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

Rewrite parser and add support for Nix 2.3 #27

Merged
merged 14 commits into from
May 2, 2021
Merged

Conversation

JojOatXGME
Copy link
Contributor

This pull request rewrites the whole lexer and parser. I will try to make a field test before I merge the pull request. The new implementation should

  • correctly parse all valid files supported by Nix 2.3,
  • provide better error recovery for invalid files,
  • generate a meaningful and shallow AST, and
  • have proper test coverage.

The third point is not visible to the user but shall provide the foundation for more advanced features that need to analyze the content of a file. The new implementation no longer has special tokens for certain built-ins like true or import. As a result, built-ins are no longer highlighted. I think the right way to implement the highlighting of built-ins is to use an annotator. I may look into that later.

I decided to rewrite the whole parser and lexer because many issues I noticed while working on the test coverage (#24) were not easily fixable without major changes on the previous implementation. While it was a quite steep learning curve, I think I finally finished my implementation. I am aware that this is a huge pull request.

PS: There are still some issues with the new implementation which I was not able to fix using Grammar-Kit. This might include strange error messages in some (hopefully rare) cases. For more details, you might look at the links below. It might also be possible that the lexer, parser, brace matcher, and maybe code folding (once we implement it) get out of sync in invalid files since they all have their own implementation to analyze the structure of the file. I think trying to ensure they are all always in sync (even in invalid files) is not maintainable.

The Gradle plugin for Grammar-Kit does not support the usage of Util
classes. See the following issues from the Gradle plugin.

JetBrains/gradle-grammar-kit-plugin#23
JetBrains/gradle-grammar-kit-plugin#3
The test cases are mostly based on the following resources:

https://nixos.org/guides/nix-pills/basics-of-language.html
https://nixos.org/manual/nix/stable/#ch-expression-language

Note that the outcome expected by the tests is not based on the behavior
described in the mentioned resources but on the current behavior of the
parser. The expected outcome will be corrected in upcoming commits when
the related bugs of the parser are fixed. This approach shall make
bugfixes on the parser easier to review by showing the old and new
behavior in the diff.
The tests are based on src/libexpr/parser.y of commit 75efa421340 in
https://github.com/NixOS/nix.
Commit 75efa421340b of https://github.com/NixOS/nix is used as
reference.
IDEA might start lexing from the middle of a file for syntax
highlighting. In such case, IDEA calls Lexer.start (which calls
FlexLexer.reset) to restore the state of the lexer. However, the
implementation previous to this commit did only restore parts of the
state.
Remove 'NixTokenType.' prefix from expected tokens to improve
readability of error messages. For example the error message

    NixTokenType.., NixTokenType.; or NixTokenType.or expected, got '}'

becomes

    '.', ';' or 'or' expected, got '}'
@Mic92
Copy link
Member

Mic92 commented Apr 6, 2021

I don't think I can provide a meaningful review for this but maybe @Rizary or @jD91mZM2 can.
Otherwise merge at your own will.

@JojOatXGME
Copy link
Contributor Author

I have used the plugin with the updated parser for a few days on a personal project. I did not notice any serious problem. I found IDEA-267912, but it was not a big issue.

@JojOatXGME JojOatXGME merged commit d1adafc into master May 2, 2021
@JojOatXGME JojOatXGME deleted the parser_rewrite branch May 2, 2021 17:31
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants