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

DLangParser should be tested against phobos sources #868

Open
SingingBush opened this issue May 31, 2023 · 2 comments
Open

DLangParser should be tested against phobos sources #868

SingingBush opened this issue May 31, 2023 · 2 comments
Labels

Comments

@SingingBush
Copy link
Member

Got a sentry report of the parser breaking on dmd/current/import/phobos/etc/c/curl.d:

com.intellij.diagnostic.PluginException: Failed to build index 'DModuleIndex' for file file:///snap/dmd/current/import/phobos/etc/c/curl.d (id = 20066) (file type = D file) [Plugin: net.masterthought.dlanguage]

This is from IntelliJ IDEA Ultimate Edition 2023.1.2 on Linux with plugin version 1.30.1:

java.lang.IllegalArgumentException: Could not find single match for token: 'identifier'. 0 matches found
    at io.github.intellij.dlanguage.parser.DLangParser.tok(DLangParser.java:9465)
    at io.github.intellij.dlanguage.parser.DLangParser.advance(DLangParser.java:9487)
    at io.github.intellij.dlanguage.parser.DLangParser.parseEnumBody(DLangParser.java:3536)
    at io.github.intellij.dlanguage.parser.DLangParser.parseEnumDeclaration(DLangParser.java:3687)
    at io.github.intellij.dlanguage.parser.DLangParser.parseDeclaration(DLangParser.java:2966)
    at io.github.intellij.dlanguage.parser.DLangParser.parseDeclaration(DLangParser.java:2807)
    at io.github.intellij.dlanguage.parser.DLangParser.parseModule(DLangParser.java:5655)
    at io.github.intellij.dlanguage.parser.ParserWrapper.parseLight(ParserWrapper.java:439)
    at io.github.intellij.dlanguage.parser.ParserWrapper.parse(ParserWrapper.java:448)

It would be good to get some tests in place which run the parser against phobos. Potentially it could be an optional test that only runs if a property is set that point to the root of the systems dmd install. That way it would be easy to opt-in to running the test and easy to setup on Github Action once it's possible to run with failing

@etienne02
Copy link
Contributor

IMHO we should change the parser logic to use the real token instead of using the dict we use (aka stop using tok()).
The problem is not related with phobos but just with the order of initialization of the map. This issue is not reproducible every times but it sometimes appear and sometimes not. I got it for some files in other project but just restarting the IDE allowed me to not have the issue after restart. It’s really inconsistent.

@SingingBush
Copy link
Member Author

Potentially worth having another issue for any changes to the way in which the parser works. This issue is just for testing purposes. I think it would be worth aiming to get to a point in which the automated tests that run against a push/PR make sure that the parser is able to run against the phobos sources without problems. It's a large codebase that should continue to be valid D syntax so will make for a good test case.

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