Skip to content

Commit

Permalink
fix!: Ensure catch block after return in try is correct.
Browse files Browse the repository at this point in the history
Fixes #17579
  • Loading branch information
nzakas committed Oct 3, 2023
1 parent 2665552 commit 285204d
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 8 deletions.
32 changes: 25 additions & 7 deletions lib/linter/code-path-analysis/code-path-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -1030,12 +1030,12 @@ class CodePathState {
* @returns {ForkContext} The last context.
*/
popForkContext() {
const lastContext = this.forkContext;
const poppedForkContext = this.forkContext;

this.forkContext = lastContext.upper;
this.forkContext.replaceHead(lastContext.makeNext(0, -1));
this.forkContext = poppedForkContext.upper;
this.forkContext.replaceHead(poppedForkContext.makeNext(0, -1));

return lastContext;
return poppedForkContext;
}

/**
Expand Down Expand Up @@ -1612,14 +1612,32 @@ class CodePathState {
* @returns {void}
*/
makeCatchBlock() {

/*
* We have now traversed into a catch block, so we need to set up
* the segments to represent this part of try statement.
*/
const context = this.tryContext;
const forkContext = this.forkContext;
const originalThrownForkContext = context.thrownForkContext;

/*
* We are now in a catch block so we need to update the context
* with that information. This includes creating a new fork
* context in case we encounter any `throw` statements here.
* Special case: If the try block ends with a return statement,
* code path analysis automatically creates an unreachable segment
* after the return. Because it's unreachable, we don't want to
* continue the path into the catch block. Instead, we want to
* skip over that unreachable segment so the end of the try block
* goes right to the start of the catch block. To do that, we remove
* the unreachable segment from the fork context.
*/
if (!forkContext.reachable && something_that_indicates_a_problem) {
forkContext.segmentsList[0] = forkContext.segmentsList[0][0].prevSegments;
}

/*
* Update the `TryContext` to reflect the current state of the world
* This includes creating a new fork context in case we encounter any
* `throw` statements here.
*/
context.position = "catch";
context.thrownForkContext = ForkContext.newEmpty(forkContext);
Expand Down
43 changes: 43 additions & 0 deletions tests/fixtures/code-path-analysis/try--try-catch-return.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*expected
initial->s2_1->s2_2->s2_3->s2_4->s2_5;
s2_1->s2_4;
s2_2->final;
s2_5->final;
*/

/*expected
initial->s1_1->final;
*/

function foo() {
try {
bar();
return;
} catch {}
baz();
}

/*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\nTryStatement:enter\nBlockStatement:enter\nExpressionStatement:enter\nCallExpression:enter\nIdentifier (bar)"];
s2_2[label="CallExpression:exit\nExpressionStatement:exit\nReturnStatement"];
s2_3[style="rounded,dashed,filled",fillcolor="#FF9800",label="<<unreachable>>\nBlockStatement:exit"];
s2_4[label="CatchClause:enter\nBlockStatement\nCatchClause:exit"];
s2_5[label="TryStatement:exit\nExpressionStatement:enter\nCallExpression:enter\nIdentifier (baz)\nCallExpression:exit\nExpressionStatement:exit\nBlockStatement:exit\nFunctionDeclaration:exit"];
initial->s2_1->s2_2->s2_3->s2_4->s2_5;
s2_1->s2_4;
s2_2->final;
s2_5->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\nProgram:exit"];
initial->s1_1->final;
}
*/
4 changes: 3 additions & 1 deletion tests/lib/linter/code-path-analysis/code-path-analyzer.js
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,9 @@ describe("CodePathAnalyzer", () => {
const testDataFiles = fs.readdirSync(testDataDir);

testDataFiles.forEach(file => {
it(file, () => {

if (!file.includes('try--try-catch-return')) return;
it.only(file, () => {
const source = fs.readFileSync(path.join(testDataDir, file), { encoding: "utf8" });
const expected = getExpectedDotArrows(source);
const actual = [];
Expand Down

0 comments on commit 285204d

Please sign in to comment.