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
  • Loading branch information
mdjermanovic committed Mar 1, 2020
1 parent c615eae commit 6c1dd9e
Show file tree
Hide file tree
Showing 2 changed files with 648 additions and 103 deletions.
260 changes: 157 additions & 103 deletions lib/rules/id-blacklist.js
Expand Up @@ -6,6 +6,111 @@

"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const { flatten } = require("lodash");

//------------------------------------------------------------------------------
// 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 +140,61 @@ module.exports = {

create(context) {


//--------------------------------------------------------------------------
// Helpers
//--------------------------------------------------------------------------

const blacklist = context.options;
const blacklist = new Set(context.options);
const reportedNodes = new Set();

// `Identifier` nodes that represent references to global variables.
let globalReferenceIdentifiers;

/**
* 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.
* @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;

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
)
);
function isReferenceToGlobalVariable(node) {
return globalReferenceIdentifiers.has(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 +219,30 @@ 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() {

/*
* Find identifiers that represent references to global variables that are not declared in the given source code.
* These identifiers will be allowed, as it is assumed that user has no control over the names of external global variables.
* There can be hundreds of globals in some environments, so filter out unrelated variables from the start, to optimize.
*/
const globalScope = context.getScope();
const globalReferences = flatten(
globalScope.variables
.filter(v => v.references.length > 0 && v.defs.length === 0 && isBlacklisted(v.name))
.map(v => v.references)
);

globalReferenceIdentifiers = new Set(
globalReferences.map(ref => ref.identifier)
);
},

} else if (shouldReport(node)) {
Identifier(node) {
if (isBlacklisted(node.name) && shouldCheck(node)) {
report(node);
}
}

};

}
};

0 comments on commit 6c1dd9e

Please sign in to comment.