Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow * 1 when followed by / in no-implicit-coercion #16522

Merged
merged 2 commits into from Nov 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/src/rules/no-implicit-coercion.md
Expand Up @@ -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
```

:::
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"
}]
}
]
});