Skip to content

Enhancement: Tests that visitor keys visit fields in source code order #11282

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.

Sign up for GitHub

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

Jump to bottom
Open
4 tasks done
overlookmotel opened this issue Jun 7, 2025 · 0 comments
Open
4 tasks done

Enhancement: Tests that visitor keys visit fields in source code order #11282

overlookmotel opened this issue Jun 7, 2025 · 0 comments
Labels
enhancement New feature or request triage Waiting for team members to take a look

Comments

@overlookmotel
Copy link
Contributor

overlookmotel commented Jun 7, 2025 •
edited
Loading

Before You File a Proposal Please Confirm You Have Done The Following...

Relevant Package

visitor-keys

My proposal is suitable for this project

  • I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Description

A comment in visitor-keys package states that keys "should be sorted in the order that they appear in the source code".

typescript-eslint/packages/visitor-keys/src/visitor-keys.ts

Lines 111 to 132 in 0f5c59c

/*
********************************** IMPORTANT NOTE ********************************
* *
* The key arrays should be sorted in the order in which you would want to visit *
* the child keys. *
* *
* DO NOT SORT THEM ALPHABETICALLY! *
* *
* They should be sorted in the order that they appear in the source code. *
* For example: *
* *
* class Foo extends Bar { prop: 1 } *
* ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ClassDeclaration *
* ^^^ id ^^^ superClass *
* ^^^^^^^^^^^ body *
* *
* It would be incorrect to provide the visitor keys ['body', 'id', 'superClass'] *
* because the body comes AFTER everything else in the source code. *
* Instead the correct ordering would be ['id', 'superClass', 'body']. *
* *
**********************************************************************************
*/

#11276 related to 5 types where this was not the case (fixed in #11279).

The field orders for standard ESTree types are inherited from eslint-visitor-keys, which are outside of TS-ESLint's control.

After #11279 and eslint/js#656, visitor keys for all types are in source code order, as intended, but there are no tests for this. It would be ideal to have tests, to ensure this rule doesn't get broken again unintentionally in future.

Additional Info

I'm not sure how such tests would be implemented. Ideally you'd probably want to codegen them so they automatically cover any new AST nodes added in future, but then you'd need to run those tests on a large corpus of test fixtures to make sure they cover all syntax. But a vast corpus might make those tests extremely slow to run, so maybe it'd be something of a sledgehammer to crack a nut situation.
@overlookmotel overlookmotel added enhancement New feature or request triage Waiting for team members to take a look labels Jun 7, 2025
@overlookmotel overlookmotel mentioned this issue Jun 7, 2025
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request triage Waiting for team members to take a look
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@overlookmotel