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

Update: support numeric separators (refs #13568) #13581

Merged
merged 3 commits into from Aug 29, 2020
Merged

Update: support numeric separators (refs #13568) #13581

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9fd6fc4
update astUtils.isDecimalInteger and astUtils.isDecimalIntegerNumeric…
mdjermanovic Aug 18, 2020
ff0a646
add tests and comment for prefer-numeric-literals
mdjermanovic Aug 18, 2020
785a7cc
add tests for quote-props
mdjermanovic Aug 18, 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: 10 additions & 0 deletions lib/rules/prefer-numeric-literals.js
View file
Open in desktop
Expand Up @@ -103,6 +103,16 @@ module.exports = {
/*
* If the newly-produced literal would be invalid, (e.g. 0b1234),
* or it would yield an incorrect parseInt result for some other reason, don't make a fix.
*
* If `str` had numeric separators, `+replacement` will evaluate to `NaN` because unary `+`
* per the specification doesn't support numeric separators. Thus, the above condition will be `true`
* (`NaN !== anything` is always `true`) regardless of the `parseInt(str, radix)` value.
* Consequently, no autofixes will be made. This is correct behavior because `parseInt` also
* doesn't support numeric separators, but it does parse part of the string before the first `_`,
* so the autofix would be invalid:
*
* parseInt("1_1", 2) // === 1
* 0b1_1 // === 3
*/
return null;
}
Expand Down
31 changes: 20 additions & 11 deletions lib/rules/utils/ast-utils.js
View file
Open in desktop
Expand Up @@ -37,7 +37,7 @@ const LINEBREAKS = new Set(["\r\n", "\r", "\n", "\u2028", "\u2029"]);
// A set of node types that can contain a list of statements
const STATEMENT_LIST_PARENTS = new Set(["Program", "BlockStatement", "SwitchCase"]);

const DECIMAL_INTEGER_PATTERN = /^(0|[1-9]\d*)$/u;
const DECIMAL_INTEGER_PATTERN = /^(0|[1-9](?:_?\d)*)$/u;
Copy link
Member Author

Choose a reason for hiding this comment

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

This actually had a bug: it doesn't recognize NonOctalDecimalIntegerLiteral literals, like 08.

I think we can fix this later since it's an existing edge-case bug that isn't related to numeric separators and requires a non-trivial change.

Copy link
Member

Choose a reason for hiding this comment

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

Mind making an issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

Opened #13588

const OCTAL_ESCAPE_PATTERN = /^(?:[^\\]|\\[^0-7]|\\0(?![0-9]))*\\(?:[1-7]|0[0-9])/u;

/**
Expand Down Expand Up @@ -1228,16 +1228,25 @@ module.exports = {
* @returns {boolean} `true` if this node is a decimal integer.
* @example
*
* 5 // true
* 5. // false
* 5.0 // false
* 05 // false
* 0x5 // false
* 0b101 // false
* 0o5 // false
* 5e0 // false
* '5' // false
* 5n // false
* 0 // true
* 5 // true
* 50 // true
* 5_000 // true
* 1_234_56 // true
* 5. // false
* .5 // false
* 5.0 // false
* 5.00_00 // false
* 05 // false
* 0x5 // false
* 0b101 // false
* 0b11_01 // false
* 0o5 // false
* 5e0 // false
* 5e1_000 // false
* 5n // false
* 1_000n // false
* '5' // false
*/
isDecimalInteger(node) {
return node.type === "Literal" && typeof node.value === "number" &&
Expand Down
28 changes: 28 additions & 0 deletions tests/lib/rules/dot-location.js
View file
Open in desktop
Expand Up @@ -231,6 +231,34 @@ ruleTester.run("dot-location", rule, {
options: ["object"],
errors: [{ messageId: "expectedDotAfterObject", type: "MemberExpression", line: 2, column: 1 }]
},
{
code: "5_000\n.toExponential()",
output: "5_000 .\ntoExponential()",
options: ["object"],
parserOptions: { ecmaVersion: 2021 },
errors: [{ messageId: "expectedDotAfterObject", type: "MemberExpression", line: 2, column: 1 }]
},
{
code: "5_000_00\n.toExponential()",
output: "5_000_00 .\ntoExponential()",
options: ["object"],
parserOptions: { ecmaVersion: 2021 },
errors: [{ messageId: "expectedDotAfterObject", type: "MemberExpression", line: 2, column: 1 }]
},
{
code: "5.000_000\n.toExponential()",
output: "5.000_000.\ntoExponential()",
options: ["object"],
parserOptions: { ecmaVersion: 2021 },
errors: [{ messageId: "expectedDotAfterObject", type: "MemberExpression", line: 2, column: 1 }]
},
{
code: "0b1010_1010\n.toExponential()",
output: "0b1010_1010.\ntoExponential()",
options: ["object"],
parserOptions: { ecmaVersion: 2021 },
errors: [{ messageId: "expectedDotAfterObject", type: "MemberExpression", line: 2, column: 1 }]
},
{
code: "foo /* a */ . /* b */ \n /* c */ bar",
output: "foo /* a */ /* b */ \n /* c */ .bar",
Expand Down
34 changes: 34 additions & 0 deletions tests/lib/rules/dot-notation.js
View file
Open in desktop
Expand Up @@ -219,6 +219,40 @@ ruleTester.run("dot-notation", rule, {
options: [{ allowKeywords: false }],
errors: [{ messageId: "useBrackets", data: { key: "if" } }]
},
{
code: "5['prop']",
output: "5 .prop",
errors: [{ messageId: "useDot", data: { key: q("prop") } }]
},
{
code: "-5['prop']",
output: "-5 .prop",
errors: [{ messageId: "useDot", data: { key: q("prop") } }]
},
{
code: "5_000['prop']",
output: "5_000 .prop",
parserOptions: { ecmaVersion: 2021 },
errors: [{ messageId: "useDot", data: { key: q("prop") } }]
},
{
code: "5_000_00['prop']",
output: "5_000_00 .prop",
parserOptions: { ecmaVersion: 2021 },
errors: [{ messageId: "useDot", data: { key: q("prop") } }]
},
{
code: "5.000_000['prop']",
output: "5.000_000.prop",
parserOptions: { ecmaVersion: 2021 },
errors: [{ messageId: "useDot", data: { key: q("prop") } }]
},
{
code: "0b1010_1010['prop']",
output: "0b1010_1010.prop",
parserOptions: { ecmaVersion: 2021 },
errors: [{ messageId: "useDot", data: { key: q("prop") } }]
},

// Optional chaining
{
Expand Down
6 changes: 5 additions & 1 deletion tests/lib/rules/no-extra-parens.js
View file
Open in desktop
Expand Up @@ -44,7 +44,7 @@ function invalid(code, output, type, line, config) {

const ruleTester = new RuleTester({
parserOptions: {
ecmaVersion: 2020,
ecmaVersion: 2021,
ecmaFeatures: {
jsx: true
}
Expand Down Expand Up @@ -190,6 +190,8 @@ ruleTester.run("no-extra-parens", rule, {

// special cases
"(0).a",
"(5_000).a",
"(5_000_00).a",
"(function(){ }())",
"({a: function(){}}.a());",
"({a:0}.a ? b : c)",
Expand Down Expand Up @@ -775,7 +777,9 @@ ruleTester.run("no-extra-parens", rule, {
invalid("(a).b", "a.b", "Identifier"),
invalid("(0)[a]", "0[a]", "Literal"),
invalid("(0.0).a", "0.0.a", "Literal"),
invalid("(0.0_0).a", "0.0_0.a", "Literal"),
invalid("(0xBEEF).a", "0xBEEF.a", "Literal"),
invalid("(0xBE_EF).a", "0xBE_EF.a", "Literal"),
invalid("(1e6).a", "1e6.a", "Literal"),
invalid("(0123).a", "0123.a", "Literal"),
invalid("a[(function() {})]", "a[function() {}]", "FunctionExpression"),
Expand Down
36 changes: 36 additions & 0 deletions tests/lib/rules/no-whitespace-before-property.js
View file
Open in desktop
Expand Up @@ -842,6 +842,24 @@ ruleTester.run("no-whitespace-before-property", rule, {
data: { propName: "toExponential" }
}]
},
{
code: "5_000 .toExponential()",
output: null, // Not fixed,
parserOptions: { ecmaVersion: 2021 },
errors: [{
messageId: "unexpectedWhitespace",
data: { propName: "toExponential" }
}]
},
{
code: "5_000_00 .toExponential()",
output: null, // Not fixed,
parserOptions: { ecmaVersion: 2021 },
errors: [{
messageId: "unexpectedWhitespace",
data: { propName: "toExponential" }
}]
},
{
code: "5. .toExponential()",
output: "5..toExponential()",
Expand All @@ -858,6 +876,15 @@ ruleTester.run("no-whitespace-before-property", rule, {
data: { propName: "toExponential" }
}]
},
{
code: "5.0_0 .toExponential()",
output: "5.0_0.toExponential()",
parserOptions: { ecmaVersion: 2021 },
errors: [{
messageId: "unexpectedWhitespace",
data: { propName: "toExponential" }
}]
},
{
code: "0x5 .toExponential()",
output: "0x5.toExponential()",
Expand All @@ -866,6 +893,15 @@ ruleTester.run("no-whitespace-before-property", rule, {
data: { propName: "toExponential" }
}]
},
{
code: "0x56_78 .toExponential()",
output: "0x56_78.toExponential()",
parserOptions: { ecmaVersion: 2021 },
errors: [{
messageId: "unexpectedWhitespace",
data: { propName: "toExponential" }
}]
},
{
code: "5e0 .toExponential()",
output: "5e0.toExponential()",
Expand Down
26 changes: 25 additions & 1 deletion tests/lib/rules/prefer-numeric-literals.js
View file
Open in desktop
Expand Up @@ -16,7 +16,7 @@ const rule = require("../../../lib/rules/prefer-numeric-literals"),
// Tests
//------------------------------------------------------------------------------

const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2020 } });
const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2021 } });

ruleTester.run("prefer-numeric-literals", rule, {
valid: [
Expand Down Expand Up @@ -345,6 +345,30 @@ ruleTester.run("prefer-numeric-literals", rule, {
code: "(Number?.parseInt)?.(\"1F7\", 16) === 255;",
output: "0x1F7 === 255;",
errors: [{ message: "Use hexadecimal literals instead of Number?.parseInt()." }]
},

// `parseInt` doesn't support numeric separators. The rule shouldn't autofix in those cases.
{
code: "parseInt('1_0', 2);",
output: null,
errors: [{ message: "Use binary literals instead of parseInt()." }]
},
{
code: "Number.parseInt('5_000', 8);",
output: null,
errors: [{ message: "Use octal literals instead of Number.parseInt()." }]
},
{
code: "parseInt('0_1', 16);",
output: null,
errors: [{ message: "Use hexadecimal literals instead of parseInt()." }]
},
{

// this would be indeed the same as `0x0_0`, but there's no need to autofix this edge case that looks more like a mistake.
Copy link
Member

Choose a reason for hiding this comment

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

Agreed that we don't need to handle this, and I appreciate that you noticed the edge case exists and added an explicit test for it 👍

code: "Number.parseInt('0_0', 16);",
output: null,
errors: [{ message: "Use hexadecimal literals instead of Number.parseInt()." }]
}
]
});
44 changes: 43 additions & 1 deletion tests/lib/rules/quote-props.js
View file
Open in desktop
Expand Up @@ -79,7 +79,13 @@ ruleTester.run("quote-props", rule, {
{ code: "({ 1n: 1 })", options: ["consistent"], parserOptions: { ecmaVersion: 2020 } },
{ code: "({ 1n: 1 })", options: ["consistent-as-needed"], parserOptions: { ecmaVersion: 2020 } },
{ code: "({ '99999999999999999': 1 })", options: ["as-needed"], parserOptions: { ecmaVersion: 2020 } },
{ code: "({ '1n': 1 })", options: ["as-needed"], parserOptions: { ecmaVersion: 2020 } }
{ code: "({ '1n': 1 })", options: ["as-needed"], parserOptions: { ecmaVersion: 2020 } },
{ code: "({ 1_0: 1 })", options: ["as-needed"], parserOptions: { ecmaVersion: 2021 } },
{ code: "({ 1_0: 1 })", options: ["as-needed", { numbers: false }], parserOptions: { ecmaVersion: 2021 } },
{ code: "({ '1_0': 1 })", options: ["as-needed"], parserOptions: { ecmaVersion: 2021 } },
{ code: "({ '1_0': 1 })", options: ["as-needed", { numbers: false }], parserOptions: { ecmaVersion: 2021 } },
{ code: "({ '1_0': 1 })", options: ["as-needed", { numbers: true }], parserOptions: { ecmaVersion: 2021 } },
{ code: "({ 1_0: 1, 1: 1 })", options: ["consistent-as-needed"], parserOptions: { ecmaVersion: 2021 } }
],
invalid: [{
code: "({ a: 0 })",
Expand Down Expand Up @@ -380,5 +386,41 @@ ruleTester.run("quote-props", rule, {
messageId: "unquotedNumericProperty",
data: { property: "1" }
}]
}, {
code: "({ 1_0: 1 })",
output: "({ \"10\": 1 })",
options: ["as-needed", { numbers: true }],
parserOptions: { ecmaVersion: 2021 },
errors: [{
messageId: "unquotedNumericProperty",
data: { property: "10" }
}]
}, {
code: "({ 1_2.3_4e0_2: 1 })",
output: "({ \"1234\": 1 })",
options: ["always"],
parserOptions: { ecmaVersion: 2021 },
errors: [{
messageId: "unquotedPropertyFound",
data: { property: "1234" }
}]
}, {
code: "({ 0b1_000: 1 })",
output: "({ \"8\": 1 })",
options: ["always"],
parserOptions: { ecmaVersion: 2021 },
errors: [{
messageId: "unquotedPropertyFound",
data: { property: "8" }
}]
}, {
code: "({ 1_000: a, '1_000': b })",
output: "({ \"1000\": a, '1_000': b })",
options: ["consistent-as-needed"],
parserOptions: { ecmaVersion: 2021 },
errors: [{
messageId: "inconsistentlyQuotedProperty",
data: { key: "1000" }
}]
}]
});