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

module body is mandatory #198

Closed
sguillia opened this issue Nov 26, 2021 · 2 comments · Fixed by #205
Closed

module body is mandatory #198

sguillia opened this issue Nov 26, 2021 · 2 comments · Fixed by #205
Labels

Comments

@sguillia
Copy link

Hi!

The following piece of code is invalid but it is parsed correctly:

module x

let a = 0

Here's a link to the TypeScript Playground showing that the snippet above is invalid JavaScript or TypeScript:
https://www.typescriptlang.org/play?#code/LYewJgrgNgpgBADwFBNgFzgQzgXjgBiA

The output of tree-sitter parse is the following:

(program [0, 0] - [3, 0]
  (module [0, 0] - [0, 8]
    name: (identifier [0, 7] - [0, 8]))
  (lexical_declaration [2, 0] - [2, 9]
    (variable_declarator [2, 4] - [2, 9]
      name: (identifier [2, 4] - [2, 5])
      value: (number [2, 8] - [2, 9]))))

I think body should not be optional here:

field('body', optional($.statement_block))

The issue template asks for a link to official documentation but I am not sure where to start looking

@mjambon
Copy link
Contributor

mjambon commented Nov 28, 2021

Like for #209, I don't think we have a high priority placed on failing on incorrect constructs.

I recently proposed that we even merge the javascript and typescript parsers into one so as to facilitate maintenance. This would create a lot of such cases since we'd parse javascript with a typescript/tsx parser. See #191 and maybe let us know if that would be good or bad for your application.

The issue template asks for a link to official documentation but I am not sure where to start looking

Sadly, the typescript team hasn't updated the spec since 2016.

@resolritter
Copy link
Contributor

I think it's optional for the sake of .d.ts files, it just isn't documented.

Currently having an optional node there makes the parser more resilient to errors while not compromising on parsing correctness, so I'd prefer to have it as it is over "fixing" it.

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

Successfully merging a pull request may close this issue.

3 participants