#723 requests two things: 1. keep type annotation, 2. add an option to allow non-destructuring assignment for type annotated ones.
For the first one, I just disable auto fixer for declaration with type annotation, rather than providing correct auto fix. It's just because difficulty and maitainability.
For the second one, I addenforceForDeclarationWithTypeAnnotation option. I set default valuefalse because I agree with "annotations on destructured variables aren't very pretty" in the issue.

#1931 requests type-aware version and I have implemented it for numeric index access for non-iterable object, that is the only case reported in the issue (excluding comments that are duplication of#723).
There is a notable thing for it. I treated numeric index access for non-iterable object as renamed one. So it is affected byenforceForRenamedProperties option.

seiyab added12 commits

June 11, 2023 07:58

copy prefer-destructuring from ESLint

8e328f6

rewrite schema by hand

392f132

add enforceForTypeAnnotatedProperties option

4435c44

autofix is still wrong.

add tests

e5ab24f

prevent fixing type-annotated declaration

90e094f

add tests

1619def

move baseTests into end of file

8bcb921

refactor

47b9e6c

consider numeric properties of non-itreable objects for VariableDecla…

e997503

…rator

consider numeric properties of non-itreable objects for AssignmentEx…

375f99c

…pression

fix a bug

05680aa

fix a bug

a687993

Copy link

Contributor

typescript-eslintbot commentedJun 18, 2023

Thanks for the PR,@seiyab!

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

netlifybot commentedJun 18, 2023•
edited
Loading

✅ Deploy Preview fortypescript-eslint ready!

Name	Link
🔨 Latest commit	`77b6704`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/64da426db8f1bd0008bf99dc
😎 Deploy Preview	https://deploy-preview-7117--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 configuration.

seiyab changed the title~~Prefer destructuring~~Add prefer destructuring extension

Jun 18, 2023

Copy link

nx-cloudbot commentedJun 18, 2023•
edited
Loading

☁️ Nx Cloud Report

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

Sent with 💌 fromNxCloud.

seiyab added3 commits

June 18, 2023 23:20

add "openjsf" into the dictionary

0fbc758

add prefer-destructuring into all

7951154

add doc and minor tweak

d3626c6

seiyab changed the title~~Add prefer destructuring extension~~feat(eslint-plugin): add new extended ruleprefer-destructuring

Jun 20, 2023

seiyab marked this pull request as ready for review

June 20, 2023 13:35

rubiesonthesky reviewed

Jun 20, 2023

View reviewed changes

packages/eslint-plugin/docs/rules/prefer-destructuring.md OutdatedShow resolvedHide resolved

rubiesonthesky reviewed

Jun 20, 2023

View reviewed changes

packages/eslint-plugin/docs/rules/prefer-destructuring.md OutdatedShow resolvedHide resolved

seiyab added2 commits

June 21, 2023 04:56

fix typo

93c179c

fix incorrect "correct" case

5ebadbf

Copy link

codecovbot commentedJun 20, 2023•
edited
Loading

Codecov Report

Merging#7117 (77b6704) intomain (479f9f6) willincrease coverage by0.06%.
The diff coverage is100.00%.

Additional details and impacted files

@@            Coverage Diff             @@##             main    #7117      +/-   ##==========================================+ Coverage   85.25%   85.31%   +0.06%==========================================  Files         386      387       +1       Lines       13358    13417      +59       Branches     3943     3962      +19     ==========================================+ Hits        11388    11447      +59  Misses       1593     1593                Partials      377      377

Flag	Coverage Δ
unittest	`85.31% <100.00%> (+0.06%)`	⬆️

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

Files Changed	Coverage Δ
...ckages/eslint-plugin/src/util/getESLintCoreRule.ts	`75.00% <ø> (ø)`
...es/eslint-plugin/src/rules/prefer-destructuring.ts	`100.00% <100.00%> (ø)`

seiyab added4 commits

June 21, 2023 19:06

improve test coverage

5d1e5be

improve test coverage

2d7e2af

Merge branch 'main' into prefer-destructuring

7199034

Merge branch 'main' into prefer-destructuring

854f1c1

Copy link

Member

JoshuaKGoldberg commentedAug 5, 2023

I also took the liberty of merging in frommain, sincev6 was merged & changed a few internals after this PR was sent. Cheers!

seiyaband others added13 commits

August 7, 2023 22:15

Update packages/eslint-plugin/src/rules/prefer-destructuring.ts

0102f80

Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>

rename a function

b6a1840

fix typo

4d69749

reduce baseRule.create() calls

b2de45b

lazily run baseRule.create(noFixContext(context))

c0dbe8f

lint

7e0d976

add test cases

a1e24e4

add test cases

67a18bc

Merge branch 'main' into prefer-destructuring

6b4bebe

remove tests copied from base rule

96cbada

add tests

4ca0e2a

add tests

25f780d

declare variables

7451956

Copy link

ContributorAuthor

seiyab commentedAug 11, 2023

I have responded to your requests🚀

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

Aug 12, 2023

JoshuaKGoldberg previously approved these changes

Aug 12, 2023

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.

LGTM, thanks for both sending & updating it (with so many tests)! 🔥

I'll defer to an additional review from @typescript-eslint/triage-team because this is the first instance I can find of callingbaseRule.create in two different potential ways.

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

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 12, 2023

minor improvements

6ca18e1

- naming of options- using nullish coalescing assignment

seiyab dismissedJoshuaKGoldberg’sstale review via6ca18e1

August 14, 2023 12:56

Copy link

ContributorAuthor

seiyab commentedAug 14, 2023•
edited
Loading

Thank you for your review.
I have responded to#7117 (comment) and#7117 (comment)~~but I'm wondering whycreate(context, [enabledTypes, options = {}]) { get uncovered renaming a variable~~.