From 05ca24c57f90f91421b682dca3d7a45b7957fb77 Mon Sep 17 00:00:00 2001 From: "Nicholas C. Zakas" Date: Thu, 26 Aug 2021 14:27:10 -0700 Subject: [PATCH] Update: Code path analysis for class fields (fixes #14343) (#14886) * Fix: Code path analysis for class fields (fixes #14343) * PropertyDefinition code paths * Add test and comments * Reverse order of code path closing * Added CodePath#origin * Update tests/lib/linter/code-path-analysis/code-path.js Co-authored-by: Milos Djermanovic * Update lib/linter/code-path-analysis/code-path.js Co-authored-by: Milos Djermanovic Co-authored-by: Milos Djermanovic --- .../code-path-analysis/README.md | 1 + .../code-path-analysis/code-path-analyzer.js | 150 ++++++++++++++---- lib/linter/code-path-analysis/code-path.js | 18 ++- .../class-fields-init--arrow-function.js | 37 +++++ .../class-fields-init--call-expression.js | 26 +++ .../class-fields-init--conditional.js | 32 ++++ .../class-fields-init--simple.js | 28 ++++ .../code-path-analysis/function--new.js | 30 ++++ .../object-literal--conditional.js | 20 +++ .../code-path-analysis/code-path-analyzer.js | 4 +- .../linter/code-path-analysis/code-path.js | 49 +++++- 11 files changed, 353 insertions(+), 42 deletions(-) create mode 100644 tests/fixtures/code-path-analysis/class-fields-init--arrow-function.js create mode 100644 tests/fixtures/code-path-analysis/class-fields-init--call-expression.js create mode 100644 tests/fixtures/code-path-analysis/class-fields-init--conditional.js create mode 100644 tests/fixtures/code-path-analysis/class-fields-init--simple.js create mode 100644 tests/fixtures/code-path-analysis/function--new.js create mode 100644 tests/fixtures/code-path-analysis/object-literal--conditional.js diff --git a/docs/developer-guide/code-path-analysis/README.md b/docs/developer-guide/code-path-analysis/README.md index 1c84b2e1f73..3530ff6a912 100644 --- a/docs/developer-guide/code-path-analysis/README.md +++ b/docs/developer-guide/code-path-analysis/README.md @@ -27,6 +27,7 @@ This has references of both the initial segment and the final segments of a code `CodePath` has the following properties: * `id` (`string`) - A unique string. Respective rules can use `id` to save additional information for each code path. +* `origin` (`string`) - The reason that the code path was started. May be `"program"`, `"function"`, or `"class-field-initializer"`. * `initialSegment` (`CodePathSegment`) - The initial segment of this code path. * `finalSegments` (`CodePathSegment[]`) - The final segments which includes both returned and thrown. * `returnedSegments` (`CodePathSegment[]`) - The final segments which includes only returned. diff --git a/lib/linter/code-path-analysis/code-path-analyzer.js b/lib/linter/code-path-analysis/code-path-analyzer.js index 59417ab4a4d..d66c2f1be32 100644 --- a/lib/linter/code-path-analysis/code-path-analyzer.js +++ b/lib/linter/code-path-analysis/code-path-analyzer.js @@ -29,6 +29,18 @@ function isCaseNode(node) { return Boolean(node.test); } +/** + * Checks if a given node appears as the value of a PropertyDefinition node. + * @param {ASTNode} node THe node to check. + * @returns {boolean} `true` if the node is a PropertyDefinition value, + * false if not. + */ +function isPropertyDefinitionValue(node) { + const parent = node.parent; + + return parent && parent.type === "PropertyDefinition" && parent.value === node; +} + /** * Checks whether the given logical operator is taken into account for the code * path analysis. @@ -138,6 +150,7 @@ function isIdentifierReference(node) { return parent.id !== node; case "Property": + case "PropertyDefinition": case "MethodDefinition": return ( parent.key !== node || @@ -388,29 +401,64 @@ function processCodePathToEnter(analyzer, node) { let state = codePath && CodePath.getState(codePath); const parent = node.parent; + /** + * Creates a new code path and trigger the onCodePathStart event + * based on the currently selected node. + * @param {string} origin The reason the code path was started. + * @returns {void} + */ + function startCodePath(origin) { + if (codePath) { + + // Emits onCodePathSegmentStart events if updated. + forwardCurrentToHead(analyzer, node); + debug.dumpState(node, state, false); + } + + // Create the code path of this scope. + codePath = analyzer.codePath = new CodePath({ + id: analyzer.idGenerator.next(), + origin, + upper: codePath, + onLooped: analyzer.onLooped + }); + state = CodePath.getState(codePath); + + // Emits onCodePathStart events. + debug.dump(`onCodePathStart ${codePath.id}`); + analyzer.emitter.emit("onCodePathStart", codePath, node); + } + + /* + * Special case: The right side of class field initializer is considered + * to be its own function, so we need to start a new code path in this + * case. + */ + if (isPropertyDefinitionValue(node)) { + startCodePath("class-field-initializer"); + + /* + * Intentional fall through because `node` needs to also be + * processed by the code below. For example, if we have: + * + * class Foo { + * a = () => {} + * } + * + * In this case, we also need start a second code path. + */ + + } + switch (node.type) { case "Program": + startCodePath("program"); + break; + case "FunctionDeclaration": case "FunctionExpression": case "ArrowFunctionExpression": - if (codePath) { - - // Emits onCodePathSegmentStart events if updated. - forwardCurrentToHead(analyzer, node); - debug.dumpState(node, state, false); - } - - // Create the code path of this scope. - codePath = analyzer.codePath = new CodePath( - analyzer.idGenerator.next(), - codePath, - analyzer.onLooped - ); - state = CodePath.getState(codePath); - - // Emits onCodePathStart events. - debug.dump(`onCodePathStart ${codePath.id}`); - analyzer.emitter.emit("onCodePathStart", codePath, node); + startCodePath("function"); break; case "ChainExpression": @@ -503,6 +551,7 @@ function processCodePathToEnter(analyzer, node) { * @returns {void} */ function processCodePathToExit(analyzer, node) { + const codePath = analyzer.codePath; const state = CodePath.getState(codePath); let dontForward = false; @@ -627,28 +676,38 @@ function processCodePathToExit(analyzer, node) { * @returns {void} */ function postprocess(analyzer, node) { + + /** + * Ends the code path for the current node. + * @returns {void} + */ + function endCodePath() { + let codePath = analyzer.codePath; + + // Mark the current path as the final node. + CodePath.getState(codePath).makeFinal(); + + // Emits onCodePathSegmentEnd event of the current segments. + leaveFromCurrentSegment(analyzer, node); + + // Emits onCodePathEnd event of this code path. + debug.dump(`onCodePathEnd ${codePath.id}`); + analyzer.emitter.emit("onCodePathEnd", codePath, node); + debug.dumpDot(codePath); + + codePath = analyzer.codePath = analyzer.codePath.upper; + if (codePath) { + debug.dumpState(node, CodePath.getState(codePath), true); + } + + } + switch (node.type) { case "Program": case "FunctionDeclaration": case "FunctionExpression": case "ArrowFunctionExpression": { - let codePath = analyzer.codePath; - - // Mark the current path as the final node. - CodePath.getState(codePath).makeFinal(); - - // Emits onCodePathSegmentEnd event of the current segments. - leaveFromCurrentSegment(analyzer, node); - - // Emits onCodePathEnd event of this code path. - debug.dump(`onCodePathEnd ${codePath.id}`); - analyzer.emitter.emit("onCodePathEnd", codePath, node); - debug.dumpDot(codePath); - - codePath = analyzer.codePath = analyzer.codePath.upper; - if (codePath) { - debug.dumpState(node, CodePath.getState(codePath), true); - } + endCodePath(); break; } @@ -662,6 +721,27 @@ function postprocess(analyzer, node) { default: break; } + + /* + * Special case: The right side of class field initializer is considered + * to be its own function, so we need to end a code path in this + * case. + * + * We need to check after the other checks in order to close the + * code paths in the correct order for code like this: + * + * + * class Foo { + * a = () => {} + * } + * + * In this case, The ArrowFunctionExpression code path is closed first + * and then we need to close the code path for the PropertyDefinition + * value. + */ + if (isPropertyDefinitionValue(node)) { + endCodePath(); + } } //------------------------------------------------------------------------------ diff --git a/lib/linter/code-path-analysis/code-path.js b/lib/linter/code-path-analysis/code-path.js index b9f72912fba..f225c09a1d5 100644 --- a/lib/linter/code-path-analysis/code-path.js +++ b/lib/linter/code-path-analysis/code-path.js @@ -22,11 +22,14 @@ const IdGenerator = require("./id-generator"); class CodePath { /** - * @param {string} id An identifier. - * @param {CodePath|null} upper The code path of the upper function scope. - * @param {Function} onLooped A callback function to notify looping. + * Creates a new instance. + * @param {Object} options Options for the function (see below). + * @param {string} options.id An identifier. + * @param {string} options.origin The type of code path origin. + * @param {CodePath|null} options.upper The code path of the upper function scope. + * @param {Function} options.onLooped A callback function to notify looping. */ - constructor(id, upper, onLooped) { + constructor({ id, origin, upper, onLooped }) { /** * The identifier of this code path. @@ -35,6 +38,13 @@ class CodePath { */ this.id = id; + /** + * The reason that this code path was started. May be "program", + * "function", or "class-field-initializer". + * @type {string} + */ + this.origin = origin; + /** * The code path of the upper function scope. * @type {CodePath|null} diff --git a/tests/fixtures/code-path-analysis/class-fields-init--arrow-function.js b/tests/fixtures/code-path-analysis/class-fields-init--arrow-function.js new file mode 100644 index 00000000000..9005f131c4b --- /dev/null +++ b/tests/fixtures/code-path-analysis/class-fields-init--arrow-function.js @@ -0,0 +1,37 @@ +/*expected +initial->s3_1->final; +*/ +/*expected +initial->s2_1->final; +*/ +/*expected +initial->s1_1->final; +*/ + +class Foo { a = () => b } + +/*DOT +digraph { +node[shape=box,style="rounded,filled",fillcolor=white]; +initial[label="",shape=circle,style=filled,fillcolor=black,width=0.25,height=0.25]; +final[label="",shape=doublecircle,style=filled,fillcolor=black,width=0.25,height=0.25]; +s3_1[label="ArrowFunctionExpression:enter\nIdentifier (b)\nArrowFunctionExpression:exit"]; +initial->s3_1->final; +} + +digraph { + node[shape=box,style="rounded,filled",fillcolor=white]; + initial[label="",shape=circle,style=filled,fillcolor=black,width=0.25,height=0.25]; + final[label="",shape=doublecircle,style=filled,fillcolor=black,width=0.25,height=0.25]; + s2_1[label="ArrowFunctionExpression"]; + initial->s2_1->final; +} + +digraph { + node[shape=box,style="rounded,filled",fillcolor=white]; + initial[label="",shape=circle,style=filled,fillcolor=black,width=0.25,height=0.25]; + final[label="",shape=doublecircle,style=filled,fillcolor=black,width=0.25,height=0.25]; + s1_1[label="Program:enter\nClassDeclaration:enter\nIdentifier (Foo)\nClassBody:enter\nPropertyDefinition:enter\nIdentifier (a)\nArrowFunctionExpression\nPropertyDefinition:exit\nClassBody:exit\nClassDeclaration:exit\nProgram:exit"]; + initial->s1_1->final; +} +*/ diff --git a/tests/fixtures/code-path-analysis/class-fields-init--call-expression.js b/tests/fixtures/code-path-analysis/class-fields-init--call-expression.js new file mode 100644 index 00000000000..ec43f0a8625 --- /dev/null +++ b/tests/fixtures/code-path-analysis/class-fields-init--call-expression.js @@ -0,0 +1,26 @@ +/*expected +initial->s2_1->final; +*/ +/*expected +initial->s1_1->final; +*/ + +class Foo { a = b() } + +/*DOT +digraph { + node[shape=box,style="rounded,filled",fillcolor=white]; + initial[label="",shape=circle,style=filled,fillcolor=black,width=0.25,height=0.25]; + final[label="",shape=doublecircle,style=filled,fillcolor=black,width=0.25,height=0.25]; + s2_1[label="CallExpression:enter\nIdentifier (b)\nCallExpression:exit"]; + initial->s2_1->final; +} + +digraph { + node[shape=box,style="rounded,filled",fillcolor=white]; + initial[label="",shape=circle,style=filled,fillcolor=black,width=0.25,height=0.25]; + final[label="",shape=doublecircle,style=filled,fillcolor=black,width=0.25,height=0.25]; + s1_1[label="Program:enter\nClassDeclaration:enter\nIdentifier (Foo)\nClassBody:enter\nPropertyDefinition:enter\nIdentifier (a)\nCallExpression\nPropertyDefinition:exit\nClassBody:exit\nClassDeclaration:exit\nProgram:exit"]; + initial->s1_1->final; +} +*/ diff --git a/tests/fixtures/code-path-analysis/class-fields-init--conditional.js b/tests/fixtures/code-path-analysis/class-fields-init--conditional.js new file mode 100644 index 00000000000..e763ca9760d --- /dev/null +++ b/tests/fixtures/code-path-analysis/class-fields-init--conditional.js @@ -0,0 +1,32 @@ +/*expected +initial->s2_1->s2_2->s2_4; +s2_1->s2_3->s2_4->final; +*/ +/*expected +initial->s1_1->final; +*/ + + +class Foo { a = b ? c : d } + +/*DOT +digraph { + node[shape=box,style="rounded,filled",fillcolor=white]; + initial[label="",shape=circle,style=filled,fillcolor=black,width=0.25,height=0.25]; + final[label="",shape=doublecircle,style=filled,fillcolor=black,width=0.25,height=0.25]; + s2_1[label="ConditionalExpression:enter\nIdentifier (b)"]; + s2_2[label="Identifier (c)"]; + s2_4[label="ConditionalExpression:exit"]; + s2_3[label="Identifier (d)"]; + initial->s2_1->s2_2->s2_4; + s2_1->s2_3->s2_4->final; +} + +digraph { + node[shape=box,style="rounded,filled",fillcolor=white]; + initial[label="",shape=circle,style=filled,fillcolor=black,width=0.25,height=0.25]; + final[label="",shape=doublecircle,style=filled,fillcolor=black,width=0.25,height=0.25]; + s1_1[label="Program:enter\nClassDeclaration:enter\nIdentifier (Foo)\nClassBody:enter\nPropertyDefinition:enter\nIdentifier (a)\nConditionalExpression\nPropertyDefinition:exit\nClassBody:exit\nClassDeclaration:exit\nProgram:exit"]; + initial->s1_1->final; +} +*/ diff --git a/tests/fixtures/code-path-analysis/class-fields-init--simple.js b/tests/fixtures/code-path-analysis/class-fields-init--simple.js new file mode 100644 index 00000000000..fd249c5d464 --- /dev/null +++ b/tests/fixtures/code-path-analysis/class-fields-init--simple.js @@ -0,0 +1,28 @@ +/*expected +initial->s2_1->final; +*/ +/*expected +initial->s1_1->final; +*/ + +class Foo { + a = b; +} + +/*DOT +digraph { + node[shape=box,style="rounded,filled",fillcolor=white]; + initial[label="",shape=circle,style=filled,fillcolor=black,width=0.25,height=0.25]; + final[label="",shape=doublecircle,style=filled,fillcolor=black,width=0.25,height=0.25]; + s2_1[label="Identifier (b)"]; + initial->s2_1->final; +} + +digraph { + node[shape=box,style="rounded,filled",fillcolor=white]; + initial[label="",shape=circle,style=filled,fillcolor=black,width=0.25,height=0.25]; + final[label="",shape=doublecircle,style=filled,fillcolor=black,width=0.25,height=0.25]; + s1_1[label="Program:enter\nClassDeclaration:enter\nIdentifier (Foo)\nClassBody:enter\nPropertyDefinition:enter\nIdentifier (a)\nIdentifier (b)\nPropertyDefinition:exit\nClassBody:exit\nClassDeclaration:exit\nProgram:exit"]; + initial->s1_1->final; +} +*/ diff --git a/tests/fixtures/code-path-analysis/function--new.js b/tests/fixtures/code-path-analysis/function--new.js new file mode 100644 index 00000000000..1aa471e3ccf --- /dev/null +++ b/tests/fixtures/code-path-analysis/function--new.js @@ -0,0 +1,30 @@ +/*expected +initial->s2_1->s2_2->s2_4; +s2_1->s2_3->s2_4->final; +*/ +/*expected +initial->s1_1->final; +*/ +function Foo() { this.a = b ? c : d }; new Foo() + +/*DOT +digraph { + node[shape=box,style="rounded,filled",fillcolor=white]; + initial[label="",shape=circle,style=filled,fillcolor=black,width=0.25,height=0.25]; + final[label="",shape=doublecircle,style=filled,fillcolor=black,width=0.25,height=0.25]; + s2_1[label="FunctionDeclaration:enter\nIdentifier (Foo)\nBlockStatement:enter\nExpressionStatement:enter\nAssignmentExpression:enter\nMemberExpression:enter\nThisExpression\nIdentifier (a)\nMemberExpression:exit\nConditionalExpression:enter\nIdentifier (b)"]; + s2_2[label="Identifier (c)"]; + s2_4[label="ConditionalExpression:exit\nAssignmentExpression:exit\nExpressionStatement:exit\nBlockStatement:exit\nFunctionDeclaration:exit"]; + s2_3[label="Identifier (d)"]; + initial->s2_1->s2_2->s2_4; + s2_1->s2_3->s2_4->final; +} + +digraph { + node[shape=box,style="rounded,filled",fillcolor=white]; + initial[label="",shape=circle,style=filled,fillcolor=black,width=0.25,height=0.25]; + final[label="",shape=doublecircle,style=filled,fillcolor=black,width=0.25,height=0.25]; + s1_1[label="Program:enter\nFunctionDeclaration\nEmptyStatement\nExpressionStatement:enter\nNewExpression:enter\nIdentifier (Foo)\nNewExpression:exit\nExpressionStatement:exit\nProgram:exit"]; + initial->s1_1->final; +} +*/ diff --git a/tests/fixtures/code-path-analysis/object-literal--conditional.js b/tests/fixtures/code-path-analysis/object-literal--conditional.js new file mode 100644 index 00000000000..c5a4145a8be --- /dev/null +++ b/tests/fixtures/code-path-analysis/object-literal--conditional.js @@ -0,0 +1,20 @@ +/*expected +initial->s1_1->s1_2->s1_4; +s1_1->s1_3->s1_4->final; +*/ + +x = { a: b ? c : d } + +/*DOT +digraph { + node[shape=box,style="rounded,filled",fillcolor=white]; + initial[label="",shape=circle,style=filled,fillcolor=black,width=0.25,height=0.25]; + final[label="",shape=doublecircle,style=filled,fillcolor=black,width=0.25,height=0.25]; + s1_1[label="Program:enter\nExpressionStatement:enter\nAssignmentExpression:enter\nIdentifier (x)\nObjectExpression:enter\nProperty:enter\nIdentifier (a)\nConditionalExpression:enter\nIdentifier + (b)"]; + s1_2[label="Identifier (c)"]; + s1_4[label="ConditionalExpression:exit\nProperty:exit\nObjectExpression:exit\nAssignmentExpression:exit\nExpressionStatement:exit\nProgram:exit"]; + s1_3[label="Identifier (d)"]; + initial->s1_1->s1_2->s1_4; + s1_1->s1_3->s1_4->final; +}*/ diff --git a/tests/lib/linter/code-path-analysis/code-path-analyzer.js b/tests/lib/linter/code-path-analysis/code-path-analyzer.js index b2356f24691..0b5dd33aab8 100644 --- a/tests/lib/linter/code-path-analysis/code-path-analyzer.js +++ b/tests/lib/linter/code-path-analysis/code-path-analyzer.js @@ -561,11 +561,11 @@ describe("CodePathAnalyzer", () => { } })); const messages = linter.verify(source, { - parserOptions: { ecmaVersion: 2021 }, + parserOptions: { ecmaVersion: 2022 }, rules: { test: 2 } }); - assert.strictEqual(messages.length, 0); + assert.strictEqual(messages.length, 0, "Unexpected linting error in code."); assert.strictEqual(actual.length, expected.length, "a count of code paths is wrong."); for (let i = 0; i < actual.length; ++i) { diff --git a/tests/lib/linter/code-path-analysis/code-path.js b/tests/lib/linter/code-path-analysis/code-path.js index c75b125022f..42a63a280df 100644 --- a/tests/lib/linter/code-path-analysis/code-path.js +++ b/tests/lib/linter/code-path-analysis/code-path.js @@ -30,7 +30,11 @@ function parseCodePaths(code) { retv.push(codePath); } })); - linter.verify(code, { rules: { test: 2 } }); + + linter.verify(code, { + rules: { test: 2 }, + parserOptions: { ecmaVersion: "latest" } + }); return retv; } @@ -62,7 +66,50 @@ function getOrderOfTraversing(codePath, options, callback) { //------------------------------------------------------------------------------ describe("CodePathAnalyzer", () => { + + /* + * If you need to output the code paths and DOT graph information for a + * particular piece of code, udpate and uncomment the following test and + * then run: + * DEBUG=eslint:code-path npx mocha tests/lib/linter/code-path-analysis/ + * + * All the information you need will be output to the console. + */ + /* + * it.only("test", () => { + * const codePaths = parseCodePaths("class Foo { a = () => b }"); + * }); + */ + + describe("CodePath#origin", () => { + + it("should be 'program' when code path starts at root node", () => { + const codePath = parseCodePaths("foo(); bar(); baz();")[0]; + + assert.strictEqual(codePath.origin, "program"); + }); + + it("should be 'function' when code path starts inside a function", () => { + const codePath = parseCodePaths("function foo() {}")[1]; + + assert.strictEqual(codePath.origin, "function"); + }); + + it("should be 'function' when code path starts inside an arrow function", () => { + const codePath = parseCodePaths("let foo = () => {}")[1]; + + assert.strictEqual(codePath.origin, "function"); + }); + + it("should be 'class-field-initializer' when code path starts inside a class field initializer", () => { + const codePath = parseCodePaths("class Foo { a=1; }")[1]; + + assert.strictEqual(codePath.origin, "class-field-initializer"); + }); + }); + describe(".traverseSegments()", () => { + describe("should traverse segments from the first to the end:", () => { /* eslint-disable internal-rules/multiline-comment-style -- Commenting out */ it("simple", () => {