From e76c3827727b48c16af8467c02c31160e5595d83 Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Fri, 11 Nov 2022 01:54:12 +0100 Subject: [PATCH] fix: allow `* 1` when followed by `/` in no-implicit-coercion (#16522) * fix: allow `* 1` when followed by `/` in no-implicit-coercion For example, `foo * 1 / bar` will be allowed as it can be logically interpreted as `foo * (1 / bar)`, although it's technically `(foo * 1) / bar` where `foo * 1` would be flagged as implicit coercion. Fixes #16373 * add one more regression test --- docs/src/rules/no-implicit-coercion.md | 2 ++ lib/rules/no-implicit-coercion.js | 21 +++++++++++- tests/lib/rules/no-implicit-coercion.js | 45 ++++++++++++++++++++++++- 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/docs/src/rules/no-implicit-coercion.md b/docs/src/rules/no-implicit-coercion.md index dd4d3689ba7..a7d30542059 100644 --- a/docs/src/rules/no-implicit-coercion.md +++ b/docs/src/rules/no-implicit-coercion.md @@ -102,6 +102,8 @@ Examples of **correct** code for the default `{ "number": true }` option: var n = Number(foo); var n = parseFloat(foo); var n = parseInt(foo, 10); + +var n = foo * 1/4; // `* 1` is allowed when followed by the `/` operator ``` ::: diff --git a/lib/rules/no-implicit-coercion.js b/lib/rules/no-implicit-coercion.js index c2367715d9d..d4126112597 100644 --- a/lib/rules/no-implicit-coercion.js +++ b/lib/rules/no-implicit-coercion.js @@ -71,6 +71,24 @@ function isMultiplyByOne(node) { ); } +/** + * Checks whether the given node logically represents multiplication by a fraction of `1`. + * For example, `a * 1` in `a * 1 / b` is technically multiplication by `1`, but the + * whole expression can be logically interpreted as `a * (1 / b)` rather than `(a * 1) / b`. + * @param {BinaryExpression} node A BinaryExpression node to check. + * @param {SourceCode} sourceCode The source code object. + * @returns {boolean} Whether or not the node is a multiplying by a fraction of `1`. + */ +function isMultiplyByFractionOfOne(node, sourceCode) { + return node.type === "BinaryExpression" && + node.operator === "*" && + (node.right.type === "Literal" && node.right.value === 1) && + node.parent.type === "BinaryExpression" && + node.parent.operator === "/" && + node.parent.left === node && + !astUtils.isParenthesised(sourceCode, node); +} + /** * Checks whether the result of a node is numeric or not * @param {ASTNode} node The node to test @@ -290,7 +308,8 @@ module.exports = { // 1 * foo operatorAllowed = options.allow.includes("*"); - const nonNumericOperand = !operatorAllowed && options.number && isMultiplyByOne(node) && getNonNumericOperand(node); + const nonNumericOperand = !operatorAllowed && options.number && isMultiplyByOne(node) && !isMultiplyByFractionOfOne(node, sourceCode) && + getNonNumericOperand(node); if (nonNumericOperand) { const recommendation = `Number(${sourceCode.getText(nonNumericOperand)})`; diff --git a/tests/lib/rules/no-implicit-coercion.js b/tests/lib/rules/no-implicit-coercion.js index f7ca9dcff3f..e935081e6f3 100644 --- a/tests/lib/rules/no-implicit-coercion.js +++ b/tests/lib/rules/no-implicit-coercion.js @@ -104,7 +104,12 @@ ruleTester.run("no-implicit-coercion", rule, { { code: "String(foo) + ``", parserOptions: { ecmaVersion: 6 } }, { code: "`${'foo'}`", options: [{ disallowTemplateShorthand: true }], parserOptions: { ecmaVersion: 6 } }, { code: "`${`foo`}`", options: [{ disallowTemplateShorthand: true }], parserOptions: { ecmaVersion: 6 } }, - { code: "`${String(foo)}`", options: [{ disallowTemplateShorthand: true }], parserOptions: { ecmaVersion: 6 } } + { code: "`${String(foo)}`", options: [{ disallowTemplateShorthand: true }], parserOptions: { ecmaVersion: 6 } }, + + // https://github.com/eslint/eslint/issues/16373 + "console.log(Math.PI * 1/4)", + "a * 1 / 2", + "a * 1 / b" ], invalid: [ { @@ -426,6 +431,44 @@ ruleTester.run("no-implicit-coercion", rule, { data: { recommendation: "(foo?.indexOf)(1) !== -1" }, type: "UnaryExpression" }] + }, + + // https://github.com/eslint/eslint/issues/16373 regression tests + { + code: "1 * a / 2", + output: "Number(a) / 2", + errors: [{ + messageId: "useRecommendation", + data: { recommendation: "Number(a)" }, + type: "BinaryExpression" + }] + }, + { + code: "(a * 1) / 2", + output: "(Number(a)) / 2", + errors: [{ + messageId: "useRecommendation", + data: { recommendation: "Number(a)" }, + type: "BinaryExpression" + }] + }, + { + code: "a * 1 / (b * 1)", + output: "a * 1 / (Number(b))", + errors: [{ + messageId: "useRecommendation", + data: { recommendation: "Number(b)" }, + type: "BinaryExpression" + }] + }, + { + code: "a * 1 + 2", + output: "Number(a) + 2", + errors: [{ + messageId: "useRecommendation", + data: { recommendation: "Number(a)" }, + type: "BinaryExpression" + }] } ] });