Feature/refactor cpp2 language module #1167
Conversation
|
With this change, how could I add semantic information to the tokens as is done in the TokenGeneratingTreeScanner? How could I track variables, for instance? |
|
Right now that is not possible, but I will write a small expansion to the abstract antlr utils, that as far as I can tell should suffice. I just did not want to make this pr any bigger. |
tsaglam
changed the title
|
@TwoOfTwelve could you extend the PR description a bit? List the fixes to the cpp2 language explicitly (e.g., which tokens/syntax elements were affected). Also, for testing the token extraction quickly, you can always use the token printer to print the token before and after your change with a large cpp file and then to a text diff. Finally, the expansion for semantic token information would be of the highest priority right now. |
tsaglam
added
enhancement
|
I compared the old and new versions. Aside from some formatting and the wrong else token mentioned in the description the tokens are the same (My testfile had about 3000 lines of code). The semantics stuff is mostly done already I just need to write the javadoc and some JUnit tests mostly. I think I can create the PR today. |
languages/cpp2/src/main/antlr4/de/jplag/cpp2/grammar/CPP14Parser.g4
Outdated
Show resolved
Hide resolved
language-antlr-utils/src/main/java/de/jplag/antlr/AbstractAntlrListener.java
Outdated
Show resolved
Hide resolved
language-antlr-utils/src/main/java/de/jplag/antlr/AbstractAntlrListener.java
Outdated
Show resolved
Hide resolved
language-antlr-utils/src/main/java/de/jplag/antlr/AbstractAntlrListener.java
Outdated
Show resolved
Hide resolved
language-antlr-utils/src/main/java/de/jplag/antlr/AbstractAntlrListener.java
Outdated
Show resolved
Hide resolved
| public CPPListener(TokenCollector collector, File currentFile) { | ||
| super(collector, currentFile); | ||
|
|
||
| createStartStopMapping(ClassSpecifierContext.class, UNION_BEGIN, UNION_END, rule -> rule.classHead().Union() != null); |
There was a problem hiding this comment.
looks clean, but since create...Mapping is repeated so often it should be less verbose, my suggestion would be simply map..., in this case mapStartStop, or, in this case specifically even simpler, mapScope
There was a problem hiding this comment.
I agree that this naming would be better, but since these methods were introduced before this PR and it is not directly connected to the cpp2 module, I would prefer to change them in a separate PR.
There was a problem hiding this comment.
Right, makes sense. Do you have time to make this PR? Otherwise I can do it as well. But I should really get started on the semantic token info 😅
There was a problem hiding this comment.
I can do it, but it will have to wait until this evening. If you would like it before then go for it, but I think it is not that urgent.
| createStartStopMapping(SelectionStatementContext.class, IF_BEGIN, IF_END, rule -> rule.If() != null); | ||
| createTerminalMapping(CPP14Parser.Else, ELSE); | ||
|
|
||
| createStartMapping(LabeledStatementContext.class, CASE, rule -> rule.Case() != null); |
There was a problem hiding this comment.
I feel like it should be enter/exit instead of start/stop, so mapEnter here
|
[JPlag Plagiarism Detector] Kudos, SonarCloud Quality Gate passed! |
This PR refactors the cpp2 tests and language module.
Also contains minor improvements for the abstract antlr utils and test utils.
The refactoring of the cpp2 module fixes a couple of issues with the module. As far as I can tell all tokens are still extracted by the same rules, although it would be nice if someone could verify that.
The fixed errors are:
-Nested if-else statements lead to the wrong line being assigned to the second else token
-The Antlr grammar was missing DivAssign in the list of operators for operator overloading
The things changed in the testutils:
-The tokenprinter is now called for the tests (should we maybe only do this if the test fails?)
-The configured tests are now passed as a list instead of a set internally, so the order of tests does not change between runs.