feat(eslint-plugin): add new extended rule prefer-destructuring (#7117

)

* copy prefer-destructuring from ESLint

* rewrite schema by hand

* add enforceForTypeAnnotatedProperties option

autofix is still wrong.

* add tests

* prevent fixing type-annotated declaration

* add tests

* move baseTests into end of file

* refactor

* consider numeric properties of non-itreable objects for VariableDeclarator

* consider numeric properties of non-itreable objects for AssignmentExpression

* fix a bug

* fix a bug

* add "openjsf" into the dictionary

* add prefer-destructuring into all

* add doc and minor tweak

* fix typo

* fix incorrect "correct" case

* improve test coverage

* improve test coverage

* fix: bring in adjustments from main branch

* Updated snapshot

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

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

* rename a function

* fix typo

* reduce baseRule.create() calls

* lazily run baseRule.create(noFixContext(context))

* lint

* add test cases

* add test cases

* remove tests copied from base rule

* add tests

* add tests

* declare variables

* minor improvements

- naming of options
- using nullish coalescing assignment

* improve type and coverage

---------

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

packages/eslint-plugin/docs/rules/prefer-destructuring.md

@@ -0,0 +1,91 @@
+---
+description: 'Require destructuring from arrays and/or objects.'
+---
+
+> 🛑 This file is source code, not the primary documentation location! 🛑
+>
+> See **https://typescript-eslint.io/rules/prefer-destructuring** for documentation.
+
+## Examples
+
+This rule extends the base [`eslint/prefer-destructuring`](https://eslint.org/docs/latest/rules/prefer-destructuring) rule.
+It adds support for TypeScript's type annotations in variable declarations.
+
+<!-- tabs -->
+
+### `eslint/prefer-destructuring`
+
+```ts
+const x: string = obj.x; // This is incorrect and the auto fixer provides following untyped fix.
+// const { x } = obj;
+```
+
+### `@typescript-eslint/prefer-destructuring`
+
+```ts
+const x: string = obj.x; // This is correct by default. You can also forbid this by an option.
+```
+
+<!-- /tabs -->
+
+And it infers binding patterns more accurately thanks to the type checker.
+
+<!-- tabs -->
+
+### ❌ Incorrect
+
+```ts
+const x = ['a'];
+const y = x[0];
+```
+
+### ✅ Correct
+
+```ts
+const x = { 0: 'a' };
+const y = x[0];
+```
+
+It is correct when `enforceForRenamedProperties` is not true.
+Valid destructuring syntax is renamed style like `{ 0: y } = x` rather than `[y] = x` because `x` is not iterable.
+
+## Options
+
+This rule adds the following options:
+
+```ts
+type Options = [
+  BasePreferDestructuringOptions[0],
+  BasePreferDestructuringOptions[1] & {
+    enforceForDeclarationWithTypeAnnotation?: boolean;
+  },
+];
+
+const defaultOptions: Options = [
+  basePreferDestructuringDefaultOptions[0],
+  {
+    ...basePreferDestructuringDefaultOptions[1],
+    enforceForDeclarationWithTypeAnnotation: false,
+  },
+];
+```
+
+### `enforceForDeclarationWithTypeAnnotation`
+
+When set to `true`, type annotated variable declarations are enforced to use destructuring assignment.
+
+Examples with `{ enforceForDeclarationWithTypeAnnotation: true }`:
+
+<!--tabs-->
+
+### ❌ Incorrect
+
+```ts
+const x: string = obj.x;
+```
+
+### ✅ Correct
+
+```ts
+const { x }: { x: string } = obj;
+```

packages/eslint-plugin/src/configs/all.ts

@@ -141,6 +141,8 @@ export = {
     '@typescript-eslint/parameter-properties': 'error',
     '@typescript-eslint/prefer-as-const': 'error',
     '@typescript-eslint/prefer-enum-initializers': 'error',
+    'prefer-destructuring': 'off',
+    '@typescript-eslint/prefer-destructuring': 'error',
     '@typescript-eslint/prefer-for-of': 'error',
     '@typescript-eslint/prefer-function-type': 'error',
     '@typescript-eslint/prefer-includes': 'error',

packages/eslint-plugin/src/rules/index.ts

@@ -97,6 +97,7 @@ import objectCurlySpacing from './object-curly-spacing';
 import paddingLineBetweenStatements from './padding-line-between-statements';
 import parameterProperties from './parameter-properties';
 import preferAsConst from './prefer-as-const';
+import preferDestructuring from './prefer-destructuring';
 import preferEnumInitializers from './prefer-enum-initializers';
 import preferForOf from './prefer-for-of';
 import preferFunctionType from './prefer-function-type';
@@ -232,6 +233,7 @@ export default {
   'padding-line-between-statements': paddingLineBetweenStatements,
   'parameter-properties': parameterProperties,
   'prefer-as-const': preferAsConst,
+  'prefer-destructuring': preferDestructuring,
   'prefer-enum-initializers': preferEnumInitializers,
   'prefer-for-of': preferForOf,
   'prefer-function-type': preferFunctionType,

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

@@ -0,0 +1,237 @@
+import type { TSESLint, TSESTree } from '@typescript-eslint/utils';
+import { AST_NODE_TYPES } from '@typescript-eslint/utils';
+import type { JSONSchema4 } from '@typescript-eslint/utils/json-schema';
+import * as tsutils from 'ts-api-utils';
+import type * as ts from 'typescript';
+
+import type {
+  InferMessageIdsTypeFromRule,
+  InferOptionsTypeFromRule,
+} from '../util';
+import { createRule, getParserServices, isTypeAnyType } from '../util';
+import { getESLintCoreRule } from '../util/getESLintCoreRule';
+
+const baseRule = getESLintCoreRule('prefer-destructuring');
+
+type BaseOptions = InferOptionsTypeFromRule<typeof baseRule>;
+type EnforcementOptions = BaseOptions[1] & {
+  enforceForDeclarationWithTypeAnnotation?: boolean;
+};
+type Options = [BaseOptions[0], EnforcementOptions];
+
+type MessageIds = InferMessageIdsTypeFromRule<typeof baseRule>;
+
+const destructuringTypeConfig: JSONSchema4 = {
+  type: 'object',
+  properties: {
+    array: {
+      type: 'boolean',
+    },
+    object: {
+      type: 'boolean',
+    },
+  },
+  additionalProperties: false,
+};
+
+const schema: readonly JSONSchema4[] = [
+  {
+    oneOf: [
+      {
+        type: 'object',
+        properties: {
+          VariableDeclarator: destructuringTypeConfig,
+          AssignmentExpression: destructuringTypeConfig,
+        },
+        additionalProperties: false,
+      },
+      destructuringTypeConfig,
+    ],
+  },
+  {
+    type: 'object',
+    properties: {
+      enforceForRenamedProperties: {
+        type: 'boolean',
+      },
+      enforceForDeclarationWithTypeAnnotation: {
+        type: 'boolean',
+      },
+    },
+  },
+];
+
+export default createRule<Options, MessageIds>({
+  name: 'prefer-destructuring',
+  meta: {
+    type: 'suggestion',
+    docs: {
+      description: 'Require destructuring from arrays and/or objects',
+      extendsBaseRule: true,
+      requiresTypeChecking: true,
+    },
+    schema,
+    fixable: baseRule.meta.fixable,
+    hasSuggestions: baseRule.meta.hasSuggestions,
+    messages: baseRule.meta.messages,
+  },
+  defaultOptions: [
+    {
+      VariableDeclarator: {
+        array: true,
+        object: true,
+      },
+      AssignmentExpression: {
+        array: true,
+        object: true,
+      },
+    },
+    {},
+  ],
+  create(context, [enabledTypes, options]) {
+    const {
+      enforceForRenamedProperties = false,
+      enforceForDeclarationWithTypeAnnotation = false,
+    } = options;
+    const { program, esTreeNodeToTSNodeMap } = getParserServices(context);
+    const typeChecker = program.getTypeChecker();
+    const baseRules = baseRule.create(context);
+    let baseRulesWithoutFixCache: typeof baseRules | null = null;
+
+    return {
+      VariableDeclarator(node): void {
+        performCheck(node.id, node.init, node);
+      },
+      AssignmentExpression(node): void {
+        if (node.operator !== '=') {
+          return;
+        }
+        performCheck(node.left, node.right, node);
+      },
+    };
+
+    function performCheck(
+      leftNode: TSESTree.BindingName | TSESTree.Expression,
+      rightNode: TSESTree.Expression | null,
+      reportNode: TSESTree.VariableDeclarator | TSESTree.AssignmentExpression,
+    ): void {
+      const rules =
+        leftNode.type === AST_NODE_TYPES.Identifier &&
+        leftNode.typeAnnotation === undefined
+          ? baseRules
+          : baseRulesWithoutFix();
+      if (
+        'typeAnnotation' in leftNode &&
+        leftNode.typeAnnotation !== undefined &&
+        !enforceForDeclarationWithTypeAnnotation
+      ) {
+        return;
+      }
+
+      if (
+        rightNode != null &&
+        isArrayLiteralIntegerIndexAccess(rightNode) &&
+        rightNode.object.type !== AST_NODE_TYPES.Super
+      ) {
+        const tsObj = esTreeNodeToTSNodeMap.get(rightNode.object);
+        const objType = typeChecker.getTypeAtLocation(tsObj);
+        if (!isTypeAnyOrIterableType(objType, typeChecker)) {
+          if (
+            !enforceForRenamedProperties ||
+            !getNormalizedEnabledType(reportNode.type, 'object')
+          ) {
+            return;
+          }
+          context.report({
+            node: reportNode,
+            messageId: 'preferDestructuring',
+            data: { type: 'object' },
+          });
+          return;
+        }
+      }
+
+      if (reportNode.type === AST_NODE_TYPES.AssignmentExpression) {
+        rules.AssignmentExpression(reportNode);
+      } else {
+        rules.VariableDeclarator(reportNode);
+      }
+    }
+
+    function getNormalizedEnabledType(
+      nodeType:
+        | AST_NODE_TYPES.VariableDeclarator
+        | AST_NODE_TYPES.AssignmentExpression,
+      destructuringType: 'array' | 'object',
+    ): boolean | undefined {
+      if ('object' in enabledTypes || 'array' in enabledTypes) {
+        return enabledTypes[destructuringType];
+      }
+      return enabledTypes[nodeType as keyof typeof enabledTypes][
+        destructuringType as keyof (typeof enabledTypes)[keyof typeof enabledTypes]
+      ];
+    }
+
+    function baseRulesWithoutFix(): ReturnType<typeof baseRule.create> {
+      baseRulesWithoutFixCache ??= baseRule.create(noFixContext(context));
+      return baseRulesWithoutFixCache;
+    }
+  },
+});
+
+type Context = TSESLint.RuleContext<MessageIds, Options>;
+
+function noFixContext(context: Context): Context {
+  const customContext: {
+    report: Context['report'];
+  } = {
+    report: (descriptor): void => {
+      context.report({
+        ...descriptor,
+        fix: undefined,
+      });
+    },
+  };
+
+  // we can't directly proxy `context` because its `report` property is non-configurable
+  // and non-writable. So we proxy `customContext` and redirect all
+  // property access to the original context except for `report`
+  return new Proxy<Context>(customContext as typeof context, {
+    get(target, path, receiver): unknown {
+      if (path !== 'report') {
+        return Reflect.get(context, path, receiver);
+      }
+      return Reflect.get(target, path, receiver);
+    },
+  });
+}
+
+function isTypeAnyOrIterableType(
+  type: ts.Type,
+  typeChecker: ts.TypeChecker,
+): boolean {
+  if (isTypeAnyType(type)) {
+    return true;
+  }
+  if (!type.isUnion()) {
+    const iterator = tsutils.getWellKnownSymbolPropertyOfType(
+      type,
+      'iterator',
+      typeChecker,
+    );
+    return iterator !== undefined;
+  }
+  return type.types.every(t => isTypeAnyOrIterableType(t, typeChecker));
+}
+
+function isArrayLiteralIntegerIndexAccess(
+  node: TSESTree.Expression,
+): node is TSESTree.MemberExpression {
+  if (node.type !== AST_NODE_TYPES.MemberExpression) {
+    return false;
+  }
+  if (node.property.type !== AST_NODE_TYPES.Literal) {
+    return false;
+  }
+  return Number.isInteger(node.property.value);
+}

packages/eslint-plugin/src/util/getESLintCoreRule.ts

@@ -34,6 +34,7 @@ interface RuleMap {
   'no-restricted-globals': typeof import('eslint/lib/rules/no-restricted-globals');
   'object-curly-spacing': typeof import('eslint/lib/rules/object-curly-spacing');
   'prefer-const': typeof import('eslint/lib/rules/prefer-const');
+  'prefer-destructuring': typeof import('eslint/lib/rules/prefer-destructuring');
   quotes: typeof import('eslint/lib/rules/quotes');
   semi: typeof import('eslint/lib/rules/semi');
   'space-before-blocks': typeof import('eslint/lib/rules/space-before-blocks');