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

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

Merged
merged 41 commits into from Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
8e328f6
copy prefer-destructuring from ESLint
seiyab Jun 10, 2023
392f132
rewrite schema by hand
seiyab Jun 12, 2023
4435c44
add enforceForTypeAnnotatedProperties option
seiyab Jun 12, 2023
e5ab24f
add tests
seiyab Jun 12, 2023
90e094f
prevent fixing type-annotated declaration
seiyab Jun 12, 2023
1619def
add tests
seiyab Jun 13, 2023
8bcb921
move baseTests into end of file
seiyab Jun 13, 2023
47b9e6c
refactor
seiyab Jun 15, 2023
e997503
consider numeric properties of non-itreable objects for VariableDecla…
seiyab Jun 17, 2023
375f99c
consider numeric properties of non-itreable objects for AssignmentEx…
seiyab Jun 18, 2023
05680aa
fix a bug
seiyab Jun 18, 2023
a687993
fix a bug
seiyab Jun 18, 2023
0fbc758
add "openjsf" into the dictionary
seiyab Jun 18, 2023
7951154
add prefer-destructuring into all
seiyab Jun 19, 2023
d3626c6
add doc and minor tweak
seiyab Jun 20, 2023
93c179c
fix typo
seiyab Jun 20, 2023
5ebadbf
fix incorrect "correct" case
seiyab Jun 20, 2023
5d1e5be
improve test coverage
seiyab Jun 21, 2023
2d7e2af
improve test coverage
seiyab Jun 21, 2023
7199034
Merge branch 'main' into prefer-destructuring
seiyab Jun 21, 2023
854f1c1
Merge branch 'main' into prefer-destructuring
seiyab Jul 4, 2023
2b92380
Merge branch 'main' into prefer-destructuring
seiyab Jul 6, 2023
2b4fdc1
Merge branch 'main' into prefer-destructuring
JoshuaKGoldberg Aug 5, 2023
521506c
fix: bring in adjustments from main branch
JoshuaKGoldberg Aug 5, 2023
044d503
Updated snapshot
JoshuaKGoldberg Aug 5, 2023
0102f80
Update packages/eslint-plugin/src/rules/prefer-destructuring.ts
seiyab Aug 7, 2023
b6a1840
rename a function
seiyab Aug 7, 2023
4d69749
fix typo
seiyab Aug 8, 2023
b2de45b
reduce baseRule.create() calls
seiyab Aug 8, 2023
c0dbe8f
lazily run baseRule.create(noFixContext(context))
seiyab Aug 8, 2023
7e0d976
lint
seiyab Aug 9, 2023
a1e24e4
add test cases
seiyab Aug 9, 2023
67a18bc
add test cases
seiyab Aug 9, 2023
6b4bebe
Merge branch 'main' into prefer-destructuring
seiyab Aug 10, 2023
96cbada
remove tests copied from base rule
seiyab Aug 10, 2023
4ca0e2a
add tests
seiyab Aug 10, 2023
25f780d
add tests
seiyab Aug 10, 2023
7451956
declare variables
seiyab Aug 11, 2023
6ca18e1
minor improvements
seiyab Aug 14, 2023
3c23be8
improve type and coverage
seiyab Aug 14, 2023
77b6704
Merge branch 'main' into prefer-destructuring
seiyab Aug 14, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
91 changes: 91 additions & 0 deletions 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;
```
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/configs/all.ts
Expand Up @@ -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',
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
237 changes: 237 additions & 0 deletions 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;
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
}
if (!type.isUnion()) {
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
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);
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
}
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/util/getESLintCoreRule.ts
Expand Up @@ -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');
Expand Down