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 tonull/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.

feat/issue5245-negated-or-optional-chaining

cfded63

Copy link

nx-cloudbot commentedJun 27, 2022•
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commitcf8c41b. 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 💌 fromNxCloud.

Copy link

Contributor

typescript-eslintbot commentedJun 27, 2022

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 visitorsper day.

Copy link

netlifybot commentedJun 27, 2022•
edited
Loading

✅ Deploy Preview fortypescript-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 yourNetlify site settings.

bradzacher added the enhancement: plugin rule optionNew 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

Copy link

codecovbot commentedJun 28, 2022•
edited
Loading

Codecov Report

Merging#5266 (cf8c41b) intomain (c950051) willincrease coverage by2.34%.
The diff coverage is96.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%> (ø)`
... and153 more

Omri Luzon added4 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

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

Comment on lines +68 to +72

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

Copy link

ContributorAuthor

omril1Jul 2, 2022•
edited
Loading

Choose a reason for hiding this comment

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

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

Omri Luzon added3 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 added2 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

Copy link

Member

JoshuaKGoldberg left a comment•
edited
Loading

Choose a reason for hiding this comment

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

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 OutdatedShow resolvedHide resolved

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

packages/eslint-plugin/src/rules/prefer-optional-chain.tsShow resolvedHide resolved

JoshuaKGoldberg marked this pull request as draft

July 29, 2022 05:02

Omri Luzon added4 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

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

Comment on lines +135 to +139

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

Copy link

ContributorAuthor

omril1Jul 29, 2022

Choose a reason for hiding this comment

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

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

Copy link

Member

JoshuaKGoldbergAug 7, 2022

Choose a reason for hiding this comment

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

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

Copy link

ContributorAuthor

omril1Aug 8, 2022

Choose a reason for hiding this comment

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

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

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

6b1244c

JoshuaKGoldberg previously approved these changes

Aug 7, 2022

View reviewed changes

Copy link

Member

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

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.

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 dismissedJoshuaKGoldberg’sstale review via1f35a88

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 commitaca935c intotypescript-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

crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request

Sep 1, 2022

Update typescript-eslint monorepo to v5.35.1 (#1519)

bcad84e

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 (@&#8203;typescript-eslint/eslint-plugin)</summary>### [`v5.35.1`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#&#8203;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 ([#&#8203;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#&#8203;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 ([#&#8203;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#&#8203;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 ([#&#8203;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 ([#&#8203;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) ([#&#8203;5453](typescript-eslint/typescript-eslint#5453)) ([d023910](typescript-eslint/typescript-eslint@d023910))</details><details><summary>typescript-eslint/typescript-eslint (@&#8203;typescript-eslint/parser)</summary>### [`v5.35.1`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#&#8203;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 [@&#8203;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#&#8203;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 [@&#8203;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#&#8203;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 [@&#8203;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 [@&#8203;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] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.---This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4xNjkuMSIsInVwZGF0ZWRJblZlciI6IjMyLjE3My4wIn0=-->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/1519Reviewed-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>

crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request

Sep 1, 2022

Update typescript-eslint monorepo to v5.36.1 (#1530)

3e9e9f3

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 (@&#8203;typescript-eslint/eslint-plugin)</summary>### [`v5.36.1`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#&#8203;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 [@&#8203;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#&#8203;5360-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5351v5360-2022-08-30)[Compare Source](typescript-eslint/typescript-eslint@v5.35.1...v5.36.0)##### Bug Fixes-   **eslint-plugin:** revert [#&#8203;5266](typescript-eslint/typescript-eslint#5266) ([#&#8203;5564](typescript-eslint/typescript-eslint#5564)) ([7a8afe2](typescript-eslint/typescript-eslint@7a8afe2))##### Features-   support TypeScript 4.8 ([#&#8203;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 ([#&#8203;5531](typescript-eslint/typescript-eslint#5531)) ([dbf8b56](typescript-eslint/typescript-eslint@dbf8b56))</details><details><summary>typescript-eslint/typescript-eslint (@&#8203;typescript-eslint/parser)</summary>### [`v5.36.1`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#&#8203;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 [@&#8203;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#&#8203;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 [@&#8203;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 [@&#8203;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.--- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.---This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4xODIuNCIsInVwZGF0ZWRJblZlciI6IjMyLjE4Mi40In0=-->Co-authored-by: cabr2-bot <cabr2.help@gmail.com>Reviewed-on:https://codeberg.org/Calciumdibromid/CaBr2/pulls/1530Reviewed-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>

github-actionsbot locked asresolvedand limited conversation to collaborators

Sep 21, 2022

Labels

1 approval

>=1 team member has approved this PR; we're now leaving it open for more reviews before we merge

enhancement: plugin rule option

New rule option for an existing eslint-plugin rule

3 participants

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

Movatterモバイル変換

Uh oh!

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

Uh oh!

Conversation

omril1 commentedJun 27, 2022• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

PR Checklist

Overview

Uh oh!

nx-cloudbot commentedJun 27, 2022• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

☁️ Nx Cloud Report

Uh oh!

typescript-eslintbot commentedJun 27, 2022

Uh oh!

netlifybot commentedJun 27, 2022• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

✅ Deploy Preview fortypescript-eslint ready!

Uh oh!

codecovbot commentedJun 28, 2022• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Codecov Report

Uh oh!

omril1Jul 2, 2022• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoshuaKGoldberg left a comment• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

omril1Jul 29, 2022

Choose a reason for hiding this comment

Uh oh!

JoshuaKGoldbergAug 7, 2022

Choose a reason for hiding this comment

Uh oh!

omril1Aug 8, 2022

Choose a reason for hiding this comment

Uh oh!

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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 commentedJun 27, 2022•
edited
Loading

nx-cloudbot commentedJun 27, 2022•
edited
Loading

netlifybot commentedJun 27, 2022•
edited
Loading

codecovbot commentedJun 28, 2022•
edited
Loading

omril1Jul 2, 2022•
edited
Loading

JoshuaKGoldberg left a comment•
edited
Loading