From f1fde01025b219e870b895197f6530b4301a39d3 Mon Sep 17 00:00:00 2001
From: Linus Groh <mail@linusgroh.de>
Date: Sat, 3 Apr 2021 14:56:54 +0200
Subject: [PATCH] LibJS: Fix returning from try statement

Not sure if this regressed at some point or just never worked, it
definitely wasn't tested at all. We would always return undefined when
returning from a try statement block, handler, or finalizer.
---
 Userland/Libraries/LibJS/AST.cpp              | 12 +++----
 .../LibJS/Tests/try-catch-finally-return.js   | 34 +++++++++++++++++++
 2 files changed, 40 insertions(+), 6 deletions(-)
 create mode 100644 Userland/Libraries/LibJS/Tests/try-catch-finally-return.js

diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp
index be1b71d3b4..f488ca7a54 100644
--- a/Userland/Libraries/LibJS/AST.cpp
+++ b/Userland/Libraries/LibJS/AST.cpp
@@ -1,6 +1,6 @@
 /*
  * Copyright (c) 2020, Andreas Kling <kling@serenityos.org>
- * Copyright (c) 2020, Linus Groh <mail@linusgroh.de>
+ * Copyright (c) 2020-2021, Linus Groh <mail@linusgroh.de>
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -1961,7 +1961,7 @@ Value TryStatement::execute(Interpreter& interpreter, GlobalObject& global_objec
 {
     InterpreterNodeScope node_scope { interpreter, *this };
 
-    interpreter.execute_statement(global_object, m_block, ScopeType::Try);
+    auto result = interpreter.execute_statement(global_object, m_block, ScopeType::Try);
     if (auto* exception = interpreter.exception()) {
         if (m_handler) {
             interpreter.vm().clear_exception();
@@ -1970,7 +1970,7 @@ Value TryStatement::execute(Interpreter& interpreter, GlobalObject& global_objec
             parameters.set(m_handler->parameter(), Variable { exception->value(), DeclarationKind::Var });
             auto* catch_scope = interpreter.heap().allocate<LexicalEnvironment>(global_object, move(parameters), interpreter.vm().call_frame().scope);
             TemporaryChange<ScopeObject*> scope_change(interpreter.vm().call_frame().scope, catch_scope);
-            interpreter.execute_statement(global_object, m_handler->body());
+            result = interpreter.execute_statement(global_object, m_handler->body());
         }
     }
 
@@ -1980,16 +1980,16 @@ Value TryStatement::execute(Interpreter& interpreter, GlobalObject& global_objec
         auto* previous_exception = interpreter.exception();
         interpreter.vm().clear_exception();
         interpreter.vm().stop_unwind();
-        m_finalizer->execute(interpreter, global_object);
+        result = m_finalizer->execute(interpreter, global_object);
         // If we previously had an exception and the finalizer didn't
         // throw a new one, restore the old one.
         // FIXME: This will print debug output in throw_exception() for
-        // a seconds time with m_should_log_exceptions enabled.
+        // a second time with m_should_log_exceptions enabled.
         if (previous_exception && !interpreter.exception())
             interpreter.vm().throw_exception(previous_exception);
     }
 
-    return js_undefined();
+    return result;
 }
 
 Value CatchClause::execute(Interpreter& interpreter, GlobalObject&) const
diff --git a/Userland/Libraries/LibJS/Tests/try-catch-finally-return.js b/Userland/Libraries/LibJS/Tests/try-catch-finally-return.js
new file mode 100644
index 0000000000..b64ea1120e
--- /dev/null
+++ b/Userland/Libraries/LibJS/Tests/try-catch-finally-return.js
@@ -0,0 +1,34 @@
+test("return from try block", () => {
+    function foo() {
+        try {
+            return "foo";
+        } catch {
+            return "bar";
+        }
+    }
+    expect(foo()).toBe("foo");
+});
+
+test("return from catch block", () => {
+    function foo() {
+        try {
+            throw "foo";
+        } catch {
+            return "bar";
+        }
+    }
+    expect(foo()).toBe("bar");
+});
+
+test("return from finally block", () => {
+    function foo() {
+        try {
+            return "foo";
+        } catch {
+            return "bar";
+        } finally {
+            return "baz";
+        }
+    }
+    expect(foo()).toBe("baz");
+});