From 86cf9c0aeb62d864c7b92a3ff99b3f00385a2642 Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Tue, 8 Nov 2022 18:14:41 +0100 Subject: [PATCH 1/2] 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 --- docs/src/rules/no-implicit-coercion.md | 2 ++ lib/rules/no-implicit-coercion.js | 21 ++++++++++++++- tests/lib/rules/no-implicit-coercion.js | 36 ++++++++++++++++++++++++- 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/docs/src/rules/no-implicit-coercion.md b/docs/src/rules/no-implicit-coercion.md index 746c9889f5a..e1a7c3b02c0 100644 --- a/docs/src/rules/no-implicit-coercion.md +++ b/docs/src/rules/no-implicit-coercion.md @@ -103,6 +103,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..c6ee117817c 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..111430378f7 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,35 @@ ruleTester.run("no-implicit-coercion", rule, { data: { recommendation: "(foo?.indexOf)(1) !== -1" }, type: "UnaryExpression" }] + }, + + // https://github.com/eslint/eslint/issues/16373 + { + 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" + }] } ] }); From 254de1ad0ac6104c3f6c2581f94947ed08b9c5a0 Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Tue, 8 Nov 2022 19:07:46 +0100 Subject: [PATCH 2/2] add one more regression test --- lib/rules/no-implicit-coercion.js | 2 +- tests/lib/rules/no-implicit-coercion.js | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/rules/no-implicit-coercion.js b/lib/rules/no-implicit-coercion.js index c6ee117817c..d4126112597 100644 --- a/lib/rules/no-implicit-coercion.js +++ b/lib/rules/no-implicit-coercion.js @@ -76,7 +76,7 @@ function isMultiplyByOne(node) { * 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 + * @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) { diff --git a/tests/lib/rules/no-implicit-coercion.js b/tests/lib/rules/no-implicit-coercion.js index 111430378f7..e935081e6f3 100644 --- a/tests/lib/rules/no-implicit-coercion.js +++ b/tests/lib/rules/no-implicit-coercion.js @@ -433,7 +433,7 @@ ruleTester.run("no-implicit-coercion", rule, { }] }, - // https://github.com/eslint/eslint/issues/16373 + // https://github.com/eslint/eslint/issues/16373 regression tests { code: "1 * a / 2", output: "Number(a) / 2", @@ -460,6 +460,15 @@ ruleTester.run("no-implicit-coercion", rule, { data: { recommendation: "Number(b)" }, type: "BinaryExpression" }] + }, + { + code: "a * 1 + 2", + output: "Number(a) + 2", + errors: [{ + messageId: "useRecommendation", + data: { recommendation: "Number(a)" }, + type: "BinaryExpression" + }] } ] });