diff --git a/crates/nu-parser/src/hir/tokens_iterator/debug/expand_trace.rs b/crates/nu-parser/src/hir/tokens_iterator/debug/expand_trace.rs index ea99f4e445..5330d97544 100644 --- a/crates/nu-parser/src/hir/tokens_iterator/debug/expand_trace.rs +++ b/crates/nu-parser/src/hir/tokens_iterator/debug/expand_trace.rs @@ -254,7 +254,7 @@ pub struct ExpandTracer { impl ExpandTracer { pub fn print(&self, source: Text) -> PrintTracer { - let root = self.frame_stack.get(0).unwrap().to_tree_frame(&source); + let root = self.frame_stack[0].to_tree_frame(&source); PrintTracer { root, source } } diff --git a/crates/nu-parser/src/parse/operator.rs b/crates/nu-parser/src/parse/operator.rs index ed1c8b28e9..6270fcb9eb 100644 --- a/crates/nu-parser/src/parse/operator.rs +++ b/crates/nu-parser/src/parse/operator.rs @@ -42,7 +42,11 @@ impl CompareOperator { impl From<&str> for CompareOperator { fn from(input: &str) -> CompareOperator { - CompareOperator::from_str(input).unwrap() + if let Ok(output) = CompareOperator::from_str(input) { + output + } else { + unreachable!("Internal error: CompareOperator from failed") + } } } @@ -90,7 +94,11 @@ impl EvaluationOperator { impl From<&str> for EvaluationOperator { fn from(input: &str) -> EvaluationOperator { - EvaluationOperator::from_str(input).unwrap() + if let Ok(output) = EvaluationOperator::from_str(input) { + output + } else { + unreachable!("Internal error: EvaluationOperator 'from' failed") + } } } diff --git a/crates/nu-parser/src/parse/parser.rs b/crates/nu-parser/src/parse/parser.rs index 6d111ec284..f67d354fd7 100644 --- a/crates/nu-parser/src/parse/parser.rs +++ b/crates/nu-parser/src/parse/parser.rs @@ -148,13 +148,21 @@ macro_rules! primitive_decimal { $( impl From<$ty> for Number { fn from(decimal: $ty) -> Number { - Number::Decimal(BigDecimal::$from(decimal).unwrap()) + if let Some(num) = BigDecimal::$from(decimal) { + Number::Decimal(num) + } else { + unreachable!("Internal error: BigDecimal 'from' failed") + } } } impl From<&$ty> for Number { fn from(decimal: &$ty) -> Number { - Number::Decimal(BigDecimal::$from(*decimal).unwrap()) + if let Some(num) = BigDecimal::$from(*decimal) { + Number::Decimal(num) + } else { + unreachable!("Internal error: BigDecimal 'from' failed") + } } } )* @@ -909,11 +917,13 @@ pub fn module(input: NomSpan) -> IResult { } fn parse_int(frag: &str, neg: Option) -> i64 { - let int = FromStr::from_str(frag).unwrap(); - - match neg { - None => int, - Some(_) => -int, + if let Ok(int) = FromStr::from_str(frag) { + match neg { + None => int, + Some(_) => -int, + } + } else { + unreachable!("Internal error: parse_int failed"); } } diff --git a/crates/nu-parser/src/parse/token_tree_builder.rs b/crates/nu-parser/src/parse/token_tree_builder.rs index c7fd5e5afb..131dbd5c81 100644 --- a/crates/nu-parser/src/parse/token_tree_builder.rs +++ b/crates/nu-parser/src/parse/token_tree_builder.rs @@ -323,10 +323,13 @@ impl TokenTreeBuilder { let mut input = input.into_iter(); - let head = input.next().unwrap(); - let tail = input.collect(); + if let Some(head) = input.next() { + let tail = input.collect(); - CallNode::new(Box::new(head), tail).spanned(span.into()) + CallNode::new(Box::new(head), tail).spanned(span.into()) + } else { + unreachable!("Internal error: spanned_call failed") + } } fn consume_delimiter( diff --git a/crates/nu-parser/src/parse/tokens.rs b/crates/nu-parser/src/parse/tokens.rs index 4cb958c957..0b15694699 100644 --- a/crates/nu-parser/src/parse/tokens.rs +++ b/crates/nu-parser/src/parse/tokens.rs @@ -88,9 +88,19 @@ impl RawNumber { pub(crate) fn to_number(self, source: &Text) -> Number { match self { - RawNumber::Int(tag) => Number::Int(BigInt::from_str(tag.slice(source)).unwrap()), + RawNumber::Int(tag) => { + if let Ok(int) = BigInt::from_str(tag.slice(source)) { + Number::Int(int) + } else { + unreachable!("Internal error: to_number failed") + } + } RawNumber::Decimal(tag) => { - Number::Decimal(BigDecimal::from_str(tag.slice(source)).unwrap()) + if let Ok(decimal) = BigDecimal::from_str(tag.slice(source)) { + Number::Decimal(decimal) + } else { + unreachable!("Internal error: to_number failed") + } } } } diff --git a/crates/nu-plugin/src/test_helpers.rs b/crates/nu-plugin/src/test_helpers.rs index c568dd8657..addc13d54e 100644 --- a/crates/nu-plugin/src/test_helpers.rs +++ b/crates/nu-plugin/src/test_helpers.rs @@ -124,14 +124,14 @@ impl CallStub { self } - pub fn with_parameter(&mut self, name: &str) -> &mut Self { + pub fn with_parameter(&mut self, name: &str) -> Result<&mut Self, ShellError> { let fields: Vec = name .split('.') .map(|s| UntaggedValue::string(s.to_string()).into_value(Tag::unknown())) .collect(); - self.positionals.push(value::column_path(&fields)); - self + self.positionals.push(value::column_path(&fields)?); + Ok(self) } pub fn create(&self) -> CallInfo { @@ -156,7 +156,11 @@ pub fn expect_return_value_at( }; if idx == at { - return item.raw_value().unwrap(); + if let Some(value) = item.raw_value() { + return value; + } else { + panic!("Internal error: could not get raw value in expect_return_value_at") + } } } @@ -168,6 +172,7 @@ pub fn expect_return_value_at( } pub mod value { + use nu_errors::ShellError; use nu_protocol::{Primitive, TaggedDictBuilder, UntaggedValue, Value}; use nu_source::Tag; use nu_value_ext::ValueExt; @@ -199,10 +204,10 @@ pub mod value { UntaggedValue::table(list).into_untagged_value() } - pub fn column_path(paths: &[Value]) -> Value { - UntaggedValue::Primitive(Primitive::ColumnPath( - table(&paths.to_vec()).as_column_path().unwrap().item, + pub fn column_path(paths: &[Value]) -> Result { + Ok(UntaggedValue::Primitive(Primitive::ColumnPath( + table(&paths.to_vec()).as_column_path()?.item, )) - .into_untagged_value() + .into_untagged_value()) } } diff --git a/crates/nu-test-support/src/fs.rs b/crates/nu-test-support/src/fs.rs index dc3c26b336..5297c222cf 100644 --- a/crates/nu-test-support/src/fs.rs +++ b/crates/nu-test-support/src/fs.rs @@ -30,7 +30,11 @@ impl AbsoluteFile { } pub fn dir(&self) -> AbsolutePath { - AbsolutePath::new(self.inner.parent().unwrap()) + AbsolutePath::new(if let Some(parent) = self.inner.parent() { + parent + } else { + unreachable!("Internal error: could not get parent in dir") + }) } } diff --git a/crates/nu_plugin_inc/src/nu_plugin_inc/tests.rs b/crates/nu_plugin_inc/src/nu_plugin_inc/tests.rs index e4c9496623..391e00e73d 100644 --- a/crates/nu_plugin_inc/src/nu_plugin_inc/tests.rs +++ b/crates/nu_plugin_inc/src/nu_plugin_inc/tests.rs @@ -1,6 +1,7 @@ mod integration { use crate::inc::{Action, SemVerAction}; use crate::Inc; + use nu_errors::ShellError; use nu_plugin::test_helpers::value::{column_path, string}; use nu_plugin::test_helpers::{plugin, CallStub}; @@ -52,16 +53,21 @@ mod integration { } #[test] - fn picks_up_argument_for_field() { + fn picks_up_argument_for_field() -> Result<(), ShellError> { plugin(&mut Inc::new()) - .args(CallStub::new().with_parameter("package.version").create()) + .args(CallStub::new().with_parameter("package.version")?.create()) .setup(|plugin, _| { - plugin.expect_field(column_path(&[string("package"), string("version")])) + //FIXME: this will need to be updated + if let Ok(column_path) = column_path(&[string("package"), string("version")]) { + plugin.expect_field(column_path) + } }); + Ok(()) } mod sem_ver { use crate::Inc; + use nu_errors::ShellError; use nu_plugin::test_helpers::value::{get_data, string, structured_sample_record}; use nu_plugin::test_helpers::{expect_return_value_at, plugin, CallStub}; @@ -70,12 +76,12 @@ mod integration { } #[test] - fn major_input_using_the_field_passed_as_parameter() { + fn major_input_using_the_field_passed_as_parameter() -> Result<(), ShellError> { let run = plugin(&mut Inc::new()) .args( CallStub::new() .with_long_flag("major") - .with_parameter("version") + .with_parameter("version")? .create(), ) .input(cargo_sample_record("0.1.3")) @@ -85,15 +91,16 @@ mod integration { let actual = expect_return_value_at(run, 0); assert_eq!(get_data(actual, "version"), string("1.0.0")); + Ok(()) } #[test] - fn minor_input_using_the_field_passed_as_parameter() { + fn minor_input_using_the_field_passed_as_parameter() -> Result<(), ShellError> { let run = plugin(&mut Inc::new()) .args( CallStub::new() .with_long_flag("minor") - .with_parameter("version") + .with_parameter("version")? .create(), ) .input(cargo_sample_record("0.1.3")) @@ -103,15 +110,16 @@ mod integration { let actual = expect_return_value_at(run, 0); assert_eq!(get_data(actual, "version"), string("0.2.0")); + Ok(()) } #[test] - fn patch_input_using_the_field_passed_as_parameter() { + fn patch_input_using_the_field_passed_as_parameter() -> Result<(), ShellError> { let run = plugin(&mut Inc::new()) .args( CallStub::new() .with_long_flag("patch") - .with_parameter("version") + .with_parameter("version")? .create(), ) .input(cargo_sample_record("0.1.3")) @@ -121,6 +129,7 @@ mod integration { let actual = expect_return_value_at(run, 0); assert_eq!(get_data(actual, "version"), string("0.1.4")); + Ok(()) } } } diff --git a/crates/nu_plugin_str/src/nu_plugin_str/tests.rs b/crates/nu_plugin_str/src/nu_plugin_str/tests.rs index 2a51a0f7be..5d33dbb9b4 100644 --- a/crates/nu_plugin_str/src/nu_plugin_str/tests.rs +++ b/crates/nu_plugin_str/src/nu_plugin_str/tests.rs @@ -1,6 +1,7 @@ mod integration { use crate::strutils::{Action, ReplaceAction}; use crate::Str; + use nu_errors::ShellError; use nu_plugin::test_helpers::value::{ column_path, get_data, int, string, structured_sample_record, table, unstructured_sample_record, @@ -83,16 +84,21 @@ mod integration { } #[test] - fn picks_up_argument_for_field() { + fn picks_up_argument_for_field() -> Result<(), ShellError> { plugin(&mut Str::new()) .args( CallStub::new() - .with_parameter("package.description") + .with_parameter("package.description")? .create(), ) .setup(|plugin, _| { - plugin.expect_field(column_path(&[string("package"), string("description")])) + //FIXME: this is possibly not correct + if let Ok(column_path) = column_path(&[string("package"), string("description")]) { + plugin.expect_field(column_path) + } }); + + Ok(()) } #[test] @@ -115,12 +121,12 @@ mod integration { } #[test] - fn upcases_the_input_using_the_field_passed_as_parameter() { + fn upcases_the_input_using_the_field_passed_as_parameter() -> Result<(), ShellError> { let run = plugin(&mut Str::new()) .args( CallStub::new() .with_long_flag("upcase") - .with_parameter("name") + .with_parameter("name")? .create(), ) .input(structured_sample_record("name", "jotandrehuda")) @@ -130,15 +136,16 @@ mod integration { let actual = expect_return_value_at(run, 0); assert_eq!(get_data(actual, "name"), string("JOTANDREHUDA")); + Ok(()) } #[test] - fn downcases_the_input_using_the_field_passed_as_parameter() { + fn downcases_the_input_using_the_field_passed_as_parameter() -> Result<(), ShellError> { let run = plugin(&mut Str::new()) .args( CallStub::new() .with_long_flag("downcase") - .with_parameter("name") + .with_parameter("name")? .create(), ) .input(structured_sample_record("name", "JOTANDREHUDA")) @@ -148,15 +155,17 @@ mod integration { let actual = expect_return_value_at(run, 0); assert_eq!(get_data(actual, "name"), string("jotandrehuda")); + Ok(()) } #[test] - fn converts_the_input_to_integer_using_the_field_passed_as_parameter() { + fn converts_the_input_to_integer_using_the_field_passed_as_parameter() -> Result<(), ShellError> + { let run = plugin(&mut Str::new()) .args( CallStub::new() .with_long_flag("to-int") - .with_parameter("Nu_birthday") + .with_parameter("Nu_birthday")? .create(), ) .input(structured_sample_record("Nu_birthday", "10")) @@ -166,14 +175,15 @@ mod integration { let actual = expect_return_value_at(run, 0); assert_eq!(get_data(actual, "Nu_birthday"), int(10)); + Ok(()) } #[test] - fn replaces_the_input_using_the_field_passed_as_parameter() { + fn replaces_the_input_using_the_field_passed_as_parameter() -> Result<(), ShellError> { let run = plugin(&mut Str::new()) .args( CallStub::new() - .with_parameter("rustconf") + .with_parameter("rustconf")? .with_named_parameter("replace", string("22nd August 2019")) .create(), ) @@ -184,14 +194,15 @@ mod integration { let actual = expect_return_value_at(run, 0); assert_eq!(get_data(actual, "rustconf"), string("22nd August 2019")); + Ok(()) } #[test] - fn find_and_replaces_the_input_using_the_field_passed_as_parameter() { + fn find_and_replaces_the_input_using_the_field_passed_as_parameter() -> Result<(), ShellError> { let run = plugin(&mut Str::new()) .args( CallStub::new() - .with_parameter("staff") + .with_parameter("staff")? .with_named_parameter( "find-replace", table(&[string("kittens"), string("jotandrehuda")]), @@ -205,6 +216,7 @@ mod integration { let actual = expect_return_value_at(run, 0); assert_eq!(get_data(actual, "staff"), string("wyjotandrehuda")); + Ok(()) } #[test] diff --git a/src/cli.rs b/src/cli.rs index 0691fc0f98..db2244b8e8 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -384,7 +384,7 @@ pub async fn cli() -> Result<(), Box> { continue; } - let cwd = context.shell_manager.path(); + let cwd = context.shell_manager.path()?; rl.set_helper(Some(crate::shell::Helper::new(context.clone()))); @@ -452,7 +452,7 @@ pub async fn cli() -> Result<(), Box> { context.with_host(|host| { print_err(err, host, &Text::from(line.clone())); - }); + })?; context.maybe_print_errors(Text::from(line.clone())); } @@ -474,7 +474,7 @@ pub async fn cli() -> Result<(), Box> { let _ = rl.save_history(&History::path()); std::process::exit(0); } else { - context.with_host(|host| host.stdout("CTRL-C pressed (again to quit)")); + context.with_host(|host| host.stdout("CTRL-C pressed (again to quit)"))?; ctrlcbreak = true; continue; } @@ -634,13 +634,13 @@ pub fn classify_pipeline( let result = expand_syntax( &PipelineShape, &mut iterator, - &context.expand_context(source), + &context.expand_context(source)?, ) .map_err(|err| err.into()); if log_enabled!(target: "nu::expand_syntax", log::Level::Debug) { outln!(""); - ptree::print_tree(&iterator.expand_tracer().print(source.clone())).unwrap(); + let _ = ptree::print_tree(&iterator.expand_tracer().print(source.clone())); outln!(""); } diff --git a/src/commands/classified/external.rs b/src/commands/classified/external.rs index 75cde671b3..3b88467a52 100644 --- a/src/commands/classified/external.rs +++ b/src/commands/classified/external.rs @@ -148,7 +148,7 @@ pub(crate) async fn run_external_command( } } - process = process.cwd(context.shell_manager.path()); + process = process.cwd(context.shell_manager.path()?); trace!(target: "nu::run::external", "cwd = {:?}", context.shell_manager.path()); diff --git a/src/commands/classified/internal.rs b/src/commands/classified/internal.rs index 0fe5d6d410..e9ade195ed 100644 --- a/src/commands/classified/internal.rs +++ b/src/commands/classified/internal.rs @@ -46,11 +46,11 @@ pub(crate) async fn run_internal_command( match item { Ok(ReturnSuccess::Action(action)) => match action { CommandAction::ChangePath(path) => { - context.shell_manager.set_path(path); + context.shell_manager.set_path(path)?; } CommandAction::Exit => std::process::exit(0), // TODO: save history.txt CommandAction::Error(err) => { - context.error(err); + context.error(err)?; break; } CommandAction::AutoConvert(tagged_contents, extension) => { @@ -99,39 +99,43 @@ pub(crate) async fn run_internal_command( value: UntaggedValue::Primitive(Primitive::String(cmd)), tag, } => { - context.shell_manager.insert_at_current(Box::new( + let result = context.shell_manager.insert_at_current(Box::new( HelpShell::for_command( UntaggedValue::string(cmd).into_value(tag), &context.registry(), )?, )); + + result? } _ => { - context.shell_manager.insert_at_current(Box::new( - HelpShell::index(&context.registry()).unwrap(), + let result = context.shell_manager.insert_at_current(Box::new( + HelpShell::index(&context.registry())?, )); + + result? } } } CommandAction::EnterValueShell(value) => { context .shell_manager - .insert_at_current(Box::new(ValueShell::new(value))); + .insert_at_current(Box::new(ValueShell::new(value)))?; } CommandAction::EnterShell(location) => { context.shell_manager.insert_at_current(Box::new( FilesystemShell::with_location(location, context.registry().clone()).unwrap(), - )); + ))?; } CommandAction::PreviousShell => { - context.shell_manager.prev(); + context.shell_manager.prev()?; } CommandAction::NextShell => { - context.shell_manager.next(); + context.shell_manager.next()?; } CommandAction::LeaveShell => { - context.shell_manager.remove_at_current(); - if context.shell_manager.is_empty() { + context.shell_manager.remove_at_current()?; + if context.shell_manager.is_empty()? { std::process::exit(0); // TODO: save history.txt } } @@ -149,7 +153,7 @@ pub(crate) async fn run_internal_command( let mut buffer = termcolor::Buffer::ansi(); let _ = doc.render_raw( - context.with_host(|host| host.width() - 5), + context.with_host(|host| host.width() - 5)?, &mut nu_source::TermColored::new(&mut buffer), ); @@ -159,7 +163,7 @@ pub(crate) async fn run_internal_command( } Err(err) => { - context.error(err); + context.error(err)?; break; } } diff --git a/src/commands/clip.rs b/src/commands/clip.rs index a2778c86bf..19f43001a4 100644 --- a/src/commands/clip.rs +++ b/src/commands/clip.rs @@ -55,35 +55,52 @@ pub mod clipboard { } async fn inner_clip(input: Vec, name: Tag) -> OutputStream { - let mut clip_context: ClipboardContext = ClipboardProvider::new().unwrap(); - let mut new_copy_data = String::new(); + if let Ok(clip_context) = ClipboardProvider::new() { + let mut clip_context: ClipboardContext = clip_context; + let mut new_copy_data = String::new(); - if !input.is_empty() { - let mut first = true; - for i in input.iter() { - if !first { - new_copy_data.push_str("\n"); - } else { - first = false; - } - - let string: String = match i.as_string() { - Ok(string) => string.to_string(), - Err(_) => { - return OutputStream::one(Err(ShellError::labeled_error( - "Given non-string data", - "expected strings from pipeline", - name, - ))) + if !input.is_empty() { + let mut first = true; + for i in input.iter() { + if !first { + new_copy_data.push_str("\n"); + } else { + first = false; } - }; - new_copy_data.push_str(&string); + let string: String = match i.as_string() { + Ok(string) => string.to_string(), + Err(_) => { + return OutputStream::one(Err(ShellError::labeled_error( + "Given non-string data", + "expected strings from pipeline", + name, + ))) + } + }; + + new_copy_data.push_str(&string); + } } + + match clip_context.set_contents(new_copy_data) { + Ok(_) => {} + Err(_) => { + return OutputStream::one(Err(ShellError::labeled_error( + "Could not set contents of clipboard", + "could not set contents of clipboard", + name, + ))); + } + } + + OutputStream::empty() + } else { + OutputStream::one(Err(ShellError::labeled_error( + "Could not open clipboard", + "could not open clipboard", + name, + ))) } - - clip_context.set_contents(new_copy_data).unwrap(); - - OutputStream::empty() } } diff --git a/src/commands/enter.rs b/src/commands/enter.rs index 1a0670d25a..80b7075855 100644 --- a/src/commands/enter.rs +++ b/src/commands/enter.rs @@ -72,7 +72,7 @@ impl PerItemCommand for Enter { // If it's a file, attempt to open the file as a value and enter it let cwd = raw_args.shell_manager.path(); - let full_path = std::path::PathBuf::from(cwd); + let full_path = std::path::PathBuf::from(cwd?); let (file_extension, contents, contents_tag) = crate::commands::open::fetch( diff --git a/src/commands/evaluate_by.rs b/src/commands/evaluate_by.rs index 526f1ab34a..b982f98cf4 100644 --- a/src/commands/evaluate_by.rs +++ b/src/commands/evaluate_by.rs @@ -45,7 +45,6 @@ pub fn evaluate_by( let stream = async_stream! { let values: Vec = input.values.collect().await; - if values.is_empty() { yield Err(ShellError::labeled_error( "Expected table from pipeline", @@ -141,6 +140,7 @@ mod tests { use crate::data::value; use crate::prelude::*; use indexmap::IndexMap; + use nu_errors::ShellError; use nu_protocol::{UntaggedValue, Value}; use nu_source::TaggedItem; @@ -160,21 +160,20 @@ mod tests { UntaggedValue::table(list).into_untagged_value() } - fn nu_releases_sorted_by_date() -> Value { + fn nu_releases_sorted_by_date() -> Result { let key = String::from("date"); t_sort( Some(key), None, - &nu_releases_grouped_by_date(), + &nu_releases_grouped_by_date()?, Tag::unknown(), ) - .unwrap() } - fn nu_releases_grouped_by_date() -> Value { + fn nu_releases_grouped_by_date() -> Result { let key = String::from("date").tagged_unknown(); - group(&key, nu_releases_commiters(), Tag::unknown()).unwrap() + group(&key, nu_releases_commiters(), Tag::unknown()) } fn nu_releases_commiters() -> Vec { @@ -230,28 +229,32 @@ mod tests { } #[test] - fn evaluates_the_tables() { + fn evaluates_the_tables() -> Result<(), ShellError> { assert_eq!( - evaluate(&nu_releases_sorted_by_date(), None, Tag::unknown()).unwrap(), + evaluate(&nu_releases_sorted_by_date()?, None, Tag::unknown())?, table(&[table(&[ table(&[int(1), int(1), int(1)]), table(&[int(1), int(1), int(1)]), table(&[int(1), int(1), int(1)]), ]),]) ); + + Ok(()) } #[test] - fn evaluates_the_tables_with_custom_evaluator() { + fn evaluates_the_tables_with_custom_evaluator() -> Result<(), ShellError> { let eval = String::from("name"); assert_eq!( - evaluate(&nu_releases_sorted_by_date(), Some(eval), Tag::unknown()).unwrap(), + evaluate(&nu_releases_sorted_by_date()?, Some(eval), Tag::unknown())?, table(&[table(&[ table(&[string("AR"), string("JT"), string("YK")]), table(&[string("AR"), string("YK"), string("JT")]), table(&[string("YK"), string("JT"), string("AR")]), ]),]) ); + + Ok(()) } } diff --git a/src/commands/open.rs b/src/commands/open.rs index 91a5aa031e..1ac00d8324 100644 --- a/src/commands/open.rs +++ b/src/commands/open.rs @@ -40,7 +40,7 @@ impl PerItemCommand for Open { fn run(call_info: &CallInfo, raw_args: &RawCommandArgs) -> Result { let shell_manager = &raw_args.shell_manager; - let cwd = PathBuf::from(shell_manager.path()); + let cwd = PathBuf::from(shell_manager.path()?); let full_path = cwd; let path = call_info.args.nth(0).ok_or_else(|| { diff --git a/src/commands/save.rs b/src/commands/save.rs index 85b8314c22..144a8cc83e 100644 --- a/src/commands/save.rs +++ b/src/commands/save.rs @@ -130,7 +130,7 @@ fn save( }: RunnableContext, raw_args: RawCommandArgs, ) -> Result { - let mut full_path = PathBuf::from(shell_manager.path()); + let mut full_path = PathBuf::from(shell_manager.path()?); let name_tag = name.clone(); let stream = async_stream! { diff --git a/src/commands/shells.rs b/src/commands/shells.rs index 7faf9f6e7e..5e9f73cb85 100644 --- a/src/commands/shells.rs +++ b/src/commands/shells.rs @@ -32,7 +32,16 @@ fn shells(args: CommandArgs, _registry: &CommandRegistry) -> Result Result>, ShellError> { + let registry = self.registry.lock().map_err(|_| { + ShellError::untagged_runtime_error("Internal error: names could not get mutex") + })?; + Ok(registry.clone()) + } } #[derive(Clone)] @@ -114,12 +121,12 @@ impl Context { pub(crate) fn expand_context<'context>( &'context self, source: &'context Text, - ) -> ExpandContext<'context> { - ExpandContext::new( + ) -> Result, ShellError> { + Ok(ExpandContext::new( Box::new(self.registry.clone()), source, - self.shell_manager.homedir(), - ) + self.shell_manager.homedir()?, + )) } pub(crate) fn basic() -> Result> { @@ -133,7 +140,7 @@ impl Context { }) } - pub(crate) fn error(&mut self, error: ShellError) { + pub(crate) fn error(&mut self, error: ShellError) -> Result<(), ShellError> { self.with_errors(|errors| errors.push(error)) } @@ -177,16 +184,30 @@ impl Context { result } - pub(crate) fn with_host(&mut self, block: impl FnOnce(&mut dyn Host) -> T) -> T { - let mut host = self.host.lock().unwrap(); - - block(&mut *host) + pub(crate) fn with_host( + &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", + )) + } } - pub(crate) fn with_errors(&mut self, block: impl FnOnce(&mut Vec) -> T) -> T { - let mut errors = self.current_errors.lock().unwrap(); - - block(&mut *errors) + pub(crate) fn with_errors( + &mut self, + block: impl FnOnce(&mut Vec) -> T, + ) -> Result { + if let Ok(mut errors) = self.current_errors.lock() { + Ok(block(&mut *errors)) + } else { + Err(ShellError::untagged_runtime_error( + "Internal error: could not lock host in with_errors", + )) + } } pub fn add_commands(&mut self, commands: Vec>) -> Result<(), ShellError> { diff --git a/src/data/base/shape.rs b/src/data/base/shape.rs index 101f0058c2..5dc34a84f9 100644 --- a/src/data/base/shape.rs +++ b/src/data/base/shape.rs @@ -359,12 +359,14 @@ impl Shapes { pub fn to_values(&self) -> Vec { if self.shapes.len() == 1 { - let shape = self.shapes.keys().nth(0).unwrap(); - - let mut tagged_dict = TaggedDictBuilder::new(Tag::unknown()); - tagged_dict.insert_untagged("type", shape.to_value()); - tagged_dict.insert_untagged("rows", UntaggedValue::string("all")); - vec![tagged_dict.into_value()] + if let Some(shape) = self.shapes.keys().nth(0) { + let mut tagged_dict = TaggedDictBuilder::new(Tag::unknown()); + tagged_dict.insert_untagged("type", shape.to_value()); + tagged_dict.insert_untagged("rows", UntaggedValue::string("all")); + vec![tagged_dict.into_value()] + } else { + unreachable!("Internal error: impossible state in to_values") + } } else { self.shapes .iter() diff --git a/src/format/entries.rs b/src/format/entries.rs index 6ead501fea..604256fc4d 100644 --- a/src/format/entries.rs +++ b/src/format/entries.rs @@ -39,10 +39,10 @@ impl RenderView for EntriesView { return Ok(()); } - let max_name_size: usize = self.entries.iter().map(|(n, _)| n.len()).max().unwrap(); - - for (name, value) in &self.entries { - outln!("{:width$} : {}", name, value, width = max_name_size) + if let Some(max_name_size) = self.entries.iter().map(|(n, _)| n.len()).max() { + for (name, value) in &self.entries { + outln!("{:width$} : {}", name, value, width = max_name_size) + } } Ok(()) diff --git a/src/format/table.rs b/src/format/table.rs index fc450bdbc1..49984d6b96 100644 --- a/src/format/table.rs +++ b/src/format/table.rs @@ -376,7 +376,7 @@ impl RenderView for TableView { )); } - table.print_term(&mut *host.out_terminal()).unwrap(); + 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)"))?; Ok(()) } diff --git a/src/prelude.rs b/src/prelude.rs index 307907e980..cc5d1c196f 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -129,7 +129,15 @@ where { fn to_input_stream(self) -> InputStream { InputStream { - values: self.map(|item| item.into().unwrap()).boxed(), + values: self + .map(|item| { + if let Ok(result) = item.into() { + result + } else { + unreachable!("Internal errors: to_input_stream in inconsistent state") + } + }) + .boxed(), } } } diff --git a/src/shell/help_shell.rs b/src/shell/help_shell.rs index 46e315351a..ab8de2fba1 100644 --- a/src/shell/help_shell.rs +++ b/src/shell/help_shell.rs @@ -25,18 +25,21 @@ impl HelpShell { let mut cmds = TaggedDictBuilder::new(Tag::unknown()); let mut specs = Vec::new(); - for cmd in registry.names()? { - let mut spec = TaggedDictBuilder::new(Tag::unknown()); - let value = command_dict(registry.get_command(&cmd)?.unwrap(), Tag::unknown()); + let snapshot = registry.snapshot()?; - spec.insert_untagged("name", cmd); + for (name, cmd) in snapshot.iter() { + let mut spec = TaggedDictBuilder::new(Tag::unknown()); + let value = command_dict(cmd.clone(), Tag::unknown()); + + spec.insert_untagged("name", name.to_string()); spec.insert_untagged( "description", value .get_data_by_key("usage".spanned_unknown()) - .unwrap() - .as_string() - .unwrap(), + .ok_or_else(|| { + ShellError::untagged_runtime_error("Internal error: expected to find usage") + })? + .as_string()?, ); spec.insert_value("details", value); @@ -77,7 +80,8 @@ impl HelpShell { match p { x if x == sep => {} step => { - let value = viewed.get_data_by_key(step.to_str().unwrap().spanned_unknown()); + let step: &str = &step.to_string_lossy().to_string(); + let value = viewed.get_data_by_key(step.spanned_unknown()); if let Some(v) = value { viewed = v.clone(); } diff --git a/src/shell/helper.rs b/src/shell/helper.rs index 09b6d8f65c..a4ef738d07 100644 --- a/src/shell/helper.rs +++ b/src/shell/helper.rs @@ -39,7 +39,10 @@ impl Completer for Helper { impl Hinter for Helper { fn hint(&self, line: &str, pos: usize, ctx: &rustyline::Context<'_>) -> Option { - self.context.shell_manager.hint(line, pos, ctx) + match self.context.shell_manager.hint(line, pos, ctx) { + Ok(output) => output, + Err(e) => Some(format!("{}", e)), + } } } @@ -78,31 +81,43 @@ impl Highlighter for Helper { let mut tokens = TokensIterator::all(&tokens[..], Text::from(line), v.span()); let text = Text::from(line); - let expand_context = self.context.expand_context(&text); + match self.context.expand_context(&text) { + Ok(expand_context) => { + let shapes = { + // We just constructed a token list that only contains a pipeline, so it can't fail + if let Err(err) = + color_fallible_syntax(&PipelineShape, &mut tokens, &expand_context) + { + let error_msg = format!("{}", err); + return Cow::Owned(error_msg); + } + tokens.with_color_tracer(|_, tracer| tracer.finish()); - let shapes = { - // We just constructed a token list that only contains a pipeline, so it can't fail - color_fallible_syntax(&PipelineShape, &mut tokens, &expand_context).unwrap(); - tokens.with_color_tracer(|_, tracer| tracer.finish()); + tokens.state().shapes() + }; - tokens.state().shapes() - }; + trace!(target: "nu::color_syntax", "{:#?}", tokens.color_tracer()); - trace!(target: "nu::color_syntax", "{:#?}", tokens.color_tracer()); + if log_enabled!(target: "nu::color_syntax", log::Level::Debug) { + outln!(""); + let _ = ptree::print_tree( + &tokens.color_tracer().clone().print(Text::from(line)), + ); + outln!(""); + } - if log_enabled!(target: "nu::color_syntax", log::Level::Debug) { - outln!(""); - ptree::print_tree(&tokens.color_tracer().clone().print(Text::from(line))) - .unwrap(); - outln!(""); + for shape in shapes { + let styled = paint_flat_shape(&shape, line); + out.push_str(&styled); + } + + Cow::Owned(out) + } + Err(err) => { + let error_msg = format!("{}", err); + Cow::Owned(error_msg) + } } - - for shape in shapes { - let styled = paint_flat_shape(&shape, line); - out.push_str(&styled); - } - - Cow::Owned(out) } } } diff --git a/src/shell/shell_manager.rs b/src/shell/shell_manager.rs index 6dd0a66a40..273a0f06c9 100644 --- a/src/shell/shell_manager.rs +++ b/src/shell/shell_manager.rs @@ -30,53 +30,95 @@ impl ShellManager { }) } - pub fn insert_at_current(&mut self, shell: Box) { - self.shells.lock().unwrap().push(shell); - self.current_shell - .store(self.shells.lock().unwrap().len() - 1, Ordering::SeqCst); - self.set_path(self.path()); + pub fn insert_at_current(&mut self, shell: Box) -> Result<(), ShellError> { + if let Ok(mut shells) = self.shells.lock() { + shells.push(shell); + } else { + return Err(ShellError::untagged_runtime_error( + "Internal error: could not lock shells ring buffer", + )); + } + + let shells_len = if let Ok(shells) = self.shells.lock() { + shells.len() + } else { + return Err(ShellError::untagged_runtime_error( + "Internal error: could not lock shells ring buffer", + )); + }; + + self.current_shell.store(shells_len - 1, Ordering::SeqCst); + self.set_path(self.path()?) } pub fn current_shell(&self) -> usize { self.current_shell.load(Ordering::SeqCst) } - pub fn remove_at_current(&mut self) { + pub fn remove_at_current(&mut self) -> Result<(), ShellError> { { - let mut shells = self.shells.lock().unwrap(); - if shells.len() > 0 { - if self.current_shell() == shells.len() - 1 { - shells.pop(); - let new_len = shells.len(); - if new_len > 0 { - self.current_shell.store(new_len - 1, Ordering::SeqCst); + if let Ok(mut shells) = self.shells.lock() { + if shells.len() > 0 { + if self.current_shell() == shells.len() - 1 { + shells.pop(); + let new_len = shells.len(); + if new_len > 0 { + self.current_shell.store(new_len - 1, Ordering::SeqCst); + } else { + return Ok(()); + } } else { - return; + shells.remove(self.current_shell()); } - } else { - shells.remove(self.current_shell()); } + } else { + return Err(ShellError::untagged_runtime_error( + "Internal error: could not lock shells ring buffer", + )); } } - self.set_path(self.path()); + self.set_path(self.path()?) } - pub fn is_empty(&self) -> bool { - self.shells.lock().unwrap().is_empty() + pub fn is_empty(&self) -> Result { + if let Ok(shells) = self.shells.lock() { + Ok(shells.is_empty()) + } else { + Err(ShellError::untagged_runtime_error( + "Internal error: could not lock shells ring buffer (is_empty)", + )) + } } - pub fn path(&self) -> String { - self.shells.lock().unwrap()[self.current_shell()].path() + pub fn path(&self) -> Result { + if let Ok(shells) = self.shells.lock() { + Ok(shells[self.current_shell()].path()) + } else { + Err(ShellError::untagged_runtime_error( + "Internal error: could not lock shells ring buffer (path)", + )) + } } pub fn pwd(&self, args: EvaluatedWholeStreamCommandArgs) -> Result { - let env = self.shells.lock().unwrap(); - - env[self.current_shell()].pwd(args) + if let Ok(shells) = self.shells.lock() { + shells[self.current_shell()].pwd(args) + } else { + Err(ShellError::untagged_runtime_error( + "Internal error: could not lock shells ring buffer (pwd)", + )) + } } - pub fn set_path(&mut self, path: String) { - self.shells.lock().unwrap()[self.current_shell()].set_path(path) + pub fn set_path(&mut self, path: String) -> Result<(), ShellError> { + if let Ok(mut shells) = self.shells.lock() { + shells[self.current_shell()].set_path(path); + Ok(()) + } else { + Err(ShellError::untagged_runtime_error( + "Internal error: could not lock shells ring buffer (set_path)", + )) + } } pub fn complete( @@ -85,43 +127,77 @@ impl ShellManager { pos: usize, ctx: &rustyline::Context<'_>, ) -> Result<(usize, Vec), rustyline::error::ReadlineError> { - self.shells.lock().unwrap()[self.current_shell()].complete(line, pos, ctx) + if let Ok(shells) = self.shells.lock() { + shells[self.current_shell()].complete(line, pos, ctx) + } else { + Err(rustyline::error::ReadlineError::Io(std::io::Error::new( + std::io::ErrorKind::Other, + "Internal error: could not lock shells ring buffer (complete)", + ))) + } } - pub fn hint(&self, line: &str, pos: usize, ctx: &rustyline::Context<'_>) -> Option { - self.shells.lock().unwrap()[self.current_shell()].hint(line, pos, ctx) + pub fn hint( + &self, + line: &str, + pos: usize, + ctx: &rustyline::Context<'_>, + ) -> Result, ShellError> { + if let Ok(shells) = self.shells.lock() { + Ok(shells[self.current_shell()].hint(line, pos, ctx)) + } else { + Err(ShellError::untagged_runtime_error( + "Internal error: could not lock shells ring buffer (hint)", + )) + } } - pub fn next(&mut self) { + pub fn next(&mut self) -> Result<(), ShellError> { { - let shell_len = self.shells.lock().unwrap().len(); - if self.current_shell() == (shell_len - 1) { - self.current_shell.store(0, Ordering::SeqCst); + if let Ok(shells) = self.shells.lock() { + let shell_len = shells.len(); + if self.current_shell() == (shell_len - 1) { + self.current_shell.store(0, Ordering::SeqCst); + } else { + self.current_shell + .store(self.current_shell() + 1, Ordering::SeqCst); + } } else { - self.current_shell - .store(self.current_shell() + 1, Ordering::SeqCst); + return Err(ShellError::untagged_runtime_error( + "Internal error: could not lock shells ring buffer (next)", + )); } } - self.set_path(self.path()); + self.set_path(self.path()?) } - pub fn prev(&mut self) { + pub fn prev(&mut self) -> Result<(), ShellError> { { - let shell_len = self.shells.lock().unwrap().len(); - if self.current_shell() == 0 { - self.current_shell.store(shell_len - 1, Ordering::SeqCst); + if let Ok(shells) = self.shells.lock() { + let shell_len = shells.len(); + if self.current_shell() == 0 { + self.current_shell.store(shell_len - 1, Ordering::SeqCst); + } else { + self.current_shell + .store(self.current_shell() - 1, Ordering::SeqCst); + } } else { - self.current_shell - .store(self.current_shell() - 1, Ordering::SeqCst); + return Err(ShellError::untagged_runtime_error( + "Internal error: could not lock shells ring buffer (prev)", + )); } } - self.set_path(self.path()); + self.set_path(self.path()?) } - pub fn homedir(&self) -> Option { - let env = self.shells.lock().unwrap(); - - env[self.current_shell()].homedir() + pub fn homedir(&self) -> Result, ShellError> { + if let Ok(shells) = self.shells.lock() { + Ok(shells[self.current_shell()].homedir()) + } else { + Err(ShellError::untagged_runtime_error( + "Internal error: could not lock shells ring buffer (homedir)", + )) + } } pub fn ls( @@ -130,15 +206,23 @@ impl ShellManager { context: &RunnableContext, full: bool, ) -> Result { - let env = self.shells.lock().unwrap(); - - env[self.current_shell()].ls(path, context, full) + if let Ok(shells) = self.shells.lock() { + shells[self.current_shell()].ls(path, context, full) + } else { + Err(ShellError::untagged_runtime_error( + "Internal error: could not lock shells ring buffer (ls)", + )) + } } pub fn cd(&self, args: EvaluatedWholeStreamCommandArgs) -> Result { - let env = self.shells.lock().unwrap(); - - env[self.current_shell()].cd(args) + if let Ok(shells) = self.shells.lock() { + shells[self.current_shell()].cd(args) + } else { + Err(ShellError::untagged_runtime_error( + "Internal error: could not lock shells ring buffer (cd)", + )) + } } pub fn cp( @@ -146,9 +230,9 @@ impl ShellManager { args: CopyArgs, context: &RunnablePerItemContext, ) -> Result { - let env = self.shells.lock(); + let shells = self.shells.lock(); - match env { + match shells { Ok(x) => { let path = x[self.current_shell()].path(); x[self.current_shell()].cp(args, context.name.clone(), &path) @@ -166,9 +250,9 @@ impl ShellManager { args: RemoveArgs, context: &RunnablePerItemContext, ) -> Result { - let env = self.shells.lock(); + let shells = self.shells.lock(); - match env { + match shells { Ok(x) => { let path = x[self.current_shell()].path(); x[self.current_shell()].rm(args, context.name.clone(), &path) @@ -186,9 +270,9 @@ impl ShellManager { args: MkdirArgs, context: &RunnablePerItemContext, ) -> Result { - let env = self.shells.lock(); + let shells = self.shells.lock(); - match env { + match shells { Ok(x) => { let path = x[self.current_shell()].path(); x[self.current_shell()].mkdir(args, context.name.clone(), &path) @@ -206,9 +290,9 @@ impl ShellManager { args: MoveArgs, context: &RunnablePerItemContext, ) -> Result { - let env = self.shells.lock(); + let shells = self.shells.lock(); - match env { + match shells { Ok(x) => { let path = x[self.current_shell()].path(); x[self.current_shell()].mv(args, context.name.clone(), &path) diff --git a/src/shell/value_shell.rs b/src/shell/value_shell.rs index 77e3507f2d..896b16d537 100644 --- a/src/shell/value_shell.rs +++ b/src/shell/value_shell.rs @@ -44,7 +44,8 @@ impl ValueShell { match p { x if x == sep => {} step => { - let value = viewed.get_data_by_key(step.to_str().unwrap().spanned_unknown()); + let name: &str = &step.to_string_lossy().to_string(); + let value = viewed.get_data_by_key(name.spanned_unknown()); if let Some(v) = value { viewed = v.clone(); }