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.

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

Add prefer-modern-math-apis rule #1780

Merged
merged 16 commits into from
Apr 1, 2022
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)),
});