tree-sitter / tree-sitter-typescript

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

Fix extends #204

Closed resolritter closed 2 years ago

resolritter commented 2 years ago

Checklist:

close #173

mjambon commented 2 years ago

thank you! You have to regenerate the files.

quolpr commented 2 years ago

@mjambon, could you check? 🙂 I would like this PR to be merged

hendrikvanantwerpen commented 2 years ago

I think this is fixed in the wrong place. The abstract keyword should not be part of the extends construct, but of new types. The following example is accepted by TSC.

// abstract can appear in *any* new signatures,
// not just in extends positions
type AbsCtor<T> = abstract new () => T;

It doesn't work with the current grammar, but I think will also not work with this change. I think the fix needs to be in the production for constructor_type.

resolritter commented 2 years ago

I think this is fixed in the wrong place. The abstract keyword should not be part of the extends construct, but of new types. The following example is accepted by TSC.

// abstract can appear in *any* new signatures,
// not just in extends positions
type AbsCtor<T> = abstract new () => T;

It doesn't work with the current grammar, but I think will also not work with this change. I think the fix needs to be in the production for constructor_type.

I've added a test for type Foo<T> = abstract new () => T; and moved the abstract to constructor_type and construct_signature as suggested.

resolritter commented 2 years ago

I don't know why CI is failing in https://github.com/tree-sitter/tree-sitter-typescript/runs/5040605929?check_suite_focus=true. This is the command I'm using locally before the commit

yarn build && yarn test

hendrikvanantwerpen commented 2 years ago

Does npm run bild && npm run test help?

resolritter commented 2 years ago

Using npm makes no difference. I cleared the cache with yarn cache clean and did a clean install with yarn install --frozen-lockfile but it still does not work. If it's relevant:

> uname -a
Linux user 5.16.1-arch1-1 #1 SMP PREEMPT Sun, 16 Jan 2022 11:39:23 +0000 x86_64 GNU/Linux
hendrikvanantwerpen commented 2 years ago

In that case I don't know. Hopefully one of the maintainers can help you out eventually.

If they do and you have the time, you could consider adding a CONTRIBUTING.md file to your PR with the proper build instructions to save others the trouble in the future.

mjambon commented 2 years ago

@resolritter maybe you're using a different version of tree-sitter than the one in CI (which says 0.20.0)? I don't know how CI is set up. Once you find out, it would be good to place your findings in a CONTRIBUTING.md file.

aryx commented 2 years ago

@resolritter would you mind rebasing and resubmit? We had a few people who run in parsing bugs that would be fixed by this PR!

resolritter commented 2 years ago

would you mind rebasing and resubmit?

Done, but I think you could have done that yourself since I have enabled "Allow edits and access to secrets by maintainers".