Skip to content

Commit

Permalink
Merge branch 'main' into no-identical-alternate
Browse files Browse the repository at this point in the history
  • Loading branch information
axetroy authored Aug 23, 2024
2 parents 44e442a + 7369077 commit 7f729df
Show file tree
Hide file tree
Showing 7 changed files with 762 additions and 1 deletion.
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 @@ -194,6 +194,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,
},
};
3 changes: 2 additions & 1 deletion test/package.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ const RULES_WITHOUT_PASS_FAIL_SECTIONS = new Set([
'filename-case',
// Intended to not use `pass`/`fail` section in this rule.
'prefer-modern-math-apis',
'no-identical-alternatives-in-ternary',
'prefer-math-min-max',
'no-identical-alternatives-in-ternary',

Check failure on line 34 in test/package.mjs

View workflow job for this annotation

GitHub Actions / lint-test (ubuntu-latest)

Expected indentation of 1 tab but found 2 spaces.
]);

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 7f729df

Please sign in to comment.