feat(eslint-plugin): [prefer-optional-chain] support suggesting `!foo || !foo.bar` as a valid match for the rule #5266

omril1 · 2022-06-27T23:02:33Z

PR Checklist

Addresses an existing open issue: fixes Enhancement: [prefer-optional-chain] Suggest !foo || !foo.bar as a valid match for the rule #5245
That issue was marked as accepting prs
Steps in CONTRIBUTING.md were taken

Overview

The code mostly duplicates the logic of the existing && case, so extracted the common pieces to functions, the hard part was overcoming the scoping and type inference when the control flow is moved to smaller functions.
I'm not sure yet if I should handle comparisons to null/undefined like the && matches, for now, it's not supported.
I think I found a bug with the "pre optional chained" code, it doesn't work when the first node is optionally chained.

nx-cloud · 2022-06-27T23:02:35Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit cf8c41b. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 47 targets

Sent with 💌 from NxCloud.

typescript-eslint · 2022-06-27T23:02:35Z

Thanks for the PR, @omril1!

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. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

netlify · 2022-06-27T23:02:39Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`cf8c41b`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/62ff996367db7a0009601d3f
😎 Deploy Preview	https://deploy-preview-5266--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

…ted-or-optional-chaining

codecov · 2022-06-28T04:48:49Z

Codecov Report

Merging #5266 (cf8c41b) into main (c950051) will increase coverage by 2.34%.
The diff coverage is 96.36%.

@@            Coverage Diff             @@
##             main    #5266      +/-   ##
==========================================
+ Coverage   91.57%   93.92%   +2.34%     
==========================================
  Files         132      290     +158     
  Lines        1496    10017    +8521     
  Branches      226     3016    +2790     
==========================================
+ Hits         1370     9408    +8038     
- Misses         62      330     +268     
- Partials       64      279     +215

Flag	Coverage Δ
unittest	`93.92% <96.36%> (+2.34%)`	⬆️

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

Impacted Files	Coverage Δ
...-plugin/src/rules/explicit-function-return-type.ts	`97.72% <0.00%> (ø)`
packages/eslint-plugin/src/rules/require-await.ts	`90.47% <0.00%> (ø)`
.../eslint-plugin/src/rules/no-useless-constructor.ts	`95.23% <100.00%> (ø)`
...s/eslint-plugin/src/rules/prefer-optional-chain.ts	`98.60% <100.00%> (ø)`
packages/eslint-plugin/src/rules/return-await.ts	`99.05% <100.00%> (ø)`
...-plugin/src/rules/restrict-template-expressions.ts	`100.00% <0.00%> (ø)`
packages/eslint-plugin/src/rules/no-unsafe-call.ts	`100.00% <0.00%> (ø)`
...plugin/src/rules/naming-convention-utils/schema.ts	`100.00% <0.00%> (ø)`
...ckages/eslint-plugin/src/util/getESLintCoreRule.ts	`75.00% <0.00%> (ø)`
... and 153 more

…ted-or-optional-chaining

omril1 · 2022-07-02T14:29:05Z

packages/eslint-plugin/docs/rules/prefer-optional-chain.md

+// With negated `or`s
+!foo || !foo.bar;
+!foo || !foo[bar];
+!foo || !foo.bar || !foo.bar.baz || !foo.bar.baz();
+


I think the examples should be arranged in a table, need some opinions

…ted-or-optional-chaining

JoshuaKGoldberg

Thanks for starting on this work!

The PR description right now mentions you're still working on some things, and there are some TODOs commented in. I'll convert this to draft so we don't review before it's ready.

If you'd find it helpful, you can always comment on individual lines to ask us questions!

packages/eslint-plugin/src/rules/require-await.ts

packages/eslint-plugin/src/rules/prefer-optional-chain.ts

…ted-or-optional-chaining

omril1 · 2022-07-29T14:06:37Z

packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts

+  // TODO: deepest left node already pre-optional chained
+  // {
+  //   code: 'foo?.bar && foo.bar?.() && foo.bar?.().baz',
+  //   output: 'foo?.bar?.()?.baz',
+  // },


@JoshuaKGoldberg This TODO is a false-negative that I found in the existing && matches, not related to the !x || matches that I'm adding

Oh? Is this a new issue that should be filed, then?

I'll open one and see when I can attend to it

JoshuaKGoldberg

Okie dokie! This is a big PR surface area with lots of potential edge cases to consider, but you've done a great job covering that logic. Very nicely done! Thanks for your perseverance @omril1! 👏

Marking as approved, but since I'm not 100% confident and this PR has a lot of logic, I think someone else should also approve before merging.

…ted-or-optional-chaining

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.33.1` -> `5.35.1`](https://renovatebot.com/diffs/npm/@typescript-eslint%2feslint-plugin/5.33.1/5.35.1) | | [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.33.1` -> `5.35.1`](https://renovatebot.com/diffs/npm/@typescript-eslint%2fparser/5.33.1/5.35.1) | --- ### Release Notes <details> <summary>typescript-eslint/typescript-eslint (@typescript-eslint/eslint-plugin)</summary> ### [`v5.35.1`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#5351-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5350v5351-2022-08-24) [Compare Source](typescript-eslint/typescript-eslint@v5.35.0...v5.35.1) ##### Bug Fixes - **eslint-plugin:** correct rule schemas to pass ajv validation ([#5531](typescript-eslint/typescript-eslint#5531)) ([dbf8b56](typescript-eslint/typescript-eslint@dbf8b56)) ### [`v5.35.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#5350-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5340v5350-2022-08-24) [Compare Source](typescript-eslint/typescript-eslint@v5.34.0...v5.35.0) ##### Features - **eslint-plugin:** \[explicit-member-accessibility] suggest adding explicit accessibility specifiers ([#5492](typescript-eslint/typescript-eslint#5492)) ([0edb94a](typescript-eslint/typescript-eslint@0edb94a)) ### [`v5.34.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#5340-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5331v5340-2022-08-22) [Compare Source](typescript-eslint/typescript-eslint@v5.33.1...v5.34.0) ##### Bug Fixes - **eslint-plugin:** \[no-useless-constructor] handle parameter decorator ([#5450](typescript-eslint/typescript-eslint#5450)) ([864dbcf](typescript-eslint/typescript-eslint@864dbcf)) ##### Features - **eslint-plugin:** \[prefer-optional-chain] support suggesting `!foo || !foo.bar` as a valid match for the rule ([#5266](typescript-eslint/typescript-eslint#5266)) ([aca935c](typescript-eslint/typescript-eslint@aca935c)) #### [5.33.1](typescript-eslint/typescript-eslint@v5.33.0...v5.33.1) (2022-08-15) ##### Bug Fixes - missing placeholders in violation messages for `no-unnecessary-type-constraint` and `no-unsafe-argument` (and enable `eslint-plugin/recommended` rules internally) ([#5453](typescript-eslint/typescript-eslint#5453)) ([d023910](typescript-eslint/typescript-eslint@d023910)) </details> <details> <summary>typescript-eslint/typescript-eslint (@typescript-eslint/parser)</summary> ### [`v5.35.1`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#5351-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5350v5351-2022-08-24) [Compare Source](typescript-eslint/typescript-eslint@v5.35.0...v5.35.1) **Note:** Version bump only for package [@typescript-eslint/parser](https://github.com/typescript-eslint/parser) ### [`v5.35.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#5350-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5340v5350-2022-08-24) [Compare Source](typescript-eslint/typescript-eslint@v5.34.0...v5.35.0) **Note:** Version bump only for package [@typescript-eslint/parser](https://github.com/typescript-eslint/parser) ### [`v5.34.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#5340-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5331v5340-2022-08-22) [Compare Source](typescript-eslint/typescript-eslint@v5.33.1...v5.34.0) **Note:** Version bump only for package [@typescript-eslint/parser](https://github.com/typescript-eslint/parser) #### [5.33.1](typescript-eslint/typescript-eslint@v5.33.0...v5.33.1) (2022-08-15) **Note:** Version bump only for package [@typescript-eslint/parser](https://github.com/typescript-eslint/parser) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [x] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Co-authored-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1519 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.35.1` -> `5.36.1`](https://renovatebot.com/diffs/npm/@typescript-eslint%2feslint-plugin/5.35.1/5.36.1) | | [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.35.1` -> `5.36.1`](https://renovatebot.com/diffs/npm/@typescript-eslint%2fparser/5.35.1/5.36.1) | --- ### Release Notes <details> <summary>typescript-eslint/typescript-eslint (@typescript-eslint/eslint-plugin)</summary> ### [`v5.36.1`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#5361-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5360v5361-2022-08-30) [Compare Source](typescript-eslint/typescript-eslint@v5.36.0...v5.36.1) **Note:** Version bump only for package [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/eslint-plugin) ### [`v5.36.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#5360-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5351v5360-2022-08-30) [Compare Source](typescript-eslint/typescript-eslint@v5.35.1...v5.36.0) ##### Bug Fixes - **eslint-plugin:** revert [#5266](typescript-eslint/typescript-eslint#5266) ([#5564](typescript-eslint/typescript-eslint#5564)) ([7a8afe2](typescript-eslint/typescript-eslint@7a8afe2)) ##### Features - support TypeScript 4.8 ([#5551](typescript-eslint/typescript-eslint#5551)) ([81450ed](typescript-eslint/typescript-eslint@81450ed)) #### [5.35.1](typescript-eslint/typescript-eslint@v5.35.0...v5.35.1) (2022-08-24) ##### Bug Fixes - **eslint-plugin:** correct rule schemas to pass ajv validation ([#5531](typescript-eslint/typescript-eslint#5531)) ([dbf8b56](typescript-eslint/typescript-eslint@dbf8b56)) </details> <details> <summary>typescript-eslint/typescript-eslint (@typescript-eslint/parser)</summary> ### [`v5.36.1`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#5361-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5360v5361-2022-08-30) [Compare Source](typescript-eslint/typescript-eslint@v5.36.0...v5.36.1) **Note:** Version bump only for package [@typescript-eslint/parser](https://github.com/typescript-eslint/parser) ### [`v5.36.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#5360-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5351v5360-2022-08-30) [Compare Source](typescript-eslint/typescript-eslint@v5.35.1...v5.36.0) **Note:** Version bump only for package [@typescript-eslint/parser](https://github.com/typescript-eslint/parser) #### [5.35.1](typescript-eslint/typescript-eslint@v5.35.0...v5.35.1) (2022-08-24) **Note:** Version bump only for package [@typescript-eslint/parser](https://github.com/typescript-eslint/parser) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1530 Reviewed-by: crapStone <crapstone@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

feat/issue5245-negated-or-optional-chaining

cfded63

bradzacher added the enhancement: plugin rule option New rule option for an existing eslint-plugin rule label Jun 27, 2022

Merge remote-tracking branch 'upstream/main' into feat/issue5245-nega…

741d5e3

…ted-or-optional-chaining

Omri Luzon added 4 commits June 29, 2022 05:14

Merge remote-tracking branch 'upstream/main' into feat/issue5245-nega…

5f6cb28

…ted-or-optional-chaining

fix lint errors with the updated rule

931274c

Merge remote-tracking branch 'upstream/main' into feat/issue5245-nega…

d0f7c3a

…ted-or-optional-chaining

reduce duplication

fdd99e0

omril1 commented Jul 2, 2022

View reviewed changes

Omri Luzon added 3 commits July 2, 2022 21:43

coverage

92f4285

coverage

e63bd2f

coverage

393d182

omril1 marked this pull request as ready for review July 3, 2022 01:37

Omri Luzon added 2 commits July 17, 2022 20:54

Merge branch 'main' into feat/issue5245-negated-or-optional-chaining

7a1fa82

Merge remote-tracking branch 'upstream/main' into feat/issue5245-nega…

70319e9

…ted-or-optional-chaining

JoshuaKGoldberg reviewed Jul 29, 2022

View reviewed changes

JoshuaKGoldberg marked this pull request as draft July 29, 2022 05:02

Omri Luzon added 4 commits July 29, 2022 14:35

CR fixes

aa0063f

Merge remote-tracking branch 'upstream/main' into feat/issue5245-nega…

54c2ba7

…ted-or-optional-chaining

refactor more duplications

265a1e8

better

f946a99

omril1 marked this pull request as ready for review July 29, 2022 14:02

Remove commented " == undefined" tests in the "negated or" rule

078bd0d

omril1 commented Jul 29, 2022

View reviewed changes

Merge branch 'main' into feat/issue5245-negated-or-optional-chaining

6b1244c

JoshuaKGoldberg previously approved these changes Aug 7, 2022

View reviewed changes

JoshuaKGoldberg added the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Aug 7, 2022

Merge remote-tracking branch 'upstream/main' into feat/issue5245-nega…

1f35a88

…ted-or-optional-chaining

omril1 dismissed JoshuaKGoldberg’s stale review via 1f35a88 August 19, 2022 12:59

fix description conflict in md

cf8c41b

bradzacher approved these changes Aug 21, 2022

View reviewed changes

bradzacher changed the title ~~feat(eslint-plugin): [prefer-optional-chain] Support suggesting !foo || !foo.bar as a valid match for the rule~~ feat(eslint-plugin): [prefer-optional-chain] support suggesting !foo || !foo.bar as a valid match for the rule Aug 21, 2022

bradzacher merged commit aca935c into typescript-eslint:main Aug 21, 2022

omril1 deleted the feat/issue5245-negated-or-optional-chaining branch August 21, 2022 16:36

bradzacher mentioned this pull request Aug 29, 2022

Rule: "@typescript-eslint/prefer-optional-chain" fails with "Error: Unexpected member object type: TSNonNullExpression" #5553

Closed

4 tasks

bradzacher added a commit that referenced this pull request Aug 29, 2022

feat(eslint-plugin): revert #5266

07f92dd

bradzacher mentioned this pull request Aug 29, 2022

fix(eslint-plugin): revert #5266 #5564

Merged

3 tasks

bradzacher added a commit that referenced this pull request Aug 29, 2022

fix(eslint-plugin): revert #5266

38d70a0

bradzacher added a commit that referenced this pull request Aug 29, 2022

fix(eslint-plugin): revert #5266 (#5564)

7a8afe2

bradzacher mentioned this pull request Aug 29, 2022

Enhancement: [prefer-optional-chain] Suggest !foo || !foo.bar as a valid match for the rule #5245

Closed

4 tasks

github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(eslint-plugin): [prefer-optional-chain] support suggesting `!foo || !foo.bar` as a valid match for the rule #5266

feat(eslint-plugin): [prefer-optional-chain] support suggesting `!foo || !foo.bar` as a valid match for the rule #5266

omril1 commented Jun 27, 2022 •

edited

Loading

nx-cloud bot commented Jun 27, 2022 •

edited

Loading

typescript-eslint bot commented Jun 27, 2022

netlify bot commented Jun 27, 2022 •

edited

Loading

codecov bot commented Jun 28, 2022 •

edited

Loading

omril1 Jul 2, 2022 •

edited

Loading

JoshuaKGoldberg left a comment •

edited

Loading

omril1 Jul 29, 2022

JoshuaKGoldberg Aug 7, 2022

omril1 Aug 8, 2022

JoshuaKGoldberg left a comment

feat(eslint-plugin): [prefer-optional-chain] support suggesting !foo || !foo.bar as a valid match for the rule #5266

feat(eslint-plugin): [prefer-optional-chain] support suggesting !foo || !foo.bar as a valid match for the rule #5266

Conversation

omril1 commented Jun 27, 2022 • edited Loading

PR Checklist

Overview

nx-cloud bot commented Jun 27, 2022 • edited Loading

☁️ Nx Cloud Report

typescript-eslint bot commented Jun 27, 2022

netlify bot commented Jun 27, 2022 • edited Loading

✅ Deploy Preview for typescript-eslint ready!

codecov bot commented Jun 28, 2022 • edited Loading

Codecov Report

omril1 Jul 2, 2022 • edited Loading

Choose a reason for hiding this comment

JoshuaKGoldberg left a comment • edited Loading

Choose a reason for hiding this comment

omril1 Jul 29, 2022

Choose a reason for hiding this comment

JoshuaKGoldberg Aug 7, 2022

Choose a reason for hiding this comment

omril1 Aug 8, 2022

Choose a reason for hiding this comment

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

feat(eslint-plugin): [prefer-optional-chain] support suggesting `!foo || !foo.bar` as a valid match for the rule #5266

feat(eslint-plugin): [prefer-optional-chain] support suggesting `!foo || !foo.bar` as a valid match for the rule #5266

omril1 commented Jun 27, 2022 •

edited

Loading

nx-cloud bot commented Jun 27, 2022 •

edited

Loading

netlify bot commented Jun 27, 2022 •

edited

Loading

codecov bot commented Jun 28, 2022 •

edited

Loading

omril1 Jul 2, 2022 •

edited

Loading

JoshuaKGoldberg left a comment •

edited

Loading