elm-tooling / tree-sitter-elm

Tree sitter implementation for elm
https://elm-tooling.github.io/tree-sitter-elm/
MIT License
74 stars 12 forks source link

tree-sitter v0.20.8 causes several tests to fail #126

Closed SiriusStarr closed 2 years ago

SiriusStarr commented 2 years ago

The offending commit is https://github.com/tree-sitter/tree-sitter/commit/d223a81b5064587af3a5f61f52d519670ba8995f with the following failures:

3 failures:

expected / actual

  1. Simple glsl function no end:

    (file
      (value_declaration
      (ERROR
        (function_declaration_left
          (lower_case_identifier))
        (eq)
        (glsl_code_expr
          (glsl_content)
          (MISSING _glsl_end))))
        (glsl_content)))

  2. Incomplete model access:

    (file
      (value_declaration
        (function_declaration_left
          (lower_case_identifier)
          (lower_pattern
            (lower_case_identifier)))
        (eq)
        (function_call_expr
          (value_expr
            (value_qid
              (lower_case_identifier)))
          (list_expr)
          (list_expr
            (ERROR
            (field_access_expr
              (value_expr
                (value_qid
                  (lower_case_identifier)))
              (dot))))))
              (dot)
              (MISSING lower_case_identifier))))))

  3. Incomplete model access:

    (file
      (value_declaration
        (function_declaration_left
          (lower_case_identifier)
          (lower_pattern
            (lower_case_identifier)))
        (eq)
        (function_call_expr
          (value_expr
            (value_qid
              (lower_case_identifier)))
          (list_expr)
          (list_expr
            (ERROR
            (field_access_expr
              (value_expr
                (value_qid
                  (lower_case_identifier)))
              (dot))))))
              (dot)
              (MISSING lower_case_identifier))))))

This is likely causing a freeze downstream in Helix per helix-editor/helix#2997

razzeee commented 2 years ago

How did you get those tests to fail?

SiriusStarr commented 2 years ago

With a tree-sitter of lib version 0.28.0, tree-sitter generate -> tree-sitter test.

Here's the full process in a temp directory (assumes you have cargo, so nix-shell -p cargo or the equivalent for non-Nix):

git clone https://github.com/tree-sitter/tree-sitter
cd tree-sitter
make
cargo build --release
cd ..
git clone https://github.com/elm-tooling/tree-sitter-elm
cp ./tree-sitter/target/release/tree-sitter ./tree-sitter-elm/
cd tree-sitter-elm
./tree-sitter generate
./tree-sitter test

(I just tested this with the latest commit, which at this time is tree-sitter/tree-sitter@1f1b1eb4501ed0a2d195d37f7de15f72aa10acd0, but you can git checkout 01df16ca9faa1635909ebea242deac200624d9ee for the exact commit of lib 0.28.0 if you'd like before building.)

razzeee commented 2 years ago

My current feeling is, that it might be a lsp server problem. It doesn't expect the new tree's shape and that might cause our code to run in a loop?

the-mikedavis commented 2 years ago

The freeze in helix is caused by that linked change in tree-sitter. There was a similar freeze with the org parser which has a very similar scanner implementation: https://github.com/milisims/tree-sitter-org/issues/34, and their fix: https://github.com/milisims/tree-sitter-org/commit/698bb1a34331e68f83fc24bdd1b6f97016bb30de

It looks the failing tests are fixed by applying a similar patch to the scanner here. I haven't been able to reproduce the hang but I suspect it would be fixed by that patch too.