diff --git a/.azure/azure-pipelines.yml b/.azure/azure-pipelines.yml index aa3cb8be6..fb58390aa 100644 --- a/.azure/azure-pipelines.yml +++ b/.azure/azure-pipelines.yml @@ -45,13 +45,13 @@ steps: - bash: RUSTFLAGS="-D warnings" cargo test --all --features=stable condition: eq(variables['style'], 'unflagged') displayName: Run tests - - bash: RUSTFLAGS="-D warnings" cargo clippy --all --features=stable + - bash: RUSTFLAGS="-D warnings" cargo clippy --all --features=stable -- -D clippy::result_unwrap_used -D clippy::option_unwrap_used condition: eq(variables['style'], 'unflagged') displayName: Check clippy lints - bash: NUSHELL_ENABLE_ALL_FLAGS=1 RUSTFLAGS="-D warnings" cargo test --all --features=stable condition: eq(variables['style'], 'canary') displayName: Run tests - - bash: NUSHELL_ENABLE_ALL_FLAGS=1 RUSTFLAGS="-D warnings" cargo clippy --all --features=stable + - bash: NUSHELL_ENABLE_ALL_FLAGS=1 RUSTFLAGS="-D warnings" cargo clippy --all --features=stable -- -D clippy::result_unwrap_used -D clippy::option_unwrap_used condition: eq(variables['style'], 'canary') displayName: Check clippy lints - bash: cargo fmt --all -- --check diff --git a/Cargo.lock b/Cargo.lock index 30d3256bb..d3f6d84c5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1817,6 +1817,15 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ae91b68aebc4ddb91978b11a1b02ddd8602a05ec19002801c5666000e05e0f83" +[[package]] +name = "lock_api" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e57b3997725d2b60dbec1297f6c2e2957cc383db1cebd6be812163f969c7d586" +dependencies = [ + "scopeguard", +] + [[package]] name = "log" version = "0.4.8" @@ -2170,6 +2179,7 @@ dependencies = [ "num-bigint", "num-traits 0.2.10", "onig_sys", + "parking_lot", "pin-utils", "pretty-hex", "pretty_assertions", @@ -2720,6 +2730,30 @@ dependencies = [ "winapi 0.3.8", ] +[[package]] +name = "parking_lot" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "92e98c49ab0b7ce5b222f2cc9193fc4efe11c6d0bd4f648e374684a6857b1cfc" +dependencies = [ + "lock_api", + "parking_lot_core", +] + +[[package]] +name = "parking_lot_core" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7582838484df45743c8434fbff785e8edf260c28748353d44bc0da32e0ceabf1" +dependencies = [ + "cfg-if", + "cloudabi", + "libc", + "redox_syscall", + "smallvec", + "winapi 0.3.8", +] + [[package]] name = "path-slash" version = "0.1.1" diff --git a/Cargo.toml b/Cargo.toml index 7b8229ba8..715b3b8a3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -123,6 +123,7 @@ umask = "0.1" futures-util = "0.3.1" termcolor = "1.0.5" natural = "0.3.0" +parking_lot = "0.10.0" clipboard = {version = "0.5", optional = true } ptree = {version = "0.2" } diff --git a/crates/nu-parser/src/parse/unit.rs b/crates/nu-parser/src/parse/unit.rs index 1d695a73a..9b238eda4 100644 --- a/crates/nu-parser/src/parse/unit.rs +++ b/crates/nu-parser/src/parse/unit.rs @@ -34,8 +34,20 @@ impl PrettyDebug for Unit { fn convert_number_to_u64(number: &Number) -> u64 { match number { - Number::Int(big_int) => big_int.to_u64().unwrap(), - Number::Decimal(big_decimal) => big_decimal.to_u64().unwrap(), + Number::Int(big_int) => { + if let Some(x) = big_int.to_u64() { + x + } else { + unreachable!("Internal error: convert_number_to_u64 given incompatible number") + } + } + Number::Decimal(big_decimal) => { + if let Some(x) = big_decimal.to_u64() { + x + } else { + unreachable!("Internal error: convert_number_to_u64 given incompatible number") + } + } } } diff --git a/crates/nu-protocol/src/type_shape.rs b/crates/nu-protocol/src/type_shape.rs index a40fa825e..aa864416f 100644 --- a/crates/nu-protocol/src/type_shape.rs +++ b/crates/nu-protocol/src/type_shape.rs @@ -235,7 +235,7 @@ impl PrettyDebug for Type { (b::kind("table") + b::space() + b::keyword("of")).group() + b::space() + (if group.len() == 1 { - let (doc, _) = group.into_iter().nth(0).unwrap(); + let (doc, _) = group.into_iter().collect::>()[0].clone(); DebugDocBuilder::from_doc(doc) } else { b::intersperse( diff --git a/crates/nu-protocol/src/value/primitive.rs b/crates/nu-protocol/src/value/primitive.rs index 171bd0059..8311ad13c 100644 --- a/crates/nu-protocol/src/value/primitive.rs +++ b/crates/nu-protocol/src/value/primitive.rs @@ -64,7 +64,11 @@ impl From for Primitive { impl From for Primitive { fn from(float: f64) -> Primitive { - Primitive::Decimal(BigDecimal::from_f64(float).unwrap()) + if let Some(f) = BigDecimal::from_f64(float) { + Primitive::Decimal(f) + } else { + unreachable!("Internal error: protocol did not use f64-compatible decimal") + } } } diff --git a/src/commands/autoview.rs b/src/commands/autoview.rs index 0b728ea45..cfcf61ea6 100644 --- a/src/commands/autoview.rs +++ b/src/commands/autoview.rs @@ -100,13 +100,7 @@ pub fn autoview( let first = &input[0]; let mut host = context.host.clone(); - let mut host = match host.lock() { - Err(err) => { - errln!("Unexpected error acquiring host lock: {:?}", err); - return; - } - Ok(val) => val - }; + let mut host = host.lock(); crate::cli::print_err(first.value.expect_error(), &*host, &context.source); return; diff --git a/src/commands/classified/external.rs b/src/commands/classified/external.rs index 3b88467a5..f14fde52d 100644 --- a/src/commands/classified/external.rs +++ b/src/commands/classified/external.rs @@ -196,7 +196,13 @@ pub(crate) async fn run_external_command( })?; let file = futures::io::AllowStdIo::new(stdout); let stream = Framed::new(file, LinesCodec {}); - let stream = stream.map(move |line| line.unwrap().into_value(&name_tag)); + let stream = stream.map(move |line| { + if let Ok(line) = line { + line.into_value(&name_tag) + } else { + panic!("Internal error: could not read lines of text from stdin") + } + }); Ok(ClassifiedInputStream::from_input_stream( stream.boxed() as BoxStream<'static, Value> )) diff --git a/src/commands/classified/internal.rs b/src/commands/classified/internal.rs index e9ade195e..f2c37653f 100644 --- a/src/commands/classified/internal.rs +++ b/src/commands/classified/internal.rs @@ -124,7 +124,7 @@ pub(crate) async fn run_internal_command( } CommandAction::EnterShell(location) => { context.shell_manager.insert_at_current(Box::new( - FilesystemShell::with_location(location, context.registry().clone()).unwrap(), + FilesystemShell::with_location(location, context.registry().clone()), ))?; } CommandAction::PreviousShell => { diff --git a/src/commands/command.rs b/src/commands/command.rs index 184c3fee5..89d6db2e1 100644 --- a/src/commands/command.rs +++ b/src/commands/command.rs @@ -63,7 +63,7 @@ impl CallInfoExt for CallInfo { #[derive(Getters)] #[get = "pub(crate)"] pub struct CommandArgs { - pub host: Arc>>, + pub host: Arc>>, pub ctrl_c: Arc, pub shell_manager: ShellManager, pub call_info: UnevaluatedCallInfo, @@ -73,7 +73,7 @@ pub struct CommandArgs { #[derive(Getters, Clone)] #[get = "pub(crate)"] pub struct RawCommandArgs { - pub host: Arc>>, + pub host: Arc>>, pub ctrl_c: Arc, pub shell_manager: ShellManager, pub call_info: UnevaluatedCallInfo, @@ -219,7 +219,7 @@ pub struct RunnablePerItemContext { pub struct RunnableContext { pub input: InputStream, pub shell_manager: ShellManager, - pub host: Arc>>, + pub host: Arc>>, pub source: Text, pub ctrl_c: Arc, pub commands: CommandRegistry, @@ -286,7 +286,7 @@ impl Deref for EvaluatedWholeStreamCommandArgs { impl EvaluatedWholeStreamCommandArgs { pub fn new( - host: Arc>, + host: Arc>, ctrl_c: Arc, shell_manager: ShellManager, call_info: CallInfo, @@ -335,7 +335,7 @@ impl Deref for EvaluatedFilterCommandArgs { impl EvaluatedFilterCommandArgs { pub fn new( - host: Arc>, + host: Arc>, ctrl_c: Arc, shell_manager: ShellManager, call_info: CallInfo, @@ -354,7 +354,7 @@ impl EvaluatedFilterCommandArgs { #[derive(Getters, new)] #[get = "pub(crate)"] pub struct EvaluatedCommandArgs { - pub host: Arc>, + pub host: Arc>, pub ctrl_c: Arc, pub shell_manager: ShellManager, pub call_info: CallInfo, @@ -555,7 +555,7 @@ impl WholeStreamCommand for FnFilterCommand { input, } = args; - let host: Arc> = host.clone(); + let host: Arc> = host.clone(); let registry: CommandRegistry = registry.clone(); let func = self.func; diff --git a/src/commands/from_yaml.rs b/src/commands/from_yaml.rs index 915c9fa08..f8f329702 100644 --- a/src/commands/from_yaml.rs +++ b/src/commands/from_yaml.rs @@ -51,31 +51,49 @@ impl WholeStreamCommand for FromYML { } } -fn convert_yaml_value_to_nu_value(v: &serde_yaml::Value, tag: impl Into) -> Value { +fn convert_yaml_value_to_nu_value( + v: &serde_yaml::Value, + tag: impl Into, +) -> Result { let tag = tag.into(); - match v { + Ok(match v { serde_yaml::Value::Bool(b) => UntaggedValue::boolean(*b).into_value(tag), serde_yaml::Value::Number(n) if n.is_i64() => { - UntaggedValue::int(n.as_i64().unwrap()).into_value(tag) + UntaggedValue::int(n.as_i64().ok_or_else(|| { + ShellError::labeled_error( + "Expected a compatible number", + "expected a compatible number", + &tag, + ) + })?) + .into_value(tag) } serde_yaml::Value::Number(n) if n.is_f64() => { - UntaggedValue::decimal(n.as_f64().unwrap()).into_value(tag) + UntaggedValue::decimal(n.as_f64().ok_or_else(|| { + ShellError::labeled_error( + "Expected a compatible number", + "expected a compatible number", + &tag, + ) + })?) + .into_value(tag) } serde_yaml::Value::String(s) => UntaggedValue::string(s).into_value(tag), - serde_yaml::Value::Sequence(a) => UntaggedValue::Table( - a.iter() + serde_yaml::Value::Sequence(a) => { + let result: Result, ShellError> = a + .iter() .map(|x| convert_yaml_value_to_nu_value(x, &tag)) - .collect(), - ) - .into_value(tag), + .collect(); + UntaggedValue::Table(result?).into_value(tag) + } serde_yaml::Value::Mapping(t) => { let mut collected = TaggedDictBuilder::new(&tag); for (k, v) in t.iter() { match k { serde_yaml::Value::String(k) => { - collected.insert_value(k.clone(), convert_yaml_value_to_nu_value(v, &tag)); + collected.insert_value(k.clone(), convert_yaml_value_to_nu_value(v, &tag)?); } _ => unimplemented!("Unknown key type"), } @@ -85,12 +103,19 @@ fn convert_yaml_value_to_nu_value(v: &serde_yaml::Value, tag: impl Into) -> } serde_yaml::Value::Null => UntaggedValue::Primitive(Primitive::Nothing).into_value(tag), x => unimplemented!("Unsupported yaml case: {:?}", x), - } + }) } -pub fn from_yaml_string_to_value(s: String, tag: impl Into) -> serde_yaml::Result { - let v: serde_yaml::Value = serde_yaml::from_str(&s)?; - Ok(convert_yaml_value_to_nu_value(&v, tag)) +pub fn from_yaml_string_to_value(s: String, tag: impl Into) -> Result { + let tag = tag.into(); + let v: serde_yaml::Value = serde_yaml::from_str(&s).map_err(|x| { + ShellError::labeled_error( + format!("Could not load yaml: {}", x), + "could not load yaml from text", + &tag, + ) + })?; + Ok(convert_yaml_value_to_nu_value(&v, tag)?) } fn from_yaml(args: CommandArgs, registry: &CommandRegistry) -> Result { diff --git a/src/commands/table.rs b/src/commands/table.rs index 854967e24..01f1533f0 100644 --- a/src/commands/table.rs +++ b/src/commands/table.rs @@ -53,7 +53,7 @@ fn table(args: CommandArgs, registry: &CommandRegistry) -> Result = args.input.into_vec().await; if input.len() > 0 { - let mut host = host.lock().unwrap(); + let mut host = host.lock(); let view = TableView::from_list(&input, start_number); if let Some(view) = view { diff --git a/src/commands/to_bson.rs b/src/commands/to_bson.rs index 3b783b7af..478571e5a 100644 --- a/src/commands/to_bson.rs +++ b/src/commands/to_bson.rs @@ -49,7 +49,15 @@ pub fn value_to_bson_value(v: &Value) -> Result { UntaggedValue::Primitive(Primitive::Date(d)) => Bson::UtcDatetime(*d), UntaggedValue::Primitive(Primitive::EndOfStream) => Bson::Null, UntaggedValue::Primitive(Primitive::BeginningOfStream) => Bson::Null, - UntaggedValue::Primitive(Primitive::Decimal(d)) => Bson::FloatingPoint(d.to_f64().unwrap()), + UntaggedValue::Primitive(Primitive::Decimal(d)) => { + Bson::FloatingPoint(d.to_f64().ok_or_else(|| { + ShellError::labeled_error( + "Could not convert value to decimal", + "could not convert to decimal", + &v.tag, + ) + })?) + } UntaggedValue::Primitive(Primitive::Int(i)) => { Bson::I64(i.tagged(&v.tag).coerce_into("converting to BSON")?) } @@ -145,9 +153,9 @@ fn object_value_to_bson(o: &Dictionary) -> Result { let bst = get_binary_subtype(tagged_binary_subtype_value); let bin: Result, _> = tagged_bin_value.try_into(); - match bst { - Err(_) => generic_object_value_to_bson(o), - Ok(v) => Ok(Bson::Binary(v, bin.unwrap())), + match (bin, bst) { + (Ok(bin), Ok(v)) => Ok(Bson::Binary(v, bin)), + _ => generic_object_value_to_bson(o), } } _ => generic_object_value_to_bson(o), diff --git a/src/commands/to_json.rs b/src/commands/to_json.rs index e82455160..4293203a3 100644 --- a/src/commands/to_json.rs +++ b/src/commands/to_json.rs @@ -98,8 +98,17 @@ pub fn value_to_json_value(v: &Value) -> Result { UntaggedValue::Primitive(Primitive::Binary(b)) => serde_json::Value::Array( b.iter() .map(|x| { - serde_json::Value::Number(serde_json::Number::from_f64(*x as f64).unwrap()) + serde_json::Number::from_f64(*x as f64).ok_or_else(|| { + ShellError::labeled_error( + "Can not convert number from floating point", + "can not convert to number", + &v.tag, + ) + }) }) + .collect::, ShellError>>()? + .into_iter() + .map(serde_json::Value::Number) .collect(), ), UntaggedValue::Row(o) => { diff --git a/src/commands/to_yaml.rs b/src/commands/to_yaml.rs index f0c1c0da1..19ea75f31 100644 --- a/src/commands/to_yaml.rs +++ b/src/commands/to_yaml.rs @@ -31,16 +31,34 @@ pub fn value_to_yaml_value(v: &Value) -> Result { Ok(match &v.value { UntaggedValue::Primitive(Primitive::Boolean(b)) => serde_yaml::Value::Bool(*b), UntaggedValue::Primitive(Primitive::Bytes(b)) => { - serde_yaml::Value::Number(serde_yaml::Number::from(b.to_f64().unwrap())) - } - UntaggedValue::Primitive(Primitive::Duration(secs)) => { - serde_yaml::Value::Number(serde_yaml::Number::from(secs.to_f64().unwrap())) + serde_yaml::Value::Number(serde_yaml::Number::from(b.to_f64().ok_or_else(|| { + ShellError::labeled_error( + "Could not convert to bytes", + "could not convert to bytes", + &v.tag, + ) + })?)) } + UntaggedValue::Primitive(Primitive::Duration(secs)) => serde_yaml::Value::Number( + serde_yaml::Number::from(secs.to_f64().ok_or_else(|| { + ShellError::labeled_error( + "Could not convert to duration", + "could not convert to duration", + &v.tag, + ) + })?), + ), UntaggedValue::Primitive(Primitive::Date(d)) => serde_yaml::Value::String(d.to_string()), UntaggedValue::Primitive(Primitive::EndOfStream) => serde_yaml::Value::Null, UntaggedValue::Primitive(Primitive::BeginningOfStream) => serde_yaml::Value::Null, UntaggedValue::Primitive(Primitive::Decimal(f)) => { - serde_yaml::Value::Number(serde_yaml::Number::from(f.to_f64().unwrap())) + serde_yaml::Value::Number(serde_yaml::Number::from(f.to_f64().ok_or_else(|| { + ShellError::labeled_error( + "Could not convert to decimal", + "could not convert to decimal", + &v.tag, + ) + })?)) } UntaggedValue::Primitive(Primitive::Int(i)) => { serde_yaml::Value::Number(serde_yaml::Number::from(CoerceInto::::coerce_into( diff --git a/src/context.rs b/src/context.rs index 19ca43039..806c31fd1 100644 --- a/src/context.rs +++ b/src/context.rs @@ -107,7 +107,7 @@ impl CommandRegistry { #[derive(Clone)] pub struct Context { pub registry: CommandRegistry, - pub host: Arc>>, + pub host: Arc>>, pub current_errors: Arc>>, pub ctrl_c: Arc, pub(crate) shell_manager: ShellManager, @@ -133,7 +133,9 @@ impl Context { let registry = CommandRegistry::new(); Ok(Context { registry: registry.clone(), - host: Arc::new(Mutex::new(Box::new(crate::env::host::BasicHost))), + host: Arc::new(parking_lot::Mutex::new(Box::new( + crate::env::host::BasicHost, + ))), current_errors: Arc::new(Mutex::new(vec![])), ctrl_c: Arc::new(AtomicBool::new(false)), shell_manager: ShellManager::basic(registry)?, @@ -161,14 +163,7 @@ impl Context { ); result = false; } - (_, Err(err)) => { - errln!( - "Unexpected error attempting to acquire the lock of the current errors: {:?}", - err - ); - result = false; - } - (Ok(mut errors), Ok(host)) => { + (Ok(mut errors), host) => { if errors.len() > 0 { let error = errors[0].clone(); *errors = vec![]; @@ -188,13 +183,8 @@ impl Context { &mut self, block: impl FnOnce(&mut dyn Host) -> T, ) -> Result { - if let Ok(mut host) = self.host.lock() { - Ok(block(&mut *host)) - } else { - Err(ShellError::untagged_runtime_error( - "Internal error: could not lock host in with_host", - )) - } + let mut host = self.host.lock(); + Ok(block(&mut *host)) } pub(crate) fn with_errors( diff --git a/src/env/host.rs b/src/env/host.rs index b8fd61d26..554ec324b 100644 --- a/src/env/host.rs +++ b/src/env/host.rs @@ -4,8 +4,8 @@ use nu_errors::ShellError; use std::fmt::Debug; pub trait Host: Debug + Send { - fn out_terminal(&self) -> Box; - fn err_terminal(&self) -> Box; + fn out_terminal(&self) -> Option>; + fn err_terminal(&self) -> Option>; fn out_termcolor(&self) -> termcolor::StandardStream; fn err_termcolor(&self) -> termcolor::StandardStream; @@ -17,11 +17,11 @@ pub trait Host: Debug + Send { } impl Host for Box { - fn out_terminal(&self) -> Box { + fn out_terminal(&self) -> Option> { (**self).out_terminal() } - fn err_terminal(&self) -> Box { + fn err_terminal(&self) -> Option> { (**self).err_terminal() } @@ -50,12 +50,12 @@ impl Host for Box { pub struct BasicHost; impl Host for BasicHost { - fn out_terminal(&self) -> Box { - term::stdout().unwrap() + fn out_terminal(&self) -> Option> { + term::stdout() } - fn err_terminal(&self) -> Box { - term::stderr().unwrap() + fn err_terminal(&self) -> Option> { + term::stderr() } fn stdout(&mut self, out: &str) { diff --git a/src/format/table.rs b/src/format/table.rs index 49984d6b9..9e317135c 100644 --- a/src/format/table.rs +++ b/src/format/table.rs @@ -326,13 +326,16 @@ impl RenderView for TableView { let mut table = Table::new(); - let table_mode = crate::data::config::config(Tag::unknown())? - .get("table_mode") - .map(|s| match s.as_string().unwrap().as_ref() { - "light" => TableMode::Light, + let table_mode = crate::data::config::config(Tag::unknown()); + + let table_mode = if let Some(s) = table_mode?.get("table_mode") { + match s.as_string() { + Ok(typ) if typ == "light" => TableMode::Light, _ => TableMode::Normal, - }) - .unwrap_or(TableMode::Normal); + } + } else { + TableMode::Normal + }; match table_mode { TableMode::Light => { @@ -376,7 +379,8 @@ impl RenderView for TableView { )); } - table.print_term(&mut *host.out_terminal()).map_err(|_| ShellError::untagged_runtime_error("Internal error: could not print to terminal (for unix systems check to make sure TERM is set)"))?; + table.print_term(&mut *host.out_terminal().ok_or_else(|| ShellError::untagged_runtime_error("Could not open terminal for output"))?) + .map_err(|_| ShellError::untagged_runtime_error("Internal error: could not print to terminal (for unix systems check to make sure TERM is set)"))?; Ok(()) } diff --git a/src/prelude.rs b/src/prelude.rs index cc5d1c196..2037bbf6d 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -99,7 +99,7 @@ pub(crate) use num_traits::cast::ToPrimitive; pub(crate) use serde::Deserialize; pub(crate) use std::collections::VecDeque; pub(crate) use std::future::Future; -pub(crate) use std::sync::{Arc, Mutex}; +pub(crate) use std::sync::Arc; pub(crate) use itertools::Itertools; diff --git a/src/shell/filesystem_shell.rs b/src/shell/filesystem_shell.rs index ead246d37..5750d7b12 100644 --- a/src/shell/filesystem_shell.rs +++ b/src/shell/filesystem_shell.rs @@ -59,12 +59,9 @@ impl FilesystemShell { }) } - pub fn with_location( - path: String, - commands: CommandRegistry, - ) -> Result { + pub fn with_location(path: String, commands: CommandRegistry) -> FilesystemShell { let last_path = path.clone(); - Ok(FilesystemShell { + FilesystemShell { path, last_path, completer: NuCompleter { @@ -72,7 +69,7 @@ impl FilesystemShell { commands, }, hinter: HistoryHinter {}, - }) + } } }