From 7350589a5bdfc9d75d1ff19364f476eec44c3911 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Sat, 18 Jan 2020 00:55:17 +0900 Subject: [PATCH] Breaking: some rules recognize bigint literals (fixes #11803) (#12701) * update no-magic-numbers to recognize bigint * update yoda to recognize bigint * add a no-extend-native test * update ci.yml temporary (this PR is blocked by #12700) * add astUtils.isNumericLiteral and use it in some rules * update no-dupe-class-members * update no-magic-number to support bigint in options * update some rules to use getStaticPropertyName * update quote-props * revert no-useless-computed-key change * revert "allowing {type: 'bigint'}" and update no-magic-number * no-magic-number 'ignores' allows negative bigint --- .eslintrc.js | 3 ++ docs/rules/no-magic-numbers.md | 11 ++++ lib/rules/key-spacing.js | 3 +- lib/rules/new-cap.js | 10 ++-- lib/rules/no-dupe-class-members.js | 19 ++----- lib/rules/no-magic-numbers.js | 32 +++++++----- lib/rules/no-useless-computed-key.js | 11 ++-- lib/rules/quote-props.js | 7 +-- lib/rules/utils/ast-utils.js | 16 ++++++ lib/rules/yoda.js | 3 +- tests/lib/rules/no-extend-native.js | 8 +++ tests/lib/rules/no-magic-numbers.js | 61 ++++++++++++++++++++++ tests/lib/rules/no-useless-computed-key.js | 11 +++- tests/lib/rules/quote-props.js | 20 ++++++- tests/lib/rules/yoda.js | 42 +++++++++++++++ 15 files changed, 206 insertions(+), 51 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index e57ac92af78..c347bf858f9 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -19,6 +19,9 @@ module.exports = { "eslint", "plugin:eslint-plugin/recommended" ], + parserOptions: { + ecmaVersion: 2020 + }, rules: { "eslint-plugin/consistent-output": "error", "eslint-plugin/no-deprecated-context-methods": "error", diff --git a/docs/rules/no-magic-numbers.md b/docs/rules/no-magic-numbers.md index 7d1e5a5583b..f6e1bacbd51 100644 --- a/docs/rules/no-magic-numbers.md +++ b/docs/rules/no-magic-numbers.md @@ -56,6 +56,9 @@ var dutyFreePrice = 100, An array of numbers to ignore. It's set to `[]` by default. If provided, it must be an `Array`. +The array can contain values of `number` and `string` types. +If it's a string, the text must be parsed as `bigint` literal (e.g., `"100n"`). + Examples of **correct** code for the sample `{ "ignore": [1] }` option: ```js @@ -65,6 +68,14 @@ var data = ['foo', 'bar', 'baz']; var dataLast = data.length && data[data.length - 1]; ``` +Examples of **correct** code for the sample `{ "ignore": ["1n"] }` option: + +```js +/*eslint no-magic-numbers: ["error", { "ignore": ["1n"] }]*/ + +foo(1n); +``` + ### ignoreArrayIndexes A boolean to specify if numbers used as array indexes are considered okay. `false` by default. diff --git a/lib/rules/key-spacing.js b/lib/rules/key-spacing.js index 6d1b9121c78..c405043794c 100644 --- a/lib/rules/key-spacing.js +++ b/lib/rules/key-spacing.js @@ -414,8 +414,7 @@ module.exports = { if (property.computed) { return sourceCode.getText().slice(key.range[0], key.range[1]); } - - return property.key.name || property.key.value; + return astUtils.getStaticPropertyName(property); } /** diff --git a/lib/rules/new-cap.js b/lib/rules/new-cap.js index cee979310ea..7cce968c5ae 100644 --- a/lib/rules/new-cap.js +++ b/lib/rules/new-cap.js @@ -9,6 +9,8 @@ // Requirements //------------------------------------------------------------------------------ +const astUtils = require("./utils/ast-utils"); + //------------------------------------------------------------------------------ // Helpers //------------------------------------------------------------------------------ @@ -160,13 +162,7 @@ module.exports = { let name = ""; if (node.callee.type === "MemberExpression") { - const property = node.callee.property; - - if (property.type === "Literal" && (typeof property.value === "string")) { - name = property.value; - } else if (property.type === "Identifier" && !node.callee.computed) { - name = property.name; - } + name = astUtils.getStaticPropertyName(node.callee) || ""; } else { name = node.callee.name; } diff --git a/lib/rules/no-dupe-class-members.js b/lib/rules/no-dupe-class-members.js index 6041e9e371c..55639746b27 100644 --- a/lib/rules/no-dupe-class-members.js +++ b/lib/rules/no-dupe-class-members.js @@ -5,6 +5,8 @@ "use strict"; +const astUtils = require("./utils/ast-utils"); + //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ @@ -53,21 +55,6 @@ module.exports = { return stateMap[key][isStatic ? "static" : "nonStatic"]; } - /** - * Gets the name text of a given node. - * @param {ASTNode} node A node to get the name. - * @returns {string} The name text of the node. - */ - function getName(node) { - switch (node.type) { - case "Identifier": return node.name; - case "Literal": return String(node.value); - - /* istanbul ignore next: syntax error */ - default: return ""; - } - } - return { // Initializes the stack of state of member declarations. @@ -91,7 +78,7 @@ module.exports = { return; } - const name = getName(node.key); + const name = astUtils.getStaticPropertyName(node) || ""; const state = getState(name, node.static); let isDuplicate = false; diff --git a/lib/rules/no-magic-numbers.js b/lib/rules/no-magic-numbers.js index 0909e3166d9..4bf24996cb8 100644 --- a/lib/rules/no-magic-numbers.js +++ b/lib/rules/no-magic-numbers.js @@ -5,10 +5,24 @@ "use strict"; +const { isNumericLiteral } = require("./utils/ast-utils"); + //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ +/** + * Convert the value to bigint if it's a string. Otherwise return the value as-is. + * @param {bigint|number|string} x The value to normalize. + * @returns {bigint|number} The normalized value. + */ +function normalizeIgnoreValue(x) { + if (typeof x === "string") { + return BigInt(x.slice(0, -1)); + } + return x; +} + module.exports = { meta: { type: "suggestion", @@ -34,7 +48,10 @@ module.exports = { ignore: { type: "array", items: { - type: "number" + anyOf: [ + { type: "number" }, + { type: "string", pattern: "^[+-]?(?:0|[1-9][0-9]*)n$" } + ] }, uniqueItems: true }, @@ -56,18 +73,9 @@ module.exports = { const config = context.options[0] || {}, detectObjects = !!config.detectObjects, enforceConst = !!config.enforceConst, - ignore = config.ignore || [], + ignore = (config.ignore || []).map(normalizeIgnoreValue), ignoreArrayIndexes = !!config.ignoreArrayIndexes; - /** - * Returns whether the node is number literal - * @param {Node} node the node literal being evaluated - * @returns {boolean} true if the node is a number literal - */ - function isNumber(node) { - return typeof node.value === "number"; - } - /** * Returns whether the number should be ignored * @param {number} num the number @@ -113,7 +121,7 @@ module.exports = { Literal(node) { const okTypes = detectObjects ? [] : ["ObjectExpression", "Property", "AssignmentExpression"]; - if (!isNumber(node)) { + if (!isNumericLiteral(node)) { return; } diff --git a/lib/rules/no-useless-computed-key.js b/lib/rules/no-useless-computed-key.js index b5e53174e42..0e0acbea7c3 100644 --- a/lib/rules/no-useless-computed-key.js +++ b/lib/rules/no-useless-computed-key.js @@ -71,14 +71,11 @@ module.exports = { message: MESSAGE_UNNECESSARY_COMPUTED, data: { property: sourceCode.getText(key) }, fix(fixer) { - const leftSquareBracket = sourceCode.getFirstToken(node, astUtils.isOpeningBracketToken); - const rightSquareBracket = sourceCode.getFirstTokenBetween(node.key, node.value, astUtils.isClosingBracketToken); - const tokensBetween = sourceCode.getTokensBetween(leftSquareBracket, rightSquareBracket, 1); + const leftSquareBracket = sourceCode.getTokenBefore(key, astUtils.isOpeningBracketToken); + const rightSquareBracket = sourceCode.getTokenAfter(key, astUtils.isClosingBracketToken); - if (tokensBetween.slice(0, -1).some((token, index) => - sourceCode.getText().slice(token.range[1], tokensBetween[index + 1].range[0]).trim())) { - - // If there are comments between the brackets and the property name, don't do a fix. + // If there are comments between the brackets and the property name, don't do a fix. + if (sourceCode.commentsExistBetween(leftSquareBracket, rightSquareBracket)) { return null; } diff --git a/lib/rules/quote-props.js b/lib/rules/quote-props.js index ab09b8fa938..4cc53b988f6 100644 --- a/lib/rules/quote-props.js +++ b/lib/rules/quote-props.js @@ -8,8 +8,9 @@ // Requirements //------------------------------------------------------------------------------ -const espree = require("espree"), - keywords = require("./utils/keywords"); +const espree = require("espree"); +const astUtils = require("./utils/ast-utils"); +const keywords = require("./utils/keywords"); //------------------------------------------------------------------------------ // Rule Definition @@ -177,7 +178,7 @@ module.exports = { data: { property: key.name }, fix: fixer => fixer.replaceText(key, getQuotedKey(key)) }); - } else if (NUMBERS && key.type === "Literal" && typeof key.value === "number") { + } else if (NUMBERS && key.type === "Literal" && astUtils.isNumericLiteral(key)) { context.report({ node, message: MESSAGE_NUMERIC, diff --git a/lib/rules/utils/ast-utils.js b/lib/rules/utils/ast-utils.js index b4e8e29806d..e10544dd61e 100644 --- a/lib/rules/utils/ast-utils.js +++ b/lib/rules/utils/ast-utils.js @@ -884,6 +884,9 @@ module.exports = { if (node.regex) { return `/${node.regex.pattern}/${node.regex.flags}`; } + if (node.bigint) { + return node.bigint; + } // Otherwise, this is an unknown literal. The function will return null. @@ -1014,6 +1017,7 @@ module.exports = { * 0o5 // false * 5e0 // false * '5' // false + * 5n // false */ isDecimalInteger(node) { return node.type === "Literal" && typeof node.value === "number" && @@ -1333,6 +1337,18 @@ module.exports = { return node.type === "Literal" && node.value === null && !node.regex && !node.bigint; }, + /** + * Check if a given node is a numeric literal or not. + * @param {ASTNode} node The node to check. + * @returns {boolean} `true` if the node is a number or bigint literal. + */ + isNumericLiteral(node) { + return ( + node.type === "Literal" && + (typeof node.value === "number" || Boolean(node.bigint)) + ); + }, + /** * Determines whether two tokens can safely be placed next to each other without merging into a single token * @param {Token|string} leftValue The left token. If this is a string, it will be tokenized and the last token will be used. diff --git a/lib/rules/yoda.js b/lib/rules/yoda.js index b00acf82c70..be5c59ce072 100644 --- a/lib/rules/yoda.js +++ b/lib/rules/yoda.js @@ -53,8 +53,7 @@ function looksLikeLiteral(node) { return (node.type === "UnaryExpression" && node.operator === "-" && node.prefix && - node.argument.type === "Literal" && - typeof node.argument.value === "number"); + astUtils.isNumericLiteral(node.argument)); } /** diff --git a/tests/lib/rules/no-extend-native.js b/tests/lib/rules/no-extend-native.js index 7c1b2514a29..a38177431d2 100644 --- a/tests/lib/rules/no-extend-native.js +++ b/tests/lib/rules/no-extend-native.js @@ -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: [{ diff --git a/tests/lib/rules/no-magic-numbers.js b/tests/lib/rules/no-magic-numbers.js index 8ebd8c93d2a..bda02662edf 100644 --- a/tests/lib/rules/no-magic-numbers.js +++ b/tests/lib/rules/no-magic-numbers.js @@ -75,6 +75,16 @@ ruleTester.run("no-magic-numbers", rule, { jsx: true } } + }, + { + code: "f(100n)", + options: [{ ignore: ["100n"] }], + parserOptions: { ecmaVersion: 2020 } + }, + { + code: "f(-100n)", + options: [{ ignore: ["-100n"] }], + parserOptions: { ecmaVersion: 2020 } } ], invalid: [ @@ -93,6 +103,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: [ @@ -227,6 +257,37 @@ ruleTester.run("no-magic-numbers", rule, { { messageId: "noMagic", data: { raw: "10" }, line: 1 }, { messageId: "noMagic", data: { raw: "4" }, line: 1 } ] + }, + { + code: "f(100n)", + options: [{ ignore: [100] }], + parserOptions: { ecmaVersion: 2020 }, + errors: [ + { messageId: "noMagic", data: { raw: "100n" }, line: 1 } + ] + }, + { + code: "f(-100n)", + options: [{ ignore: ["100n"] }], + parserOptions: { ecmaVersion: 2020 }, + errors: [ + { messageId: "noMagic", data: { raw: "-100n" }, line: 1 } + ] + }, + { + code: "f(100n)", + options: [{ ignore: ["-100n"] }], + parserOptions: { ecmaVersion: 2020 }, + errors: [ + { messageId: "noMagic", data: { raw: "100n" }, line: 1 } + ] + }, + { + code: "f(100)", + options: [{ ignore: ["100n"] }], + errors: [ + { messageId: "noMagic", data: { raw: "100" }, line: 1 } + ] } ] }); diff --git a/tests/lib/rules/no-useless-computed-key.js b/tests/lib/rules/no-useless-computed-key.js index cd33cc732ca..a5907aede0d 100644 --- a/tests/lib/rules/no-useless-computed-key.js +++ b/tests/lib/rules/no-useless-computed-key.js @@ -40,7 +40,16 @@ ruleTester.run("no-useless-computed-key", rule, { { code: "class Foo { ['x']() {} }", options: [{ enforceForClassMembers: false }] }, { code: "(class { ['x']() {} })", options: [{ enforceForClassMembers: false }] }, { code: "class Foo { static ['constructor']() {} }", options: [{ enforceForClassMembers: false }] }, - { code: "class Foo { ['prototype']() {} }", options: [{ enforceForClassMembers: false }] } + { code: "class Foo { ['prototype']() {} }", options: [{ enforceForClassMembers: false }] }, + + /* + * Well-known browsers throw syntax error bigint literals on property names, + * so, this rule doesn't touch those for now. + */ + { + code: "({ [99999999999999999n]: 0 })", + parserOptions: { ecmaVersion: 2020 } + } ], invalid: [ { diff --git a/tests/lib/rules/quote-props.js b/tests/lib/rules/quote-props.js index c1cd10e50f0..9d033dfb85a 100644 --- a/tests/lib/rules/quote-props.js +++ b/tests/lib/rules/quote-props.js @@ -73,7 +73,13 @@ ruleTester.run("quote-props", rule, { { code: "({1: 1, x: 2})", options: ["consistent-as-needed", { numbers: true }] }, { code: "({ ...x })", options: ["as-needed"], parserOptions: { ecmaVersion: 2018 } }, { code: "({ ...x })", options: ["consistent"], parserOptions: { ecmaVersion: 2018 } }, - { code: "({ ...x })", options: ["consistent-as-needed"], parserOptions: { ecmaVersion: 2018 } } + { code: "({ ...x })", options: ["consistent-as-needed"], parserOptions: { ecmaVersion: 2018 } }, + { code: "({ 1n: 1 })", options: ["as-needed"], parserOptions: { ecmaVersion: 2020 } }, + { code: "({ 1n: 1 })", options: ["as-needed", { numbers: false }], parserOptions: { ecmaVersion: 2020 } }, + { 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 } } ], invalid: [{ code: "({ a: 0 })", @@ -304,5 +310,17 @@ ruleTester.run("quote-props", rule, { errors: [{ message: "Unquoted property '5' found." }] + }, { + code: "({ 1n: 1 })", + output: "({ \"1\": 1 })", + options: ["always"], + parserOptions: { ecmaVersion: 2020 }, + errors: [{ message: "Unquoted property '1' found." }] + }, { + code: "({ 1n: 1 })", + output: "({ \"1\": 1 })", + options: ["as-needed", { numbers: true }], + parserOptions: { ecmaVersion: 2020 }, + errors: [{ message: "Unquoted number literal '1' used as key." }] }] }); diff --git a/tests/lib/rules/yoda.js b/tests/lib/rules/yoda.js index 414cb5a0460..72f01396479 100644 --- a/tests/lib/rules/yoda.js +++ b/tests/lib/rules/yoda.js @@ -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 } }, { code: "if (1 <= a['/(?0)/'] && a[/(?0)/] <= 100) {}", options: ["never", { exceptRange: true }], @@ -140,6 +156,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) {}", @@ -236,6 +265,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) {}",