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 23 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
29 changes: 27 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,31 @@ foo(function (err, result) {
});
```

### ignoreOnInitialization

The `ignoreOnInitialization` option is `false` by default.
If it is `true`, the rule prevents reporting variables on initialization statements, the shadowed variable must be on the left side and the shadowing variable must be on the right side.

Examples of **incorrect** code for the default `{ "ignoreOnInitialization": "true" }` option:
boutahlilsoufiane marked this conversation as resolved.
Show resolved Hide resolved

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

var x = x => x;
```

Because the shadowing variable x will shadow an initialized shadowed variable x.
boutahlilsoufiane marked this conversation as resolved.
Show resolved Hide resolved

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

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

var x = foo(x => x)
```

Because the shadowing variable x will shadow an uninitialized shadowed variable x.
boutahlilsoufiane marked this conversation as resolved.
Show resolved Hide resolved

## Related Rules

* [no-shadow-restricted-names](no-shadow-restricted-names.md)
Expand Down
154 changes: 138 additions & 16 deletions lib/rules/no-shadow.js
Expand Up @@ -11,6 +11,15 @@

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

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

const FUN_NODES_TYPES = ["ArrowFunctionExpression", "FunctionExpression"];
boutahlilsoufiane marked this conversation as resolved.
Show resolved Hide resolved
const CALL_EXP_NODE_TYPE = ["CallExpression"];
boutahlilsoufiane marked this conversation as resolved.
Show resolved Hide resolved
const SENTINEL_TYPE = /^(?:(?:Function|Class)(?:Declaration|Expression)|ArrowFunctionExpression|CatchClause|ImportDeclaration|ExportNamedDeclaration)$/u;
const FOR_IN_OF_TYPE = /^For(?:In|Of)Statement$/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,120 @@ 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];
}

/**
* Checks whether or not the nodes types include the node type.
* @param {ASTNode} node a node to get.
* @param {[string]} types an array of nodes types.
* @returns {boolean} True if nodes types include the node type.
*/
function isNodeMatchesTypes(node, types) {
return types.includes(node.type);
}

/**
* Searches from the current node through its ancestry to find a matching node.
* @param {ASTNode} node a node to get.
* @param {MatchCallback} match a callback that checks whether or not the node verifies its condition or not.
* @param {StopSearchingCallback} stopSearching a callback that checks whether or not the searching should be stopped or not.
* @returns {ASTNode} the matching node.
*/
function findSelfOrAncestor(node, match, stopSearching) {
let currentNode = node;

while (currentNode && !match(currentNode)) {
if (stopSearching && stopSearching(currentNode)) {
return null;
}
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];
const { variableScope } = variable.scope;
boutahlilsoufiane marked this conversation as resolved.
Show resolved Hide resolved


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

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

const callExpression = findSelfOrAncestor(
parent,
node => isNodeMatchesTypes(node, CALL_EXP_NODE_TYPE),
node => isNodeMatchesTypes(node, FUN_NODES_TYPES)
boutahlilsoufiane marked this conversation as resolved.
Show resolved Hide resolved
);

if (!callExpression) {
return false;
}
boutahlilsoufiane marked this conversation as resolved.
Show resolved Hide resolved

let node = outerDef && outerDef.name;
boutahlilsoufiane marked this conversation as resolved.
Show resolved Hide resolved
const innerDef = variable.defs[0];
const location = innerDef && innerDef.name.range[1];
boutahlilsoufiane marked this conversation as resolved.
Show resolved Hide resolved

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 +220,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 +275,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 +296,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 +306,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") &&
boutahlilsoufiane marked this conversation as resolved.
Show resolved Hide resolved
!(options.hoist !== "all" && isInTdz(variable, shadowed))
) {
const location = getDeclaredLocation(shadowed);
const messageId = location.global ? "noShadowGlobal" : "noShadow";
Expand Down