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.

Sign up for GitHub

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

Jump to bottom

Breaking: some rules recognize bigint literals (fixes #11803) #12701

Merged
merged 14 commits into from Jan 17, 2020
Merged

Breaking: some rules recognize bigint literals (fixes #11803) #12701

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
debb953
update no-magic-numbers to recognize bigint
mysticatea Dec 22, 2019
4c12c64
update yoda to recognize bigint
mysticatea Dec 22, 2019
a6758e0
add a no-extend-native test
mysticatea Dec 22, 2019
3c3cd00
update ci.yml temporary (this PR is blocked by #12700)
mysticatea Dec 22, 2019
513f507
add astUtils.isNumericLiteral and use it in some rules
mysticatea Dec 24, 2019
4475a58
Merge branch 'master' into issue11803
mysticatea Dec 25, 2019
339f932
update no-dupe-class-members
mysticatea Dec 25, 2019
4bfdcb1
update no-magic-number to support bigint in options
mysticatea Dec 26, 2019
7ae3a85
update some rules to use getStaticPropertyName
mysticatea Dec 26, 2019
0921b79
update quote-props
mysticatea Dec 26, 2019
97f38f1
revert no-useless-computed-key change
mysticatea Dec 27, 2019
fbcddfc
revert "allowing {type: 'bigint'}" and update no-magic-number
mysticatea Dec 27, 2019
49b7c70
Merge remote-tracking branch 'origin/master' into issue11803
mysticatea Jan 7, 2020
3695321
no-magic-number 'ignores' allows negative bigint
mysticatea Jan 7, 2020
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
10 changes: 3 additions & 7 deletions .github/workflows/ci.yml
View file
Open in desktop
Expand Up @@ -26,20 +26,16 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest, windows-latest, macOS-latest]
node: [13.x, 12.x, 10.x, 8.x, "8.10.0"]
node: [13.x, 12.x, 10.x, "10.12.0"]
exclude:
- os: windows-latest
node: "8.10.0"
- os: windows-latest
node: 8.x
node: "10.12.0"
- os: windows-latest
node: 10.x
- os: windows-latest
node: 13.x
- os: macOS-latest
node: "8.10.0"
- os: macOS-latest
node: 8.x
node: "10.12.0"
- os: macOS-latest
node: 10.x
- os: macOS-latest
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/no-magic-numbers.js
View file
Open in desktop
Expand Up @@ -65,7 +65,7 @@ module.exports = {
* @returns {boolean} true if the node is a number literal
*/
function isNumber(node) {
return typeof node.value === "number";
return typeof node.value === "number" || Boolean(node.bigint);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a blocker: For my own understanding, is there a reason we're checking the presence of this property versus using typeof like we are below? I wonder if it would be nice to add astUtils.isNumber() and use it everywhere?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have noticed after I wrote this, it's no problem if it's typeof node.value === "number" || typeof node.value === "bigint".

The reason for checking node.bigint was that node.value will be null if the runtime doesn't support bigint natively. However, Node.js 10.4.0 supports bigint and ESLint 7 requires 10.11.0, so it's not a problem.

astUtils.isNumeric() or someting like is a good idea.

Copy link
Sponsor Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does bigint support matter tho? typeof x === 'bigint' will just be false in an engine that doesn't support bigint.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a source code 1n is given, ESLint expects the AST to be { type: "Literal", value: 1n }. However, if the runtime doesn't support BigInt natively, the AST will be { type: "Literal", value: null } then typeof node.value becomes a wrong value.

Copy link
Sponsor Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that seems like a pretty big flaw in the thing that generates the AST - it should probably supply enough information to know that the value isn't a literal null.

}

/**
Expand Down
7 changes: 6 additions & 1 deletion lib/rules/yoda.js
View file
Open in desktop
Expand Up @@ -54,7 +54,12 @@ function looksLikeLiteral(node) {
node.operator === "-" &&
node.prefix &&
node.argument.type === "Literal" &&
typeof node.argument.value === "number");
(
typeof node.argument.value === "number" ||

// Since Node.js 10.4.0, it supports BigInt, so this operation is safe.
typeof node.argument.value === "bigint"
));
}

/**
Expand Down
8 changes: 8 additions & 0 deletions tests/lib/rules/no-extend-native.js
View file
Open in desktop
Expand Up @@ -56,6 +56,14 @@ ruleTester.run("no-extend-native", rule, {
data: { builtin: "Object" },
type: "AssignmentExpression"
}]
}, {
code: "BigInt.prototype.p = 0",
env: { es2020: true },
errors: [{
messageId: "unexpected",
data: { builtin: "BigInt" },
type: "AssignmentExpression"
}]
}, {
code: "Function.prototype['p'] = 0",
errors: [{
Expand Down
20 changes: 20 additions & 0 deletions tests/lib/rules/no-magic-numbers.js
View file
Open in desktop
Expand Up @@ -93,6 +93,26 @@ ruleTester.run("no-magic-numbers", rule, {
{ messageId: "noMagic", data: { raw: "1" } }
]
},
{
code: "var foo = 42n",
options: [{
enforceConst: true
}],
parserOptions: {
ecmaVersion: 2020
},
errors: [{ messageId: "useConst" }]
},
{
code: "var foo = 0n + 1n;",
parserOptions: {
ecmaVersion: 2020
},
errors: [
{ messageId: "noMagic", data: { raw: "0n" } },
{ messageId: "noMagic", data: { raw: "1n" } }
]
},
{
code: "a = a + 5;",
errors: [
Expand Down
42 changes: 42 additions & 0 deletions tests/lib/rules/yoda.js
View file
Open in desktop
Expand Up @@ -91,6 +91,22 @@ ruleTester.run("yoda", rule, {
}, {
code: "if (0 <= a.b && a[\"b\"] <= 100) {}",
options: ["never", { exceptRange: true }]
}, {
code: "if (-1n < x && x <= 1n) {}",
options: ["never", { exceptRange: true }],
parserOptions: { ecmaVersion: 2020 }
}, {
code: "if (x < -1n || 1n <= x) {}",
options: ["never", { exceptRange: true }],
parserOptions: { ecmaVersion: 2020 }
}, {
code: "if (-1n <= x && x < 1n) {}",
options: ["always", { exceptRange: true }],
parserOptions: { ecmaVersion: 2020 }
}, {
code: "if (x < -1n || 1n <= x) {}",
options: ["always", { exceptRange: true }],
parserOptions: { ecmaVersion: 2020 }
},

// onlyEquality
Expand Down Expand Up @@ -136,6 +152,19 @@ ruleTester.run("yoda", rule, {
}
]
},
{
code: "if (5n != value) {}",
output: "if (value != 5n) {}",
options: ["never"],
parserOptions: { ecmaVersion: 2020 },
errors: [
{
messageId: "expected",
data: { expectedSide: "right", operator: "!=" },
type: "BinaryExpression"
}
]
},
{
code: "if (null !== value) {}",
output: "if (value !== null) {}",
Expand Down Expand Up @@ -232,6 +261,19 @@ ruleTester.run("yoda", rule, {
}
]
},
{
code: "if (value === 5n) {}",
output: "if (5n === value) {}",
options: ["always"],
parserOptions: { ecmaVersion: 2020 },
errors: [
{
messageId: "expected",
data: { expectedSide: "left", operator: "===" },
type: "BinaryExpression"
}
]
},
{
code: "if (a < 0 && 0 <= b && b < 1) {}",
output: "if (a < 0 && b >= 0 && b < 1) {}",
Expand Down