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

Top-level expressions cannot be parsed #198

Closed
keynmol opened this issue Apr 13, 2023 · 2 comments · Fixed by #271
Closed

Top-level expressions cannot be parsed #198

keynmol opened this issue Apr 13, 2023 · 2 comments · Fixed by #271

Comments

@keynmol
Copy link
Collaborator

keynmol commented Apr 13, 2023

Commit of tree-sitter-scala you tested this on

HEAD

A code sample showing the error

test-plugins.sbt

addSbtPlugin("com.indoorvivants.vcpkg" % "sbt-vcpkg-native" % "0.0.11")
// hello

Show the error node

Run with npm exec -- tree-sitter parse test-plugins.sbt

(compilation_unit [0, 0] - [2, 0]
  (ERROR [0, 0] - [2, 0]
    (comment [1, 0] - [1, 8])))
test-plugins.sbt        0 ms    (ERROR [0, 0] - [2, 0])

What do you expect the tree to look like

Correctly parsed

Where are you experiencing this error?

No response


Notes

This is technically not valid Scala, you can't have top level expressions:

val x = 25
println(x)

@main def hello = ???
Compiling project (Scala 3.2.0, JVM)
[error] ./test.scala:2:1: Illegal start of toplevel definition
[error] println(x)
[error] ^^^^^^^
Error compiling project (Scala 3.2.0, JVM)
Compilation failed

But it's valid in the context of SBT.

@olafurpg patched it in Sourcegraph's fork, but that breaks most tests: sourcegraph@6cd3cb6 so we will likely need to rejig other parts of the grammar.

The question of whether we allow it unconditionally is open, but for the purposes of syntax highlighting IMO we should.

@bishabosha
Copy link

I think given scala script files with scala-cli are now SIP approved, this is certainly more critical

@susliko
Copy link
Collaborator

susliko commented May 24, 2023

I took a look at the test suite errors arising from including $.expression into $._top_level_definition. It seems, that roughly a half of them are related to the left-associativity of class/object/trait definition (introduced in #102 in order to miminize memory usage). For example

class Foo() {}

is parsed as

(compilation_unit [0, 0] - [2, 0]
  (class_definition [0, 0] - [0, 11]
    name: (identifier [0, 6] - [0, 9])
    class_parameters: (class_parameters [0, 9] - [0, 11]))
  (block [0, 12] - [0, 14]))

(Now $.block is a valid top-level node as a part of $._simple_expression)

@eed3si9n fyi

susliko added a commit to susliko/tree-sitter-scala-1 that referenced this issue May 29, 2023
Summary
-------
`$.compilation_unit` is now a sequence of top-level stats separated by
`$.semicolon`

This change has two effects:
- Grammar optimization:
  - ~10% faster generation time
  - lower number of parser states ([before](https://gist.github.com/susliko/f950b997a98c54bbfd88969a949346fd), [after](https://gist.github.com/susliko/236a85dce46219c5868c494d7f5cf629))
  - parser size reduction from 43M to 36M
- It seems to me, that handling `$._automatic_semicolon` on the top level is a
  prerequisite to support top-level expressions (tree-sitter#198) and leading infix operators (tree-sitter#141)
susliko added a commit to susliko/tree-sitter-scala-1 that referenced this issue May 29, 2023
Summary
-------
`$.compilation_unit` is now a sequence of top-level stats separated by
`$.semicolon`

This change has two effects:
- Grammar optimization:
  - ~10% faster generation time
  - lower number of parser states ([before](https://gist.github.com/susliko/f950b997a98c54bbfd88969a949346fd), [after](https://gist.github.com/susliko/236a85dce46219c5868c494d7f5cf629))
  - parser size reduction from 43M to 36M
- It seems to me, that handling `$._automatic_semicolon` on the top level is a
  prerequisite to support top-level expressions (tree-sitter#198) and leading infix operators (tree-sitter#141)
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