fix(rule-tester): fixes on the same line are not fixed properly #8252
Conversation
|
Thanks for the PR, @StyleShit! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. |
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -1069,7 +1069,7 @@ class PanCamera extends FreeCamera { | |||
| }, | |||
| { | |||
| code: 'a;\n++b;', | |||
| output: 'a\n++b;', | |||
There was a problem hiding this comment.
This was a bug in the tests. See other tests above.
| @@ -1022,7 +1022,7 @@ ruleTester.run('space-infix-ops', rule, { | |||
| type Test=|string|(((() => void)))|string; | |||
| `, | |||
| output: ` | |||
| type Test = |string | (((() => void))) | string; | |||
| type Test = | string | (((() => void))) | string; | |||
There was a problem hiding this comment.
This should be the real fix. See this playground.
The test currently fails because it's being fixed to:
type Test = | string | (((() => void))) | string;(notice there are 2 spaces after the =)
The reason is probably because there are 2 fixes that add spaces before and after the relevant characters (= & |), which then results in 2 spaces.
Not sure how to approach this. I have 2 options in mind:
- Trimming the spaces after each fix somehow (probably a bad idea)
- Re-running the rule tester on the same line after each fix (is that what happens in real life? not sure if/how to do it though)
WDYT?
| @@ -63,7 +63,7 @@ export function applyFixes( | |||
| const end = fix.range[1]; | |||
|
|
|||
| // Remain it as a problem if it's overlapped or it's a negative range | |||
| if (lastPos >= start || start > end) { | |||
| if (lastPos > start || start > end) { | |||
There was a problem hiding this comment.
The actual fix that causes all of this mess 😅
There was a problem hiding this comment.
As per the comment at the top of the file - this code is directly forked from eslint core.
If it's broken here then this only changes tests run using our rule tester. Code run through eslint will have the old logic.
So either you've found a bug upstream or tests are running correctly, but correctly, but differently to how you expect.
If it's the former - the fix needs to land upstream before we land it here.
|
Closing for now as per above. |
Closes #8251