Skip to content

Commit

Permalink
refactor: clean up unwrap and clone (denoland#17282)
Browse files Browse the repository at this point in the history
Co-authored-by: Divy Srivastava <[email protected]>
  • Loading branch information
attila-lin and littledivy committed Jan 15, 2023
1 parent 05ef925 commit fd85f84
Show file tree
Hide file tree
Showing 15 changed files with 75 additions and 94 deletions.
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
74 changes: 31 additions & 43 deletions cli/args/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Vec<PathBuf>> {
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![]),
}
}

Expand Down Expand Up @@ -583,15 +583,15 @@ 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);
} 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() {
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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());
Expand All @@ -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 });
Expand Down
2 changes: 1 addition & 1 deletion cli/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
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 @@ -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
Expand Down
23 changes: 8 additions & 15 deletions cli/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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.
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 @@ -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;
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
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
6 changes: 3 additions & 3 deletions runtime/permissions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ impl UnaryPermission<ReadDescriptor> {
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 {
Expand All @@ -446,7 +446,7 @@ impl UnaryPermission<ReadDescriptor> {
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),
Expand Down Expand Up @@ -600,7 +600,7 @@ impl UnaryPermission<WriteDescriptor> {
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 {
Expand Down

0 comments on commit fd85f84

Please sign in to comment.