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

[prefer-optional-chain] fixer produces wrong logic #1438

Closed
bradleyayers opened this issue Jan 12, 2020 · 2 comments · Fixed by #5919
Closed

[prefer-optional-chain] fixer produces wrong logic #1438

bradleyayers opened this issue Jan 12, 2020 · 2 comments · Fixed by #5919
Labels
accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@bradleyayers
Copy link
Contributor

Repro

{
  "rules": {
    "@typescript-eslint/prefer-optional-chain": ["error"]
  }
}
declare const a: { b: string | null } | null;
a !== null && a.b !== null && a.b.length > 0;
/*
^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using an optional chain expression instead, as it's more concise and easier to read.
*/

Expected Result

a?.b != null && a.b.length > 0;

Actual Result

a?.b !== null && a.b.length > 0;
/*
                 ^ Object is possibly 'null'.
*/

Additional Info

Versions

package version
@typescript-eslint/eslint-plugin 2.15.0
@typescript-eslint/parser 2.15.0
TypeScript 3.7.3
ESLint 6.1.0
node 10.16.0
npm 6.9.0
@bradleyayers bradleyayers added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels Jan 12, 2020
@bradzacher
Copy link
Member

interesting.
Types wise, this doesn't work because of the !==, ?. produces an undefined. If you instead use != null, then it's fine.

The root cause of this is due to a limitation in the logic that I have been wanting (needing) to fix in that it only allows !=/!== binary expressions.

@bradzacher bradzacher added bug Something isn't working and removed triage Waiting for maintainers to take a look labels Jan 12, 2020
@glen-84
Copy link
Contributor

glen-84 commented Aug 3, 2021

I have a similar issue, but I use strict eqeqeq, so I wouldn't want the fix to use !=.

if (coreStore.activeSiteSection && coreStore.activeSiteSection.backgroundImageUrl !== null) {}

Wants to change to:

if (coreStore.activeSiteSection?.backgroundImageUrl !== null) {}

... but this has different behaviour, because undefined !== null, so it would enter the block. It shouldn't flag this case.

@JoshuaKGoldberg JoshuaKGoldberg added the accepting prs Go ahead, send a pull request that resolves this issue label Oct 25, 2021
JoshuaKGoldberg added a commit that referenced this issue Jan 26, 2023
#5919)

* fix(eslint-plugin): [prefer-optional-chain] fixer produces wrong logic (#1438)

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

* fix(eslint-plugin): [prefer-optional-chain] fix tests

* fix(eslint-plugin): [prefer-optional-chain] fix tests

Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com>
Co-authored-by: Святослав Зайцев <sz@agentapp.ru>
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this issue Feb 2, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.48.2` -> `5.50.0`](https://renovatebot.com/diffs/npm/@typescript-eslint%2feslint-plugin/5.48.2/5.50.0) |
| [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.48.2` -> `5.50.0`](https://renovatebot.com/diffs/npm/@typescript-eslint%2fparser/5.48.2/5.50.0) |

---

### Release Notes

<details>
<summary>typescript-eslint/typescript-eslint (@&#8203;typescript-eslint/eslint-plugin)</summary>

### [`v5.50.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#&#8203;5500-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5490v5500-2023-01-31)

[Compare Source](typescript-eslint/typescript-eslint@v5.49.0...v5.50.0)

##### Bug Fixes

-   **eslint-plugin:** \[ban-ts-comment] counts graphemes instead of `String.prototype.length` ([#&#8203;5704](typescript-eslint/typescript-eslint#5704)) ([09d57ce](typescript-eslint/typescript-eslint@09d57ce))
-   **eslint-plugin:** \[prefer-optional-chain] fix `ThisExpression` and `PrivateIdentifier` errors ([#&#8203;6028](typescript-eslint/typescript-eslint#6028)) ([85e783c](typescript-eslint/typescript-eslint@85e783c))
-   **eslint-plugin:** \[prefer-optional-chain] fixer produces wrong logic ([#&#8203;5919](typescript-eslint/typescript-eslint#5919)) ([b0f6c8e](typescript-eslint/typescript-eslint@b0f6c8e)), closes [#&#8203;1438](typescript-eslint/typescript-eslint#1438)

##### Features

-   **eslint-plugin:** add `key-spacing` rule extension for interface & type declarations ([#&#8203;6211](typescript-eslint/typescript-eslint#6211)) ([67706e7](typescript-eslint/typescript-eslint@67706e7))

### [`v5.49.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#&#8203;5490-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5482v5490-2023-01-23)

[Compare Source](typescript-eslint/typescript-eslint@v5.48.2...v5.49.0)

##### Features

-   **eslint-plugin:** \[naming-convention] add support for `#private` modifier on class members ([#&#8203;6259](typescript-eslint/typescript-eslint#6259)) ([c8a6d80](typescript-eslint/typescript-eslint@c8a6d80))

#### [5.48.2](typescript-eslint/typescript-eslint@v5.48.1...v5.48.2) (2023-01-16)

**Note:** Version bump only for package [@&#8203;typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/eslint-plugin)

#### [5.48.1](typescript-eslint/typescript-eslint@v5.48.0...v5.48.1) (2023-01-09)

**Note:** Version bump only for package [@&#8203;typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/eslint-plugin)

</details>

<details>
<summary>typescript-eslint/typescript-eslint (@&#8203;typescript-eslint/parser)</summary>

### [`v5.50.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#&#8203;5500-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5490v5500-2023-01-31)

[Compare Source](typescript-eslint/typescript-eslint@v5.49.0...v5.50.0)

**Note:** Version bump only for package [@&#8203;typescript-eslint/parser](https://github.com/typescript-eslint/parser)

### [`v5.49.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#&#8203;5490-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5482v5490-2023-01-23)

[Compare Source](typescript-eslint/typescript-eslint@v5.48.2...v5.49.0)

**Note:** Version bump only for package [@&#8203;typescript-eslint/parser](https://github.com/typescript-eslint/parser)

#### [5.48.2](typescript-eslint/typescript-eslint@v5.48.1...v5.48.2) (2023-01-16)

**Note:** Version bump only for package [@&#8203;typescript-eslint/parser](https://github.com/typescript-eslint/parser)

#### [5.48.1](typescript-eslint/typescript-eslint@v5.48.0...v5.48.1) (2023-01-09)

**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, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xMTQuMSIsInVwZGF0ZWRJblZlciI6IjM0LjExOS4yIn0=-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1757
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>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants