Repository: denoland/deno_lint (rust)

Issue:
Problem in `finally` block
Not a big issue but I don't think `deno lint` should throw a `no-unreachable` problem here. I could prevent the problem by moving the `return` statement outside the block.

### Lint Name
`no-unreachable`

### Code Snippet

```ts
private async handleContext(ctx: Context<S>): Promise<Response> {
    try {
      ctx = await this.invokeHandlers(ctx, this.routes);
    } catch (error) {
      ctx.error = error;
      ctx = await this.invokeHandlers(ctx, this.catchRoutes);
    } finally {
      ctx = await this.invokeHandlers(ctx, this.finallyRoutes);
      return ctx.response;
    }
  }
```

### Expected Result

Nothing being thrown

### Version

deno 1.20.1 (release, x86_64-unknown-linux-gnu)
v8 10.0.139.6
typescript 4.6.2


Pull request:
fix(control-flow): analyze try statements in correct order
Closes #1031 


Patch:
diff --git a/src/control_flow/mod.rs b/src/control_flow/mod.rs
index f7554df..c5120d9 100644
--- a/src/control_flow/mod.rs
+++ b/src/control_flow/mod.rs
@@ -307,14 +307,6 @@ impl Analyzer<'_> {
         }
         BlockKind::Finally => {
           self.mark_as_end(lo, end);
-          match end {
-            e if matches!(e, End::Forced { .. }) => {
-              self.scope.end = Some(e);
-            }
-            _ => {
-              self.scope.end = prev_end;
-            }
-          }
         }
       }
     }
@@ -737,11 +729,6 @@ impl Visit for Analyzer<'_> {
   }
 
   fn visit_try_stmt(&mut self, n: &TryStmt) {
-    if let Some(finalizer) = &n.finalizer {
-      self.with_child_scope(BlockKind::Finally, finalizer.span.lo, |a| {
-        n.finalizer.visit_with(a);
-      });
-    }
     let old_throw = self.scope.may_throw;
 
     let prev_end = self.scope.end;
@@ -749,53 +736,64 @@ impl Visit for Analyzer<'_> {
     self.scope.may_throw = false;
     n.block.visit_with(self);
 
-    let mut try_block_end = None;
+    let try_block_end = self.scope.end;
+    let try_block_may_throw = self.scope.may_throw;
 
-    if self.scope.may_throw {
-      if let Some(end) = self.scope.end {
-        try_block_end = Some(end);
+    if let Some(handler) = &n.handler {
+      if try_block_may_throw {
         self.scope.end = prev_end;
       }
-    } else if let Some(end) = self.scope.end {
-      try_block_end = Some(end);
-      self.mark_as_end(n.span.lo, end);
-    }
-
-    if let Some(handler) = &n.handler {
       self.scope.may_throw = false;
       handler.visit_with(self);
 
-      match (try_block_end, self.scope.end) {
-        (Some(x), Some(y)) if x.is_forced() && y.is_forced() => {
-          // This `unwrap` is safe; `x` and `y` are surely `Some(End::Forced { .. })`
-          self.mark_as_end(n.span.lo, x.merge_forced(y).unwrap());
+      if try_block_may_throw {
+        match (try_block_end, self.scope.end) {
+          (
+            Some(End::Forced {
+              ret: false,
+              throw: true,
+              infinite_loop: false,
+            }),
+            _,
+          ) => {}
+          (Some(x), Some(y)) if x.is_forced() && y.is_forced() => {
+            // This `unwrap` is safe; `x` and `y` are surely `Some(End::Forced { .. })`
+            self.scope.end = Some(x.merge_forced(y).unwrap());
+          }
+          (_, Some(y)) if y.is_forced() => {
+            self.scope.end = try_block_end;
+          }
+          (None | Some(End::Continue), Some(End::Break)) => {
+            self.scope.end = try_block_end;
+          }
+          _ => {}
         }
+      } else {
+        self.scope.end = try_block_end;
+      }
+    }
+
+    if let Some(finalizer) = &n.finalizer {
+      let try_catch_end = self.scope.end;
+      self.scope.end = prev_end;
+      self.with_child_scope(BlockKind::Finally, finalizer.span.lo, |a| {
+        n.finalizer.visit_with(a);
+      });
+      match (try_catch_end, self.scope.end) {
         (Some(x), Some(End::Break)) if x.is_forced() => {
-          self.mark_as_end(n.span.lo, End::Break);
+          self.scope.end = Some(x);
         }
-        _ => {
-          self.mark_as_end(n.span.lo, End::Continue);
-          self.scope.end = prev_end;
+        (Some(x), None | Some(End::Continue)) => {
+          self.scope.end = Some(x);
         }
+        _ => {}
       }
-    } else if matches!(
-      try_block_end,
-      Some(End::Forced { .. }) | Some(End::Break)
-    ) {
-      // This `unwrap` is safe; `try_block_end` is surely wrapped in `Some`
-      self.mark_as_end(n.span.lo, try_block_end.unwrap());
-    } else if let Some(finalizer) = &n.finalizer {
-      self.mark_as_end(
-        n.span.lo,
-        self
-          .get_end_reason(finalizer.span.lo)
-          .unwrap_or(End::Continue),
-      );
-      self.scope.end = prev_end;
-      self.scope.may_throw = old_throw;
-    } else {
-      self.scope.end = prev_end;
     }
+
+    if let Some(end) = self.scope.end {
+      self.mark_as_end(n.span.lo, end);
+    }
+    self.scope.may_throw |= old_throw;
   }
 
   fn visit_labeled_stmt(&mut self, n: &LabeledStmt) {
diff --git a/src/rules/no_unreachable.rs b/src/rules/no_unreachable.rs
index 173095d..cc21a36 100644
--- a/src/rules/no_unreachable.rs
+++ b/src/rules/no_unreachable.rs
@@ -174,6 +174,10 @@ function foo() {
       "function* foo() { try { yield 1; return; } catch (err) { return err; } }",
       "function foo() { try { bar(); return; } catch (err) { return err; } }",
       "function foo() { try { a.b.c = 1; return; } catch (err) { return err; } }",
+      "function foo() { try { a.b.c = 1; } catch (err) { c.b.a = 1; } finally { return; } }",
+      "function foo() { try { a.b.c = 1; } catch (err) { return; } finally { return; } }",
+      "function foo() { try { bar(); return; } catch (err) { a.b = 1; } finally { return; } }",
+      "function foo() { try { bar(); return; } catch (err) { return; } finally { return; } }",
 
       r#"
 function normalize(type: string): string | undefined {


Test patch:
diff --git a/src/control_flow/analyze_test.rs b/src/control_flow/analyze_test.rs
index c7a301b..2903c99 100644
--- a/src/control_flow/analyze_test.rs
+++ b/src/control_flow/analyze_test.rs
@@ -503,7 +503,7 @@ function foo() {
     false,
     Some(End::Forced {
       ret: true,
-      throw: true,
+      throw: false,
       infinite_loop: false
     })
   ); // TryStmt
@@ -585,7 +585,7 @@ function foo() {
     false,
     Some(End::Forced {
       ret: true,
-      throw: true,
+      throw: false,
       infinite_loop: false
     })
   ); // TryStmt
@@ -700,6 +700,28 @@ bar();
   assert_flow!(flow, 74, false, None); // `bar();`
 }
 
+#[test]
+fn try_11() {
+  let src = r#"
+try {
+  throw 0;
+} catch (e) {
+  break;
+} finally {
+  return;
+}
+"#;
+  let flow = analyze_flow(src);
+  assert_flow!(flow, 1, false, Some(End::forced_return())); // try stmt
+  assert_flow!(flow, 5, false, Some(End::forced_throw())); // BlockStmt of try
+  assert_flow!(flow, 9, false, Some(End::forced_throw())); // throw stmt
+  assert_flow!(flow, 20, false, Some(End::Break)); // catch
+  assert_flow!(flow, 30, false, Some(End::Break)); // BloskStmt of catch
+  assert_flow!(flow, 34, false, Some(End::Break)); // break stmt
+  assert_flow!(flow, 51, false, Some(End::forced_return())); // finally
+  assert_flow!(flow, 55, false, Some(End::forced_return())); // return stmt
+}
+
 #[test]
 fn if_1() {
   let src = r#"