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.

Sign up for GitHub

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

Jump to bottom

prefer-number-properties: Check any use of global functions #1834

Merged
merged 4 commits into from May 29, 2022
Merged

prefer-number-properties: Check any use of global functions #1834

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7f5cf47
`prefer-number-properties`: Check any use of methods
fisker May 27, 2022
dc925dc
Fix
fisker May 27, 2022
9e31434
Add test
fisker May 27, 2022
c84f8d1
Lint
fisker May 27, 2022
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
87 changes: 35 additions & 52 deletions rules/prefer-number-properties.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
Binary file not shown.