Skip to content

Commit

Permalink
feat: Add ignoreOnInitialization option to no-shadow rule (#14963)
Browse files Browse the repository at this point in the history 
* Fix: Remove warning  in initialized variables (fixes #12687)

* Fix: Remove warning  in initialized variables (fixes #12687)

* Fix: Remove warning  in initialized variables (fixes #12687)

* Docs: add invalid ambiguous example in doc (fixes #12687)

* Docs: add invalid ambiguous example in doc (fixes #12687)

* Fix: Adding option for variables on intialization (fixes #12687)

* Fix: Adding option for variables on intialization (fixes #12687)

* Fix: Adding option for variables on intialization (fixes #12687)

* Fix: Adding option for variables on intialization (fixes #12687)

* Fix: Adding option for variables on intialization (fixes #12687)

* Fix: Adding option for variables on intialization (fixes #12687)

* Fix: Adding option for variables on intialization (fixes #12687)

* Fix: Adding option for variables on intialization (fixes #12687)

* Fix: Adding option for variables on intialization (fixes #12687)

* Fix: Adding option for variables on intialization (fixes #12687)

* feat: Adding option for variables on intialization (fixes #12687)

* feat: Adding option for variables on intialization (fixes #12687)

* feat: Adding option for variables on intialization (fixes #12687)

* feat: Adding option for variables on intialization (fixes #12687)

* feat: Adding option for variables on intialization (fixes #12687)

* feat: Adding option for variables on intialization (fixes #12687)

* feat: Adding option for variables on intialization (fixes #12687)

* feat: Adding option for variables on intialization (fixes #12687)

* feat: Adding option for variables on intialization (fixes #12687)

* feat: Adding option for variables on intialization (fixes #12687)

* feat: Adding option for variables on intialization (fixes #12687)

* feat: Adding option for variables on intialization (fixes #12687)
  • Loading branch information
@boutahlilsoufiane
boutahlilsoufiane authored Feb 25, 2022
1 parent 1005bd5 commit 6e2c325
Show file tree
Hide file tree
Showing 3 changed files with 412 additions and 19 deletions.
32 changes: 30 additions & 2 deletions docs/rules/no-shadow.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ if (true) {

## Options

This rule takes one option, an object, with properties `"builtinGlobals"`, `"hoist"` and `"allow"`.
This rule takes one option, an object, with properties `"builtinGlobals"`, `"hoist"`, `"allow"` and `"ignoreOnInitialization"`.

```json
{
"no-shadow": ["error", { "builtinGlobals": false, "hoist": "functions", "allow": [] }]
"no-shadow": ["error", { "builtinGlobals": false, "hoist": "functions", "allow": [], "ignoreOnInitialization": false }]
}
```

Expand Down Expand Up @@ -166,6 +166,34 @@ foo(function (err, result) {
});
```

### ignoreOnInitialization

The `ignoreOnInitialization` option is `false` by default. If it is `true`, it prevents reporting shadowing of variables in their initializers when the shadowed variable is presumably still uninitialized.

The shadowed variable must be on the left side. The shadowing variable must be on the right side and declared in a callback function or in an IIFE.

Examples of **incorrect** code for the `{ "ignoreOnInitialization": "true" }` option:

```js
/*eslint no-shadow: ["error", { "ignoreOnInitialization": true }]*/

var x = x => x;
```

Because the shadowing variable `x` will shadow the already initialized shadowed variable `x`.

Examples of **correct** code for the `{ "ignoreOnInitialization": true }` option:

```js
/*eslint no-shadow: ["error", { "ignoreOnInitialization": true }]*/

var x = foo(x => x)

var y = (y => y)()
```

The rationale for callback functions is the assumption that they will be called during the initialization, so that at the time when the shadowing variable will be used, the shadowed variable has not yet been initialized.

## Related Rules

* [no-shadow-restricted-names](no-shadow-restricted-names.md)
Expand Down
143 changes: 127 additions & 16 deletions lib/rules/no-shadow.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,15 @@

const astUtils = require("./utils/ast-utils");

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

const FUNC_EXPR_NODE_TYPES = ["ArrowFunctionExpression", "FunctionExpression"];
const CALL_EXPR_NODE_TYPE = ["CallExpression"];
const FOR_IN_OF_TYPE = /^For(?:In|Of)Statement$/u;
const SENTINEL_TYPE = /^(?:(?:Function|Class)(?:Declaration|Expression)|ArrowFunctionExpression|CatchClause|ImportDeclaration|ExportNamedDeclaration)$/u;

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand All @@ -37,7 +46,8 @@ module.exports = {
items: {
type: "string"
}
}
},
ignoreOnInitialization: { type: "boolean", default: false }
},
additionalProperties: false
}
Expand All @@ -54,9 +64,109 @@ module.exports = {
const options = {
builtinGlobals: context.options[0] && context.options[0].builtinGlobals,
hoist: (context.options[0] && context.options[0].hoist) || "functions",
allow: (context.options[0] && context.options[0].allow) || []
allow: (context.options[0] && context.options[0].allow) || [],
ignoreOnInitialization: context.options[0] && context.options[0].ignoreOnInitialization
};

/**
* Checks whether or not a given location is inside of the range of a given node.
* @param {ASTNode} node An node to check.
* @param {number} location A location to check.
* @returns {boolean} `true` if the location is inside of the range of the node.
*/
function isInRange(node, location) {
return node && node.range[0] <= location && location <= node.range[1];
}

/**
* Searches from the current node through its ancestry to find a matching node.
* @param {ASTNode} node a node to get.
* @param {(node: ASTNode) => boolean} match a callback that checks whether or not the node verifies its condition or not.
* @returns {ASTNode|null} the matching node.
*/
function findSelfOrAncestor(node, match) {
let currentNode = node;

while (currentNode && !match(currentNode)) {
currentNode = currentNode.parent;
}
return currentNode;
}

/**
* Finds function's outer scope.
* @param {Scope} scope Function's own scope.
* @returns {Scope} Function's outer scope.
*/
function getOuterScope(scope) {
const upper = scope.upper;

if (upper.type === "function-expression-name") {
return upper.upper;
}
return upper;
}

/**
* Checks if a variable and a shadowedVariable have the same init pattern ancestor.
* @param {Object} variable a variable to check.
* @param {Object} shadowedVariable a shadowedVariable to check.
* @returns {boolean} Whether or not the variable and the shadowedVariable have the same init pattern ancestor.
*/
function isInitPatternNode(variable, shadowedVariable) {
const outerDef = shadowedVariable.defs[0];

if (!outerDef) {
return false;
}

const { variableScope } = variable.scope;


if (!(FUNC_EXPR_NODE_TYPES.includes(variableScope.block.type) && getOuterScope(variableScope) === shadowedVariable.scope)) {
return false;
}

const fun = variableScope.block;
const { parent } = fun;

const callExpression = findSelfOrAncestor(
parent,
node => CALL_EXPR_NODE_TYPE.includes(node.type)
);

if (!callExpression) {
return false;
}

let node = outerDef.name;
const location = callExpression.range[1];

while (node) {
if (node.type === "VariableDeclarator") {
if (isInRange(node.init, location)) {
return true;
}
if (FOR_IN_OF_TYPE.test(node.parent.parent.type) &&
isInRange(node.parent.parent.right, location)
) {
return true;
}
break;
} else if (node.type === "AssignmentPattern") {
if (isInRange(node.right, location)) {
return true;
}
} else if (SENTINEL_TYPE.test(node.type)) {
break;
}

node = node.parent;
}

return false;
}

/**
* Check if variable name is allowed.
* @param {ASTNode} variable The variable to check.
Expand Down Expand Up @@ -99,11 +209,11 @@ module.exports = {

return (
outer &&
inner &&
outer[0] < inner[0] &&
inner[1] < outer[1] &&
((innerDef.type === "FunctionName" && innerDef.node.type === "FunctionExpression") || innerDef.node.type === "ClassExpression") &&
outerScope === innerScope.upper
inner &&
outer[0] < inner[0] &&
inner[1] < outer[1] &&
((innerDef.type === "FunctionName" && innerDef.node.type === "FunctionExpression") || innerDef.node.type === "ClassExpression") &&
outerScope === innerScope.upper
);
}

Expand Down Expand Up @@ -154,11 +264,11 @@ module.exports = {

return (
inner &&
outer &&
inner[1] < outer[0] &&
outer &&
inner[1] < outer[0] &&

// Excepts FunctionDeclaration if is {"hoist":"function"}.
(options.hoist !== "functions" || !outerDef || outerDef.node.type !== "FunctionDeclaration")
// Excepts FunctionDeclaration if is {"hoist":"function"}.
(options.hoist !== "functions" || !outerDef || outerDef.node.type !== "FunctionDeclaration")
);
}

Expand All @@ -175,8 +285,8 @@ module.exports = {

// Skips "arguments" or variables of a class name in the class scope of ClassDeclaration.
if (variable.identifiers.length === 0 ||
isDuplicatedClassNameVariable(variable) ||
isAllowed(variable)
isDuplicatedClassNameVariable(variable) ||
isAllowed(variable)
) {
continue;
}
Expand All @@ -185,9 +295,10 @@ module.exports = {
const shadowed = astUtils.getVariableByName(scope.upper, variable.name);

if (shadowed &&
(shadowed.identifiers.length > 0 || (options.builtinGlobals && "writeable" in shadowed)) &&
!isOnInitializer(variable, shadowed) &&
!(options.hoist !== "all" && isInTdz(variable, shadowed))
(shadowed.identifiers.length > 0 || (options.builtinGlobals && "writeable" in shadowed)) &&
!isOnInitializer(variable, shadowed) &&
!(options.ignoreOnInitialization && isInitPatternNode(variable, shadowed)) &&
!(options.hoist !== "all" && isInTdz(variable, shadowed))
) {
const location = getDeclaredLocation(shadowed);
const messageId = location.global ? "noShadowGlobal" : "noShadow";
Expand Down
Loading

0 comments on commit 6e2c325

Please sign in to comment.