refactor(cli): Refactor execution & microtasks into CliRunner struct

andreubotella · andreubotella · commit 192a470faefb · 2024-07-21T23:56:55.000+02:00
In order to add a repl to the cli, trynova#323 moved the `initialize_global_object` and `exit_with_parse_errors` functions into a new `helper` module. However, this still left duplicate code between the `eval` and `repl` commands. This patch moves the isolate and realm initialization, as well as the script execution and microtask checkpoint, to a new `CliRunner` struct that lives in the `helper` module. Aside from just refactoring, this patch makes two visible changes related to promise jobs: - When passing multiple files to `eval`, now promise jobs run after each script execution, rather than at the end. - In `repl` mode it used to be that promise jobs were never run. Now, all promise jobs enqueued when running a line of JS are run before the result is printed.
diff --git a/nova_cli/Cargo.toml b/nova_cli/Cargo.toml
@@ -10,6 +10,7 @@ cliclack = { workspace = true }
 console = { workspace = true }
 miette = { workspace = true }
 nova_vm = { path = "../nova_vm" }
+oxc_allocator = { workspace = true }
 oxc_ast = { workspace = true }
 oxc_parser = { workspace = true }
 oxc_semantic = { workspace = true }
diff --git a/nova_cli/src/helper.rs b/nova_cli/src/helper.rs
@@ -1,12 +1,17 @@
 use nova_vm::ecmascript::{
     builtins::{create_builtin_function, ArgumentsList, Behaviour, BuiltinFunctionArgs},
-    execution::{Agent, JsResult},
+    execution::{
+        agent::{HostHooks, Job, Options},
+        initialize_host_defined_realm, Agent, JsResult, Realm, RealmIdentifier,
+    },
+    scripts_and_modules::script::{parse_script, script_evaluation},
     types::{InternalMethods, IntoValue, Object, PropertyDescriptor, PropertyKey, Value},
 };
 use oxc_diagnostics::OxcDiagnostic;
+use std::{cell::RefCell, collections::VecDeque, fmt::Debug};
 
 /// Initialize the global object with the built-in functions.
-pub fn initialize_global_object(agent: &mut Agent, global: Object) {
+fn initialize_global_object(agent: &mut Agent, global: Object) {
     // `print` function
     fn print(agent: &mut Agent, _this: Value, args: ArgumentsList) -> JsResult<Value> {
         if args.len() == 0 {
@@ -59,3 +64,106 @@ pub fn exit_with_parse_errors(errors: Vec<OxcDiagnostic>, source_path: &str, sou
 
     std::process::exit(1);
 }
+
+#[derive(Default)]
+struct CliHostHooks {
+    promise_job_queue: RefCell<VecDeque<Job>>,
+}
+
+// RefCell doesn't implement Debug
+impl Debug for CliHostHooks {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("CliHostHooks")
+            //.field("promise_job_queue", &*self.promise_job_queue.borrow())
+            .finish()
+    }
+}
+
+impl CliHostHooks {
+    fn pop_promise_job(&self) -> Option<Job> {
+        self.promise_job_queue.borrow_mut().pop_front()
+    }
+}
+
+impl HostHooks for CliHostHooks {
+    fn enqueue_promise_job(&self, job: Job) {
+        self.promise_job_queue.borrow_mut().push_back(job);
+    }
+}
+
+pub struct CliRunner {
+    allocator: oxc_allocator::Allocator,
+    host_hooks: &'static CliHostHooks,
+    agent: Option<Agent>,
+    realm_id: RealmIdentifier,
+}
+
+impl CliRunner {
+    pub fn new(print_internals: bool) -> Self {
+        let host_hooks: &CliHostHooks = &*Box::leak(Box::default());
+        let mut agent = Agent::new(
+            Options {
+                disable_gc: false,
+                print_internals,
+            },
+            host_hooks,
+        );
+
+        {
+            let create_global_object: Option<fn(&mut Realm) -> Object> = None;
+            let create_global_this_value: Option<fn(&mut Realm) -> Object> = None;
+            initialize_host_defined_realm(
+                &mut agent,
+                create_global_object,
+                create_global_this_value,
+                Some(initialize_global_object),
+            );
+        }
+
+        let realm_id = agent.current_realm_id();
+        Self {
+            allocator: Default::default(),
+            host_hooks,
+            agent: Some(agent),
+            realm_id,
+        }
+    }
+
+    pub fn run_script_and_microtasks(
+        &mut self,
+        script: Box<str>,
+        script_path: &str,
+        allow_loose_mode: bool,
+    ) -> JsResult<Value> {
+        let script = match parse_script(
+            &self.allocator,
+            script,
+            self.realm_id,
+            !allow_loose_mode,
+            None,
+        ) {
+            Ok(script) => script,
+            Err((file, errors)) => exit_with_parse_errors(errors, script_path, &file),
+        };
+
+        let result = script_evaluation(self.agent(), script)?;
+
+        while let Some(job) = self.host_hooks.pop_promise_job() {
+            job.run(self.agent())?;
+        }
+
+        Ok(result)
+    }
+
+    pub fn agent(&mut self) -> &mut Agent {
+        self.agent.as_mut().unwrap()
+    }
+}
+
+impl Drop for CliRunner {
+    fn drop(&mut self) {
+        // The agent unsafely borrows the allocator and the host hooks, so it
+        // has to be dropped first.
+        self.agent.take();
+    }
+}
diff --git a/nova_cli/src/main.rs b/nova_cli/src/main.rs
@@ -4,19 +4,10 @@
 mod helper;
 mod theme;
 
-use std::{cell::RefCell, collections::VecDeque, fmt::Debug};
-
 use clap::{Parser as ClapParser, Subcommand};
 use cliclack::{input, intro, set_theme};
-use helper::{exit_with_parse_errors, initialize_global_object};
-use nova_vm::ecmascript::{
-    execution::{
-        agent::{HostHooks, Job, Options},
-        initialize_host_defined_realm, Agent, Realm,
-    },
-    scripts_and_modules::script::{parse_script, script_evaluation},
-    types::{Object, Value},
-};
+use helper::{exit_with_parse_errors, CliRunner};
+use nova_vm::ecmascript::types::Value;
 use oxc_parser::Parser;
 use oxc_semantic::{SemanticBuilder, SemanticBuilderReturn};
 use oxc_span::SourceType;
@@ -56,32 +47,6 @@ enum Command {
     Repl {},
 }
 
-#[derive(Default)]
-struct CliHostHooks {
-    promise_job_queue: RefCell<VecDeque<Job>>,
-}
-
-// RefCell doesn't implement Debug
-impl Debug for CliHostHooks {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.debug_struct("CliHostHooks")
-            //.field("promise_job_queue", &*self.promise_job_queue.borrow())
-            .finish()
-    }
-}
-
-impl CliHostHooks {
-    fn pop_promise_job(&self) -> Option<Job> {
-        self.promise_job_queue.borrow_mut().pop_front()
-    }
-}
-
-impl HostHooks for CliHostHooks {
-    fn enqueue_promise_job(&self, job: Job) {
-        self.promise_job_queue.borrow_mut().push_back(job);
-    }
-}
-
 fn main() -> Result<(), Box<dyn std::error::Error>> {
     let args = Cli::parse();
 
@@ -112,27 +77,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
             no_strict,
             paths,
         } => {
-            let allocator = Default::default();
-
-            let host_hooks: &CliHostHooks = &*Box::leak(Box::default());
-            let mut agent = Agent::new(
-                Options {
-                    disable_gc: false,
-                    print_internals: verbose,
-                },
-                host_hooks,
-            );
-            {
-                let create_global_object: Option<fn(&mut Realm) -> Object> = None;
-                let create_global_this_value: Option<fn(&mut Realm) -> Object> = None;
-                initialize_host_defined_realm(
-                    &mut agent,
-                    create_global_object,
-                    create_global_this_value,
-                    Some(initialize_global_object),
-                );
-            }
-            let realm = agent.current_realm_id();
+            let mut cli_runner = CliRunner::new(verbose);
 
             // `final_result` will always be overwritten in the paths loop, but
             // we populate it with a dummy value here so rustc won't complain.
@@ -141,25 +86,12 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
             assert!(!paths.is_empty());
             for path in paths {
                 let file = std::fs::read_to_string(&path)?;
-                let script = match parse_script(&allocator, file.into(), realm, !no_strict, None) {
-                    Ok(script) => script,
-                    Err((file, errors)) => exit_with_parse_errors(errors, &path, &file),
-                };
-                final_result = script_evaluation(&mut agent, script);
+                final_result = cli_runner.run_script_and_microtasks(file.into(), &path, no_strict);
                 if final_result.is_err() {
                     break;
                 }
             }
 
-            if final_result.is_ok() {
-                while let Some(job) = host_hooks.pop_promise_job() {
-                    if let Err(err) = job.run(&mut agent) {
-                        final_result = Err(err);
-                        break;
-                    }
-                }
-            }
-
             match final_result {
                 Ok(result) => {
                     if verbose {
@@ -169,33 +101,18 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
                 Err(error) => {
                     eprintln!(
                         "Uncaught exception: {}",
-                        error.value().string_repr(&mut agent).as_str(&agent)
+                        error
+                            .value()
+                            .string_repr(cli_runner.agent())
+                            .as_str(cli_runner.agent())
                     );
                     std::process::exit(1);
                 }
             }
+            std::process::exit(0);
         }
         Command::Repl {} => {
-            let allocator = Default::default();
-            let host_hooks: &CliHostHooks = &*Box::leak(Box::default());
-            let mut agent = Agent::new(
-                Options {
-                    disable_gc: false,
-                    print_internals: true,
-                },
-                host_hooks,
-            );
-            {
-                let create_global_object: Option<fn(&mut Realm) -> Object> = None;
-                let create_global_this_value: Option<fn(&mut Realm) -> Object> = None;
-                initialize_host_defined_realm(
-                    &mut agent,
-                    create_global_object,
-                    create_global_this_value,
-                    Some(initialize_global_object),
-                );
-            }
-            let realm = agent.current_realm_id();
+            let mut cli_runner = CliRunner::new(false);
 
             set_theme(DefaultTheme);
             println!("\n\n");
@@ -209,21 +126,18 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
                     std::process::exit(0);
                 }
                 placeholder = input.to_string();
-                let script = match parse_script(&allocator, input.into(), realm, true, None) {
-                    Ok(script) => script,
-                    Err((file, errors)) => {
-                        exit_with_parse_errors(errors, "<stdin>", &file);
-                    }
-                };
-                let result = script_evaluation(&mut agent, script);
-                match result {
+
+                match cli_runner.run_script_and_microtasks(input.into(), "<stdin>", false) {
                     Ok(result) => {
                         println!("{:?}\n", result);
                     }
                     Err(error) => {
                         eprintln!(
                             "Uncaught exception: {}",
-                            error.value().string_repr(&mut agent).as_str(&agent)
+                            error
+                                .value()
+                                .string_repr(cli_runner.agent())
+                                .as_str(cli_runner.agent())
                         );
                     }
                 }
diff --git a/nova_vm/src/ecmascript/execution.rs b/nova_vm/src/ecmascript/execution.rs
@@ -17,8 +17,8 @@ pub(crate) use environments::{
     PrivateEnvironmentIndex, ThisBindingStatus,
 };
 pub(crate) use execution_context::*;
+pub(crate) use realm::ProtoIntrinsics;
 pub use realm::{
     create_realm, initialize_default_realm, initialize_host_defined_realm, set_realm_global_object,
-    Realm,
+    Realm, RealmIdentifier,
 };
-pub(crate) use realm::{ProtoIntrinsics, RealmIdentifier};
