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

prefer-math-trunc: Use suggestion instead of fix for x | 0 #1014

Merged
merged 9 commits into from Jan 13, 2021
Merged

prefer-math-trunc: Use suggestion instead of fix for x | 0 #1014

merged 9 commits into from Jan 13, 2021

Conversation

noftaly
Copy link
Contributor

@noftaly noftaly commented Jan 12, 2021

Use a suggestion rather than an autofix for the | 0 case, to prevent incorrectly transforming Int32's casts.

I've only done it for | 0 but I can do it for all the cases too (~~x, x >> 0, x << 0, x ^ 0).

I've not updated the test because I have no idea how to do so with the "visual tester". Does it support suggestions? If no, do I have to move all tests containing | 0 to the invalid array?
I've already done something like this, taking inspiration from the prefer-node-remove rule ; but I don't know if I should really move all of them, if this is the right thing to do, and mostly, how to deal with the "multiple-in-one" tests, such as const foo = ~~(bar| 0);

With the new suggestion support to the visuazizetester, this is no longer applicable

Fixes #911

@fisker
Copy link
Collaborator

fisker commented Jan 12, 2021

Suggestion support for visualizeTester was added few hours ago #1008, please rebase and update snapshots.

@noftaly
Copy link
Contributor Author

noftaly commented Jan 12, 2021

Suggestion support for visualizeTester was added few hours ago #1008, please rebase and update snapshots.

Yes I'm doing this right now, although I've never done it before, so it might take a little moment 😄

Update: I don't really know what I've done, a mix of multiple rebases, pulls, merge conflicts etc, but it seems like it worked!

@fisker fisker self-requested a review January 13, 2021 01:18
@sindresorhus sindresorhus merged commit 74b1b2d into sindresorhus:master Jan 13, 2021
@sindresorhus
Copy link
Owner

@noftaly Thanks for the PR 👍🏻

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.

Incorrect autofix for prefer-math-trunc rule
3 participants