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

A PR to add field names to the grammar #72

Closed
vladimir-reshetnikov opened this issue Jan 26, 2023 · 2 comments
Closed

A PR to add field names to the grammar #72

vladimir-reshetnikov opened this issue Jan 26, 2023 · 2 comments

Comments

@vladimir-reshetnikov
Copy link

Dear fwcd,

My name is Vladimir Reshetnikov, I'm a software engineer at Relyance AI, currently working on static analysis of Kotlin code. In the past, I worked at JetBrains as Language Specification Lead on the Kotlin team, and was actively involved in design and specification of Kotlin.

In our project, we are using fwcd / tree-sitter-kotlin to parse Kotlin code. I appreciate all the effort you put into creating and maintaining this project, it has been an invaluable tool that I rely on.

To facilitate navigation through the AST and extraction of certain nodes, I decided to add named fields by wrapping parts of grammar rules into field('name', …), similar to how it is done in other tree-sitter parsers, such as tree-sitter / tree-sitter-java. For that purpose, I created a fork of your project and made necessary changes in it: vladimir-reshetnikov / tree-sitter-kotlin. None of the changes modify the shape of the generated AST in any other way, except for adding named fields. A typical example of the changes I made is included below this message.

I would like to submit a pull request to your repository with the changes I've made. Given that Kotlin is still being actively developed, I anticipate possible changes in the grammar in the future, and I would like to avoid divergent histories between the main project and my fork, and reduce the risk of future merge conflicts. I believe that the changes I'm proposing could be beneficial for other users of your parser as well. Please let me know if you are open to this discussion, and if so, then what are the requirements regarding the coding style, stability of field names, consistency of field names with other tree-sitter parsers, etc. I will make all required corrections.

Kind regards,
Vladimir Reshetnikov
Email: [email protected], [email protected]

--- a/grammar.js
+++ b/grammar.js
@@ -171,16 +171,16 @@ module.exports = grammar({
       $._semi
     ),

-    import_alias: $ => seq("as", alias($.simple_identifier, $.type_identifier)),
+    import_alias: $ => seq("as", field('name', alias($.simple_identifier, $.type_identifier))),

-    top_level_object: $ => seq($._declaration, optional($._semi)),
+    top_level_object: $ => seq(field('declaration', $._declaration), optional($._semi)),

     type_alias: $ => seq(
       optional(field('modifiers', $.modifiers)),
       "typealias",
-      alias($.simple_identifier, $.type_identifier),
+      field('name', alias($.simple_identifier, $.type_identifier)),
       "=",
-      $._type
+      field('type', $._type)
     ),

Here is a patch containing only the changes in grammar.js file:
grammar.js.patch

The PR I'm proposing will contain all the changes, including those in the generated parser code.

@VladimirReshetnikov
Copy link

Dear fwcd,

I wanted to let you know that, due to recent layoffs, I am no longer employed at Relyance AI, so my work account vladimir-reshetnikov and email [email protected] are no longer active, and I no longer have control over the fork vladimir-reshetnikov / tree-sitter-kotlin. I apologize for any confusion this may have caused with regard to this pending PR. It's my understanding that someone from Relyance AI will take over and may get in touch with you regarding this matter.

Kind regards,
Vladimir Reshetnikov
Email: [email protected]

@VladimirMakaev
Copy link
Collaborator

#106 was merged

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

No branches or pull requests

3 participants