Skip to content

Commit

Permalink
Add prefer-modern-math-apis rule (#1780)
Browse files Browse the repository at this point in the history
  • Loading branch information
fisker authored Apr 1, 2022
1 parent ce8a4b7 commit 6dfdeb0
Show file tree
Hide file tree
Showing 8 changed files with 534 additions and 1 deletion.
1 change: 1 addition & 0 deletions configs/recommended.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ module.exports = {
'unicorn/prefer-keyboard-event-key': 'error',
'unicorn/prefer-math-trunc': 'error',
'unicorn/prefer-modern-dom-apis': 'error',
'unicorn/prefer-modern-math-apis': 'error',
'unicorn/prefer-module': 'error',
'unicorn/prefer-negative-index': 'error',
'unicorn/prefer-node-protocol': 'error',
Expand Down
64 changes: 64 additions & 0 deletions docs/rules/prefer-modern-math-apis.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# Prefer modern `Math` APIs over legacy patterns

<!-- Do not manually modify RULE_NOTICE part. Run: `npm run generate-rule-notices` -->
<!-- RULE_NOTICE -->
*This rule is part of the [recommended](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config) config.*

🔧 *This rule is [auto-fixable](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems).*
<!-- /RULE_NOTICE -->

Math additions in ES2015:

- [Math.sign()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/sign)
- [Math.trunc()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/trunc)
- [Math.cbrt()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/cbrt)
- [Math.expm1()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/expm1)
- [Math.log1p()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/log1p)
- [Math.log10()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/log10)
- [Math.log2()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/log2)
- [Math.sinh()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/sinh)
- [Math.cosh()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/cosh)
- [Math.tanh()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/tanh)
- [Math.asinh()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/asinh)
- [Math.acosh()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/acosh)
- [Math.atanh()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/atanh)
- [Math.hypot()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/hypot)
- [Math.clz32()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/clz32)
- [Math.imul()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/imul)
- [Math.fround()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/fround)

Currently, we only check a few known cases, but we are open to add more patterns.

If you find a suitable case for this rule, please [open an issue](https://github.com/sindresorhus/eslint-plugin-unicorn/issues/new?title=%20%60prefer-modern-math-apis%60%20%20change%20request&labels=evaluating).

## Prefer `Math.log10(x)` over

```js
Math.log(x) * Math.LOG10E
```

```js
Math.LOG10E * Math.log(x)
```

```js
Math.log(x) / Math.LN10
```

## Prefer `Math.log2(x)` over

```js
Math.log(x) * Math.LOG2E
```

```js
Math.LOG2E * Math.log(x)
```

```js
Math.log(x) / Math.LN2
```

## Separate rule for `Math.trunc()`

See [`unicorn/prefer-math-trunc`](./prefer-math-trunc.md) rule.
1 change: 1 addition & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ Each rule has emojis denoting:
| [prefer-keyboard-event-key](docs/rules/prefer-keyboard-event-key.md) | Prefer `KeyboardEvent#key` over `KeyboardEvent#keyCode`. || 🔧 | |
| [prefer-math-trunc](docs/rules/prefer-math-trunc.md) | Enforce the use of `Math.trunc` instead of bitwise operators. || 🔧 | 💡 |
| [prefer-modern-dom-apis](docs/rules/prefer-modern-dom-apis.md) | Prefer `.before()` over `.insertBefore()`, `.replaceWith()` over `.replaceChild()`, prefer one of `.before()`, `.after()`, `.append()` or `.prepend()` over `insertAdjacentText()` and `insertAdjacentElement()`. || 🔧 | |
| [prefer-modern-math-apis](docs/rules/prefer-modern-math-apis.md) | Prefer modern `Math` APIs over legacy patterns. || 🔧 | |
| [prefer-module](docs/rules/prefer-module.md) | Prefer JavaScript modules (ESM) over CommonJS. || 🔧 | 💡 |
| [prefer-negative-index](docs/rules/prefer-negative-index.md) | Prefer negative index over `.length - index` for `{String,Array,TypedArray}#slice()`, `Array#splice()` and `Array#at()`. || 🔧 | |
| [prefer-node-protocol](docs/rules/prefer-node-protocol.md) | Prefer using the `node:` protocol when importing Node.js builtin modules. || 🔧 | |
Expand Down
138 changes: 138 additions & 0 deletions rules/prefer-modern-math-apis.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
'use strict';
const {getParenthesizedText} = require('./utils/parentheses.js');

const MESSAGE_ID = 'prefer-modern-math-apis';
const messages = {
[MESSAGE_ID]: 'Prefer `{{replacement}}` over `{{description}}`.',
};

const isMathProperty = (node, property) =>
node.type === 'MemberExpression'
&& !node.optional
&& !node.computed
&& node.object.type === 'Identifier'
&& node.object.name === 'Math'
&& node.property.type === 'Identifier'
&& node.property.name === property;

const isMathMethodCall = (node, method) =>
node.type === 'CallExpression'
&& !node.optional
&& isMathProperty(node.callee, method)
&& node.arguments.length === 1
&& node.arguments[0].type !== 'SpreadElement';

// `Math.log(x) * Math.LOG10E` -> `Math.log10(x)`
// `Math.LOG10E * Math.log(x)` -> `Math.log10(x)`
// `Math.log(x) * Math.LOG2E` -> `Math.log2(x)`
// `Math.LOG2E * Math.log(x)` -> `Math.log2(x)`
function createLogCallTimesConstantCheck({constantName, replacementMethod}) {
const replacement = `Math.${replacementMethod}(…)`;

return function (node, context) {
if (!(node.type === 'BinaryExpression' && node.operator === '*')) {
return;
}

let mathLogCall;
let description;
if (isMathMethodCall(node.left, 'log') && isMathProperty(node.right, constantName)) {
mathLogCall = node.left;
description = `Math.log(…) * Math.${constantName}`;
} else if (isMathMethodCall(node.right, 'log') && isMathProperty(node.left, constantName)) {
mathLogCall = node.right;
description = `Math.${constantName} * Math.log(…)`;
}

if (!mathLogCall) {
return;
}

const [valueNode] = mathLogCall.arguments;

return {
node,
messageId: MESSAGE_ID,
data: {
replacement,
description,
},
fix: fixer => fixer.replaceText(node, `Math.${replacementMethod}(${getParenthesizedText(valueNode, context.getSourceCode())})`),
};
};
}

// `Math.log(x) / Math.LN10` -> `Math.log10(x)`
// `Math.log(x) / Math.LN2` -> `Math.log2(x)`
function createLogCallDivideConstantCheck({constantName, replacementMethod}) {
const message = {
messageId: MESSAGE_ID,
data: {
replacement: `Math.${replacementMethod}(…)`,
description: `Math.log(…) / Math.${constantName}`,
},
};

return function (node, context) {
if (
!(
node.type === 'BinaryExpression'
&& node.operator === '/'
&& isMathMethodCall(node.left, 'log')
&& isMathProperty(node.right, constantName)
)
) {
return;
}

const [valueNode] = node.left.arguments;

return {
...message,
node,
fix: fixer => fixer.replaceText(node, `Math.${replacementMethod}(${getParenthesizedText(valueNode, context.getSourceCode())})`),
};
};
}

const checkFunctions = [
createLogCallTimesConstantCheck({constantName: 'LOG10E', replacementMethod: 'log10'}),
createLogCallTimesConstantCheck({constantName: 'LOG2E', replacementMethod: 'log2'}),
createLogCallDivideConstantCheck({constantName: 'LN10', replacementMethod: 'log10'}),
createLogCallDivideConstantCheck({constantName: 'LN2', replacementMethod: 'log2'}),
];

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => {
const nodes = [];

return {
BinaryExpression(node) {
nodes.push(node);
},
* 'Program:exit'() {
for (const node of nodes) {
for (const getProblem of checkFunctions) {
const problem = getProblem(node, context);

if (problem) {
yield problem;
}
}
}
},
};
};

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
create,
meta: {
type: 'suggestion',
docs: {
description: 'Prefer modern `Math` APIs over legacy patterns.',
},
fixable: 'code',
messages,
},
};
5 changes: 4 additions & 1 deletion test/package.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ function getNamedOptions(jsonSchema) {
}

const RULES_WITHOUT_PASS_FAIL_SECTIONS = new Set([
'filename-case', // Doesn't show code samples since it's just focused on filenames.
// Doesn't show code samples since it's just focused on filenames.
'filename-case',
// Intended to not use `pass`/`fail` section in this rule.
'prefer-modern-math-apis',
]);

test('Every rule is defined in index file in alphabetical order', t => {
Expand Down
67 changes: 67 additions & 0 deletions test/prefer-modern-math-apis.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import outdent from 'outdent';
import {getTester} from './utils/test.mjs';

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

// `Math.log10()` and `Math.log2()`
const duplicateLog10Test = code => [
code,
// `Math.log2()` test
code.replace(/Math\.LOG10E/g, 'Math.LOG2E').replace(/Math\.LN10/g, 'Math.LN2'),
];
test.snapshot({
valid: [
'Math.log(x) * Math.log(x)',

'Math.LOG10E * Math.LOG10E',
'Math.log(x) * Math[LOG10E]',
'Math.log(x) * LOG10E',
'Math[log](x) * Math.LOG10E',
'foo.Math.log(x) * Math.LOG10E',
'Math.log(x) * foo.Math.LOG10E',
'Math.log(x) * Math.NOT_LOG10E',
'Math.log(x) * Math?.LOG10E',
'Math?.log(x) * Math.LOG10E',
'log(x) * Math.LOG10E',
'new Math.log(x) * Math.LOG10E',
'Math.not_log(x) + Math.LOG10E',
'Math.log(x)[Math.LOG10E]',
'Math.log() * Math.LOG10E',
'Math.log(x, extraArgument) * Math.LOG10E',
'Math.log(...x) * Math.LOG10E',

'Math.LN10 / Math.LN10',
'Math.log(x) /Math[LN10]',
'Math.log(x) / LN10',
'Math[log](x) / Math.LN10',
'foo.Math.log(x) / Math.LN10',
'Math.log(x) / foo.Math.LN10',
'Math.log(x) / Math.log(x)',
'Math.log(x) / Math.NOT_LN10',
'Math.log(x) / Math?.LN10',
'Math?.log(x) / Math.LN10',
'log(x) / Math.LN10',
'new Math.log(x) / Math.LN10',
'Math.not_log(x) + Math.LN10',
'Math.log(x)[Math.LN10]',
'Math.log() / Math.LN10',
'Math.log(x, extraArgument) / Math.LN10',
'Math.log(...x) / Math.LN10',
].flatMap(code => duplicateLog10Test(code)),
invalid: [
'Math.log(x) * Math.LOG10E',
'Math.LOG10E * Math.log(x)',
'Math.log(x) / Math.LN10',
'Math.log((( 0,x ))) * Math.LOG10E',
'Math.LOG10E * Math.log((( 0,x )))',
'Math.log((( 0,x ))) / Math.LN10',
outdent`
function foo(x) {
return (
Math.log(x)
/ Math.LN10
);
}
`,
].flatMap(code => duplicateLog10Test(code)),
});
Loading

0 comments on commit 6dfdeb0

Please sign in to comment.