[DO NOT MERGE] fix(kotlin): classes with newlines #477
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 task
amchiclet
requested review from
nmote,
ajbt200128,
aryx and
brandonspark
and removed request for
a team and
ajbt200128
|
I will merge commits from this PR into Brandon's original PR: #476 (comment). Not closing this PR yet. I'd like the branch to still be alive while merging changes. |
amchiclet
changed the title
amchiclet
added a commit
to semgrep/semgrep
that referenced
this pull request
Mar 27, 2024
Fixes: SAF-899 Fixes: SAF-923 Requires: semgrep/ocaml-tree-sitter-semgrep#477 The current version of Kotlin's tree-sitter doesn't support newlines when declaring classes. It has to do with automatic semicolons. For example, the following won't parse: ``` class C constructor(arg:Int){} ``` but the following parses fine: ``` class C constructor(arg:Int){} ``` The fix requires a hack in the grammar. (Ideally the scanner seems to be the right place for a fix, but modifying the scanner is complicated.) The hack works as follows. When the parser sees the code: ``` class C constructor(arg:Int){} ``` The first line is parsed as a class declaration node, and the second line is parsed as a partial class declaration node. These need to be consolidated during CST -> Generic AST. This PR implements this merge. Test plan: * `make core-test` with new tests * ran `semgrep-core -lang kotlin -dump_ast` with some examples that have new lines in class declarations, including the one reported on Linear. The output AST shows that there's a single class definition that is correctly populated.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR refactors Brandon's PR, so the CST to Generic AST process becomes a bit easier. The rest of the description is copied and pasted from #476
What:
This PR makes it so we can parse Kotlin classes that have a newline between the class identifier and constructor.
Why:
Parse rate.
How:
The problem is in cases like:
Here, we insert an automatic semicolon between
Fooandconstructor. This leaves us able to parseclass Fooas aclass_declaration, butconstructor Bar ...is not allowed on its own.We simply allow
constructor Bar ...to be a standalone statement, and resolve to stitch them together at parsing time.Security