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

Improve test coverage (and rewrite parser) #24

Closed
JojOatXGME opened this issue Jan 10, 2021 · 3 comments · Fixed by #27
Closed

Improve test coverage (and rewrite parser) #24

JojOatXGME opened this issue Jan 10, 2021 · 3 comments · Fixed by #27
Assignees

Comments

@JojOatXGME
Copy link
Contributor

The project currently lacks automatic tests for the Java implementation. Within this ticket, I want to add tests for the most important components.

@JojOatXGME JojOatXGME self-assigned this Jan 10, 2021
@JojOatXGME
Copy link
Contributor Author

JojOatXGME commented Jan 26, 2021

While working on this ticket, I noticed some issues with the parser. I initially wanted to fix the issues incrementally, but I eventually decided to rewrite the parser. I guess I might create a pull request at the end of the week.

Anyway, I noticed that the parser of the plugin had support for some expressions and keywords I couldn't find in the Nix language itself. The related keywords are

  • require,
  • requires,
  • import, and
  • imports.

@pSub or @Mic92, do you have any idea where these keywords are coming from? Did Nix had any breaking changes which might have removed these keywords?

@edwtjo, I mention you because you have created the initial version of the parser six years ago according to GitHub. The initial version already supported these keywords. Maybe you can still remember the background?

EDIT: Well, I guess import just refers to the built-in function (which is not a keyword). Strange that I didn't think about that until after writing this comment. 😅 I could not find functions for the other terms in the manual, though.

@Mic92
Copy link
Member

Mic92 commented Jan 26, 2021

Imports is used only in NixOS modules where it is not a module but an attribute in your configuration attribute set. import is a builtin function as you found out. I never saw require or requires being used for anything.

@JojOatXGME JojOatXGME changed the title Improve test coverage Improve test coverage (and rewrite parser) Feb 1, 2021
@JojOatXGME
Copy link
Contributor Author

Just a short update for everyone interested:

My new parser implementation seems to work fine as long as the file is valid. I currently try to find a good approach to recover from syntax errors. I actually created a post on intellij-support.jetbrains.com about one of my problems. I will invest some more time and try to find a good solution. Anyway, I hope that even without reliable error recovery, my new parser implementation (with test coverage and Nix 2.3-support) would be an improvement.

My current status can be found on the tests branch of my fork. However, if you look at this branch, be aware that I rebase this branch from time to time.

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

Successfully merging a pull request may close this issue.

2 participants