From fd85f840cd707c31d08fa836562127e249c9ff62 Mon Sep 17 00:00:00 2001 From: Yiyu Lin Date: Sun, 15 Jan 2023 12:06:46 +0800 Subject: [PATCH] refactor: clean up `unwrap` and `clone` (#17282) Co-authored-by: Divy Srivastava --- cli/args/config_file.rs | 2 +- cli/args/flags.rs | 74 ++++++++++++---------------- cli/build.rs | 2 +- cli/deno_std.rs | 2 +- cli/http_util.rs | 2 +- cli/npm/resolution/mod.rs | 4 +- cli/proc_state.rs | 8 +-- cli/worker.rs | 23 +++------ core/lib.rs | 5 +- core/module_specifier.rs | 2 +- ext/broadcast_channel/lib.rs | 7 +-- ext/cache/lib.rs | 15 +++--- ext/cache/sqlite.rs | 14 +++--- runtime/ops/web_worker/sync_fetch.rs | 3 +- runtime/permissions/mod.rs | 6 +-- 15 files changed, 75 insertions(+), 94 deletions(-) diff --git a/cli/args/config_file.rs b/cli/args/config_file.rs index 47f8e9daa4..199f84709c 100644 --- a/cli/args/config_file.rs +++ b/cli/args/config_file.rs @@ -213,7 +213,7 @@ impl TsConfig { } pub fn as_bytes(&self) -> Vec { - let map = self.0.as_object().unwrap(); + let map = self.0.as_object().expect("invalid tsconfig"); let ordered: BTreeMap<_, _> = map.iter().collect(); let value = json!(ordered); value.to_string().as_bytes().to_owned() diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 84c1d083ba..ef1aedce4c 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -465,30 +465,30 @@ impl Flags { /// If it returns None, the config file shouldn't be discovered at all. pub fn config_path_args(&self) -> Option> { use DenoSubcommand::*; - if let Fmt(FmtFlags { files, .. }) = &self.subcommand { - Some(files.include.clone()) - } else if let Lint(LintFlags { files, .. }) = &self.subcommand { - Some(files.include.clone()) - } else if let Run(RunFlags { script }) = &self.subcommand { - if let Ok(module_specifier) = deno_core::resolve_url_or_path(script) { - if module_specifier.scheme() == "file" - || module_specifier.scheme() == "npm" - { - if let Ok(p) = module_specifier.to_file_path() { - Some(vec![p]) + + match &self.subcommand { + Fmt(FmtFlags { files, .. }) => Some(files.include.clone()), + Lint(LintFlags { files, .. }) => Some(files.include.clone()), + Run(RunFlags { script }) => { + if let Ok(module_specifier) = deno_core::resolve_url_or_path(script) { + if module_specifier.scheme() == "file" + || module_specifier.scheme() == "npm" + { + if let Ok(p) = module_specifier.to_file_path() { + Some(vec![p]) + } else { + Some(vec![]) + } } else { - Some(vec![]) + // When the entrypoint doesn't have file: scheme (it's the remote + // script), then we don't auto discover config file. + None } } else { - // When the entrypoint doesn't have file: scheme (it's the remote - // script), then we don't auto discover config file. - None + Some(vec![]) } - } else { - Some(vec![]) } - } else { - Some(vec![]) + _ => Some(vec![]), } } @@ -583,15 +583,15 @@ pub fn flags_from_vec(args: Vec) -> clap::Result { if matches.is_present("unstable") { flags.unstable = true; } - if matches.is_present("log-level") { - flags.log_level = match matches.value_of("log-level").unwrap() { - "debug" => Some(Level::Debug), - "info" => Some(Level::Info), - _ => unreachable!(), - }; - } + if matches.is_present("quiet") { flags.log_level = Some(Level::Error); + } else { + match matches.value_of("log-level") { + Some("debug") => flags.log_level = Some(Level::Debug), + Some("info") => flags.log_level = Some(Level::Info), + _ => {} + } } match matches.subcommand() { @@ -2556,11 +2556,9 @@ fn fmt_parse(flags: &mut Flags, matches: &clap::ArgMatches) { } else { None }; - let prose_wrap = if matches.is_present("options-prose-wrap") { - Some(matches.value_of("options-prose-wrap").unwrap().to_string()) - } else { - None - }; + let prose_wrap = matches + .value_of("options-prose-wrap") + .map(ToString::to_string); flags.subcommand = DenoSubcommand::Fmt(FmtFlags { check: matches.is_present("check"), @@ -2597,12 +2595,7 @@ fn info_parse(flags: &mut Flags, matches: &clap::ArgMatches) { fn install_parse(flags: &mut Flags, matches: &clap::ArgMatches) { runtime_args_parse(flags, matches, true, true); - let root = if matches.is_present("root") { - let install_root = matches.value_of("root").unwrap(); - Some(PathBuf::from(install_root)) - } else { - None - }; + let root = matches.value_of("root").map(PathBuf::from); let force = matches.is_present("force"); let name = matches.value_of("name").map(|s| s.to_string()); @@ -2625,12 +2618,7 @@ fn install_parse(flags: &mut Flags, matches: &clap::ArgMatches) { } fn uninstall_parse(flags: &mut Flags, matches: &clap::ArgMatches) { - let root = if matches.is_present("root") { - let install_root = matches.value_of("root").unwrap(); - Some(PathBuf::from(install_root)) - } else { - None - }; + let root = matches.value_of("root").map(PathBuf::from); let name = matches.value_of("name").unwrap().to_string(); flags.subcommand = DenoSubcommand::Uninstall(UninstallFlags { name, root }); diff --git a/cli/build.rs b/cli/build.rs index 4d82630e55..03aa47c517 100644 --- a/cli/build.rs +++ b/cli/build.rs @@ -219,7 +219,7 @@ mod ts { // if it comes from an op crate, we were supplied with the path to the // file. let path = if let Some(op_crate_lib) = op_crate_libs.get(lib) { - PathBuf::from(op_crate_lib).canonicalize().unwrap() + PathBuf::from(op_crate_lib).canonicalize()? // otherwise we are will generate the path ourself } else { path_dts.join(format!("lib.{}.d.ts", lib)) diff --git a/cli/deno_std.rs b/cli/deno_std.rs index a8bee6a01d..1c99eda08c 100644 --- a/cli/deno_std.rs +++ b/cli/deno_std.rs @@ -8,4 +8,4 @@ use once_cell::sync::Lazy; static CURRENT_STD_URL_STR: &str = "https://deno.land/std@0.172.0/"; pub static CURRENT_STD_URL: Lazy = - Lazy::new(|| Url::parse(CURRENT_STD_URL_STR).unwrap()); + Lazy::new(|| Url::parse(CURRENT_STD_URL_STR).expect("invalid std url")); diff --git a/cli/http_util.rs b/cli/http_util.rs index acec734365..52d0cb6640 100644 --- a/cli/http_util.rs +++ b/cli/http_util.rs @@ -53,7 +53,7 @@ pub fn resolve_redirect_from_response( ) -> Result { debug_assert!(response.status().is_redirection()); if let Some(location) = response.headers().get(LOCATION) { - let location_string = location.to_str().unwrap(); + let location_string = location.to_str()?; log::debug!("Redirecting to {:?}...", &location_string); let new_url = resolve_url_from_location(request_url, location_string); Ok(new_url) diff --git a/cli/npm/resolution/mod.rs b/cli/npm/resolution/mod.rs index bd408e3ec1..18158bd172 100644 --- a/cli/npm/resolution/mod.rs +++ b/cli/npm/resolution/mod.rs @@ -239,7 +239,7 @@ impl NpmResolution { package_reqs: Vec, ) -> Result<(), AnyError> { // only allow one thread in here at a time - let _permit = self.update_semaphore.acquire().await.unwrap(); + let _permit = self.update_semaphore.acquire().await?; let snapshot = self.snapshot.read().clone(); let snapshot = self @@ -255,7 +255,7 @@ impl NpmResolution { package_reqs: HashSet, ) -> Result<(), AnyError> { // only allow one thread in here at a time - let _permit = self.update_semaphore.acquire().await.unwrap(); + let _permit = self.update_semaphore.acquire().await?; let snapshot = self.snapshot.read().clone(); let has_removed_package = !snapshot diff --git a/cli/proc_state.rs b/cli/proc_state.rs index ac06e6c72e..b3c3b90cd1 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -133,7 +133,7 @@ impl ProcState { // Add the extra files listed in the watch flag if let Some(watch_paths) = ps.options.watch_paths() { - files_to_watch_sender.send(watch_paths.clone()).unwrap(); + files_to_watch_sender.send(watch_paths.clone())?; } if let Ok(Some(import_map_path)) = ps @@ -141,7 +141,7 @@ impl ProcState { .resolve_import_map_specifier() .map(|ms| ms.and_then(|ref s| s.to_file_path().ok())) { - files_to_watch_sender.send(vec![import_map_path]).unwrap(); + files_to_watch_sender.send(vec![import_map_path])?; } Ok(ps) @@ -625,9 +625,9 @@ impl ProcState { // but sadly that's not the case due to missing APIs in V8. let is_repl = matches!(self.options.sub_command(), DenoSubcommand::Repl(_)); let referrer = if referrer.is_empty() && is_repl { - deno_core::resolve_url_or_path("./$deno$repl.ts").unwrap() + deno_core::resolve_url_or_path("./$deno$repl.ts")? } else { - deno_core::resolve_url_or_path(referrer).unwrap() + deno_core::resolve_url_or_path(referrer)? }; // FIXME(bartlomieju): this is another hack way to provide NPM specifier diff --git a/cli/worker.rs b/cli/worker.rs index 7e06166505..e34c65d664 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -184,14 +184,10 @@ impl CliMainWorker { // Enable op call tracing in core to enable better debugging of op sanitizer // failures. if self.ps.options.trace_ops() { - self - .worker - .js_runtime - .execute_script( - &located_script_name!(), - "Deno.core.enableOpCallTracing();", - ) - .unwrap(); + self.worker.js_runtime.execute_script( + &located_script_name!(), + "Deno.core.enableOpCallTracing();", + )?; } let mut maybe_coverage_collector = @@ -233,13 +229,10 @@ impl CliMainWorker { ) -> Result<(), AnyError> { self.enable_test(); - self - .worker - .execute_script( - &located_script_name!(), - "Deno.core.enableOpCallTracing();", - ) - .unwrap(); + self.worker.execute_script( + &located_script_name!(), + "Deno.core.enableOpCallTracing();", + )?; if mode != TestMode::Documentation { // We execute the module module as a side module so that import.meta.main is not set. diff --git a/core/lib.rs b/core/lib.rs index 308724fdd0..2333ff75f0 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -37,6 +37,8 @@ pub use sourcemap; pub use url; pub use v8; +pub use deno_ops::op; + pub use crate::async_cancel::CancelFuture; pub use crate::async_cancel::CancelHandle; pub use crate::async_cancel::CancelTryFuture; @@ -110,13 +112,12 @@ pub use crate::runtime::Snapshot; pub use crate::runtime::V8_WRAPPER_OBJECT_INDEX; pub use crate::runtime::V8_WRAPPER_TYPE_INDEX; pub use crate::source_map::SourceMapGetter; -pub use deno_ops::op; pub fn v8_version() -> &'static str { v8::V8::get_version() } -/// An internal module re-exporting funcs used by the #[op] (`deno_ops`) macro +/// An internal module re-exporting functions used by the #[op] (`deno_ops`) macro #[doc(hidden)] pub mod _ops { pub use super::bindings::throw_type_error; diff --git a/core/module_specifier.rs b/core/module_specifier.rs index 1d7abd1ca0..832208758d 100644 --- a/core/module_specifier.rs +++ b/core/module_specifier.rs @@ -142,7 +142,7 @@ pub fn resolve_path( .map_err(|_| ModuleResolutionError::InvalidPath(path_str.into()))? .join(path_str); let path = normalize_path(path); - Url::from_file_path(path.clone()) + Url::from_file_path(&path) .map_err(|()| ModuleResolutionError::InvalidPath(path)) } diff --git a/ext/broadcast_channel/lib.rs b/ext/broadcast_channel/lib.rs index 0bf359e6bf..de9bef8813 100644 --- a/ext/broadcast_channel/lib.rs +++ b/ext/broadcast_channel/lib.rs @@ -5,6 +5,10 @@ mod in_memory_broadcast_channel; pub use in_memory_broadcast_channel::InMemoryBroadcastChannel; pub use in_memory_broadcast_channel::InMemoryBroadcastChannelResource; +use std::cell::RefCell; +use std::path::PathBuf; +use std::rc::Rc; + use async_trait::async_trait; use deno_core::error::AnyError; use deno_core::include_js_files; @@ -14,9 +18,6 @@ use deno_core::OpState; use deno_core::Resource; use deno_core::ResourceId; use deno_core::ZeroCopyBuf; -use std::cell::RefCell; -use std::path::PathBuf; -use std::rc::Rc; #[async_trait] pub trait BroadcastChannel: Clone { diff --git a/ext/cache/lib.rs b/ext/cache/lib.rs index c48b7cda41..8aab33268a 100644 --- a/ext/cache/lib.rs +++ b/ext/cache/lib.rs @@ -1,8 +1,9 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. -mod sqlite; -use deno_core::ByteString; -pub use sqlite::SqliteBackedCache; +use std::cell::RefCell; +use std::path::PathBuf; +use std::rc::Rc; +use std::sync::Arc; use async_trait::async_trait; use deno_core::error::AnyError; @@ -10,15 +11,13 @@ use deno_core::include_js_files; use deno_core::op; use deno_core::serde::Deserialize; use deno_core::serde::Serialize; +use deno_core::ByteString; use deno_core::Extension; use deno_core::OpState; use deno_core::Resource; use deno_core::ResourceId; - -use std::cell::RefCell; -use std::path::PathBuf; -use std::rc::Rc; -use std::sync::Arc; +mod sqlite; +pub use sqlite::SqliteBackedCache; #[derive(Clone)] pub struct CreateCache(pub Arc C>); diff --git a/ext/cache/sqlite.rs b/ext/cache/sqlite.rs index 2d8e83f4b3..0252934e52 100644 --- a/ext/cache/sqlite.rs +++ b/ext/cache/sqlite.rs @@ -1,5 +1,12 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. +use std::borrow::Cow; +use std::path::PathBuf; +use std::rc::Rc; +use std::sync::Arc; +use std::time::SystemTime; +use std::time::UNIX_EPOCH; + use async_trait::async_trait; use deno_core::error::AnyError; use deno_core::parking_lot::Mutex; @@ -13,13 +20,6 @@ use rusqlite::OptionalExtension; use tokio::io::AsyncReadExt; use tokio::io::AsyncWriteExt; -use std::borrow::Cow; -use std::path::PathBuf; -use std::rc::Rc; -use std::sync::Arc; -use std::time::SystemTime; -use std::time::UNIX_EPOCH; - use crate::deserialize_headers; use crate::get_header; use crate::serialize_headers; diff --git a/runtime/ops/web_worker/sync_fetch.rs b/runtime/ops/web_worker/sync_fetch.rs index 9d356f5ee5..69adec0517 100644 --- a/runtime/ops/web_worker/sync_fetch.rs +++ b/runtime/ops/web_worker/sync_fetch.rs @@ -57,8 +57,7 @@ pub fn op_worker_sync_fetch( let runtime = tokio::runtime::Builder::new_current_thread() .enable_io() .enable_time() - .build() - .unwrap(); + .build()?; let handles: Vec<_> = scripts .into_iter() diff --git a/runtime/permissions/mod.rs b/runtime/permissions/mod.rs index 024aa81d9a..5424a3f36c 100644 --- a/runtime/permissions/mod.rs +++ b/runtime/permissions/mod.rs @@ -427,7 +427,7 @@ impl UnaryPermission { self.prompt, ); if prompted { - let resolved_path = resolve_from_cwd(path).unwrap(); + let resolved_path = resolve_from_cwd(path)?; if result.is_ok() { self.granted_list.insert(ReadDescriptor(resolved_path)); } else { @@ -446,7 +446,7 @@ impl UnaryPermission { display: &str, api_name: &str, ) -> Result<(), AnyError> { - let resolved_path = resolve_from_cwd(path).unwrap(); + let resolved_path = resolve_from_cwd(path)?; let (result, prompted) = self.query(Some(&resolved_path)).check( self.name, Some(api_name), @@ -600,7 +600,7 @@ impl UnaryPermission { self.prompt, ); if prompted { - let resolved_path = resolve_from_cwd(path).unwrap(); + let resolved_path = resolve_from_cwd(path)?; if result.is_ok() { self.granted_list.insert(WriteDescriptor(resolved_path)); } else {