diff --git a/lib/rules/id-blacklist.js b/lib/rules/id-blacklist.js index ddff9363aee..d77a35d41b6 100644 --- a/lib/rules/id-blacklist.js +++ b/lib/rules/id-blacklist.js @@ -6,6 +6,105 @@ "use strict"; +//------------------------------------------------------------------------------ +// Helpers +//------------------------------------------------------------------------------ + +/** + * Checks whether the given node represents assignment target in a normal assignment or destructuring. + * @param {ASTNode} node The node to check. + * @returns {boolean} `true` if the node is assignment target. + */ +function isAssignmentTarget(node) { + const parent = node.parent; + + return ( + + // normal assignment + ( + parent.type === "AssignmentExpression" && + parent.left === node + ) || + + // destructuring + parent.type === "ArrayPattern" || + parent.type === "RestElement" || + ( + parent.type === "Property" && + parent.value === node && + parent.parent.type === "ObjectPattern" + ) || + ( + parent.type === "AssignmentPattern" && + parent.left === node + ) + ); +} + +/** + * Checks whether the given node represents an imported name that is renamed in the same import/export specifier. + * + * Examples: + * import { a as b } from 'mod'; // node `a` is renamed import + * export { a as b } from 'mod'; // node `a` is renamed import + * @param {ASTNode} node `Identifier` node to check. + * @returns {boolean} `true` if the node is a renamed import. + */ +function isRenamedImport(node) { + const parent = node.parent; + + return ( + ( + parent.type === "ImportSpecifier" && + parent.imported !== parent.local && + parent.imported === node + ) || + ( + parent.type === "ExportSpecifier" && + parent.parent.source && // re-export + parent.local !== parent.exported && + parent.local === node + ) + ); +} + +/** + * Checks whether the given node is a renamed identifier node in an ObjectPattern destructuring. + * + * Examples: + * const { a : b } = foo; // node `a` is renamed node. + * @param {ASTNode} node `Identifier` node to check. + * @returns {boolean} `true` if the node is a renamed node in an ObjectPattern destructuring. + */ +function isRenamedInDestructuring(node) { + const parent = node.parent; + + return ( + ( + !parent.computed && + parent.type === "Property" && + parent.parent.type === "ObjectPattern" && + parent.value !== node && + parent.key === node + ) + ); +} + +/** + * Checks whether the given node represents shorthand definition of a property in an object literal. + * @param {ASTNode} node `Identifier` node to check. + * @returns {boolean} `true` if the node is a shorthand property definition. + */ +function isShorthandPropertyDefinition(node) { + const parent = node.parent; + + return ( + parent.type === "Property" && + parent.parent.type === "ObjectExpression" && + parent.shorthand + ); +} + //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ @@ -35,88 +134,64 @@ module.exports = { create(context) { - - //-------------------------------------------------------------------------- - // Helpers - //-------------------------------------------------------------------------- - - const blacklist = context.options; + const blacklist = new Set(context.options); const reportedNodes = new Set(); + let globalScope; /** - * Checks if a string matches the provided pattern - * @param {string} name The string to check. - * @returns {boolean} if the string is a match + * Checks whether the given name is blacklisted. + * @param {string} name The name to check. + * @returns {boolean} `true` if the name is blacklisted. * @private */ - function isInvalid(name) { - return blacklist.indexOf(name) !== -1; + function isBlacklisted(name) { + return blacklist.has(name); } /** - * Checks whether the given node represents an imported name that is renamed in the same import/export specifier. - * - * Examples: - * import { a as b } from 'mod'; // node `a` is renamed import - * export { a as b } from 'mod'; // node `a` is renamed import + * Checks whether the given node represents a reference to a global variable that is not declared in the source code. + * These identifiers will be allowed, as it is assumed that user has no control over the names of external global variables. * @param {ASTNode} node `Identifier` node to check. - * @returns {boolean} `true` if the node is a renamed import. + * @returns {boolean} `true` if the node is a reference to a global variable. */ - function isRenamedImport(node) { - const parent = node.parent; + function isReferenceToGlobalVariable(node) { + const variable = globalScope.set.get(node.name); - return ( - ( - parent.type === "ImportSpecifier" && - parent.imported !== parent.local && - parent.imported === node - ) || - ( - parent.type === "ExportSpecifier" && - parent.parent.source && // re-export - parent.local !== parent.exported && - parent.local === node - ) - ); + return variable && variable.defs.length === 0 && + variable.references.some(ref => ref.identifier === node); } /** - * Checks whether the given node is a renamed identifier node in an ObjectPattern destructuring. - * - * Examples: - * const { a : b } = foo; // node `a` is renamed node. - * @param {ASTNode} node `Identifier` node to check. - * @returns {boolean} `true` if the node is a renamed node in an ObjectPattern destructuring. + * Determines whether the given node should be checked. + * @param {ASTNode} node `Identifier` node. + * @returns {boolean} `true` if the node should be checked. */ - function isRenamedInDestructuring(node) { + function shouldCheck(node) { const parent = node.parent; - return ( - ( - !parent.computed && - parent.type === "Property" && - parent.parent.type === "ObjectPattern" && - parent.value !== node && - parent.key === node - ) - ); - } - - /** - * Verifies if we should report an error or not. - * @param {ASTNode} node The node to check - * @returns {boolean} whether an error should be reported or not - */ - function shouldReport(node) { - const parent = node.parent; + /* + * Member access has special rules for checking property names. + * Read access to a property with a blacklisted name is allowed, because it can be on an object that user has no control over. + * Write access isn't allowed, because it potentially creates a new property with a blacklisted name. + */ + if ( + parent.type === "MemberExpression" && + parent.property === node && + !parent.computed + ) { + return isAssignmentTarget(parent); + } return ( parent.type !== "CallExpression" && parent.type !== "NewExpression" && !isRenamedImport(node) && !isRenamedInDestructuring(node) && - isInvalid(node.name) + !( + isReferenceToGlobalVariable(node) && + !isShorthandPropertyDefinition(node) + ) ); } @@ -141,54 +216,15 @@ module.exports = { return { - Identifier(node) { - - // MemberExpressions get special rules - if (node.parent.type === "MemberExpression") { - const name = node.name, - effectiveParent = node.parent.parent; - - // Always check object names - if (node.parent.object.type === "Identifier" && - node.parent.object.name === name) { - if (isInvalid(name)) { - report(node); - } - - // Report AssignmentExpressions only if they are the left side of the assignment - } else if (effectiveParent.type === "AssignmentExpression" && - (effectiveParent.right.type !== "MemberExpression" || - effectiveParent.left.type === "MemberExpression" && - effectiveParent.left.property.name === name)) { - if (isInvalid(name)) { - report(node); - } - - // Report the last identifier in an ObjectPattern destructuring. - } else if ( - ( - effectiveParent.type === "Property" && - effectiveParent.value === node.parent && - effectiveParent.parent.type === "ObjectPattern" - ) || - effectiveParent.type === "RestElement" || - effectiveParent.type === "ArrayPattern" || - ( - effectiveParent.type === "AssignmentPattern" && - effectiveParent.left === node.parent - ) - ) { - if (isInvalid(name)) { - report(node); - } - } + Program() { + globalScope = context.getScope(); + }, - } else if (shouldReport(node)) { + Identifier(node) { + if (isBlacklisted(node.name) && shouldCheck(node)) { report(node); } } - }; - } }; diff --git a/tests/lib/rules/id-blacklist.js b/tests/lib/rules/id-blacklist.js index 11c85ae0885..3d40beeb578 100644 --- a/tests/lib/rules/id-blacklist.js +++ b/tests/lib/rules/id-blacklist.js @@ -156,6 +156,54 @@ ruleTester.run("id-blacklist", rule, { code: "({[obj.bar]: a = baz} = qux);", options: ["bar"], parserOptions: { ecmaVersion: 6 } + }, + + // references to global variables + { + code: "Number.parseInt()", + options: ["Number"] + }, + { + code: "x = Number.NaN;", + options: ["Number"] + }, + { + code: "var foo = undefined;", + options: ["undefined"] + }, + { + code: "if (foo === undefined);", + options: ["undefined"] + }, + { + code: "obj[undefined] = 5;", // creates obj["undefined"]. It should be disallowed, but the rule doesn't know values of globals and can't control computed access. + options: ["undefined"] + }, + { + code: "foo = { [myGlobal]: 1 };", + options: ["myGlobal"], + parserOptions: { ecmaVersion: 6 }, + globals: { myGlobal: "readonly" } + }, + { + code: "({ myGlobal } = foo);", // writability doesn't affect the logic, it's always assumed that user doesn't have control over the names of globals. + options: ["myGlobal"], + parserOptions: { ecmaVersion: 6 }, + globals: { myGlobal: "writable" } + }, + { + code: "/* global myGlobal: readonly */ myGlobal = 5;", + options: ["myGlobal"] + }, + { + code: "var foo = [Map];", + options: ["Map"], + env: { es6: true } + }, + { + code: "var foo = { bar: window.baz };", + options: ["window"], + env: { browser: true } } ], invalid: [ @@ -448,6 +496,24 @@ ruleTester.run("id-blacklist", rule, { error ] }, + { + code: "foo[bar] = baz;", + options: ["bar"], + errors: [{ + messageId: "blacklisted", + data: { name: "bar" }, + type: "Identifier" + }] + }, + { + code: "baz = foo[bar];", + options: ["bar"], + errors: [{ + messageId: "blacklisted", + data: { name: "bar" }, + type: "Identifier" + }] + }, { code: "var foo = bar.baz;", options: ["f", "fo", "foo", "b", "ba", "barr", "bazz"], @@ -855,6 +921,431 @@ ruleTester.run("id-blacklist", rule, { column: 8 } ] + }, + + // not a reference to a global variable, because it isn't a reference to a variable + { + code: "foo.undefined = 1;", + options: ["undefined"], + errors: [ + { + messageId: "blacklisted", + data: { name: "undefined" }, + type: "Identifier" + } + ] + }, + { + code: "var foo = { undefined: 1 };", + options: ["undefined"], + errors: [ + { + messageId: "blacklisted", + data: { name: "undefined" }, + type: "Identifier" + } + ] + }, + { + code: "var foo = { undefined: undefined };", + options: ["undefined"], + errors: [ + { + messageId: "blacklisted", + data: { name: "undefined" }, + type: "Identifier", + column: 13 + } + ] + }, + { + code: "var foo = { Number() {} };", + options: ["Number"], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + messageId: "blacklisted", + data: { name: "Number" }, + type: "Identifier" + } + ] + }, + { + code: "class Foo { Number() {} }", + options: ["Number"], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + messageId: "blacklisted", + data: { name: "Number" }, + type: "Identifier" + } + ] + }, + { + code: "myGlobal: while(foo) { break myGlobal; } ", + options: ["myGlobal"], + globals: { myGlobal: "readonly" }, + errors: [ + { + messageId: "blacklisted", + data: { name: "myGlobal" }, + type: "Identifier", + column: 1 + }, + { + messageId: "blacklisted", + data: { name: "myGlobal" }, + type: "Identifier", + column: 30 + } + ] + }, + + // globals declared in the given source code are not excluded from consideration + { + code: "const foo = 1; bar = foo;", + options: ["foo"], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + messageId: "blacklisted", + data: { name: "foo" }, + type: "Identifier", + column: 7 + }, + { + messageId: "blacklisted", + data: { name: "foo" }, + type: "Identifier", + column: 22 + } + ] + }, + { + code: "let foo; foo = bar;", + options: ["foo"], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + messageId: "blacklisted", + data: { name: "foo" }, + type: "Identifier", + column: 5 + }, + { + messageId: "blacklisted", + data: { name: "foo" }, + type: "Identifier", + column: 10 + } + ] + }, + { + code: "bar = foo; var foo;", + options: ["foo"], + errors: [ + { + messageId: "blacklisted", + data: { name: "foo" }, + type: "Identifier", + column: 7 + }, + { + messageId: "blacklisted", + data: { name: "foo" }, + type: "Identifier", + column: 16 + } + ] + }, + { + code: "function foo() {} var bar = foo;", + options: ["foo"], + errors: [ + { + messageId: "blacklisted", + data: { name: "foo" }, + type: "Identifier", + column: 10 + }, + { + messageId: "blacklisted", + data: { name: "foo" }, + type: "Identifier", + column: 29 + } + ] + }, + { + code: "class Foo {} var bar = Foo;", + options: ["Foo"], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + messageId: "blacklisted", + data: { name: "Foo" }, + type: "Identifier", + column: 7 + }, + { + messageId: "blacklisted", + data: { name: "Foo" }, + type: "Identifier", + column: 24 + } + ] + }, + + // redeclared globals are not excluded from consideration + { + code: "let undefined; undefined = 1;", + options: ["undefined"], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + messageId: "blacklisted", + data: { name: "undefined" }, + type: "Identifier", + column: 5 + }, + { + messageId: "blacklisted", + data: { name: "undefined" }, + type: "Identifier", + column: 16 + } + ] + }, + { + code: "foo = undefined; var undefined;", + options: ["undefined"], + errors: [ + { + messageId: "blacklisted", + data: { name: "undefined" }, + type: "Identifier", + column: 7 + }, + { + messageId: "blacklisted", + data: { name: "undefined" }, + type: "Identifier", + column: 22 + } + ] + }, + { + code: "function undefined(){} x = undefined;", + options: ["undefined"], + errors: [ + { + messageId: "blacklisted", + data: { name: "undefined" }, + type: "Identifier", + column: 10 + }, + { + messageId: "blacklisted", + data: { name: "undefined" }, + type: "Identifier", + column: 28 + } + ] + }, + { + code: "class Number {} x = Number.NaN;", + options: ["Number"], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + messageId: "blacklisted", + data: { name: "Number" }, + type: "Identifier", + column: 7 + }, + { + messageId: "blacklisted", + data: { name: "Number" }, + type: "Identifier", + column: 21 + } + ] + }, + + /* + * Assignment to a property with a blacklisted name isn't allowed, in general. + * In this case, that restriction prevents creating a global variable with a blacklisted name. + */ + { + code: "/* globals myGlobal */ window.myGlobal = 5; foo = myGlobal;", + options: ["myGlobal"], + env: { browser: true }, + errors: [ + { + messageId: "blacklisted", + data: { name: "myGlobal" }, + type: "Identifier", + column: 31 + } + ] + }, + + // disabled global variables + { + code: "var foo = undefined;", + options: ["undefined"], + globals: { undefined: "off" }, + errors: [ + { + messageId: "blacklisted", + data: { name: "undefined" }, + type: "Identifier" + } + ] + }, + { + code: "/* globals Number: off */ Number.parseInt()", + options: ["Number"], + errors: [ + { + messageId: "blacklisted", + data: { name: "Number" }, + type: "Identifier" + } + ] + }, + { + code: "var foo = [Map];", // this actually isn't a disabled global: it was never enabled because es6 environment isn't enabled + options: ["Map"], + errors: [ + { + messageId: "blacklisted", + data: { name: "Map" }, + type: "Identifier" + } + ] + }, + + // shadowed global variables + { + code: "if (foo) { let undefined; bar = undefined; }", + options: ["undefined"], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + messageId: "blacklisted", + data: { name: "undefined" }, + type: "Identifier", + column: 16 + }, + { + messageId: "blacklisted", + data: { name: "undefined" }, + type: "Identifier", + column: 33 + } + ] + }, + { + code: "function foo(Number) { var x = Number.NaN; }", + options: ["Number"], + errors: [ + { + messageId: "blacklisted", + data: { name: "Number" }, + type: "Identifier", + column: 14 + }, + { + messageId: "blacklisted", + data: { name: "Number" }, + type: "Identifier", + column: 32 + } + ] + }, + { + code: "function foo() { var myGlobal; x = myGlobal; }", + options: ["myGlobal"], + globals: { myGlobal: "readonly" }, + errors: [ + { + messageId: "blacklisted", + data: { name: "myGlobal" }, + type: "Identifier", + column: 22 + }, + { + messageId: "blacklisted", + data: { name: "myGlobal" }, + type: "Identifier", + column: 36 + } + ] + }, + { + code: "function foo(bar) { return Number.parseInt(bar); } const Number = 1;", + options: ["Number"], + parserOptions: { ecmaVersion: 6, sourceType: "module" }, + errors: [ + { + messageId: "blacklisted", + data: { name: "Number" }, + type: "Identifier", + column: 28 + }, + { + messageId: "blacklisted", + data: { name: "Number" }, + type: "Identifier", + column: 58 + } + ] + }, + { + code: "import Number from 'myNumber'; const foo = Number.parseInt(bar);", + options: ["Number"], + parserOptions: { ecmaVersion: 6, sourceType: "module" }, + errors: [ + { + messageId: "blacklisted", + data: { name: "Number" }, + type: "Identifier", + column: 8 + }, + { + messageId: "blacklisted", + data: { name: "Number" }, + type: "Identifier", + column: 44 + } + ] + }, + { + code: "var foo = function undefined() {};", + options: ["undefined"], + errors: [ + { + messageId: "blacklisted", + data: { name: "undefined" }, + type: "Identifier" + } + ] + }, + + // this is a reference to a global variable, but at the same time creates a property with a blacklisted name + { + code: "var foo = { undefined }", + options: ["undefined"], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + messageId: "blacklisted", + data: { name: "undefined" }, + type: "Identifier" + } + ] } ] });