Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix: fix wrong code-path about try-for-in (fixes #8848) (#9348)
  • Loading branch information
mysticatea authored and ilyavolodin committed Sep 27, 2017
1 parent 434d9e2 commit 38d0cb2
Show file tree
Hide file tree
Showing 6 changed files with 141 additions and 41 deletions.
78 changes: 39 additions & 39 deletions lib/code-path-analysis/code-path-segment.js
Expand Up @@ -15,43 +15,6 @@ const debug = require("./debug-helpers");
// Helpers
//------------------------------------------------------------------------------

/**
* Replaces unused segments with the previous segments of each unused segment.
*
* @param {CodePathSegment[]} segments - An array of segments to replace.
* @returns {CodePathSegment[]} The replaced array.
*/
function flattenUnusedSegments(segments) {
const done = Object.create(null);
const retv = [];

for (let i = 0; i < segments.length; ++i) {
const segment = segments[i];

// Ignores duplicated.
if (done[segment.id]) {
continue;
}

// Use previous segments if unused.
if (!segment.internal.used) {
for (let j = 0; j < segment.allPrevSegments.length; ++j) {
const prevSegment = segment.allPrevSegments[j];

if (!done[prevSegment.id]) {
done[prevSegment.id] = true;
retv.push(prevSegment);
}
}
} else {
done[segment.id] = true;
retv.push(segment);
}
}

return retv;
}

/**
* Checks whether or not a given segment is reachable.
*
Expand Down Expand Up @@ -163,7 +126,7 @@ class CodePathSegment {
static newNext(id, allPrevSegments) {
return new CodePathSegment(
id,
flattenUnusedSegments(allPrevSegments),
CodePathSegment.flattenUnusedSegments(allPrevSegments),
allPrevSegments.some(isReachable)
);
}
Expand All @@ -176,7 +139,7 @@ class CodePathSegment {
* @returns {CodePathSegment} The created segment.
*/
static newUnreachable(id, allPrevSegments) {
const segment = new CodePathSegment(id, flattenUnusedSegments(allPrevSegments), false);
const segment = new CodePathSegment(id, CodePathSegment.flattenUnusedSegments(allPrevSegments), false);

// In `if (a) return a; foo();` case, the unreachable segment preceded by
// the return statement is not used but must not be remove.
Expand Down Expand Up @@ -238,6 +201,43 @@ class CodePathSegment {
static markPrevSegmentAsLooped(segment, prevSegment) {
segment.internal.loopedPrevSegments.push(prevSegment);
}

/**
* Replaces unused segments with the previous segments of each unused segment.
*
* @param {CodePathSegment[]} segments - An array of segments to replace.
* @returns {CodePathSegment[]} The replaced array.
*/
static flattenUnusedSegments(segments) {
const done = Object.create(null);
const retv = [];

for (let i = 0; i < segments.length; ++i) {
const segment = segments[i];

// Ignores duplicated.
if (done[segment.id]) {
continue;
}

// Use previous segments if unused.
if (!segment.internal.used) {
for (let j = 0; j < segment.allPrevSegments.length; ++j) {
const prevSegment = segment.allPrevSegments[j];

if (!done[prevSegment.id]) {
done[prevSegment.id] = true;
retv.push(prevSegment);
}
}
} else {
done[segment.id] = true;
retv.push(segment);
}
}

return retv;
}
}

module.exports = CodePathSegment;
3 changes: 3 additions & 0 deletions lib/code-path-analysis/code-path-state.js
Expand Up @@ -169,6 +169,9 @@ function removeConnection(prevSegments, nextSegments) {
* @returns {void}
*/
function makeLooped(state, fromSegments, toSegments) {
fromSegments = CodePathSegment.flattenUnusedSegments(fromSegments);
toSegments = CodePathSegment.flattenUnusedSegments(toSegments);

const end = Math.min(fromSegments.length, toSegments.length);

for (let i = 0; i < end; ++i) {
Expand Down
33 changes: 33 additions & 0 deletions tests/fixtures/code-path-analysis/try--try-with-for-inof-1.js
@@ -0,0 +1,33 @@
/*expected
initial->s1_1->s1_3->s1_2->s1_5->s1_2;
s1_3->s1_6->s1_7->s1_8;
s1_5->s1_6;
s1_3->s1_7;
s1_6->s1_8->final;
*/

try {
for (let x of xs) {
}
} catch (err) {
}

/*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\nTryStatement\nBlockStatement\nForOfStatement"];
s1_3[label="Identifier (xs)\nIdentifier:exit (xs)"];
s1_2[label="VariableDeclaration\nVariableDeclarator\nIdentifier (x)\nIdentifier:exit (x)\nVariableDeclarator:exit\nVariableDeclaration:exit"];
s1_5[label="BlockStatement\nBlockStatement:exit"];
s1_6[label="ForOfStatement:exit\nBlockStatement:exit"];
s1_7[label="CatchClause\nIdentifier (err)\nBlockStatement\nIdentifier:exit (err)\nBlockStatement:exit\nCatchClause:exit"];
s1_8[label="TryStatement:exit\nProgram:exit"];
initial->s1_1->s1_3->s1_2->s1_5->s1_2;
s1_3->s1_6->s1_7->s1_8;
s1_5->s1_6;
s1_3->s1_7;
s1_6->s1_8->final;
}
*/
32 changes: 32 additions & 0 deletions tests/fixtures/code-path-analysis/try--try-with-for-inof-2.js
@@ -0,0 +1,32 @@
/*expected
initial->s1_1->s1_3->s1_4->s1_2->s1_5->s1_2;
s1_3->s1_7->s1_8;
s1_4->s1_6->s1_7;
s1_5->s1_6->s1_8->final;
*/

try {
for (let x of obj.xs) {
}
} catch (err) {
}

/*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\nTryStatement\nBlockStatement\nForOfStatement"];
s1_3[label="MemberExpression\nIdentifier (obj)\nIdentifier:exit (obj)"];
s1_4[label="Identifier (xs)\nIdentifier:exit (xs)\nMemberExpression:exit"];
s1_2[label="VariableDeclaration\nVariableDeclarator\nIdentifier (x)\nIdentifier:exit (x)\nVariableDeclarator:exit\nVariableDeclaration:exit"];
s1_5[label="BlockStatement\nBlockStatement:exit"];
s1_7[label="CatchClause\nIdentifier (err)\nBlockStatement\nIdentifier:exit (err)\nBlockStatement:exit\nCatchClause:exit"];
s1_8[label="TryStatement:exit\nProgram:exit"];
s1_6[label="ForOfStatement:exit\nBlockStatement:exit"];
initial->s1_1->s1_3->s1_4->s1_2->s1_5->s1_2;
s1_3->s1_7->s1_8;
s1_4->s1_6->s1_7;
s1_5->s1_6->s1_8->final;
}
*/
18 changes: 17 additions & 1 deletion tests/lib/rules/constructor-super.js
Expand Up @@ -77,7 +77,23 @@ ruleTester.run("constructor-super", rule, {
].join("\n"),

// https://github.com/eslint/eslint/issues/5894
"class A { constructor() { return; super(); } }"
"class A { constructor() { return; super(); } }",

// https://github.com/eslint/eslint/issues/8848
`
class A extends B {
constructor(props) {
super(props);
try {
let arr = [];
for (let a of arr) {
}
} catch (err) {
}
}
}
`
],
invalid: [

Expand Down
18 changes: 17 additions & 1 deletion tests/lib/rules/no-this-before-super.js
Expand Up @@ -77,7 +77,23 @@ ruleTester.run("no-this-before-super", rule, {

// https://github.com/eslint/eslint/issues/5894
"class A { constructor() { return; this; } }",
"class A extends B { constructor() { return; this; } }"
"class A extends B { constructor() { return; this; } }",

// https://github.com/eslint/eslint/issues/8848
`
class A extends B {
constructor(props) {
super(props);
try {
let arr = [];
for (let a of arr) {
}
} catch (err) {
}
}
}
`
],
invalid: [

Expand Down

0 comments on commit 38d0cb2

Please sign in to comment.