typescript-eslint/typescript-eslintPublic

NotificationsYou must be signed in to change notification settings
Fork2.8k
Star15.8k

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

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

JoshuaKGoldberg merged 10 commits intotypescript-eslint:mainfromMillerSvt:fix/1438

Jan 26, 2023

Merged

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

JoshuaKGoldberg merged 10 commits intotypescript-eslint:mainfromMillerSvt:fix/1438

Jan 26, 2023

Conversation

Copy link

Contributor

sviat9440 commentedNov 2, 2022

PR Checklist

Addresses an existing open issue:fixes[prefer-optional-chain] fixer produces wrong logic #1438
That issue was marked asaccepting prs
Steps inCONTRIBUTING.md were taken

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

1bcc7f7

typescript-eslint#1438)

Copy link

Contributor

typescript-eslintbot commentedNov 2, 2022

Thanks for the PR,@sviat9440!

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

nx-cloudbot commentedNov 2, 2022•
edited
Loading

☁️ Nx Cloud Report

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

netlifybot commentedNov 2, 2022•
edited
Loading

✅ Deploy Preview fortypescript-eslint ready!

Name	Link
🔨 Latest commit	`47304f3`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/63d2a37b0ba2980009afad9a
😎 Deploy Preview	https://deploy-preview-5919--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 bugSomething isn't working label

Nov 2, 2022

Copy link

codecovbot commentedNov 2, 2022•
edited
Loading

Codecov Report

Merging#5919 (47304f3) intomain (033e87c) willincrease coverage by0.00%.
The diff coverage is100.00%.

Additional details and impacted files

@@           Coverage Diff           @@##             main    #5919   +/-   ##=======================================  Coverage   91.36%   91.37%           =======================================  Files         368      368             Lines       12592    12596    +4       Branches     3707     3709    +2     =======================================+ Hits        11505    11509    +4  Misses        772      772             Partials      315      315

Flag	Coverage Δ
unittest	`91.37% <100.00%> (+<0.01%)`	⬆️

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

Impacted Files	Coverage Δ
...s/eslint-plugin/src/rules/prefer-optional-chain.ts	`98.67% <100.00%> (+0.03%)`	⬆️

JoshuaKGoldberg previously approved these changes

Nov 13, 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.

A nice targeted change - thanks@sviat9440! 🔥

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

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

27a22ae

JoshuaKGoldberg dismissed theirstale review via27a22ae

November 13, 2022 23:48

Merge branch 'main'

c0083b1

JoshuaKGoldberg reviewed

Nov 14, 2022

View reviewed changes

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

JoshuaKGoldberg added the awaiting responseIssues waiting for a reply from the OP or another party label

Nov 14, 2022

sviat9440 added2 commits

November 15, 2022 06:34

Merge branch 'main' into fix/1438

ca2f338

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

787f938

sviat9440 commented

Nov 15, 2022

View reviewed changes

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

bradzacher changed the title~~fix(eslint-plugin): [prefer-optional-chain] fixer produces wrong logic (#1438)~~fix(eslint-plugin): [prefer-optional-chain] fixer produces wrong logic

Nov 16, 2022

bradzacher added awaiting responseIssues waiting for a reply from the OP or another party and removed awaiting responseIssues waiting for a reply from the OP or another party labels

Nov 16, 2022

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

874c21a

sviat9440 requested a review frombradzacher

November 21, 2022 05:46

Merge branch 'master' into fix/1438

b965ba3

bradzacher removed the awaiting responseIssues waiting for a reply from the OP or another party label

Nov 24, 2022

sviat9440 added2 commits

November 24, 2022 08:08

Merge branch 'main' into fix/1438

eab2a11

Merge branch 'main' into fix/1438

d9e4f18

bradzacher approved these changes

Dec 16, 2022

View reviewed changes

Copy link

Member

bradzacher left a comment

Choose a reason for hiding this comment

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

it's worth mentioning that a lot of people don't like inexact equal fornull. It's a stylistic thing 🤷

Given this is a suggestion fixer - it doesn't matter that we're fixing to!= because the fixer is entirely optional and user-driven.

But it's definitely a potential future piece of work to add a second suggestion fix to use!== null && !== undefined instead.
OFC it's also a lot harder to do that fix because you need to pay attention to other logic and precedence

Copy link

Member

bradzacher commentedDec 16, 2022

oops - if you can resolve the conflicts, then we can look at merging this in.

bradzacher added the awaiting responseIssues waiting for a reply from the OP or another party label

Dec 16, 2022

Merge branch 'main' into fix/1438

47304f3

JoshuaKGoldberg merged commitb0f6c8e intotypescript-eslint:main

Jan 26, 2023

sviat9440 deleted the fix/1438 branch

January 26, 2023 17:29

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

Feb 2, 2023

Update typescript-eslint monorepo to v5.50.0 (#1757)

067929b

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/1757Reviewed-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>