Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add ignoreOnInitialization option to no-shadow rule #14963

Merged
merged 27 commits into from Feb 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
af7186c
Fix: Remove warning in initialized variables (fixes #12687)
boutahlilsoufiane Aug 22, 2021
5174969
Fix: Remove warning in initialized variables (fixes #12687)
boutahlilsoufiane Aug 26, 2021
3860b98
Fix: Remove warning in initialized variables (fixes #12687)
boutahlilsoufiane Sep 5, 2021
0832760
Docs: add invalid ambiguous example in doc (fixes #12687)
boutahlilsoufiane Sep 19, 2021
0e40f04
Docs: add invalid ambiguous example in doc (fixes #12687)
boutahlilsoufiane Sep 19, 2021
b314ffd
Fix: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Sep 26, 2021
b6a2c0f
Fix: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Nov 7, 2021
1185d09
Fix: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Nov 7, 2021
b9297d1
Fix: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Nov 8, 2021
f848418
Fix: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Nov 8, 2021
5053720
Fix: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Nov 8, 2021
9e66477
Fix: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Nov 8, 2021
5865277
Fix: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Nov 8, 2021
aec18a3
Fix: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Nov 28, 2021
60ccbf6
Fix: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Dec 19, 2021
ec734a3
feat: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Dec 19, 2021
0e74b6c
feat: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Jan 9, 2022
7875170
feat: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Jan 9, 2022
78fe1a0
feat: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Jan 9, 2022
d854d9a
feat: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Jan 9, 2022
bb58435
feat: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Jan 29, 2022
89fef64
feat: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Feb 5, 2022
5d0292b
feat: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Feb 8, 2022
97931bf
feat: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Feb 13, 2022
b7a9428
feat: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Feb 14, 2022
c9b30ec
feat: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Feb 21, 2022
c555585
feat: Adding option for variables on intialization (fixes #12687)
boutahlilsoufiane Feb 23, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 30 additions & 2 deletions docs/rules/no-shadow.md
Expand Up @@ -42,11 +42,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 @@ -164,6 +164,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
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;
}
boutahlilsoufiane marked this conversation as resolved.
Show resolved Hide resolved

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