Skip to content

Commit

Permalink
Add prefer-math-min-max (#2432)
Browse files Browse the repository at this point in the history
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
Co-authored-by: fisker <lionkay@gmail.com>
  • Loading branch information
3 people authored Aug 23, 2024
1 parent 1e367bb commit 7369077
Show file tree
Hide file tree
Showing 7 changed files with 761 additions and 0 deletions.
58 changes: 58 additions & 0 deletions docs/rules/prefer-math-min-max.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# Prefer `Math.min()` and `Math.max()` over ternaries for simple comparisons

💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#preset-configs-eslintconfigjs).

🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).

<!-- end auto-generated rule header -->
<!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` -->

This rule enforces the use of `Math.min()` and `Math.max()` functions instead of ternary expressions when performing simple comparisons, such as selecting the minimum or maximum value between two or more options.

By replacing ternary expressions with these functions, the code becomes more concise, easier to understand, and less prone to errors. It also enhances consistency across the codebase, ensuring that the same approach is used for similar operations, ultimately improving the overall readability and maintainability of the code.

## Examples

<!-- Math.min() -->

```js
height > 50 ? 50 : height; //
Math.min(height, 50); //
```

```js
height >= 50 ? 50 : height; //
Math.min(height, 50); //
```

```js
height < 50 ? height : 50; //
Math.min(height, 50); //
```

```js
height <= 50 ? height : 50; //
Math.min(height, 50); //
```

<!-- Math.max() -->

```js
height > 50 ? height : 50; //
Math.max(height, 50); //
```

```js
height >= 50 ? height : 50; //
Math.max(height, 50); //
```

```js
height < 50 ? 50 : height; //
Math.max(height, 50); //
```

```js
height <= 50 ? 50 : height; //
Math.max(height, 50); //
```
1 change: 1 addition & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ If you don't use the preset, ensure you use the same `env` and `parserOptions` c
| [prefer-json-parse-buffer](docs/rules/prefer-json-parse-buffer.md) | Prefer reading a JSON file as a buffer. | | 🔧 | |
| [prefer-keyboard-event-key](docs/rules/prefer-keyboard-event-key.md) | Prefer `KeyboardEvent#key` over `KeyboardEvent#keyCode`. || 🔧 | |
| [prefer-logical-operator-over-ternary](docs/rules/prefer-logical-operator-over-ternary.md) | Prefer using a logical operator over a ternary. || | 💡 |
| [prefer-math-min-max](docs/rules/prefer-math-min-max.md) | Prefer `Math.min()` and `Math.max()` over ternaries for simple comparisons. || 🔧 | |
| [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. || 🔧 | |
Expand Down
80 changes: 80 additions & 0 deletions rules/prefer-math-min-max.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
'use strict';
const {fixSpaceAroundKeyword} = require('./fix/index.js');

const MESSAGE_ID = 'prefer-math-min-max';
const messages = {
[MESSAGE_ID]: 'Prefer `Math.{{method}}()` to simplify ternary expressions.',
};

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => ({
/** @param {import('estree').ConditionalExpression} conditionalExpression */
ConditionalExpression(conditionalExpression) {
const {test, consequent, alternate} = conditionalExpression;

if (test.type !== 'BinaryExpression') {
return;
}

const {operator, left, right} = test;
const [leftText, rightText, alternateText, consequentText] = [left, right, alternate, consequent].map(node => context.sourceCode.getText(node));

const isGreaterOrEqual = operator === '>' || operator === '>=';
const isLessOrEqual = operator === '<' || operator === '<=';

let method;

// Prefer `Math.min()`
if (
// `height > 50 ? 50 : height`
(isGreaterOrEqual && leftText === alternateText && rightText === consequentText)
// `height < 50 ? height : 50`
|| (isLessOrEqual && leftText === consequentText && rightText === alternateText)
) {
method = 'min';
} else if (
// `height > 50 ? height : 50`
(isGreaterOrEqual && leftText === consequentText && rightText === alternateText)
// `height < 50 ? 50 : height`
|| (isLessOrEqual && leftText === alternateText && rightText === consequentText)
) {
method = 'max';
}

if (!method) {
return;
}

return {
node: conditionalExpression,
messageId: MESSAGE_ID,
data: {method},
/** @param {import('eslint').Rule.RuleFixer} fixer */
* fix(fixer) {
const {sourceCode} = context;

yield * fixSpaceAroundKeyword(fixer, conditionalExpression, sourceCode);

const argumentsText = [left, right]
.map(node => node.type === 'SequenceExpression' ? `(${sourceCode.getText(node)})` : sourceCode.getText(node))
.join(', ');

yield fixer.replaceText(conditionalExpression, `Math.${method}(${argumentsText})`);
},
};
},
});

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
create,
meta: {
type: 'problem',
docs: {
description: 'Prefer `Math.min()` and `Math.max()` over ternaries for simple comparisons.',
recommended: true,
},
fixable: 'code',
messages,
},
};
1 change: 1 addition & 0 deletions test/package.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const RULES_WITHOUT_PASS_FAIL_SECTIONS = new Set([
'filename-case',
// Intended to not use `pass`/`fail` section in this rule.
'prefer-modern-math-apis',
'prefer-math-min-max',
]);

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

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

test.snapshot({
valid: [
'height > 10 ? height : 20',
'height > 50 ? Math.min(50, height) : height',
'foo ? foo : bar',
],
invalid: [
// Prefer `Math.min()`
'height > 50 ? 50 : height',
'height >= 50 ? 50 : height',
'height < 50 ? height : 50',
'height <= 50 ? height : 50',

// Prefer `Math.min()`
'height > maxHeight ? maxHeight : height',
'height < maxHeight ? height : maxHeight',

// Prefer `Math.min()`
'window.height > 50 ? 50 : window.height',
'window.height < 50 ? window.height : 50',

// Prefer `Math.max()`
'height > 50 ? height : 50',
'height >= 50 ? height : 50',
'height < 50 ? 50 : height',
'height <= 50 ? 50 : height',

// Prefer `Math.max()`
'height > maxHeight ? height : maxHeight',
'height < maxHeight ? maxHeight : height',

// Edge test when there is no space between ReturnStatement and ConditionalExpression
outdent`
function a() {
return +foo > 10 ? 10 : +foo
}
`,
outdent`
function a() {
return+foo > 10 ? 10 : +foo
}
`,

'(0,foo) > 10 ? 10 : (0,foo)',

'foo.bar() > 10 ? 10 : foo.bar()',
outdent`
async function foo() {
return await foo.bar() > 10 ? 10 : await foo.bar()
}
`,
outdent`
async function foo() {
await(+foo > 10 ? 10 : +foo)
}
`,
outdent`
function foo() {
return(foo.bar() > 10) ? 10 : foo.bar()
}
`,
outdent`
function* foo() {
yield+foo > 10 ? 10 : +foo
}
`,
'export default+foo > 10 ? 10 : +foo',

'foo.length > bar.length ? bar.length : foo.length',
],
});
Loading

0 comments on commit 7369077

Please sign in to comment.