Skip to content

Commit e76c382

Browse files
authored
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
1 parent 68f1288 commit e76c382

3 files changed

Lines changed: 66 additions & 2 deletions

File tree

docs/src/rules/no-implicit-coercion.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ Examples of **correct** code for the default `{ "number": true }` option:
102102
var n = Number(foo);
103103
var n = parseFloat(foo);
104104
var n = parseInt(foo, 10);
105+
106+
var n = foo * 1/4; // `* 1` is allowed when followed by the `/` operator
105107
```
106108

107109
:::

lib/rules/no-implicit-coercion.js

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,24 @@ function isMultiplyByOne(node) {
7171
);
7272
}
7373

74+
/**
75+
* Checks whether the given node logically represents multiplication by a fraction of `1`.
76+
* For example, `a * 1` in `a * 1 / b` is technically multiplication by `1`, but the
77+
* whole expression can be logically interpreted as `a * (1 / b)` rather than `(a * 1) / b`.
78+
* @param {BinaryExpression} node A BinaryExpression node to check.
79+
* @param {SourceCode} sourceCode The source code object.
80+
* @returns {boolean} Whether or not the node is a multiplying by a fraction of `1`.
81+
*/
82+
function isMultiplyByFractionOfOne(node, sourceCode) {
83+
return node.type === "BinaryExpression" &&
84+
node.operator === "*" &&
85+
(node.right.type === "Literal" && node.right.value === 1) &&
86+
node.parent.type === "BinaryExpression" &&
87+
node.parent.operator === "/" &&
88+
node.parent.left === node &&
89+
!astUtils.isParenthesised(sourceCode, node);
90+
}
91+
7492
/**
7593
* Checks whether the result of a node is numeric or not
7694
* @param {ASTNode} node The node to test
@@ -290,7 +308,8 @@ module.exports = {
290308

291309
// 1 * foo
292310
operatorAllowed = options.allow.includes("*");
293-
const nonNumericOperand = !operatorAllowed && options.number && isMultiplyByOne(node) && getNonNumericOperand(node);
311+
const nonNumericOperand = !operatorAllowed && options.number && isMultiplyByOne(node) && !isMultiplyByFractionOfOne(node, sourceCode) &&
312+
getNonNumericOperand(node);
294313

295314
if (nonNumericOperand) {
296315
const recommendation = `Number(${sourceCode.getText(nonNumericOperand)})`;

tests/lib/rules/no-implicit-coercion.js

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,12 @@ ruleTester.run("no-implicit-coercion", rule, {
104104
{ code: "String(foo) + ``", parserOptions: { ecmaVersion: 6 } },
105105
{ code: "`${'foo'}`", options: [{ disallowTemplateShorthand: true }], parserOptions: { ecmaVersion: 6 } },
106106
{ code: "`${`foo`}`", options: [{ disallowTemplateShorthand: true }], parserOptions: { ecmaVersion: 6 } },
107-
{ code: "`${String(foo)}`", options: [{ disallowTemplateShorthand: true }], parserOptions: { ecmaVersion: 6 } }
107+
{ code: "`${String(foo)}`", options: [{ disallowTemplateShorthand: true }], parserOptions: { ecmaVersion: 6 } },
108+
109+
// https://github.com/eslint/eslint/issues/16373
110+
"console.log(Math.PI * 1/4)",
111+
"a * 1 / 2",
112+
"a * 1 / b"
108113
],
109114
invalid: [
110115
{
@@ -426,6 +431,44 @@ ruleTester.run("no-implicit-coercion", rule, {
426431
data: { recommendation: "(foo?.indexOf)(1) !== -1" },
427432
type: "UnaryExpression"
428433
}]
434+
},
435+
436+
// https://github.com/eslint/eslint/issues/16373 regression tests
437+
{
438+
code: "1 * a / 2",
439+
output: "Number(a) / 2",
440+
errors: [{
441+
messageId: "useRecommendation",
442+
data: { recommendation: "Number(a)" },
443+
type: "BinaryExpression"
444+
}]
445+
},
446+
{
447+
code: "(a * 1) / 2",
448+
output: "(Number(a)) / 2",
449+
errors: [{
450+
messageId: "useRecommendation",
451+
data: { recommendation: "Number(a)" },
452+
type: "BinaryExpression"
453+
}]
454+
},
455+
{
456+
code: "a * 1 / (b * 1)",
457+
output: "a * 1 / (Number(b))",
458+
errors: [{
459+
messageId: "useRecommendation",
460+
data: { recommendation: "Number(b)" },
461+
type: "BinaryExpression"
462+
}]
463+
},
464+
{
465+
code: "a * 1 + 2",
466+
output: "Number(a) + 2",
467+
errors: [{
468+
messageId: "useRecommendation",
469+
data: { recommendation: "Number(a)" },
470+
type: "BinaryExpression"
471+
}]
429472
}
430473
]
431474
});

0 commit comments

Comments
 (0)