Skip to content

Commit

Permalink
prefer-number-properties: Check any use of global functions (#1834)
Browse files Browse the repository at this point in the history
  • Loading branch information
@fisker
fisker committed May 29, 2022
1 parent 80c4af2 commit 51d7e06
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 79 deletions.
87 changes: 35 additions & 52 deletions rules/prefer-number-properties.js
View file
Expand Up @@ -3,20 +3,21 @@ const {ReferenceTracker} = require('eslint-utils');
const {replaceReferenceIdentifier} = require('./fix/index.js');
const {fixSpaceAroundKeyword} = require('./fix/index.js');

const METHOD_ERROR_MESSAGE_ID = 'method-error';
const METHOD_SUGGESTION_MESSAGE_ID = 'method-suggestion';
const PROPERTY_ERROR_MESSAGE_ID = 'property-error';
const MESSAGE_ID_ERROR = 'error';
const MESSAGE_ID_SUGGESTION = 'suggestion';
const messages = {
[METHOD_ERROR_MESSAGE_ID]: 'Prefer `Number.{{name}}()` over `{{name}}()`.',
[METHOD_SUGGESTION_MESSAGE_ID]: 'Replace `{{name}}()` with `Number.{{name}}()`.',
[PROPERTY_ERROR_MESSAGE_ID]: 'Prefer `Number.{{property}}` over `{{identifier}}`.',
[MESSAGE_ID_ERROR]: 'Prefer `Number.{{property}}` over `{{description}}`.',
[MESSAGE_ID_SUGGESTION]: 'Replace `{{description}}` with `Number.{{property}}`.',
};

const methods = {
// Safe
const globalObjects = {
// Safe to replace with `Number` properties
parseInt: true,
parseFloat: true,
// Unsafe
NaN: true,
Infinity: true,

// Unsafe to replace with `Number` properties
isNaN: false,
isFinite: false,
};
Expand All @@ -26,47 +27,14 @@ const isNegative = node => {
return parent && parent.type === 'UnaryExpression' && parent.operator === '-' && parent.argument === node;
};

function * checkMethods({sourceCode, tracker}) {
const traceMap = Object.fromEntries(
Object.keys(methods).map(name => [name, {[ReferenceTracker.CALL]: true}]),
);

for (const {node: callExpression, path: [name]} of tracker.iterateGlobalReferences(traceMap)) {
const node = callExpression.callee;
const isSafe = methods[name];

const problem = {
node,
messageId: METHOD_ERROR_MESSAGE_ID,
data: {
name,
},
};

const fix = fixer => replaceReferenceIdentifier(node, `Number.${name}`, fixer, sourceCode);

if (isSafe) {
problem.fix = fix;
} else {
problem.suggest = [
{
messageId: METHOD_SUGGESTION_MESSAGE_ID,
data: {
name,
},
fix,
},
];
}

yield problem;
function * checkProperties({sourceCode, tracker, checkInfinity}) {
let names = Object.keys(globalObjects);
if (!checkInfinity) {
names = names.filter(name => name !== 'Infinity');
}
}

function * checkProperties({sourceCode, tracker, checkInfinity}) {
const properties = checkInfinity ? ['NaN', 'Infinity'] : ['NaN'];
const traceMap = Object.fromEntries(
properties.map(name => [name, {[ReferenceTracker.READ]: true}]),
names.map(name => [name, {[ReferenceTracker.READ]: true}]),
);

for (const {node, path: [name]} of tracker.iterateGlobalReferences(traceMap)) {
Expand All @@ -79,22 +47,38 @@ function * checkProperties({sourceCode, tracker, checkInfinity}) {

const problem = {
node,
messageId: PROPERTY_ERROR_MESSAGE_ID,
messageId: MESSAGE_ID_ERROR,
data: {
identifier: name,
description: name,
property,
},
};

if (property === 'NEGATIVE_INFINITY') {
problem.node = parent;
problem.data.identifier = '-Infinity';
problem.data.description = '-Infinity';
problem.fix = function * (fixer) {
yield fixer.replaceText(parent, 'Number.NEGATIVE_INFINITY');
yield * fixSpaceAroundKeyword(fixer, parent, sourceCode);
};

yield problem;
continue;
}

const fix = fixer => replaceReferenceIdentifier(node, `Number.${property}`, fixer, sourceCode);
const isSafeToFix = globalObjects[name];

if (isSafeToFix) {
problem.fix = fix;
} else {
problem.fix = fixer => replaceReferenceIdentifier(node, `Number.${property}`, fixer, sourceCode);
problem.suggest = [
{
messageId: MESSAGE_ID_SUGGESTION,
data: problem.data,
fix,
},
];
}

yield problem;
Expand All @@ -115,7 +99,6 @@ const create = context => {
const sourceCode = context.getSourceCode();
const tracker = new ReferenceTracker(context.getScope());

yield * checkMethods({sourceCode, tracker});
yield * checkProperties({sourceCode, tracker, checkInfinity});
},
};
Expand Down
35 changes: 20 additions & 15 deletions test/prefer-number-properties.mjs
View file
Expand Up @@ -3,9 +3,8 @@ import {getTester} from './utils/test.mjs';

const {test} = getTester(import.meta);

const METHOD_ERROR_MESSAGE_ID = 'method-error';
const METHOD_SUGGESTION_MESSAGE_ID = 'method-suggestion';
const PROPERTY_ERROR_MESSAGE_ID = 'property-error';
const MESSAGE_ID_ERROR = 'error';
const MESSAGE_ID_SUGGESTION = 'suggestion';

const methods = {
parseInt: {
Expand All @@ -30,17 +29,19 @@ const createError = (name, suggestionOutput) => {
const {safe} = methods[name];

const error = {
messageId: METHOD_ERROR_MESSAGE_ID,
messageId: MESSAGE_ID_ERROR,
data: {
name,
description: name,
property: name,
},
};

const suggestions = safe ? undefined : [
{
messageId: METHOD_SUGGESTION_MESSAGE_ID,
messageId: MESSAGE_ID_SUGGESTION,
data: {
name,
description: name,
property: name,
},
output: suggestionOutput,
},
Expand Down Expand Up @@ -75,12 +76,6 @@ test({
'Number.isNaN(10);',
'Number.isFinite(10);',

// Not call
...Object.keys(methods),

// New
...Object.values(methods).map(({code}) => `new ${code}`),

// Shadowed
...Object.entries(methods).map(([name, {code}]) => outdent`
const ${name} = function() {};
Expand Down Expand Up @@ -161,9 +156,9 @@ test({
// `NaN` and `Infinity`
const errorNaN = [
{
messageId: PROPERTY_ERROR_MESSAGE_ID,
messageId: MESSAGE_ID_ERROR,
data: {
identifier: 'NaN',
description: 'NaN',
property: 'NaN',
},
},
Expand Down Expand Up @@ -401,5 +396,15 @@ test.snapshot({
'self.parseFloat(foo);',
'globalThis.NaN',
'-globalThis.Infinity',

// Not a call
outdent`
const options = {
normalize: parseFloat,
parseInt,
};
run(foo, options);
`,
],
});
67 changes: 55 additions & 12 deletions test/snapshots/prefer-number-properties.mjs.md
View file
Expand Up @@ -583,10 +583,10 @@ Generated by [AVA](https://avajs.dev).
`␊
> 1 | globalThis.isNaN(foo);␊
| ^^^^^^^^^^^^^^^^ Prefer \`Number.isNaN()\` over \`isNaN()\`.␊
| ^^^^^^^^^^^^^^^^ Prefer \`Number.isNaN\` over \`isNaN\`.␊
␊
--------------------------------------------------------------------------------␊
Suggestion 1/1: Replace \`isNaN()\` with \`Number.isNaN()\`.␊
Suggestion 1/1: Replace \`isNaN\` with \`Number.isNaN\`.␊
1 | Number.isNaN(foo);␊
`

Expand All @@ -597,10 +597,10 @@ Generated by [AVA](https://avajs.dev).
`␊
> 1 | global.isNaN(foo);␊
| ^^^^^^^^^^^^ Prefer \`Number.isNaN()\` over \`isNaN()\`.␊
| ^^^^^^^^^^^^ Prefer \`Number.isNaN\` over \`isNaN\`.␊
␊
--------------------------------------------------------------------------------␊
Suggestion 1/1: Replace \`isNaN()\` with \`Number.isNaN()\`.␊
Suggestion 1/1: Replace \`isNaN\` with \`Number.isNaN\`.␊
1 | Number.isNaN(foo);␊
`

Expand All @@ -611,10 +611,10 @@ Generated by [AVA](https://avajs.dev).
`␊
> 1 | window.isNaN(foo);␊
| ^^^^^^^^^^^^ Prefer \`Number.isNaN()\` over \`isNaN()\`.␊
| ^^^^^^^^^^^^ Prefer \`Number.isNaN\` over \`isNaN\`.␊
␊
--------------------------------------------------------------------------------␊
Suggestion 1/1: Replace \`isNaN()\` with \`Number.isNaN()\`.␊
Suggestion 1/1: Replace \`isNaN\` with \`Number.isNaN\`.␊
1 | Number.isNaN(foo);␊
`

Expand All @@ -625,10 +625,10 @@ Generated by [AVA](https://avajs.dev).
`␊
> 1 | self.isNaN(foo);␊
| ^^^^^^^^^^ Prefer \`Number.isNaN()\` over \`isNaN()\`.␊
| ^^^^^^^^^^ Prefer \`Number.isNaN\` over \`isNaN\`.␊
␊
--------------------------------------------------------------------------------␊
Suggestion 1/1: Replace \`isNaN()\` with \`Number.isNaN()\`.␊
Suggestion 1/1: Replace \`isNaN\` with \`Number.isNaN\`.␊
1 | Number.isNaN(foo);␊
`

Expand All @@ -645,7 +645,7 @@ Generated by [AVA](https://avajs.dev).
`␊
> 1 | globalThis.parseFloat(foo);␊
| ^^^^^^^^^^^^^^^^^^^^^ Prefer \`Number.parseFloat()\` over \`parseFloat()\`.␊
| ^^^^^^^^^^^^^^^^^^^^^ Prefer \`Number.parseFloat\` over \`parseFloat\`.␊
`

## Invalid #40
Expand All @@ -661,7 +661,7 @@ Generated by [AVA](https://avajs.dev).
`␊
> 1 | global.parseFloat(foo);␊
| ^^^^^^^^^^^^^^^^^ Prefer \`Number.parseFloat()\` over \`parseFloat()\`.␊
| ^^^^^^^^^^^^^^^^^ Prefer \`Number.parseFloat\` over \`parseFloat\`.␊
`

## Invalid #41
Expand All @@ -677,7 +677,7 @@ Generated by [AVA](https://avajs.dev).
`␊
> 1 | window.parseFloat(foo);␊
| ^^^^^^^^^^^^^^^^^ Prefer \`Number.parseFloat()\` over \`parseFloat()\`.␊
| ^^^^^^^^^^^^^^^^^ Prefer \`Number.parseFloat\` over \`parseFloat\`.␊
`

## Invalid #42
Expand All @@ -693,7 +693,7 @@ Generated by [AVA](https://avajs.dev).
`␊
> 1 | self.parseFloat(foo);␊
| ^^^^^^^^^^^^^^^ Prefer \`Number.parseFloat()\` over \`parseFloat()\`.␊
| ^^^^^^^^^^^^^^^ Prefer \`Number.parseFloat\` over \`parseFloat\`.␊
`

## Invalid #43
Expand Down Expand Up @@ -727,3 +727,46 @@ Generated by [AVA](https://avajs.dev).
> 1 | -globalThis.Infinity␊
| ^^^^^^^^^^^^^^^^^^^^ Prefer \`Number.NEGATIVE_INFINITY\` over \`-Infinity\`.␊
`

## Invalid #45
1 | const options = {
2 | normalize: parseFloat,
3 | parseInt,
4 | };
5 |
6 | run(foo, options);

> Output
`␊
1 | const options = {␊
2 | normalize: Number.parseFloat,␊
3 | parseInt: Number.parseInt,␊
4 | };␊
5 |␊
6 | run(foo, options);␊
`

> Error 1/2
`␊
1 | const options = {␊
> 2 | normalize: parseFloat,␊
| ^^^^^^^^^^ Prefer \`Number.parseFloat\` over \`parseFloat\`.␊
3 | parseInt,␊
4 | };␊
5 |␊
6 | run(foo, options);␊
`

> Error 2/2
`␊
1 | const options = {␊
2 | normalize: parseFloat,␊
> 3 | parseInt,␊
| ^^^^^^^^ Prefer \`Number.parseInt\` over \`parseInt\`.␊
4 | };␊
5 |␊
6 | run(foo, options);␊
`
Binary file modified test/snapshots/prefer-number-properties.mjs.snap
View file
Binary file not shown.

0 comments on commit 51d7e06

Please sign in to comment.