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

Run parser tests from the official TypeScript parser #10444

Merged
merged 2 commits into from Dec 3, 2019

Conversation

nicolo-ribaudo
Copy link
Member

Q                       A
Fixed Issues? Closes #6104
Tests Added + Pass? Yes
Any Dependency Changes? Yes, dev dependency
License MIT

The two commits can/should be reviewed one by one.

In the first commit, I abstracted the logic to run external Babel tests: I now that abstractions can introduce bugs, but I didn't want to have three different files to do basically the same thing 😛

In the second commit I added the tests from https://github.com/Microsoft/TypeScript. The whitelist is so big because many invalid JavaScript programs are accepted by the TypeScript parser without any error (because they report them in the typechecker, which is too strict compared to @babel/parser).

cc @babel/typescript

@babel-bot
Copy link
Collaborator

babel-bot commented Sep 15, 2019

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/11582/

@KFlash
Copy link

KFlash commented Sep 16, 2019

@nicolo-ribaudo Regarding the whitelist file. There are plenty of cases where the Babel TS parser doesn't fail / pass as it should which isn't related to the type checker..

Just to mention a few ... The current dynamic import isn't equal to what TS does, the subtract type node should also accept type reference and not only literal. See TS source.
There are other cases as well related to the class and function implementation etc.

I think I discovered around 60 different cases in total.

@nicolo-ribaudo
Copy link
Member Author

Yeah I know, but a significant part of them are for things like duplicate declarations, or invalid modifiers (e.g. static const x = 2 as a class field)

Makefile Outdated
test-typescript:
node scripts/tests/typescript

test-typescript-ci: bootstrap test-typescript
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will fail on make -j test-typescript-ci since test-typescript depends on bootstrap. Could you rebase on master?

const names = await fs.readdir(dir);

for (const name of names) {
//if (name !== "abstractProperty.ts") continue;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: code-like comments should be removed.

@nicolo-ribaudo nicolo-ribaudo force-pushed the ts-parser-tests branch 2 times, most recently from 75e6196 to 79fe856 Compare October 14, 2019 20:05
Copy link
Contributor

@JLHwung JLHwung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!

@nicolo-ribaudo nicolo-ribaudo force-pushed the ts-parser-tests branch 2 times, most recently from ae2b24a to 99ea52e Compare December 3, 2019 00:07
@nicolo-ribaudo
Copy link
Member Author

@nicolo-ribaudo nicolo-ribaudo merged commit e74efd2 into babel:master Dec 3, 2019
@nicolo-ribaudo nicolo-ribaudo deleted the ts-parser-tests branch December 3, 2019 00:09
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Mar 3, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: tests area: typescript outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants