New Rule: prefer-destructuring

[0xCLARITY]

I'd like to use the prefer-destructuring rule from ESLint, however, when I try to use array destructuring with an Express request object, I get the following TypeScript Error:

Type 'ParamsDictionary' must have a '[Symbol.iterator]()' method that returns an iterator.ts(2488).

That seems reasonable, but then the base ESLint prefer-destructuring rule gives me a warning, even though TypeScript claims I'm not allowed to use array destructuring here.

I think having a type-aware version of this rule would be beneficial.

[bradzacher]

could you please give me a self-contained example?

[0xCLARITY]

Does this work?

import { Request, Response, NextFunction } from 'express'

async function example(
  req: Request,
  _res: Response,
  _next: NextFunction,
): Promise<void> {
  // Type 'ParamsDictionary' must have a '[Symbol.iterator]()' method that returns an iterator
  const [param] = req.params

  console.log(param)
}

[bradzacher]

what was the code before you fixed it?
const param = req.params[0]?

[0xCLARITY]

Yes, which the base ESLint prefer-destructuring rule warns against.

[bradzacher]

Thanks.
Happy to accept a PR if you'd like to add a type-aware version of the rule to prevent array destructuring of non-iterable types.
In that case, could also implement a fixer, as we know for sure the type is fixable.

[phaux]

The correct way to destructure this is

const { 0: param } = req.params

because params is just an object with numeric properties.

[danvk]

Another issue with the eslint prefer-destructuring rule in TS is if you apply a type declaration to the resulting variable:

const obj = {xs: ['a', 'b', 'c']};  // type is {xs: string[]}
const xs: readonly string[] = obj.xs;
xs.push('d');
// ~~~~ Property 'push' does not exist on type 'readonly string[]'.

Line two gets flagged by eslint's rule:

However, applying the quick fix removes the type annotation, an unsafe operation which makes the push call pass the type checker:

const obj = {xs: ['a', 'b', 'c']};  // type is {xs: string[]}
const {xs} = obj;
xs.push('d');  // ok! oh no!

A typescript-eslint prefer-destructuring rule should ignore assignments with attached type declarations.

[rafaelss95]

Isn't it a duplication of #723? This one had a lot more interaction than the former though.

[bradzacher]

No - they both want to extend the same extension rule - but they want different things.

THIS issue is about
const x = (y as {0: unknown})[0]
being fixed to
const [x] = (y as {0: unknown})
i.e. the rule causes a type error due to attempting to convert a non-iterable to an array destructure.

#723 is about
const x: number = (y as {bar: 1}).bar
being fixed to
const {x} = (y as {bar: 1})
i.e. the rule erases the type annotation on the variable.

[rafaelss95]

Oh yes, I noticed that too, but as (#1931 (comment)) was talking about the same of #723 (and received so many upvotes), I thought all related things were being tracked here.

[iliubinskii]

Another issue with core eslint-rule:

export const construct: Function = Reflect.construct;

// Fixed to:
export const { construct } = Reflect;

As you can see, core rule auto-fix removes type definition (": Function").

[seiyab]

Hi, I'm working on this issue.
For the original problem, I want to clarify the desired behavior.
I'm considering following behavior. Is it reasonable?

const a = {0: 1};
const b = a[0]; // this is incorrect iff options[0].object is true and options[1].enforceForRenamedProperties  is true
const {0: c} = a; // this is always correct


const x = [1];
const y = x[0]; // works same as the current ESLint rule

const v: any = {0: 1};
const w = x[0]; // works same as the current ESLint rule

Showing other edge cases are welcome.

[seiyab]

I'm also working on #1931 (comment), #1931 (comment) and #723.
It is almost done in my local repository. Wait for a little while 😉 .

[JoshuaKGoldberg]

I'm considering following behavior. Is it reasonable?

Sorry, I don't follow what you're asking about @seiyab. There are no rule reports from the base prefer-destructuring rule for const b = a[0];.

[seiyab]

@JoshuaKGoldberg
I'm sorry for unclearness. (My English might often be broken.)
The base prefere-destructuring doesn't report for const b = a[0] if array in options is false, as you wrote.
For the following example, I wanted to show the behavior of the new rule I'm considering (and have already implemented as a PR).

const a = {0: 1};
const b = a[0]; // this is incorrect iff options[0].object is true and options[1].enforceForRenamedProperties  is true

In this case, the proper destructuring syntax is const { 0: b } = a rather than const [b] = a since a is not iterable. This should be the primary point in this issue.
The comment following const b = a[0] suggests:

• options[0].object should control the report. options[1].array shouldn't.
• it should report it only if options[1].enforceForRenamedProperties because it is renamed from 0 to b

Related document I have written: https://github.com/typescript-eslint/typescript-eslint/pull/7117/files#diff-0d683d5803f89de0a159ca7ef7461802afac69ae7fbf459411553aa36e5a7cbaR49-R50

And I asked that whether my thoughts make sense or not.

Does it get clear?

[seiyab]

"this will be incorrect iff ..." might be better than "this is incorrect iff ...".

[tjx666]

Example bad case for ESLint builtin prefer-destructuring:

After destructuring:

[HolgerJeromin]

Can this issue be closed? ref PR #7117