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

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

Closed
Sveder opened this issue Mar 12, 2024 · 4 comments · Fixed by #333

Comments

@Sveder
Copy link

Sveder commented Mar 12, 2024

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
Copy link
Author

Sveder commented Mar 13, 2024

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:
57ba56f

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
Copy link
Author

Sveder commented Apr 7, 2024

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
Copy link
Collaborator

damieng commented Apr 7, 2024

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
Copy link
Collaborator

tamasvajk commented Apr 19, 2024

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

  • limit what sort of variable declarations can show up in a using statement, as in Fix wrong parsing when a function call with a (one or more) variable … #327
  • another similar approach would be making sure that variable declarators in using statements have a non-optional equals_value_clause
  • restricting the current variable_declaration and declarator a bit further. I think type tuple_pattern is only valid if type is implicit_type, also, there can't be multiple tuple patterns in the same variable declaration, for example var (x,y) = (1,2), (a,b) = (3,4); is not valid.

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

Successfully merging a pull request may close this issue.

3 participants