Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: clean up unwrap and clone #17282

Merged
merged 6 commits into from
Jan 15, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
refactor(cli,core,ext,rt): clean up unwrap and clone
  • Loading branch information
attila-lin committed Jan 6, 2023
commit 672b39cd970482e8f67e999b700012ebadc712cf
2 changes: 1 addition & 1 deletion cli/args/config_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ impl TsConfig {
}

pub fn as_bytes(&self) -> Vec<u8> {
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()
Expand Down
79 changes: 36 additions & 43 deletions cli/args/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,30 +454,30 @@ impl Flags {
/// If it returns None, the config file shouldn't be discovered at all.
pub fn config_path_args(&self) -> Option<Vec<PathBuf>> {
use DenoSubcommand::*;
if let Fmt(FmtFlags { files, .. }) = &self.subcommand {
Some(files.clone())
} else if let Lint(LintFlags { files, .. }) = &self.subcommand {
Some(files.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.clone()),
Lint(LintFlags { files, .. }) => Some(files.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![]),
}
}

Expand Down Expand Up @@ -572,15 +572,14 @@ pub fn flags_from_vec(args: Vec<String>) -> clap::Result<Flags> {
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);
match matches.value_of("log-level") {
Some("debug") => flags.log_level = Some(Level::Debug),
Some("info") => flags.log_level = Some(Level::Info),
_ => {
if matches.is_present("quiet") {
flags.log_level = Some(Level::Error);
}
}
}

match matches.subcommand() {
Expand Down Expand Up @@ -2535,11 +2534,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"),
Expand Down Expand Up @@ -2577,11 +2574,9 @@ 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 = match matches.value_of("root") {
Some(install_root) => Some(PathBuf::from(install_root)),
None => None,
};

let force = matches.is_present("force");
Expand All @@ -2605,11 +2600,9 @@ 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 = match matches.value_of("root") {
Some(install_root) => Some(PathBuf::from(install_root)),
None => None,
};

let name = matches.value_of("name").unwrap().to_string();
Expand Down
2 changes: 1 addition & 1 deletion cli/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,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))
Expand Down
2 changes: 1 addition & 1 deletion cli/deno_std.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ use once_cell::sync::Lazy;
static CURRENT_STD_URL_STR: &str = "https://deno.land/[email protected]/";

pub static CURRENT_STD_URL: Lazy<Url> =
Lazy::new(|| Url::parse(CURRENT_STD_URL_STR).unwrap());
Lazy::new(|| Url::parse(CURRENT_STD_URL_STR).expect("invalid std url"));
2 changes: 1 addition & 1 deletion cli/http_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ pub fn resolve_redirect_from_response(
) -> Result<Url, AnyError> {
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)
Expand Down
4 changes: 2 additions & 2 deletions cli/npm/resolution/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ impl NpmResolution {
package_reqs: Vec<NpmPackageReq>,
) -> 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
Expand All @@ -255,7 +255,7 @@ impl NpmResolution {
package_reqs: HashSet<NpmPackageReq>,
) -> 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
Expand Down
8 changes: 4 additions & 4 deletions cli/proc_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,15 +133,15 @@ 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
.options
.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)
Expand Down Expand Up @@ -596,9 +596,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
Expand Down
23 changes: 8 additions & 15 deletions cli/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,14 +182,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 =
Expand Down Expand Up @@ -231,13 +227,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.
Expand Down
5 changes: 3 additions & 2 deletions core/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -109,13 +111,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;
Expand Down
2 changes: 1 addition & 1 deletion core/module_specifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand Down
6 changes: 4 additions & 2 deletions ext/broadcast_channel/in_memory_broadcast_channel.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.

use crate::BroadcastChannel;
use std::sync::Arc;

use async_trait::async_trait;
use deno_core::error::AnyError;
use deno_core::parking_lot::Mutex;
use std::sync::Arc;
use tokio::sync::broadcast;
use tokio::sync::mpsc;
use uuid::Uuid;

use crate::BroadcastChannel;

#[derive(Clone)]
pub struct InMemoryBroadcastChannel(Arc<Mutex<broadcast::Sender<Message>>>);

Expand Down
7 changes: 4 additions & 3 deletions ext/broadcast_channel/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down
15 changes: 7 additions & 8 deletions ext/cache/lib.rs
Original file line number Diff line number Diff line change
@@ -1,24 +1,23 @@
// 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;
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<C: Cache + 'static>(pub Arc<dyn Fn() -> C>);
Expand Down
14 changes: 7 additions & 7 deletions ext/cache/sqlite.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down
3 changes: 1 addition & 2 deletions runtime/ops/web_worker/sync_fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Loading