Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
feat(eslint-plugin): [no-magic-numbers] ignoreTypeIndexes option (#4789)
* feat(eslint-plugin): [no-magic-numbers] ignoreTypeIndexes option

Add a new `ignoreTypeIndexes` option to the `no-magic-numbers` rule to
allow magic numbers in type index access (eg. `Foo[4]`).

* chore: refactor rule to report violations early

* feat: [no-magic-numbers] handle intersections & nested typed indexed access
  • Loading branch information
davecardwell committed Apr 15, 2022
1 parent 8e17f56 commit 5e79451
Show file tree
Hide file tree
Showing 4 changed files with 407 additions and 16 deletions.
24 changes: 24 additions & 0 deletions packages/eslint-plugin/docs/rules/no-magic-numbers.md
Expand Up @@ -36,13 +36,15 @@ interface Options extends BaseNoMagicNumbersOptions {
ignoreEnums?: boolean;
ignoreNumericLiteralTypes?: boolean;
ignoreReadonlyClassProperties?: boolean;
ignoreTypeIndexes?: boolean;
}

const defaultOptions: Options = {
...baseNoMagicNumbersDefaultOptions,
ignoreEnums: false,
ignoreNumericLiteralTypes: false,
ignoreReadonlyClassProperties: false,
ignoreTypeIndexes: false,
};
```

Expand Down Expand Up @@ -118,6 +120,28 @@ class Foo {
}
```

### `ignoreTypeIndexes`

A boolean to specify if numbers used to index types are okay. `false` by default.

Examples of **incorrect** code for the `{ "ignoreTypeIndexes": false }` option:

```ts
/*eslint @typescript-eslint/no-magic-numbers: ["error", { "ignoreTypeIndexes": false }]*/

type Foo = Bar[0];
type Baz = Parameters<Foo>[2];
```

Examples of **correct** code for the `{ "ignoreTypeIndexes": true }` option:

```ts
/*eslint @typescript-eslint/no-magic-numbers: ["error", { "ignoreTypeIndexes": true }]*/

type Foo = Bar[0];
type Baz = Parameters<Foo>[2];
```

<sup>

Taken with ❤️ [from ESLint core](https://github.com/eslint/eslint/blob/main/docs/rules/no-magic-numbers.md)
Expand Down
68 changes: 52 additions & 16 deletions packages/eslint-plugin/src/rules/no-magic-numbers.ts
Expand Up @@ -24,6 +24,9 @@ const schema = util.deepMerge(
ignoreReadonlyClassProperties: {
type: 'boolean',
},
ignoreTypeIndexes: {
type: 'boolean',
},
},
},
);
Expand Down Expand Up @@ -56,29 +59,40 @@ export default util.createRule<Options, MessageIds>({

return {
Literal(node): void {
// Check if the node is a TypeScript enum declaration
if (options.ignoreEnums && isParentTSEnumDeclaration(node)) {
// If it’s not a numeric literal we’re not interested
if (typeof node.value !== 'number' && typeof node.value !== 'bigint') {
return;
}

// This will be `true` if we’re configured to ignore this case (eg. it’s
// an enum and `ignoreEnums` is `true`). It will be `false` if we’re not
// configured to ignore this case. It will remain `undefined` if this is
// not one of our exception cases, and we’ll fall back to the base rule.
let isAllowed: boolean | undefined;

// Check if the node is a TypeScript enum declaration
if (isParentTSEnumDeclaration(node)) {
isAllowed = options.ignoreEnums === true;
}
// Check TypeScript specific nodes for Numeric Literal
if (
options.ignoreNumericLiteralTypes &&
typeof node.value === 'number' &&
isTSNumericLiteralType(node)
) {
return;
else if (isTSNumericLiteralType(node)) {
isAllowed = options.ignoreNumericLiteralTypes === true;
}
// Check if the node is a type index
else if (isAncestorTSIndexedAccessType(node)) {
isAllowed = options.ignoreTypeIndexes === true;
}

// Check if the node is a readonly class property
if (
(typeof node.value === 'number' || typeof node.value === 'bigint') &&
isParentTSReadonlyPropertyDefinition(node)
) {
if (options.ignoreReadonlyClassProperties) {
return;
}
else if (isParentTSReadonlyPropertyDefinition(node)) {
isAllowed = options.ignoreReadonlyClassProperties === true;
}

// If we’ve hit a case where the ignore option is true we can return now
if (isAllowed === true) {
return;
}
// If the ignore option is *not* set we can report it now
else if (isAllowed === false) {
let fullNumberNode: TSESTree.Literal | TSESTree.UnaryExpression =
node;
let raw = node.raw;
Expand Down Expand Up @@ -216,3 +230,25 @@ function isParentTSReadonlyPropertyDefinition(node: TSESTree.Literal): boolean {

return false;
}

/**
* Checks if the node is part of a type indexed access (eg. Foo[4])
* @param node the node to be validated.
* @returns true if the node is part of an indexed access
* @private
*/
function isAncestorTSIndexedAccessType(node: TSESTree.Literal): boolean {
// Handle unary expressions (eg. -4)
let ancestor = getLiteralParent(node);

// Go up another level while we’re part of a type union (eg. 1 | 2) or
// intersection (eg. 1 & 2)
while (
ancestor?.parent?.type === AST_NODE_TYPES.TSUnionType ||
ancestor?.parent?.type === AST_NODE_TYPES.TSIntersectionType
) {
ancestor = ancestor.parent;
}

return ancestor?.parent?.type === AST_NODE_TYPES.TSIndexedAccessType;
}

0 comments on commit 5e79451

Please sign in to comment.