Skip to content

feat(eslint-plugin): add rule prefer-find #8216

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

Merged
merged 15 commits into from
Feb 3, 2024

Conversation

kirkwaiblinger
Copy link
Member

@kirkwaiblinger kirkwaiblinger commented Jan 8, 2024

PR Checklist

Overview

Adds a rule that prefers arr.find(...) over arr.filter(...)[0]. Also adds the corresponding autofix as suggestions.

@typescript-eslint
Copy link
Contributor

Thanks for the PR, @kirkwaiblinger!

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 Jan 8, 2024

Deploy Preview for typescript-eslint ready!

Name Link
🔨 Latest commit b3e6db3
🔍 Latest deploy log https://app.netlify.com/sites/typescript-eslint/deploys/65bd1b939870fb0008e6d1c9
😎 Deploy Preview https://deploy-preview-8216--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: 95 (🟢 up 5 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.

Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A solid start, thanks for sending this in! Left some requests around edge cases to work on. Have fun 😄

Phoebe from Friends laughing, slightly pained maybe. Caption: 'SO MUCH FUN!'

@JoshuaKGoldberg JoshuaKGoldberg added the awaiting response Issues waiting for a reply from the OP or another party label Jan 9, 2024
@github-actions github-actions bot removed the awaiting response Issues waiting for a reply from the OP or another party label Jan 13, 2024
@bradzacher bradzacher added the enhancement: new plugin rule New rule request for eslint-plugin label Jan 28, 2024
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking fantastic! Left a few comments, but mostly just nitpicks. Really happy about the implementation. ☺️

I think the only remaining blocker is the 0n bigint tests?

@JoshuaKGoldberg JoshuaKGoldberg added the awaiting response Issues waiting for a reply from the OP or another party label Jan 30, 2024
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait sorry, (as you pointed out in Twitch chat), the 0ns are there! I just missed them!

So I think it's just a few nitpicky comments from me left. I'll set a reminder to merge before our next release if you don't have time or interest for them.

@JoshuaKGoldberg JoshuaKGoldberg added 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge and removed awaiting response Issues waiting for a reply from the OP or another party labels Jan 30, 2024
@kirkwaiblinger kirkwaiblinger requested a review from auvred January 31, 2024 17:27
@auvred
Copy link
Member

auvred commented Jan 31, 2024

looks good to me

Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aggressively enthusiastic guy on a sports field shouting 'let's goooo' in front of a crowd of people

@JoshuaKGoldberg JoshuaKGoldberg merged commit 3bbb78d into typescript-eslint:main Feb 3, 2024
danvk pushed a commit to danvk/typescript-eslint that referenced this pull request Feb 4, 2024
* Add rule prefer-find

* address lots of stuff

* remove console statement

* tweaks

* extract fix to function

* improve behavior around nulls

* add comments around array indexing checks

* messages were backwards

* filter syntax

* formatting

* add extra comma operator test

* pr feedback round 2

* Fix the the

Co-authored-by: auvred <61150013+auvred@users.noreply.github.com>

* fix up imports

* address intersections of arrays

---------

Co-authored-by: auvred <61150013+auvred@users.noreply.github.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 11, 2024
@kirkwaiblinger kirkwaiblinger deleted the prefer-find branch February 21, 2024 21:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge enhancement: new plugin rule New rule request for eslint-plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rule proposal: Preferring .find(...) over .filter(...)[0]
5 participants