Skip to content

Commit

Permalink
fix: allow * 1 when followed by / in no-implicit-coercion (#16522)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
mdjermanovic committed Nov 11, 2022
1 parent 68f1288 commit e76c382
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 2 deletions.
2 changes: 2 additions & 0 deletions docs/src/rules/no-implicit-coercion.md
Expand Up @@ -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
```

:::
Expand Down
21 changes: 20 additions & 1 deletion lib/rules/no-implicit-coercion.js
Expand Up @@ -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
Expand Down Expand Up @@ -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)})`;
Expand Down
45 changes: 44 additions & 1 deletion tests/lib/rules/no-implicit-coercion.js
Expand Up @@ -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: [
{
Expand Down Expand Up @@ -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"
}]
}
]
});

0 comments on commit e76c382

Please sign in to comment.