tree-sitter / tree-sitter-c-sharp

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

Using statement with a function call with parameters is not parsed as invocation, instead a variable declaration #326

Closed Sveder closed 1 month ago

Sveder commented 3 months ago

I'm testing some parsing with big projects and in two instances I think I found the same parsing error. It also reproduces in the TreeSitter playground for csharp.

Simplified issue:

using (CreateAndOpenSelfHost(null, configuration))
{
.....
}

parses into a using_statement->variable_declaration

              [using_statement](https://tree-sitter.github.io/tree-sitter/playground#) [139, 12] - [151, 13]
                [variable_declaration](https://tree-sitter.github.io/tree-sitter/playground#) [139, 19] - [139, 61]
                  type: [identifier](https://tree-sitter.github.io/tree-sitter/playground#) [139, 19] - [139, 40]
                  [variable_declarator](https://tree-sitter.github.io/tree-sitter/playground#) [139, 40] - [139, 61]
                    [tuple_pattern](https://tree-sitter.github.io/tree-sitter/playground#) [139, 40] - [139, 61]
                      name: [identifier](https://tree-sitter.github.io/tree-sitter/playground#) [139, 41] - [139, 45]
                      name: [identifier](https://tree-sitter.github.io/tree-sitter/playground#) [139, 47] - [139, 60]

removing the parameters to the function results in the correct using_statement -> invocation_expression. Example code:

using (CreateAndOpenSelfHost())
{
.....
}

parses into:

             [using_statement](https://tree-sitter.github.io/tree-sitter/playground#) [139, 12] - [151, 13]
                [invocation_expression](https://tree-sitter.github.io/tree-sitter/playground#) [139, 19] - [139, 42]
                  function: [identifier](https://tree-sitter.github.io/tree-sitter/playground#) [139, 19] - [139, 40]
                  arguments: [argument_list](https://tree-sitter.github.io/tree-sitter/playground#) [139, 40] - [139, 42]

Actual file to parse/copy paste into playground: https://github.com/NancyFx/Nancy/blob/master/test/Nancy.Hosting.Self.Tests/NancySelfHostFixture.cs#L80 This file has a few using statements - some without arguments that parse to an invocation (as they should) and some, like I linked, with arguments that is parsed to a variable_declarator.

I've started looking at the grammar.js and C# spec to find the issue, it might just be a precedence issues, but wanted to open this issue to ask:

  1. Am I right that this is an error?
  2. Any advice on how to fix?

Thanks :)

Sveder commented 3 months ago

I've grown more confident this is indeed a bug. My research leads me to believe that tuple_pattern clause in variable_declarator is wrong in this case. The commit introducing it is explicitly deviating from the C# spec: https://github.com/tree-sitter/tree-sitter-c-sharp/commit/57ba56f25b415da5b3dd2e7381387fa99d71b314

My best idea is currently to have a specific variable_declarator (say a using_variable_declarator) that doesn't allow tuples inside using statements. It is hard to reason about this since it is not part of the spec, but seems like a safe change to me. Does this make sense @maxbrunsfeld?

Sveder commented 2 months ago

So is this project unmaintained? @damieng @amaanq @tamasvajk (just randomly pinging people who seem to have committed a lot lately as there is no CODEOWNERS file).

damieng commented 2 months ago

There are maintainers around but like a lot of projects we're fixing the issues we individually need/want fixing and shepherding external pull requests given we're all volunteers and not financially backed.

Not sure why your PR didn't show up on my notifications - I'll take a look.

tamasvajk commented 2 months ago

Thinking out loud, I think there are multiple ways to fix this: