tree-sitter / tree-sitter-c-sharp

C# Grammar for tree-sitter
MIT License
177 stars 48 forks source link

WIP: Remove `implicit_type` #263

Closed tamasvajk closed 1 year ago

tamasvajk commented 1 year ago

This PR removes the var implicit_type from the type system. The change would align the grammar with the one in Roslyn, where var is parsed as

 Type 
 |_ IdentifierName 
    |_ IdentifierToken

This would probably fix https://github.com/tree-sitter/tree-sitter-c-sharp/issues/163.

TODO:

damieng commented 1 year ago

I wish I knew more about how people were using the highlighting. I suspect https://github.com/tree-sitter/tree-sitter-c-sharp/pull/39/files happened because contributors wanted var to be able to be highlighted like a keyword and not a user-type.

There are a few places where we've diverged from Roslyn for practical reasons as we don't have the extra layers on top to do things like identify and colorize actual types based on resolution.

That's not to say I'm 100% opposed to this - just worth a discussion of the trade-offs and any potential workarounds.

tamasvajk commented 1 year ago

The below also parses correctly with the second commit:

class async
{
    async async async(async async)
    {
        async async = async;
    }
}
(compilation_unit [0, 0] - [6, 1]
  (class_declaration [0, 0] - [6, 1]
    name: (identifier [0, 6] - [0, 11])
    body: (declaration_list [1, 0] - [6, 1]
      (method_declaration [2, 4] - [5, 5]
        (modifier [2, 4] - [2, 9])
        type: (identifier [2, 10] - [2, 15])
        name: (identifier [2, 16] - [2, 21])
        parameters: (parameter_list [2, 21] - [2, 34]
          (parameter [2, 22] - [2, 33]
            type: (identifier [2, 22] - [2, 27])
            name: (identifier [2, 28] - [2, 33])))
        body: (block [3, 4] - [5, 5]
          (local_declaration_statement [4, 8] - [4, 28]
            (variable_declaration [4, 8] - [4, 27]
              type: (identifier [4, 8] - [4, 13])
              (variable_declarator [4, 14] - [4, 27]
                (identifier [4, 14] - [4, 19])
                (equals_value_clause [4, 20] - [4, 27]
                  (identifier [4, 22] - [4, 27]))))))))))
damieng commented 1 year ago

Addressing those two issues may well be worth the trade.

tamasvajk commented 1 year ago

I've experimented a bit with highlighting. I think we can do the var highlighting without implicit_type. We'd need to add type field name to all type child nodes, and then the below handles the coloring.

(_
  type: (identifier ("var")) @type.builtin)

(BTW, I'd need to check how different IDEs highlight var. I think VS code is using the same color as int, but sharplab.io is coloring it as any other type (and not as int). The latter would work better for us, as we can't know if class var { ... } is declared or not.)

damieng commented 1 year ago

Which tree-sitter extension are you using for VS Code? Would like to get mine setup too.

tamasvajk commented 1 year ago

Which tree-sitter extension are you using for VS Code? Would like to get mine setup too.

I'm not using any tree sitter extension for VS Code. I was checking how the Roslyn based C# extension is doing the coloring. I'm not sure what the best way of checking/changing the highlighting is. Adding tests for everything would be cumbersome. I think adding some tests as a baseline, and then improving on that would be an option.

damieng commented 1 year ago

I'll give https://marketplace.visualstudio.com/items?itemName=ms-vscode.anycode another go and see if I can get it working with a dev local copy of tree-sitter-c-sharp.

tamasvajk commented 1 year ago

I'm closing this draft PR, var has already been added to the contextual keywords in an earlier PR. async and await have a tracking issue. This PR doesn't fully solve using await everywhere, so that would anyways need more work.

I'm adding the var highlighting test in this PR.