From 6e2c325324479df1b3f868cf00a529b67d2c3d82 Mon Sep 17 00:00:00 2001 From: Soufiane Boutahlil Date: Fri, 25 Feb 2022 11:20:26 +0100 Subject: [PATCH] feat: Add `ignoreOnInitialization` option to no-shadow rule (#14963) * 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) --- docs/rules/no-shadow.md | 32 ++++- lib/rules/no-shadow.js | 143 ++++++++++++++++--- tests/lib/rules/no-shadow.js | 256 ++++++++++++++++++++++++++++++++++- 3 files changed, 412 insertions(+), 19 deletions(-) diff --git a/docs/rules/no-shadow.md b/docs/rules/no-shadow.md index 605a0eb2dfc..284b7a1b9a2 100644 --- a/docs/rules/no-shadow.md +++ b/docs/rules/no-shadow.md @@ -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 }] } ``` @@ -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) diff --git a/lib/rules/no-shadow.js b/lib/rules/no-shadow.js index bd619235ab9..43d7d738e29 100644 --- a/lib/rules/no-shadow.js +++ b/lib/rules/no-shadow.js @@ -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 //------------------------------------------------------------------------------ @@ -37,7 +46,8 @@ module.exports = { items: { type: "string" } - } + }, + ignoreOnInitialization: { type: "boolean", default: false } }, additionalProperties: false } @@ -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. @@ -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 ); } @@ -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") ); } @@ -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; } @@ -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"; diff --git a/tests/lib/rules/no-shadow.js b/tests/lib/rules/no-shadow.js index 13c4c1100d6..0afcc3ff31b 100644 --- a/tests/lib/rules/no-shadow.js +++ b/tests/lib/rules/no-shadow.js @@ -63,7 +63,35 @@ ruleTester.run("no-shadow", rule, { { code: "class C { static { let x; } static { let x; } }", parserOptions: { ecmaVersion: 2022 } }, { code: "class C { static { var x; { var x; /* redeclaration */ } } }", parserOptions: { ecmaVersion: 2022 } }, { code: "class C { static { { var x; } { var x; /* redeclaration */ } } }", parserOptions: { ecmaVersion: 2022 } }, - { code: "class C { static { { let x; } { let x; } } }", parserOptions: { ecmaVersion: 2022 } } + { code: "class C { static { { let x; } { let x; } } }", parserOptions: { ecmaVersion: 2022 } }, + { code: "const a = [].find(a => a)", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "const a = [].find(function(a) { return a; })", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "const [a = [].find(a => true)] = dummy", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "const { a = [].find(a => true) } = dummy", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "function func(a = [].find(a => true)) {}", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "for (const a in [].find(a => true)) {}", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "for (const a of [].find(a => true)) {}", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "const a = [].map(a => true).filter(a => a === 'b')", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "const a = [].map(a => true).filter(a => a === 'b').find(a => a === 'c')", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "const { a } = (({ a }) => ({ a }))();", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "const person = people.find(item => {const person = item.name; return person === 'foo'})", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "var y = bar || foo(y => y);", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "var y = bar && foo(y => y);", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "var z = bar(foo(z => z));", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "var z = boo(bar(foo(z => z)));", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "var match = function (person) { return person.name === 'foo'; };\nconst person = [].find(match);", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "const a = foo(x || (a => {}))", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "const { a = 1 } = foo(a => {})", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "const person = {...people.find((person) => person.firstName.startsWith('s'))}", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 2021 } }, + { code: "const person = { firstName: people.filter((person) => person.firstName.startsWith('s')).map((person) => person.firstName)[0]}", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 2021 } }, + { code: "() => { const y = foo(y => y); }", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "const x = (x => x)()", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "var y = bar || (y => y)();", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "var y = bar && (y => y)();", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "var x = (x => x)((y => y)());", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "const { a = 1 } = (a => {})()", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "() => { const y = (y => y)(); }", options: [{ ignoreOnInitialization: true }], parserOptions: { ecmaVersion: 6 } }, + { code: "const [x = y => y] = [].map(y => y)", parserOptions: { ecmaVersion: 6 } } ], invalid: [ { @@ -859,6 +887,232 @@ ruleTester.run("no-shadow", rule, { line: 1, column: 50 }] + }, + { + code: "let x = foo((x,y) => {});\nlet y;", + options: [{ hoist: "all" }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + messageId: "noShadow", + data: { + name: "x", + shadowedLine: 1, + shadowedColumn: 5 + }, + type: "Identifier" + }, + { + messageId: "noShadow", + data: { + name: "y", + shadowedLine: 2, + shadowedColumn: 5 + }, + type: "Identifier" + } + ] + }, + { + code: "const a = fn(()=>{ class C { fn () { const a = 42; return a } } return new C() })", + options: [{ ignoreOnInitialization: true }], + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "noShadow", + data: { + name: "a", + shadowedLine: 1, + shadowedColumn: 7 + }, + type: "Identifier", + line: 1, + column: 44 + }] + }, + { + code: "function a() {}\nfoo(a => {});", + options: [{ ignoreOnInitialization: true }], + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "noShadow", + data: { + name: "a", + shadowedLine: 1, + shadowedColumn: 10 + }, + type: "Identifier", + line: 2, + column: 5 + }] + }, + { + code: "const a = fn(()=>{ function C() { this.fn=function() { const a = 42; return a } } return new C() });", + options: [{ ignoreOnInitialization: true }], + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "noShadow", + data: { + name: "a", + shadowedLine: 1, + shadowedColumn: 7 + }, + type: "Identifier", + line: 1, + column: 62 + }] + }, + { + code: "const x = foo(() => { const bar = () => { return x => {}; }; return bar; });", + options: [{ ignoreOnInitialization: true }], + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "noShadow", + data: { + name: "x", + shadowedLine: 1, + shadowedColumn: 7 + }, + type: "Identifier", + line: 1, + column: 50 + }] + }, + { + code: "const x = foo(() => { return { bar(x) {} }; });", + options: [{ ignoreOnInitialization: true }], + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "noShadow", + data: { + name: "x", + shadowedLine: 1, + shadowedColumn: 7 + }, + type: "Identifier", + line: 1, + column: 36 + }] + }, + { + code: "const x = () => { foo(x => x); }", + options: [{ ignoreOnInitialization: true }], + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "noShadow", + data: { + name: "x", + shadowedLine: 1, + shadowedColumn: 7 + }, + type: "Identifier", + line: 1, + column: 23 + }] + }, + { + code: "const foo = () => { let x; bar(x => x); }", + options: [{ ignoreOnInitialization: true }], + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "noShadow", + data: { + name: "x", + shadowedLine: 1, + shadowedColumn: 25 + }, + type: "Identifier", + line: 1, + column: 32 + }] + }, + { + code: "foo(() => { const x = x => x; });", + options: [{ ignoreOnInitialization: true }], + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "noShadow", + data: { + name: "x", + shadowedLine: 1, + shadowedColumn: 19 + }, + type: "Identifier", + line: 1, + column: 23 + }] + }, + { + code: "const foo = (x) => { bar(x => {}) }", + options: [{ ignoreOnInitialization: true }], + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "noShadow", + data: { + name: "x", + shadowedLine: 1, + shadowedColumn: 14 + }, + type: "Identifier", + line: 1, + column: 26 + }] + }, + { + code: "let x = ((x,y) => {})();\nlet y;", + options: [{ hoist: "all" }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + messageId: "noShadow", + data: { + name: "x", + shadowedLine: 1, + shadowedColumn: 5 + }, + type: "Identifier" + }, + { + messageId: "noShadow", + data: { + name: "y", + shadowedLine: 2, + shadowedColumn: 5 + }, + type: "Identifier" + } + ] + }, + { + code: "const a = (()=>{ class C { fn () { const a = 42; return a } } return new C() })()", + options: [{ ignoreOnInitialization: true }], + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "noShadow", + data: { + name: "a", + shadowedLine: 1, + shadowedColumn: 7 + }, + type: "Identifier", + line: 1, + column: 42 + }] + }, + { + code: "const x = () => { (x => x)(); }", + options: [{ ignoreOnInitialization: true }], + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "noShadow", + data: { + name: "x", + shadowedLine: 1, + shadowedColumn: 7 + }, + type: "Identifier", + line: 1, + column: 20 + }] } ] });