Skip to content

Commit

Permalink
fix: control flow analysis on "for" statements (#481)
Browse files Browse the repository at this point in the history
  • Loading branch information
@magurotuna
magurotuna committed Oct 30, 2020
1 parent c3a1b4e commit 9f03d55
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 18 deletions.
120 changes: 102 additions & 18 deletions src/control_flow.rs
View file
Expand Up @@ -109,7 +109,7 @@ enum End {
/// Break or continue
Break,
/// Pass through a block, like a function's block statement which ends without returning a value
/// or throwing an exception. Note that a node marked as `End::Pass` won't prevent further execution, which is
/// or throwing an exception. Note that a node marked as `End::Continue` won't prevent further execution, which is
/// different from `End::Forced` or `End::Break`.
Continue,
}
Expand Down Expand Up @@ -397,7 +397,7 @@ impl Visit for Analyzer<'_> {
if let Some(end) = case_end {
self.mark_as_end(n.span.lo, end);
} else {
self.mark_as_end(n.span.lo, End::Continue); // TODO(magurotuna): is `End::Pass` suitable?
self.mark_as_end(n.span.lo, End::Continue);
}

self.scope.end = prev_end;
Expand Down Expand Up @@ -487,28 +487,36 @@ impl Visit for Analyzer<'_> {
n.update.visit_with(n, self);
n.test.visit_with(n, self);

let mut stmt_end = None;
let mut is_infinite_loop = false;

self.with_child_scope(BlockKind::Loop, n.body.span().lo, |a| {
n.body.visit_with(n, a);

if a.scope.found_break.is_none() {
if n.test.is_none() {
// Infinite loop
a.mark_as_end(n.span.lo, End::Forced);
stmt_end = Some(End::Forced);
} else if let (_, Value::Known(true)) =
n.test.as_ref().unwrap().as_bool()
{
// Infinite loop
a.mark_as_end(n.span.lo, End::Forced);
stmt_end = Some(End::Forced);
match &n.test {
None => {
// Infinite loop
a.mark_as_end(n.span.lo, End::Forced);
is_infinite_loop = true;
}
Some(test) => {
if matches!(test.as_bool(), (_, Value::Known(true))) {
// Infinite loop
a.mark_as_end(n.span.lo, End::Forced);
is_infinite_loop = true;
}
}
}
}

if !is_infinite_loop {
a.mark_as_end(n.span.lo, End::Continue);
a.scope.end = Some(End::Continue);
}
});

if let Some(end) = stmt_end {
self.scope.end = Some(end)
if is_infinite_loop {
self.scope.end = Some(End::Forced);
}
}

Expand All @@ -521,7 +529,7 @@ impl Visit for Analyzer<'_> {
n.body.visit_with(n, a);

// it's impossible to decide whether it enters loop block unconditionally, so we always mark
// it as `End::Pass`.
// it as `End::Continue`.
a.mark_as_end(body_lo, End::Continue);
a.scope.end = Some(End::Continue);
});
Expand All @@ -536,7 +544,7 @@ impl Visit for Analyzer<'_> {
n.body.visit_with(n, a);

// it's impossible to decide whether it enters loop block unconditionally, so we always mark
// it as `End::Pass`.
// it as `End::Continue`.
a.mark_as_end(body_lo, End::Continue);
a.scope.end = Some(End::Continue);
});
Expand Down Expand Up @@ -737,7 +745,7 @@ function foo() {

// BlockStmt of while
// This block contains `return 1;` but whether entering the block depends on the specific value
// of `a`, so we treat it as `End::Pass`.
// of `a`, so we treat it as `End::Continue`.
assert_flow!(flow, 30, false, Some(End::Continue));

assert_flow!(flow, 36, false, Some(End::Forced)); // return stmt
Expand Down Expand Up @@ -879,6 +887,82 @@ function foo() {
assert_flow!(flow, 55, true, Some(End::Forced)); // return stmt
}

#[test]
fn for_1() {
let src = r#"
function foo() {
for (let i = 0; f(); i++) {
return 1;
}
bar();
}
"#;
let flow = analyze_flow(src);
assert_flow!(flow, 16, false, Some(End::Continue)); // BlockStmt of `foo`

// BlockStmt of for statement
// This is marked as `End::Continue` because it's quite difficult to decide statically whether
// the program enters the block or not.
assert_flow!(flow, 46, false, Some(End::Continue));

assert_flow!(flow, 52, false, Some(End::Forced)); // return stmt
assert_flow!(flow, 68, false, None); // `bar();`
}

#[test]
fn for_2() {
// infinite loop
let src = r#"
function foo() {
for (let i = 0; true; i++) {
return 1;
}
bar();
}
"#;
let flow = analyze_flow(src);
assert_flow!(flow, 16, false, Some(End::Forced)); // BlockStmt of `foo`
assert_flow!(flow, 47, false, Some(End::Forced)); // BlockStmt of for statement
assert_flow!(flow, 53, false, Some(End::Forced)); // return stmt
assert_flow!(flow, 69, true, None); // `bar();`
}

#[test]
fn for_3() {
// infinite loop
let src = r#"
function foo() {
for (let i = 0;; i++) {
return 1;
}
bar();
}
"#;
let flow = analyze_flow(src);
assert_flow!(flow, 16, false, Some(End::Forced)); // BlockStmt of `foo`
assert_flow!(flow, 42, false, Some(End::Forced)); // BlockStmt of for statement
assert_flow!(flow, 48, false, Some(End::Forced)); // return stmt
assert_flow!(flow, 64, true, None); // `bar();`
}

#[test]
fn for_4() {
// never enter the block of for
let src = r#"
function foo() {
for (let i = 0; false; i++) {
return 1;
}
bar();
}
"#;
let flow = analyze_flow(src);
assert_flow!(flow, 16, false, Some(End::Continue)); // BlockStmt of `foo`
assert_flow!(flow, 48, false, Some(End::Continue)); // BlockStmt of for statement
assert_flow!(flow, 54, false, Some(End::Forced)); // return stmt
assert_flow!(flow, 70, false, None); // `bar();`
}

#[test]
fn for_in_1() {
let src = r#"
Expand Down
22 changes: 22 additions & 0 deletions src/rules/no_unreachable.rs
View file
Expand Up @@ -132,6 +132,27 @@ function foo() {
"#,

"function foo() { var x = 1; while (x) { return; } x = 2; }",
// https://github.com/denoland/deno_lint/issues/477
"function foo() { for (;false;) { return 0; } return 1; }",
"function foo() { var x = 1; for (let i = 0; i < bar(); i++) { return; } x = 2; }",
r#"
function foo() {
const partsA = [];
const partsB = [];
for (let i = 0; i < Math.max(partsA.length, partsB.length); i++) {
const partA = partsA[i];
const partB = partsB[i];
if (partA === undefined) return -1;
if (partB === undefined) return 1;
if (partA === partB) continue;
const priorityA = partA.startsWith(":") ? partA.endsWith("*") ? 0 : 1 : 2;
const priorityB = partB.startsWith(":") ? partB.endsWith("*") ? 0 : 1 : 2;
return Math.max(Math.min(priorityB - priorityA, 1), -1);
}
return 0;
}
"#,

"function foo() { var x = 1; for (x in {}) { return; } x = 2; }",
"function foo() { var x = 1; for (x of []) { return; } x = 2; }",
"function foo() { var x = 1; try { return; } finally { x = 2; } }",
Expand Down Expand Up @@ -350,6 +371,7 @@ console.log("unreachable???");
"function foo() { var x = 1; do { return; } while (x); x = 2; }": [{ col: 54, message: MESSAGE }],
"function foo() { var x = 1; while (x) { if (x) break; else continue; x = 2; } }": [{ col: 69, message: MESSAGE }],
"function foo() { var x = 1; for (;;) { if (x) continue; } x = 2; }": [{ col: 58, message: MESSAGE }],
"function foo() { var x = 1; for (;true;) { if (x) continue; } x = 2; }": [{ col: 62, message: MESSAGE }],
"function foo() { var x = 1; while (true) { } x = 2; }": [{ col: 45, message: MESSAGE }],
"const arrow_direction = arrow => {
switch (arrow) {
Expand Down

0 comments on commit 9f03d55

Please sign in to comment.