eslint
eslint copied to clipboard
Change Request: RuleTester check suggestion fixers
ESLint version
8.12.0
What problem do you want to solve?
The rule tester currently checks if any fixes result in a parser error but it does include suggestion fixers.
What do you think is the correct solution?
I would propose to include suggestion fixes as well by checking each suggestion in isolation.
Participation
- [X] I am willing to submit a pull request for this change.
Additional comments
No response
Hi @DMartens, thanks for the issue!
I would propose to include suggestion fixes as well by checking each suggestion in isolation.
Can you please clarify this, I think I understand but just to be sure. The proposal is to check for each suggestion whether applying the suggested fix would produce parsing errors?
Yes, you understood correctly.
I said in isolation as there may be multiple errors each with multiple suggestions.
This differs from output
as all fixes from all reported errors are applied at once.
So there are multiple ways to handle it:
- Each suggestion in isolation (my suggestion)
- One suggestion per error (this would speed up the parser fail checks as there are less reparses)
- Apply all non-overlapping suggestions at once, repeat until none left
I chose (1) as
- Debuggability: In the other cases you don't know exactly which suggestion caused the error
- Performance: The advantage of the other is that but the test cases usually are small
- Simplicity: Easy to implement
Makes sense to me :+1:
The only downside I see is that the tests will take a bit more time to execute due to the additional parsing, but this seems worth it.
Note that this change requires a RFC, but first let's see what other team members think about this proposal.
- Each suggestion in isolation (my suggestion)
This seems like the most correct solution to implement this check. :+1:
Oops! It looks like we lost track of this issue. What do we want to do here? This issue will auto-close in 7 days without an update.
I want to add my support for this request.
Ideally, we would have included this change in a related RFC that has been approved but not implemented yet:
- https://github.com/eslint/rfcs/pull/84
I opened:
- https://github.com/eslint/rfcs/pull/101 - RFC
- https://github.com/eslint/eslint/pull/16639 - draft implementation