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

feat(eslint-plugin): no-unsafe-enum-comparison handles switch cases #7898

Conversation

JoshuaKGoldberg
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg commented Nov 11, 2023

PR Checklist

Overview

Recreates #7541. Description pasted following.

Currently, the no-unsafe-enum-comparison rule handles comparisons (e.g. BinaryExpression) but it does not handle switch statements. My PR makes the rule handle both.

This was an oversight in my original design of this rule, and I consider this to be a bug. However, I have marked the PR as feat instead of fix to be more conservative; feel free to change it if you wish.

Code Change Summary

I refactored the logic in the BinaryExpression selector and put it inside of a function called isMismatchedComparison.
Then, I can call that function from multiple selectors. The changes should be pretty straightforward to understand.

Other Notes

I have reworked the documentation for this rule a bit, as the original text is now no longer accurate, because TypeScript has made number enums more safe in version 5.0. Thus, I rewrote the document to focus on the string case, adding a line to emphasize that the rule is still useful if you choose to use string enums over number enums.

@typescript-eslint
Copy link
Contributor

Thanks for the PR, @JoshuaKGoldberg!

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.

Copy link

netlify bot commented Nov 11, 2023

Deploy Preview for typescript-eslint ready!

Name Link
🔨 Latest commit a91d93a
🔍 Latest deploy log https://app.netlify.com/sites/typescript-eslint/deploys/6552211c8f48a6000872217b
😎 Deploy Preview https://deploy-preview-7898--typescript-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 98 (🔴 down 1 from production)
Accessibility: 100 (no change from production)
Best Practices: 92 (no change from production)
SEO: 98 (no change from production)
PWA: 80 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@JoshuaKGoldberg JoshuaKGoldberg added the 1 approval PR that a maintainer has LGTM'd - any maintainer can merge this when ready label Nov 11, 2023
@bradzacher bradzacher changed the title feat: no-unsafe-enum-comparison handles switch cases feat(eslint-plugin): no-unsafe-enum-comparison handles switch cases Nov 13, 2023
@JoshuaKGoldberg JoshuaKGoldberg merged commit 72cb9e4 into typescript-eslint:main Nov 13, 2023
46 of 47 checks passed
@JoshuaKGoldberg JoshuaKGoldberg deleted the enum-comparison-switch-merge branch November 13, 2023 16:02
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1 approval PR that a maintainer has LGTM'd - any maintainer can merge this when ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[no-unsafe-enum-comparison] consider evaluating switch statements
2 participants