Skip to content

Commit

Permalink
Fix: allow references to external globals in id-blacklist (fixes #12567…
Browse files Browse the repository at this point in the history 
…) (#12987)

* Fix: allow references to external globals in id-blacklist (fixes #12567)

* Simplify global reference check
  • Loading branch information
@mdjermanovic
mdjermanovic committed Mar 23, 2020
1 parent 4955c50 commit aef9488
Show file tree
Hide file tree
Showing 2 changed files with 629 additions and 102 deletions.
240 changes: 138 additions & 102 deletions lib/rules/id-blacklist.js
View file
Expand Up @@ -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
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -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)
)
);
}

Expand All @@ -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);
}
}

};

}
};

0 comments on commit aef9488

Please sign in to comment.