Materialize expression_statement nodes

tree-sitter / tree-sitter-go

Go grammar for tree-sitter

MIT License

310 stars 65 forks source link

Materialize expression_statement nodes #73

Closed adonovan closed 1 year ago

adonovan commented 2 years ago

This change causes the parser to materialize expression_statement nodes for calls and receive expressions that appear as a statement, so that every child of a block is a statement (or comment).

REVIEWERS: why does test --update make a bunch of unnecessary whitespace changes to the corpus/*.txt files? I'm using ./node_modules/tree-sitter-cli/tree-sitter (v0.19.5) for this step. Should I be doing something different?

Checklist:

[ ] All tests pass in CI.
[ ] There are sufficient tests for the new fix/feature.
[ ] Grammar rules have not been renamed unless absolutely necessary.
[ ] The conflicts section hasn't grown too much.
[ ] The parser size hasn't grown too much (check the value of STATE_COUNT in src/parser.c).

maxbrunsfeld commented 2 years ago

every child of a block is a statement

This seems like a good change.

why does test --update make a bunch of unnecessary whitespace changes?

It is reformatting the S-expressions. I'm ok with committing these changes. For now, we'll just need to verify that no ERROR or MISSING nodes have been introduced.

maxbrunsfeld commented 2 years ago

Can you upgrade to the latest Tree-sitter and regenerate with that? Currently, the latest version is 0.20.6.

josharian commented 1 year ago

Ping. We'd like to have materialized expression statements in cursorless: https://github.com/cursorless-dev/cursorless/issues/1592.

I'd weakly prefer to only include call and receive expressions (and set aside the spec restrictions around certain predeclared identifiers), but what is in this PR is definitely good enough for us for now.

(Also: hi, @adonovan! Just about every time I have a Go-related issue, I find you've already got a fix ready for it. Amazing.)

amaanq commented 1 year ago

Could you rebase?

josharian commented 1 year ago

I know this is Alan's PR, but in case it is helpful, I rebased this commit and regenerated with tree-sitter 0.20.8. (Please do double-check my work if you want to use it.)

https://github.com/josharian/tree-sitter-go/commit/309e76588c33af4b9b48ea2b37751be01b21e253

Note that I found that regenerating at master with 0.20.8 made changes as well, so those would be mixed in here.

amaanq commented 1 year ago

If you want you can PR that, but remove the comments added, especially the LL one - but otherwise I think it looks good

josharian commented 1 year ago

Sent https://github.com/tree-sitter/tree-sitter-go/pull/124.

amaanq commented 1 year ago

Superseded by #124