Skip to content

Commit

Permalink
feat(eslint-plugin): add no-magic-numbers rule
Browse files Browse the repository at this point in the history
  • Loading branch information
@scottohara
scottohara committed Mar 26, 2019
1 parent 1b3d31e commit ac78113
Show file tree
Hide file tree
Showing 5 changed files with 465 additions and 0 deletions.
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
View file
Expand Up @@ -131,6 +131,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e
| [`@typescript-eslint/no-extraneous-class`](./docs/rules/no-extraneous-class.md) | Forbids the use of classes as namespaces (`no-unnecessary-class` from TSLint) | | |
| [`@typescript-eslint/no-for-in-array`](./docs/rules/no-for-in-array.md) | Disallow iterating over an array with a for-in loop (`no-for-in-array` from TSLint) | | |
| [`@typescript-eslint/no-inferrable-types`](./docs/rules/no-inferrable-types.md) | Disallows explicit type declarations for variables or parameters initialized to a number, string, or boolean. (`no-inferrable-types` from TSLint) | :heavy_check_mark: | :wrench: |
| [`@typescript-eslint/no-magic-numbers`](./docs/rules/no-magic-numbers.md) | Disallows magic numbers. | :heavy_check_mark: | |
| [`@typescript-eslint/no-misused-new`](./docs/rules/no-misused-new.md) | Enforce valid definition of `new` and `constructor`. (`no-misused-new` from TSLint) | :heavy_check_mark: | |
| [`@typescript-eslint/no-namespace`](./docs/rules/no-namespace.md) | Disallow the use of custom TypeScript modules and namespaces (`no-namespace` from TSLint) | :heavy_check_mark: | |
| [`@typescript-eslint/no-non-null-assertion`](./docs/rules/no-non-null-assertion.md) | Disallows non-null assertions using the `!` postfix operator (`no-non-null-assertion` from TSLint) | :heavy_check_mark: | |
Expand Down
159 changes: 159 additions & 0 deletions packages/eslint-plugin/docs/rules/no-magic-numbers.md
View file
@@ -0,0 +1,159 @@
# Disallow Magic Numbers (no-magic-numbers)

'Magic numbers' are numbers that occur multiple time in code without an explicit meaning.
They should preferably be replaced by named constants.

```js
var now = Date.now(),
inOneHour = now + 60 * 60 * 1000;
```

## Rule Details

The `no-magic-numbers` rule aims to make code more readable and refactoring easier by ensuring that special numbers
are declared as constants to make their meaning explicit.

**_This rule was taken from the ESLint core rule `no-magic-numbers`._**
**_Available options and test cases may vary depending on the version of ESLint installed in the system._**

Examples of **incorrect** code for this rule:

```js
/*eslint no-magic-numbers: "error"*/

var dutyFreePrice = 100,
finalPrice = dutyFreePrice + dutyFreePrice * 0.25;
```

```js
/*eslint no-magic-numbers: "error"*/

var data = ['foo', 'bar', 'baz'];

var dataLast = data[2];
```

```js
/*eslint no-magic-numbers: "error"*/

var SECONDS;

SECONDS = 60;
```

Examples of **correct** code for this rule:

```js
/*eslint no-magic-numbers: "error"*/

var TAX = 0.25;

var dutyFreePrice = 100,
finalPrice = dutyFreePrice + dutyFreePrice * TAX;
```

## Options

```cjson
{
// note you must disable the base rule as it can report incorrect errors
"no-magic-numbers": "off",
"@typescript-eslint/no-magic-numbers": ["error", { "ignoreNumericLiteralTypes": true }]
}
```

### ignore

An array of numbers to ignore. It's set to `[]` by default.
If provided, it must be an `Array`.

Examples of **correct** code for the sample `{ "ignore": [1] }` option:

```js
/*eslint no-magic-numbers: ["error", { "ignore": [1] }]*/

var data = ['foo', 'bar', 'baz'];
var dataLast = data.length && data[data.length - 1];
```

### ignoreArrayIndexes

A boolean to specify if numbers used as array indexes are considered okay. `false` by default.

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

```js
/*eslint no-magic-numbers: ["error", { "ignoreArrayIndexes": true }]*/

var data = ['foo', 'bar', 'baz'];
var dataLast = data[2];
```

### enforceConst

A boolean to specify if we should check for the const keyword in variable declaration of numbers. `false` by default.

Examples of **incorrect** code for the `{ "enforceConst": true }` option:

```js
/*eslint no-magic-numbers: ["error", { "enforceConst": true }]*/

var TAX = 0.25;

var dutyFreePrice = 100,
finalPrice = dutyFreePrice + dutyFreePrice * TAX;
```

### detectObjects

A boolean to specify if we should detect numbers when setting object properties for example. `false` by default.

Examples of **incorrect** code for the `{ "detectObjects": true }` option:

```js
/*eslint no-magic-numbers: ["error", { "detectObjects": true }]*/

var magic = {
tax: 0.25,
};

var dutyFreePrice = 100,
finalPrice = dutyFreePrice + dutyFreePrice * magic.tax;
```

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

```js
/*eslint no-magic-numbers: ["error", { "detectObjects": true }]*/

var TAX = 0.25;

var magic = {
tax: TAX,
};

var dutyFreePrice = 100,
finalPrice = dutyFreePrice + dutyFreePrice * magic.tax;
```

### ignoreNumericLiteralTypes

A boolean to specify if numbers used in Typescript numeric literal types are considered okay. `false` by default.

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

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

type SmallPrimes = 2 | 3 | 5 | 7 | 11;
```

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

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

type SmallPrimes = 2 | 3 | 5 | 7 | 11;
```

<sup>Taken with ❤️ [from ESLint core](https://github.com/eslint/eslint/blob/master/docs/rules/no-magic-numbers.md)</sup>
149 changes: 149 additions & 0 deletions packages/eslint-plugin/src/rules/no-magic-numbers.ts
View file
@@ -0,0 +1,149 @@
/**
* @fileoverview Rule to flag statements that use magic numbers (adapted from https://github.com/danielstjules/buddy.js)
* @author Scott O'Hara
*/

import { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/typescript-estree';
import baseRule from 'eslint/lib/rules/no-magic-numbers';
import * as util from '../util';
import { JSONSchema4 } from 'json-schema';

type Options = util.InferOptionsTypeFromRule<typeof baseRule>;
type MessageIds = util.InferMessageIdsTypeFromRule<typeof baseRule>;

// Original schema properties from the base rule
const { properties } = (baseRule.meta.schema as JSONSchema4[])[0];

// Extend base schema with additional property to ignore TS numeric literal types
if (properties) {
properties.ignoreNumericLiteralTypes = {
type: 'boolean',
};
}

export default util.createRule<Options, MessageIds>({
name: 'no-magic-numbers',
meta: {
type: 'suggestion',
docs: {
description: 'Disallow magic numbers',
category: 'Best Practices',
recommended: false,
},
schema: baseRule.meta.schema,
messages: baseRule.meta.messages,
},
defaultOptions: [
{
ignore: [],
ignoreArrayIndexes: false,
enforceConst: false,
detectObjects: false,
ignoreNumericLiteralTypes: false,
},
],
create(context, [options]) {
const rules = baseRule.create(context);

/**
* Returns whether the node is number literal
* @param node the node literal being evaluated
* @returns true if the node is a number literal
*/
function isNumber(node: TSESTree.Literal): boolean {
return typeof node.value === 'number';
}

/**
* Checks if the node grandparent is a Typescript type alias declaration
* @param node the node to be validated.
* @returns true if the node grandparent is a Typescript type alias declaration
* @private
*/
function isGrandparentTSTypeAliasDeclaration(node: TSESTree.Node): boolean {
return node.parent && node.parent.parent
? AST_NODE_TYPES.TSTypeAliasDeclaration === node.parent.parent.type
: false;
}

/**
* Checks if the node grandparent is a Typescript union type and its parent is a type alias declaration
* @param node the node to be validated.
* @returns true if the node grandparent is a Typescript untion type and its parent is a type alias declaration
* @private
*/
function isGrandparentTSUnionType(node: TSESTree.Node): boolean {
if (
node.parent &&
node.parent.parent &&
AST_NODE_TYPES.TSUnionType === node.parent.parent.type
) {
return isGrandparentTSTypeAliasDeclaration(node.parent);
}

return false;
}

/**
* Checks if the node parent is a Typescript literal type
* @param node the node to be validated.
* @returns true if the node parent is a Typescript literal type
* @private
*/
function isParentTSLiteralType(node: TSESTree.Node): boolean {
return node.parent
? AST_NODE_TYPES.TSLiteralType === node.parent.type
: false;
}

/**
* Checks if the node is a valid TypeScript numeric literal type.
* @param node the node to be validated.
* @returns true if the node is a TypeScript numeric literal type.
* @private
*/
function isTSNumericLiteralType(node: TSESTree.Node): boolean {
// For negative numbers, update the parent node
if (
node.parent &&
node.parent.type === AST_NODE_TYPES.UnaryExpression &&
node.parent.operator === '-'
) {
node = node.parent;
}

// If the parent node is not a TSLiteralType, early return
if (!isParentTSLiteralType(node)) {
return false;
}

// If the grandparent is a TSTypeAliasDeclaration, ignore
if (isGrandparentTSTypeAliasDeclaration(node)) {
return true;
}

// If the grandparent is a TSUnionType and it's parent is a TSTypeAliasDeclaration, ignore
if (isGrandparentTSUnionType(node)) {
return true;
}

return false;
}

return {
Literal(node) {
// Check TypeScript specific nodes
if (
options.ignoreNumericLiteralTypes &&
isNumber(node) &&
isTSNumericLiteralType(node)
) {
return;
}

// Let the base rule deal with the rest
rules.Literal(node);
},
};
},
});

0 comments on commit ac78113

Please sign in to comment.