diff --git a/docs/rules/prefer-math-min-max.md b/docs/rules/prefer-math-min-max.md new file mode 100644 index 0000000000..c5de13f45e --- /dev/null +++ b/docs/rules/prefer-math-min-max.md @@ -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). + + + + +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 + + + +```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); // βœ… +``` + + + +```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); // βœ… +``` diff --git a/readme.md b/readme.md index df459ba791..33619961eb 100644 --- a/readme.md +++ b/readme.md @@ -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. | βœ… | πŸ”§ | | diff --git a/rules/prefer-math-min-max.js b/rules/prefer-math-min-max.js new file mode 100644 index 0000000000..fdad464fa4 --- /dev/null +++ b/rules/prefer-math-min-max.js @@ -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, + }, +}; diff --git a/test/package.mjs b/test/package.mjs index 96bfcb1351..7434537ca7 100644 --- a/test/package.mjs +++ b/test/package.mjs @@ -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 => { diff --git a/test/prefer-math-min-max.mjs b/test/prefer-math-min-max.mjs new file mode 100644 index 0000000000..6da574dfbd --- /dev/null +++ b/test/prefer-math-min-max.mjs @@ -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', + ], +}); diff --git a/test/snapshots/prefer-math-min-max.mjs.md b/test/snapshots/prefer-math-min-max.mjs.md new file mode 100644 index 0000000000..daa61b660c --- /dev/null +++ b/test/snapshots/prefer-math-min-max.mjs.md @@ -0,0 +1,545 @@ +# Snapshot report for `test/prefer-math-min-max.mjs` + +The actual snapshot is saved in `prefer-math-min-max.mjs.snap`. + +Generated by [AVA](https://avajs.dev). + +## invalid(1): height > 50 ? 50 : height + +> Input + + `␊ + 1 | height > 50 ? 50 : height␊ + ` + +> Output + + `␊ + 1 | Math.min(height, 50)␊ + ` + +> Error 1/1 + + `␊ + > 1 | height > 50 ? 50 : height␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊ + ` + +## invalid(2): height >= 50 ? 50 : height + +> Input + + `␊ + 1 | height >= 50 ? 50 : height␊ + ` + +> Output + + `␊ + 1 | Math.min(height, 50)␊ + ` + +> Error 1/1 + + `␊ + > 1 | height >= 50 ? 50 : height␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊ + ` + +## invalid(3): height < 50 ? height : 50 + +> Input + + `␊ + 1 | height < 50 ? height : 50␊ + ` + +> Output + + `␊ + 1 | Math.min(height, 50)␊ + ` + +> Error 1/1 + + `␊ + > 1 | height < 50 ? height : 50␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊ + ` + +## invalid(4): height <= 50 ? height : 50 + +> Input + + `␊ + 1 | height <= 50 ? height : 50␊ + ` + +> Output + + `␊ + 1 | Math.min(height, 50)␊ + ` + +> Error 1/1 + + `␊ + > 1 | height <= 50 ? height : 50␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊ + ` + +## invalid(5): height > maxHeight ? maxHeight : height + +> Input + + `␊ + 1 | height > maxHeight ? maxHeight : height␊ + ` + +> Output + + `␊ + 1 | Math.min(height, maxHeight)␊ + ` + +> Error 1/1 + + `␊ + > 1 | height > maxHeight ? maxHeight : height␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊ + ` + +## invalid(6): height < maxHeight ? height : maxHeight + +> Input + + `␊ + 1 | height < maxHeight ? height : maxHeight␊ + ` + +> Output + + `␊ + 1 | Math.min(height, maxHeight)␊ + ` + +> Error 1/1 + + `␊ + > 1 | height < maxHeight ? height : maxHeight␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊ + ` + +## invalid(7): window.height > 50 ? 50 : window.height + +> Input + + `␊ + 1 | window.height > 50 ? 50 : window.height␊ + ` + +> Output + + `␊ + 1 | Math.min(window.height, 50)␊ + ` + +> Error 1/1 + + `␊ + > 1 | window.height > 50 ? 50 : window.height␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊ + ` + +## invalid(8): window.height < 50 ? window.height : 50 + +> Input + + `␊ + 1 | window.height < 50 ? window.height : 50␊ + ` + +> Output + + `␊ + 1 | Math.min(window.height, 50)␊ + ` + +> Error 1/1 + + `␊ + > 1 | window.height < 50 ? window.height : 50␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊ + ` + +## invalid(9): height > 50 ? height : 50 + +> Input + + `␊ + 1 | height > 50 ? height : 50␊ + ` + +> Output + + `␊ + 1 | Math.max(height, 50)␊ + ` + +> Error 1/1 + + `␊ + > 1 | height > 50 ? height : 50␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.max()\` to simplify ternary expressions.␊ + ` + +## invalid(10): height >= 50 ? height : 50 + +> Input + + `␊ + 1 | height >= 50 ? height : 50␊ + ` + +> Output + + `␊ + 1 | Math.max(height, 50)␊ + ` + +> Error 1/1 + + `␊ + > 1 | height >= 50 ? height : 50␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.max()\` to simplify ternary expressions.␊ + ` + +## invalid(11): height < 50 ? 50 : height + +> Input + + `␊ + 1 | height < 50 ? 50 : height␊ + ` + +> Output + + `␊ + 1 | Math.max(height, 50)␊ + ` + +> Error 1/1 + + `␊ + > 1 | height < 50 ? 50 : height␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.max()\` to simplify ternary expressions.␊ + ` + +## invalid(12): height <= 50 ? 50 : height + +> Input + + `␊ + 1 | height <= 50 ? 50 : height␊ + ` + +> Output + + `␊ + 1 | Math.max(height, 50)␊ + ` + +> Error 1/1 + + `␊ + > 1 | height <= 50 ? 50 : height␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.max()\` to simplify ternary expressions.␊ + ` + +## invalid(13): height > maxHeight ? height : maxHeight + +> Input + + `␊ + 1 | height > maxHeight ? height : maxHeight␊ + ` + +> Output + + `␊ + 1 | Math.max(height, maxHeight)␊ + ` + +> Error 1/1 + + `␊ + > 1 | height > maxHeight ? height : maxHeight␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.max()\` to simplify ternary expressions.␊ + ` + +## invalid(14): height < maxHeight ? maxHeight : height + +> Input + + `␊ + 1 | height < maxHeight ? maxHeight : height␊ + ` + +> Output + + `␊ + 1 | Math.max(height, maxHeight)␊ + ` + +> Error 1/1 + + `␊ + > 1 | height < maxHeight ? maxHeight : height␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.max()\` to simplify ternary expressions.␊ + ` + +## invalid(15): function a() { return +foo > 10 ? 10 : +foo } + +> Input + + `␊ + 1 | function a() {␊ + 2 | return +foo > 10 ? 10 : +foo␊ + 3 | }␊ + ` + +> Output + + `␊ + 1 | function a() {␊ + 2 | return Math.min(+foo, 10)␊ + 3 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | function a() {␊ + > 2 | return +foo > 10 ? 10 : +foo␊ + | ^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊ + 3 | }␊ + ` + +## invalid(16): function a() { return+foo > 10 ? 10 : +foo } + +> Input + + `␊ + 1 | function a() {␊ + 2 | return+foo > 10 ? 10 : +foo␊ + 3 | }␊ + ` + +> Output + + `␊ + 1 | function a() {␊ + 2 | return Math.min(+foo, 10)␊ + 3 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | function a() {␊ + > 2 | return+foo > 10 ? 10 : +foo␊ + | ^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊ + 3 | }␊ + ` + +## invalid(17): (0,foo) > 10 ? 10 : (0,foo) + +> Input + + `␊ + 1 | (0,foo) > 10 ? 10 : (0,foo)␊ + ` + +> Output + + `␊ + 1 | Math.min((0,foo), 10)␊ + ` + +> Error 1/1 + + `␊ + > 1 | (0,foo) > 10 ? 10 : (0,foo)␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊ + ` + +## invalid(18): foo.bar() > 10 ? 10 : foo.bar() + +> Input + + `␊ + 1 | foo.bar() > 10 ? 10 : foo.bar()␊ + ` + +> Output + + `␊ + 1 | Math.min(foo.bar(), 10)␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.bar() > 10 ? 10 : foo.bar()␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊ + ` + +## invalid(19): async function foo() { return await foo.bar() > 10 ? 10 : await foo.bar() } + +> Input + + `␊ + 1 | async function foo() {␊ + 2 | return await foo.bar() > 10 ? 10 : await foo.bar()␊ + 3 | }␊ + ` + +> Output + + `␊ + 1 | async function foo() {␊ + 2 | return Math.min(await foo.bar(), 10)␊ + 3 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | async function foo() {␊ + > 2 | return await foo.bar() > 10 ? 10 : await foo.bar()␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊ + 3 | }␊ + ` + +## invalid(20): async function foo() { await(+foo > 10 ? 10 : +foo) } + +> Input + + `␊ + 1 | async function foo() {␊ + 2 | await(+foo > 10 ? 10 : +foo)␊ + 3 | }␊ + ` + +> Output + + `␊ + 1 | async function foo() {␊ + 2 | await (Math.min(+foo, 10))␊ + 3 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | async function foo() {␊ + > 2 | await(+foo > 10 ? 10 : +foo)␊ + | ^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊ + 3 | }␊ + ` + +## invalid(21): function foo() { return(foo.bar() > 10) ? 10 : foo.bar() } + +> Input + + `␊ + 1 | function foo() {␊ + 2 | return(foo.bar() > 10) ? 10 : foo.bar()␊ + 3 | }␊ + ` + +> Output + + `␊ + 1 | function foo() {␊ + 2 | return Math.min(foo.bar(), 10)␊ + 3 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | function foo() {␊ + > 2 | return(foo.bar() > 10) ? 10 : foo.bar()␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊ + 3 | }␊ + ` + +## invalid(22): function* foo() { yield+foo > 10 ? 10 : +foo } + +> Input + + `␊ + 1 | function* foo() {␊ + 2 | yield+foo > 10 ? 10 : +foo␊ + 3 | }␊ + ` + +> Output + + `␊ + 1 | function* foo() {␊ + 2 | yield Math.min(+foo, 10)␊ + 3 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | function* foo() {␊ + > 2 | yield+foo > 10 ? 10 : +foo␊ + | ^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊ + 3 | }␊ + ` + +## invalid(23): export default+foo > 10 ? 10 : +foo + +> Input + + `␊ + 1 | export default+foo > 10 ? 10 : +foo␊ + ` + +> Output + + `␊ + 1 | export default Math.min(+foo, 10)␊ + ` + +> Error 1/1 + + `␊ + > 1 | export default+foo > 10 ? 10 : +foo␊ + | ^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊ + ` + +## invalid(24): foo.length > bar.length ? bar.length : foo.length + +> Input + + `␊ + 1 | foo.length > bar.length ? bar.length : foo.length␊ + ` + +> Output + + `␊ + 1 | Math.min(foo.length, bar.length)␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.length > bar.length ? bar.length : foo.length␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`Math.min()\` to simplify ternary expressions.␊ + ` diff --git a/test/snapshots/prefer-math-min-max.mjs.snap b/test/snapshots/prefer-math-min-max.mjs.snap new file mode 100644 index 0000000000..7847c6b5bf Binary files /dev/null and b/test/snapshots/prefer-math-min-max.mjs.snap differ