Skip to content

feat: add typescript-eslint estree of typescript tests #6

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

Merged
merged 8 commits into from
Mar 14, 2025

Conversation

hi-ogawa
Copy link
Contributor

@hi-ogawa hi-ogawa commented Mar 14, 2025

Part of oxc-project/oxc#9705

I asked copilot to port TestCaseContent::make_units_from_test to js. The script is fairly rough and I haven't verified the correctness since most of estree conformance doesn't currently pass due to other reasons as well oxc-project/oxc#9774. Still this may be a good enough for starter, so opening a PR here.

Also I copied just clone-submodule from oxc repo since degit's commit hash cloning isn't working for me.

@hi-ogawa hi-ogawa marked this pull request as ready for review March 14, 2025 07:21
@Boshen
Copy link
Member

Boshen commented Mar 14, 2025

🤔

Sorry, this diff is taking too long to generate.
It may be too large to display on GitHub.

TypeScript has way too many files, perhaps Babel is a better candidate?

@hi-ogawa
Copy link
Contributor Author

hi-ogawa commented Mar 14, 2025

Right, this one is huge, but test262 is actually 4 times bigger, so I thought this might not be particularly a concern.

$ find test -type f | wc -l
44057
$ find test-typescript -type f | wc -l
10741

I'm happy to take a look babel if that's more tractable to start with.

@overlookmotel
Copy link
Contributor

overlookmotel commented Mar 14, 2025

I've reviewed locally. Personally, I think the number of files is fine. As @hi-ogawa says, Test262 produces far more files. And, from the point of view of test coverage, the more the better!

As we bring this repo into oxc as a submodule, I don't think it's a big concern.

I've pushed 1 commit just to format index.mjs, and going to merge.

If we find we have trouble with it downstream, then we can always reconsider.

@overlookmotel overlookmotel merged commit ad6f205 into main Mar 14, 2025
@overlookmotel overlookmotel deleted the feat-typescript branch March 14, 2025 08:36
@overlookmotel
Copy link
Contributor

And just to say, thanks loads @hi-ogawa for doing this.

graphite-app bot pushed a commit to oxc-project/oxc that referenced this pull request Mar 14, 2025
Part of #9705
Requires oxc-project/acorn-test262#6

Current result `Positive Passed: 32/10725 (0.30%)` 😢

What I observed from a few diffs I looked at:
- typescript-eslint parser's field ordering is not a super set of acorn (maybe keys are ordered alphabetically)
  - example: `ClassDeclaration`
    - typescript-eslint https://ast-explorer.dev/#eNolyDEKgDAMBdCrlD/rBTyIOGQJJUOlpqUpgpTe3YjrG8jYcPLNFluqHQuqQ3+q/LCK5aSfR/eY2SwcYUxSp+I0SEMgSLx4l2apKGFzyNzFOoF0Yr7njiIU
      - abstract, body, declare, decorators, id, implements, superClass, superTypeArguments, typeParameters,
    - acorn https://ast-explorer.dev/#eNolyTEKwzAMRuGrmH/OCXKOUjpoEaqGFMcyllMIxnePQrbH9wYyVvz4zy5tqx0LagCLtRIt0ZLZPX3SmHSTBQ0qKRFUdn5r880KYQ3I3NU7YXm+29FEX2fVZ+/2PXI0lYl5ATuZJ1U=
      - id, superClass, body
- typescript-eslint parser uses `undefined` instead of `null` for optional ts-specific field
  - example: `ClassDeclaration.typeParameters`
    - https://ast-explorer.dev/#eNolyDEKgDAMBdCrlD/rBTyIOGQJJUOlpqUpgpTe3YjrG8jYcPLNFluqHQuqQ3+q/LCK5aSfR/eY2SwcYUxSp+I0SEMgSLx4l2apKGFzyNzFOoF0Yr7njiIU
    - spec https://github.com/typescript-eslint/typescript-eslint/blob/2224f12ff05b59c5a459291fe93fc8099221089d/packages/ast-spec/src/base/ClassBase.ts#L62
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 this pull request may close these issues.

3 participants