From 0b78a61f084bc60648589c79f202cc63c792066e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 12 Sep 2023 13:14:45 +0200 Subject: [PATCH] refactor: rewrite cli/ ops to op2 (#20462) --- cli/build.rs | 19 +++++++++++-------- cli/lsp/tsc.rs | 30 +++++++++++++++++------------- cli/ops/bench.rs | 20 ++++++++++++-------- cli/ops/mod.rs | 5 +++-- cli/ops/testing.rs | 25 ++++++++++++++----------- cli/tsc/mod.rs | 21 +++++++++++++-------- 6 files changed, 70 insertions(+), 50 deletions(-) diff --git a/cli/build.rs b/cli/build.rs index dd8f780203..c8fca543ea 100644 --- a/cli/build.rs +++ b/cli/build.rs @@ -12,7 +12,7 @@ mod ts { use super::*; use deno_core::error::custom_error; use deno_core::error::AnyError; - use deno_core::op; + use deno_core::op2; use deno_core::OpState; use deno_runtime::deno_node::SUPPORTED_BUILTIN_NODE_MODULES; use serde::Deserialize; @@ -35,8 +35,9 @@ mod ts { node_built_in_module_names: Vec, } - #[op] - fn op_build_info<'s>(state: &mut OpState) -> BuildInfoResponse { + #[op2] + #[serde] + fn op_build_info(state: &mut OpState) -> BuildInfoResponse { let build_specifier = "asset:///bootstrap.ts".to_string(); let build_libs = state .borrow::>() @@ -54,7 +55,7 @@ mod ts { } } - #[op] + #[op2(fast)] fn op_is_node_file() -> bool { false } @@ -65,10 +66,11 @@ mod ts { specifier: String, } - #[op] + #[op2] + #[string] fn op_script_version( _state: &mut OpState, - _args: ScriptVersionArgs, + #[serde] _args: ScriptVersionArgs, ) -> Result, AnyError> { Ok(Some("1".to_string())) } @@ -81,12 +83,13 @@ mod ts { script_kind: i32, } - #[op] + #[op2] + #[serde] // using the same op that is used in `tsc.rs` for loading modules and reading // files, but a slightly different implementation at build time. fn op_load( state: &mut OpState, - args: LoadArgs, + #[serde] args: LoadArgs, ) -> Result { let op_crate_libs = state.borrow::>(); let path_dts = state.borrow::(); diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 425856d52e..278a1b5d3b 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -36,7 +36,7 @@ use deno_core::anyhow::anyhow; use deno_core::error::custom_error; use deno_core::error::AnyError; use deno_core::located_script_name; -use deno_core::op; +use deno_core::op2; use deno_core::parking_lot::Mutex; use deno_core::resolve_url; use deno_core::serde::de; @@ -3232,14 +3232,14 @@ struct SpecifierArgs { specifier: String, } -#[op] +#[op2(fast)] fn op_is_cancelled(state: &mut OpState) -> bool { let state = state.borrow_mut::(); state.token.is_cancelled() } -#[op] -fn op_is_node_file(state: &mut OpState, path: String) -> bool { +#[op2(fast)] +fn op_is_node_file(state: &mut OpState, #[string] path: String) -> bool { let state = state.borrow::(); match ModuleSpecifier::parse(&path) { Ok(specifier) => state @@ -3260,10 +3260,11 @@ struct LoadResponse { version: Option, } -#[op] +#[op2] +#[serde] fn op_load( state: &mut OpState, - args: SpecifierArgs, + #[serde] args: SpecifierArgs, ) -> Result, AnyError> { let state = state.borrow_mut::(); let mark = state.performance.mark("op_load", Some(&args)); @@ -3277,10 +3278,11 @@ fn op_load( })) } -#[op] +#[op2] +#[serde] fn op_resolve( state: &mut OpState, - args: ResolveArgs, + #[serde] args: ResolveArgs, ) -> Result>, AnyError> { let state = state.borrow_mut::(); let mark = state.performance.mark("op_resolve", Some(&args)); @@ -3314,13 +3316,14 @@ fn op_resolve( result } -#[op] -fn op_respond(state: &mut OpState, args: Response) { +#[op2] +fn op_respond(state: &mut OpState, #[serde] args: Response) { let state = state.borrow_mut::(); state.response = Some(args); } -#[op] +#[op2] +#[serde] fn op_script_names(state: &mut OpState) -> Vec { let state = state.borrow_mut::(); let documents = &state.state_snapshot.documents; @@ -3371,10 +3374,11 @@ struct ScriptVersionArgs { specifier: String, } -#[op] +#[op2] +#[string] fn op_script_version( state: &mut OpState, - args: ScriptVersionArgs, + #[serde] args: ScriptVersionArgs, ) -> Result, AnyError> { let state = state.borrow_mut::(); // this op is very "noisy" and measuring its performance is not useful, so we diff --git a/cli/ops/bench.rs b/cli/ops/bench.rs index 0f75097510..7a1572a838 100644 --- a/cli/ops/bench.rs +++ b/cli/ops/bench.rs @@ -7,6 +7,7 @@ use std::time; use deno_core::error::generic_error; use deno_core::error::AnyError; use deno_core::op; +use deno_core::op2; use deno_core::serde_v8; use deno_core::v8; use deno_core::ModuleSpecifier; @@ -47,10 +48,11 @@ deno_core::extension!(deno_bench, #[derive(Clone)] struct PermissionsHolder(Uuid, PermissionsContainer); -#[op] +#[op2] +#[serde] pub fn op_pledge_test_permissions( state: &mut OpState, - args: ChildPermissionsArg, + #[serde] args: ChildPermissionsArg, ) -> Result { let token = Uuid::new_v4(); let parent_permissions = state.borrow_mut::(); @@ -73,10 +75,10 @@ pub fn op_pledge_test_permissions( Ok(token) } -#[op] +#[op2] pub fn op_restore_test_permissions( state: &mut OpState, - token: Uuid, + #[serde] token: Uuid, ) -> Result<(), AnyError> { if let Some(permissions_holder) = state.try_take::() { if token != permissions_holder.0 { @@ -114,11 +116,12 @@ struct BenchRegisterResult { static NEXT_ID: AtomicUsize = AtomicUsize::new(0); -#[op(v8)] +#[op2] +#[serde] fn op_register_bench<'a>( scope: &mut v8::HandleScope<'a>, state: &mut OpState, - info: BenchInfo<'a>, + #[serde] info: BenchInfo<'a>, ) -> Result { let id = NEXT_ID.fetch_add(1, Ordering::SeqCst); let origin = state.borrow::().to_string(); @@ -143,8 +146,8 @@ fn op_register_bench<'a>( Ok(BenchRegisterResult { id, origin }) } -#[op] -fn op_dispatch_bench_event(state: &mut OpState, event: BenchEvent) { +#[op2] +fn op_dispatch_bench_event(state: &mut OpState, #[serde] event: BenchEvent) { assert!( matches!(event, BenchEvent::Output(_)), "Only output events are expected from JS." @@ -153,6 +156,7 @@ fn op_dispatch_bench_event(state: &mut OpState, event: BenchEvent) { sender.send(event).ok(); } +// TODO(bartlomieju): op2 forces to use bigint, but JS doesn't expect a bigint #[op] fn op_bench_now(state: &mut OpState) -> Result { let ns = state.borrow::().elapsed().as_nanos(); diff --git a/cli/ops/mod.rs b/cli/ops/mod.rs index 7af5f14af5..d4d8a84ba2 100644 --- a/cli/ops/mod.rs +++ b/cli/ops/mod.rs @@ -4,7 +4,7 @@ use std::sync::Arc; use crate::npm::CliNpmResolver; use deno_core::error::AnyError; -use deno_core::op; +use deno_core::op2; use deno_core::Extension; use deno_core::OpState; @@ -46,7 +46,8 @@ deno_core::extension!(cli, }, ); -#[op] +#[op2] +#[string] fn op_npm_process_state(state: &mut OpState) -> Result { let npm_resolver = state.borrow_mut::>(); Ok(npm_resolver.get_npm_process_state()) diff --git a/cli/ops/testing.rs b/cli/ops/testing.rs index b4d9b451a0..2f5f04e8a3 100644 --- a/cli/ops/testing.rs +++ b/cli/ops/testing.rs @@ -8,7 +8,7 @@ use crate::tools::test::TestStepDescription; use deno_core::error::generic_error; use deno_core::error::AnyError; -use deno_core::op; +use deno_core::op2; use deno_core::serde_v8; use deno_core::v8; use deno_core::ModuleSpecifier; @@ -48,10 +48,11 @@ deno_core::extension!(deno_test, #[derive(Clone)] struct PermissionsHolder(Uuid, PermissionsContainer); -#[op] +#[op2] +#[serde] pub fn op_pledge_test_permissions( state: &mut OpState, - args: ChildPermissionsArg, + #[serde] args: ChildPermissionsArg, ) -> Result { let token = Uuid::new_v4(); let parent_permissions = state.borrow_mut::(); @@ -73,10 +74,10 @@ pub fn op_pledge_test_permissions( Ok(token) } -#[op] +#[op2] pub fn op_restore_test_permissions( state: &mut OpState, - token: Uuid, + #[serde] token: Uuid, ) -> Result<(), AnyError> { if let Some(permissions_holder) = state.try_take::() { if token != permissions_holder.0 { @@ -113,11 +114,12 @@ struct TestRegisterResult { static NEXT_ID: AtomicUsize = AtomicUsize::new(0); -#[op(v8)] +#[op2] +#[serde] fn op_register_test<'a>( scope: &mut v8::HandleScope<'a>, state: &mut OpState, - info: TestInfo<'a>, + #[serde] info: TestInfo<'a>, ) -> Result { let id = NEXT_ID.fetch_add(1, Ordering::SeqCst); let origin = state.borrow::().to_string(); @@ -164,10 +166,11 @@ struct TestStepInfo { root_name: String, } -#[op] +#[op2] +#[serde] fn op_register_test_step( state: &mut OpState, - info: TestStepInfo, + #[serde] info: TestStepInfo, ) -> Result { let id = NEXT_ID.fetch_add(1, Ordering::SeqCst); let origin = state.borrow::().to_string(); @@ -186,10 +189,10 @@ fn op_register_test_step( Ok(TestRegisterResult { id, origin }) } -#[op] +#[op2] fn op_dispatch_test_event( state: &mut OpState, - event: TestEvent, + #[serde] event: TestEvent, ) -> Result<(), AnyError> { assert!( matches!(event, TestEvent::StepWait(_) | TestEvent::StepResult(..)), diff --git a/cli/tsc/mod.rs b/cli/tsc/mod.rs index af4c67c0d6..8ab35bc79d 100644 --- a/cli/tsc/mod.rs +++ b/cli/tsc/mod.rs @@ -14,6 +14,7 @@ use deno_core::ascii_str; use deno_core::error::AnyError; use deno_core::located_script_name; use deno_core::op; +use deno_core::op2; use deno_core::resolve_url_or_path; use deno_core::serde::Deserialize; use deno_core::serde::Deserializer; @@ -377,8 +378,9 @@ fn normalize_specifier( resolve_url_or_path(specifier, current_dir).map_err(|err| err.into()) } -#[op] -fn op_create_hash(s: &mut OpState, text: &str) -> String { +#[op2] +#[string] +fn op_create_hash(s: &mut OpState, #[string] text: &str) -> String { let state = s.borrow_mut::(); get_hash(text, state.hash_data) } @@ -393,8 +395,8 @@ struct EmitArgs { file_name: String, } -#[op] -fn op_emit(state: &mut OpState, args: EmitArgs) -> bool { +#[op2] +fn op_emit(state: &mut OpState, #[serde] args: EmitArgs) -> bool { let state = state.borrow_mut::(); match args.file_name.as_ref() { "internal:///.tsbuildinfo" => state.maybe_tsbuildinfo = Some(args.data), @@ -435,6 +437,7 @@ pub fn as_ts_script_kind(media_type: MediaType) -> i32 { } } +// TODO(bartlomieju): `op2` doesn't support `serde_json::Value` #[op] fn op_load(state: &mut OpState, args: Value) -> Result { let state = state.borrow_mut::(); @@ -528,10 +531,11 @@ pub struct ResolveArgs { pub specifiers: Vec, } -#[op] +#[op2] +#[serde] fn op_resolve( state: &mut OpState, - args: ResolveArgs, + #[serde] args: ResolveArgs, ) -> Result, AnyError> { let state = state.borrow_mut::(); let mut resolved: Vec<(String, String)> = @@ -713,8 +717,8 @@ fn resolve_non_graph_specifier_types( } } -#[op] -fn op_is_node_file(state: &mut OpState, path: &str) -> bool { +#[op2(fast)] +fn op_is_node_file(state: &mut OpState, #[string] path: &str) -> bool { let state = state.borrow::(); match ModuleSpecifier::parse(path) { Ok(specifier) => state @@ -732,6 +736,7 @@ struct RespondArgs { pub stats: Stats, } +// TODO(bartlomieju): `op2` doesn't support `serde_json::Value` #[op] fn op_respond(state: &mut OpState, args: RespondArgs) { let state = state.borrow_mut::();