tree-sitter / tree-sitter-python

Python grammar for tree-sitter
MIT License
364 stars 136 forks source link

tree-sitter-python is overly permissive with newlines #178

Open agirardeau opened 1 year ago

agirardeau commented 1 year ago

The following code produces a syntax error in python due to the line break before the colon, but tree-sitter-python parses it as valid code:

def foo(x)
:
    return x + 2

This happens because \s is included in the extras parameter[1], telling tree-sitter to ignore whitespace (and therefore newlines) between any two characters.

Replacing \s by \t in extras causes tree-sitter-python to correctly reject newlines such as the above[2]. However, after doing so it longer escape newlines correctly inside brackets. Consider the following valid python:

a = (
  1 +
  2
)

This fails to parse because tree-sitter does not expect newlines at the end of lines 1 and 2. The scanner.cc logic to ignore line breaks inside bracket expressions depends on close bracket being a valid token[3], which it is not following an open paren or the plus operator.

Is disallowing arbitrary newlines in general while permitting them inside brackets something that is possible to accomplish with tree-sitter?

[1] https://github.com/tree-sitter/tree-sitter-python/blob/b14614e2144b8f9ee54deed5a24f3c6f51f9ffa8/grammar.js#L32

[2] To avoid rejecting all empty lines we'd also have to replace module: $ => repeat($._statement) with something like module: $ => repeat(choice($._statement, /\r?\n/))

[3] https://github.com/tree-sitter/tree-sitter-python/blob/b14614e2144b8f9ee54deed5a24f3c6f51f9ffa8/src/scanner.cc#L157

agirardeau commented 1 year ago

Seems like the correct approach would be to track open paren/brace/brackets in the delimiter stack

amaanq commented 1 year ago

Tracking brackets/parenthesis in the delimiter stack sounds a bit complex, but it might also solve this case

def foo(a):
    return (a.
bar)

PR welcome :)

theHamsta commented 1 year ago

I thought the policy of tree-sitter was too parse all valid code, but also make the most sense out of invalid code. Though your examples show that also valid code gets rejected. If there's a solution for this issue that would fix the failure cases, but also parse some invalid cases, maybe that could get favored over something that would require more scanner logic with state.

module: $ => repeat(choice($._statement, /\r?\n/))

At the moment, ; is ignored. ; can be an alternative to a newline for a next statement