tree-sitter / tree-sitter-typescript

TypeScript grammar for tree-sitter
MIT License
341 stars 104 forks source link

module body is mandatory #198

Closed sguillia closed 2 years ago

sguillia commented 2 years ago

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: https://github.com/tree-sitter/tree-sitter-typescript/blob/111b07762e86efab9a918b7c721f720c37e76b0a/common/define-grammar.js#L482

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

mjambon commented 2 years ago

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 commented 2 years ago

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.