feat(eslint-plugin): [strict-boolean-expressions] check array predicate functions' return statements #10106

ronami · 2024-10-05T06:39:03Z

PR Checklist

Addresses an existing open issue: fixes Enhancement: [strict-boolean-expressions] Check array filter/find predicate functions as boolean locations. #8016
That issue was marked as accepting prs
Steps in Contributing were taken

Overview

This PR addresses #8016 and updates strict-boolean-expressions to check for array predicate return statements.

Two things to note:

Doesn't handle implicit returns

I didn't handle implicit return types from array predicates since I'm not sure if the rule should cover this:

[1, null].filter(x => {
  if (x) {
    return true;
  }

  // implicitly returns `undefined`
});

Please let me know if you think it should.

Somewhat of a breaking change

This is a breaking change, though the issue itself isn't marked as one, and according to the docs:

A change to the plugins shall not be considered breaking if it:

Adds additional checks to an existing rule that causes new reports in a small-to-medium set of cases in an average codebase.

Alternatively, I can put this behind a flag, which can be removed in a later version.

typescript-eslint · 2024-10-05T06:39:06Z

Thanks for the PR, @ronami!

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.

netlify · 2024-10-05T06:39:19Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`f17a05c`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/676d2a75a34d9400096b00f1
😎 Deploy Preview	https://deploy-preview-10106--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 98 (🟢 up 25 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.

nx-cloud · 2024-10-05T06:39:54Z

View your CI Pipeline Execution ↗ for commit f17a05c.

Command	Status	Duration	Result
`nx test eslint-plugin --coverage=false`	✅ Succeeded	7m 13s	View ↗
`nx test eslint-plugin`	✅ Succeeded	6m 1s	View ↗
`nx test visitor-keys --coverage=false`	✅ Succeeded	<1s	View ↗
`nx test utils --coverage=false`	✅ Succeeded	<1s	View ↗
`nx test typescript-eslint --coverage=false`	✅ Succeeded	8s	View ↗
`nx test type-utils --coverage=false`	✅ Succeeded	<1s	View ↗
`nx run types:build`	✅ Succeeded	<1s	View ↗
`nx run-many --target=build --exclude website --...`	✅ Succeeded	31s	View ↗
`Additional runs (24)`	✅ Succeeded	...	View ↗

☁️ Nx Cloud last updated this comment at 2024-12-26 10:20:31 UTC

codecov · 2024-10-05T06:53:30Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.86%. Comparing base (5a39e0c) to head (f17a05c).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10106      +/-   ##
==========================================
+ Coverage   86.84%   86.86%   +0.02%     
==========================================
  Files         445      445              
  Lines       15427    15455      +28     
  Branches     4497     4507      +10     
==========================================
+ Hits        13397    13425      +28     
  Misses       1675     1675              
  Partials      355      355

Flag	Coverage Δ
unittest	`86.86% <100.00%> (+0.02%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...int-plugin/src/rules/strict-boolean-expressions.ts	`100.00% <100.00%> (ø)`

JoshuaKGoldberg

Nice! One discussion on implicit undefined return, and some requests around testing. Direction seems great to me! 🚀

JoshuaKGoldberg · 2024-10-10T22:37:11Z

packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts

+        x.filter(t => {
+          return t ? 1 : 0;
+        });
+        x.filter(t => !t);


[Testing] We generally try to stick to one thing under test for each test case. For this one, it looks like two things are being tested:

That the allowNullableBoolean option still works...

...and that general logic holds true for all three variations of something passed to .filter

I don't think the latter is necessary. We can trim the tests down a bit:

Suggested change

x.filter(t => {

return t ? 1 : 0;

});

x.filter(t => !t);

Here and the other ones with several variations of the same, such as allowNullableString: true.

packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts

packages/eslint-plugin/src/rules/strict-boolean-expressions.ts

…e found

packages/eslint-plugin/src/rules/strict-boolean-expressions.ts

Co-authored-by: Kirk Waiblinger <kirk.waiblinger@gmail.com>

ronami · 2024-12-17T20:22:31Z

btw - sorry for taking so long to get around to a review on this!

No problem, I appreciate your assistance with the PR, it's been very helpful 👍

kirkwaiblinger

home stretch!

packages/eslint-plugin/src/rules/strict-boolean-expressions.ts

packages/eslint-plugin/src/util/misc.ts

kirkwaiblinger

I think we're there! Great work on this! I'm excited to get to use this feature ❤️

re-requesting

JoshuaKGoldberg

So thorough, love it! 🚀

@kirkwaiblinger

##### [v8.19.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8190-2024-12-30) ##### 🚀 Features - **eslint-plugin:** \[strict-boolean-expressions] check array predicate functions' return statements ([#10106](typescript-eslint/typescript-eslint#10106)) ##### 🩹 Fixes - **eslint-plugin:** \[member-ordering] ignore method overloading ([#10536](typescript-eslint/typescript-eslint#10536)) - **eslint-plugin:** \[consistent-indexed-object-style] don't report on indirect circular references ([#10537](typescript-eslint/typescript-eslint#10537)) - **eslint-plugin:** \[array-type] autofix with conditional types needs parentheses ([#10522](typescript-eslint/typescript-eslint#10522)) - **eslint-plugin:** add getConstraintInfo to handle generic constraints better ([#10496](typescript-eslint/typescript-eslint#10496)) ##### ❤️ Thank You - Karl Werner - Kirk Waiblinger [@kirkwaiblinger](https://github.com/kirkwaiblinger) - Ronen Amiel - YeonJuan [@yeonjuan](https://github.com/yeonjuan) You can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.

@kirkwaiblinger

| datasource | package | from | to | | ---------- | -------------------------------- | ------ | ------ | | npm | @typescript-eslint/eslint-plugin | 8.18.2 | 8.19.0 | | npm | @typescript-eslint/parser | 8.18.2 | 8.19.0 | ## [v8.19.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8190-2024-12-30) ##### 🚀 Features - **eslint-plugin:** \[strict-boolean-expressions] check array predicate functions' return statements ([#10106](typescript-eslint/typescript-eslint#10106)) ##### 🩹 Fixes - **eslint-plugin:** \[member-ordering] ignore method overloading ([#10536](typescript-eslint/typescript-eslint#10536)) - **eslint-plugin:** \[consistent-indexed-object-style] don't report on indirect circular references ([#10537](typescript-eslint/typescript-eslint#10537)) - **eslint-plugin:** \[array-type] autofix with conditional types needs parentheses ([#10522](typescript-eslint/typescript-eslint#10522)) - **eslint-plugin:** add getConstraintInfo to handle generic constraints better ([#10496](typescript-eslint/typescript-eslint#10496)) ##### ❤️ Thank You - Karl Werner - Kirk Waiblinger [@kirkwaiblinger](https://github.com/kirkwaiblinger) - Ronen Amiel - YeonJuan [@yeonjuan](https://github.com/yeonjuan) You can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.

ronami added 6 commits October 5, 2024 09:20

initial implementation

95525e0

tests

e6dd55c

refactor

1a898fe

some more tests

a16f46e

update docs

057a68e

update snapshots

d2ea1ab

ronami changed the title ~~Strict boolean expressions predicates~~ feat(eslint-plugin): [strict-boolean-expressions] Check array predicate return statements Oct 5, 2024

ronami changed the title ~~feat(eslint-plugin): [strict-boolean-expressions] Check array predicate return statements~~ feat(eslint-plugin): [strict-boolean-expressions] Check array predicate functions' return statements Oct 5, 2024

ronami changed the title ~~feat(eslint-plugin): [strict-boolean-expressions] Check array predicate functions' return statements~~ feat(eslint-plugin): [strict-boolean-expressions] check array predicate functions' return statements Oct 5, 2024

ronami marked this pull request as ready for review October 5, 2024 06:54

JoshuaKGoldberg previously requested changes Oct 10, 2024

View reviewed changes

JoshuaKGoldberg added the awaiting response Issues waiting for a reply from the OP or another party label Oct 10, 2024

ronami added 8 commits October 11, 2024 09:28

merge conflicts

116c80b

handle implicit or explicit undefined return types

d641dc9

refactor and update tests

8d997d1

remove unnecessary union check

47c167c

inline check

90f9ba8

cover empty return staetments

42a1336

report a function as returning undefined only if no other isssues wer…

655e52c

…e found

update comments

b8643c9

ronami commented Oct 11, 2024

View reviewed changes

packages/eslint-plugin/src/rules/strict-boolean-expressions.ts Outdated Show resolved Hide resolved

ronami added 2 commits October 11, 2024 19:02

add test

c665d82

remove unnecessary test

b3dd8be

kirkwaiblinger mentioned this pull request Oct 16, 2024

[array-callback-return] False positive when in a map that uses a switch case without default because all possibilities are handled but ESLint isn't aware of that #2841

Closed

3 tasks

ronami requested a review from JoshuaKGoldberg October 22, 2024 06:18

github-actions bot removed the awaiting response Issues waiting for a reply from the OP or another party label Oct 22, 2024

ronami and others added 4 commits December 17, 2024 21:09

use an existing helper rather than implementing one

d2a254a

update tests

6b32e50

Update packages/eslint-plugin/src/rules/strict-boolean-expressions.ts

c97d53c

Co-authored-by: Kirk Waiblinger <kirk.waiblinger@gmail.com>

fix codecov

ac6be7e

ronami requested a review from kirkwaiblinger December 17, 2024 20:22

kirkwaiblinger requested changes Dec 19, 2024

View reviewed changes

ronami added 4 commits December 19, 2024 20:35

cleanup old code, support type constraints

e94c0ee

refactor fixer

601dfba

remove unnecessary tests

1994ba6

Merge branch 'main' into strict-boolean-expressions-predicates

c71112b

ronami requested a review from kirkwaiblinger December 19, 2024 21:11

kirkwaiblinger requested changes Dec 26, 2024

View reviewed changes

packages/eslint-plugin/src/util/misc.ts Outdated Show resolved Hide resolved

ronami added 3 commits December 26, 2024 11:51

revert changes to isParenlessArrowFunction

8e54722

Merge branch 'main' into strict-boolean-expressions-predicates

abfb9f7

oops

f17a05c

ronami requested a review from kirkwaiblinger December 26, 2024 10:21

kirkwaiblinger approved these changes Dec 26, 2024

View reviewed changes

kirkwaiblinger added the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Dec 26, 2024

kirkwaiblinger requested review from JoshuaKGoldberg and removed request for JoshuaKGoldberg December 26, 2024 10:23

github-actions bot removed the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Dec 26, 2024

kirkwaiblinger added the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Dec 26, 2024

JoshuaKGoldberg approved these changes Dec 26, 2024

View reviewed changes

JoshuaKGoldberg merged commit 07d0e0d into typescript-eslint:main Dec 26, 2024
68 checks passed

ronami deleted the strict-boolean-expressions-predicates branch December 26, 2024 23:09

github-actions bot locked as resolved and limited conversation to collaborators Jan 3, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(eslint-plugin): [strict-boolean-expressions] check array predicate functions' return statements #10106

feat(eslint-plugin): [strict-boolean-expressions] check array predicate functions' return statements #10106

ronami commented Oct 5, 2024 •

edited

Loading

typescript-eslint bot commented Oct 5, 2024

netlify bot commented Oct 5, 2024 •

edited

Loading

nx-cloud bot commented Oct 5, 2024 •

edited

Loading

codecov bot commented Oct 5, 2024 •

edited

Loading

JoshuaKGoldberg left a comment

JoshuaKGoldberg Oct 10, 2024

ronami commented Dec 17, 2024

kirkwaiblinger left a comment

kirkwaiblinger left a comment

JoshuaKGoldberg left a comment

feat(eslint-plugin): [strict-boolean-expressions] check array predicate functions' return statements #10106

feat(eslint-plugin): [strict-boolean-expressions] check array predicate functions' return statements #10106

Conversation

ronami commented Oct 5, 2024 • edited Loading

PR Checklist

Overview

Doesn't handle implicit returns

Somewhat of a breaking change

typescript-eslint bot commented Oct 5, 2024

netlify bot commented Oct 5, 2024 • edited Loading

✅ Deploy Preview for typescript-eslint ready!

nx-cloud bot commented Oct 5, 2024 • edited Loading

codecov bot commented Oct 5, 2024 • edited Loading

Codecov Report

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

JoshuaKGoldberg Oct 10, 2024

Choose a reason for hiding this comment

ronami commented Dec 17, 2024

kirkwaiblinger left a comment

Choose a reason for hiding this comment

kirkwaiblinger left a comment

Choose a reason for hiding this comment

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

ronami commented Oct 5, 2024 •

edited

Loading

netlify bot commented Oct 5, 2024 •

edited

Loading

nx-cloud bot commented Oct 5, 2024 •

edited

Loading

codecov bot commented Oct 5, 2024 •

edited

Loading