From 8386bc0919c0290da8f54244514b0d06b45eb661 Mon Sep 17 00:00:00 2001 From: Eric Hodel Date: Tue, 28 Nov 2023 04:43:51 -0800 Subject: [PATCH] Convert more ShellError variants to named fields (#11173) # Description Convert these ShellError variants to named fields: * CreateNotPossible * MoveNotPossibleSingle * DirectoryNotFoundCustom * DirectoryNotFound * NotADirectory * OutOfMemoryError * PermissionDeniedError * IOErrorSpanned * IOError * IOInterrupted Also place the `span` field of `DirectoryNotFound` last to match other errors. Part of #10700 (almost half done!) # User-Facing Changes None # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` # After Submitting N/A --- crates/nu-cli/src/repl.rs | 8 +-- crates/nu-command/src/filesystem/cd.rs | 24 +++---- crates/nu-command/src/filesystem/cp.rs | 38 +++++++--- crates/nu-command/src/filesystem/mkdir.rs | 9 +-- crates/nu-command/src/filesystem/mktemp.rs | 12 ++-- crates/nu-command/src/filesystem/mv.rs | 16 ++--- crates/nu-command/src/filesystem/open.rs | 2 +- crates/nu-command/src/filesystem/save.rs | 17 +++-- crates/nu-command/src/filesystem/touch.rs | 9 +-- crates/nu-command/src/filesystem/watch.rs | 32 ++++----- crates/nu-command/src/network/http/client.rs | 12 ++-- crates/nu-command/src/path/exists.rs | 8 ++- crates/nu-command/src/platform/clear.rs | 10 ++- .../nu-command/src/platform/input/input_.rs | 4 +- crates/nu-command/src/platform/input/list.rs | 12 +++- crates/nu-command/src/system/nu_check.rs | 12 +++- crates/nu-engine/src/glob_from.rs | 8 +-- crates/nu-explore/src/nu_common/command.rs | 16 +++-- crates/nu-protocol/src/eval_const.rs | 35 ++++++--- crates/nu-protocol/src/shell_error.rs | 71 +++++++++++++++---- crates/nu-protocol/src/util.rs | 2 +- 21 files changed, 241 insertions(+), 116 deletions(-) diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index d79198f05..a069f248e 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -502,10 +502,10 @@ pub fn evaluate_repl( report_error( &working_set, - &ShellError::DirectoryNotFound( - tokens.0[0].span, - path.to_string_lossy().to_string(), - ), + &ShellError::DirectoryNotFound { + dir: path.to_string_lossy().to_string(), + span: tokens.0[0].span, + }, ); } let path = nu_path::canonicalize_with(path, &cwd) diff --git a/crates/nu-command/src/filesystem/cd.rs b/crates/nu-command/src/filesystem/cd.rs index bf02671ce..2905aba34 100644 --- a/crates/nu-command/src/filesystem/cd.rs +++ b/crates/nu-command/src/filesystem/cd.rs @@ -83,10 +83,10 @@ impl Command for Cd { let path = match nu_path::canonicalize_with(path.clone(), &cwd) { Ok(p) => p, Err(_) => { - return Err(ShellError::DirectoryNotFound( - v.span, - path.to_string_lossy().to_string(), - )); + return Err(ShellError::DirectoryNotFound { + dir: path.to_string_lossy().to_string(), + span: v.span, + }); } }; (path.to_string_lossy().to_string(), v.span) @@ -100,17 +100,17 @@ impl Command for Cd { let path = match nu_path::canonicalize_with(path_no_whitespace, &cwd) { Ok(p) => { if !p.is_dir() { - return Err(ShellError::NotADirectory(v.span)); + return Err(ShellError::NotADirectory { span: v.span }); }; p } // if canonicalize failed, let's check to see if it's abbreviated Err(_) => { - return Err(ShellError::DirectoryNotFound( - v.span, - path_no_whitespace.to_string(), - )); + return Err(ShellError::DirectoryNotFound { + dir: path_no_whitespace.to_string(), + span: v.span, + }); } }; (path.to_string_lossy().to_string(), v.span) @@ -135,9 +135,9 @@ impl Command for Cd { stack.add_env_var("PWD".into(), path_value); Ok(PipelineData::empty()) } - PermissionResult::PermissionDenied(reason) => Err(ShellError::IOError(format!( - "Cannot change directory to {path}: {reason}" - ))), + PermissionResult::PermissionDenied(reason) => Err(ShellError::IOError { + msg: format!("Cannot change directory to {path}: {reason}"), + }), } } diff --git a/crates/nu-command/src/filesystem/cp.rs b/crates/nu-command/src/filesystem/cp.rs index c92c500f7..f943c2b60 100644 --- a/crates/nu-command/src/filesystem/cp.rs +++ b/crates/nu-command/src/filesystem/cp.rs @@ -88,10 +88,10 @@ impl Command for Cp { let path_last_char = destination.as_os_str().to_string_lossy().chars().last(); let is_directory = path_last_char == Some('/') || path_last_char == Some('\\'); if is_directory && !destination.exists() { - return Err(ShellError::DirectoryNotFound( - dst.span, - destination.to_string_lossy().to_string(), - )); + return Err(ShellError::DirectoryNotFound { + dir: destination.to_string_lossy().to_string(), + span: dst.span, + }); } let ctrlc = engine_state.ctrlc.clone(); let span = call.head; @@ -577,18 +577,36 @@ fn convert_io_error(error: std::io::Error, src: PathBuf, dst: PathBuf, span: Spa ErrorKind::PermissionDenied => match std::fs::metadata(&dst) { Ok(meta) => { if meta.permissions().readonly() { - ShellError::PermissionDeniedError(message_dst, span) + ShellError::PermissionDeniedError { + msg: message_dst, + span, + } } else { - ShellError::PermissionDeniedError(message_src, span) + ShellError::PermissionDeniedError { + msg: message_src, + span, + } } } - Err(_) => ShellError::PermissionDeniedError(message_dst, span), + Err(_) => ShellError::PermissionDeniedError { + msg: message_dst, + span, + }, + }, + ErrorKind::Interrupted => ShellError::IOInterrupted { + msg: message_src, + span, + }, + ErrorKind::OutOfMemory => ShellError::OutOfMemoryError { + msg: message_src, + span, }, - ErrorKind::Interrupted => ShellError::IOInterrupted(message_src, span), - ErrorKind::OutOfMemory => ShellError::OutOfMemoryError(message_src, span), // TODO: handle ExecutableFileBusy etc. when io_error_more is stabilized // https://github.com/rust-lang/rust/issues/86442 - _ => ShellError::IOErrorSpanned(message_src, span), + _ => ShellError::IOErrorSpanned { + msg: message_src, + span, + }, }; Value::error(shell_error, span) diff --git a/crates/nu-command/src/filesystem/mkdir.rs b/crates/nu-command/src/filesystem/mkdir.rs index 703ac4898..2b99395d2 100644 --- a/crates/nu-command/src/filesystem/mkdir.rs +++ b/crates/nu-command/src/filesystem/mkdir.rs @@ -69,12 +69,13 @@ impl Command for Mkdir { let dir_res = std::fs::create_dir_all(&dir); if let Err(reason) = dir_res { - return Err(ShellError::CreateNotPossible( - format!("failed to create directory: {reason}"), - call.positional_nth(i) + return Err(ShellError::CreateNotPossible { + msg: format!("failed to create directory: {reason}"), + span: call + .positional_nth(i) .expect("already checked through directories") .span, - )); + }); } if is_verbose { diff --git a/crates/nu-command/src/filesystem/mktemp.rs b/crates/nu-command/src/filesystem/mktemp.rs index d9ab2848d..417d1c806 100644 --- a/crates/nu-command/src/filesystem/mktemp.rs +++ b/crates/nu-command/src/filesystem/mktemp.rs @@ -110,10 +110,14 @@ impl Command for Mktemp { }; let res = match uu_mktemp::mktemp(&options) { - Ok(res) => res - .into_os_string() - .into_string() - .map_err(|e| ShellError::IOErrorSpanned(e.to_string_lossy().to_string(), span))?, + Ok(res) => { + res.into_os_string() + .into_string() + .map_err(|e| ShellError::IOErrorSpanned { + msg: e.to_string_lossy().to_string(), + span, + })? + } Err(e) => { return Err(ShellError::GenericError( format!("{}", e), diff --git a/crates/nu-command/src/filesystem/mv.rs b/crates/nu-command/src/filesystem/mv.rs index 4c911a61e..3e5fd928b 100644 --- a/crates/nu-command/src/filesystem/mv.rs +++ b/crates/nu-command/src/filesystem/mv.rs @@ -260,10 +260,10 @@ fn move_file( }; if !destination_dir_exists { - return Err(ShellError::DirectoryNotFound( - to_span, - to.to_string_lossy().to_string(), - )); + return Err(ShellError::DirectoryNotFound { + dir: to.to_string_lossy().to_string(), + span: to_span, + }); } // This can happen when changing case on a case-insensitive filesystem (ex: changing foo to Foo on Windows) @@ -275,10 +275,10 @@ fn move_file( let from_file_name = match from.file_name() { Some(name) => name, None => { - return Err(ShellError::DirectoryNotFound( - to_span, - from.to_string_lossy().to_string(), - )) + return Err(ShellError::DirectoryNotFound { + dir: from.to_string_lossy().to_string(), + span: to_span, + }) } }; diff --git a/crates/nu-command/src/filesystem/open.rs b/crates/nu-command/src/filesystem/open.rs index b70ca9dca..99cd94afd 100644 --- a/crates/nu-command/src/filesystem/open.rs +++ b/crates/nu-command/src/filesystem/open.rs @@ -105,7 +105,7 @@ impl Command for Open { for path in nu_engine::glob_from(&path, &cwd, call_span, None) .map_err(|err| match err { - ShellError::DirectoryNotFound(span, _) => ShellError::FileNotFound { span }, + ShellError::DirectoryNotFound { span, .. } => ShellError::FileNotFound { span }, _ => err, })? .1 diff --git a/crates/nu-command/src/filesystem/save.rs b/crates/nu-command/src/filesystem/save.rs index 4e9a5bda8..4b02b7e7b 100644 --- a/crates/nu-command/src/filesystem/save.rs +++ b/crates/nu-command/src/filesystem/save.rs @@ -162,9 +162,13 @@ impl Command for Save { )?; for val in ls { file.write_all(&value_to_bytes(val)?) - .map_err(|err| ShellError::IOError(err.to_string()))?; + .map_err(|err| ShellError::IOError { + msg: err.to_string(), + })?; file.write_all("\n".as_bytes()) - .map_err(|err| ShellError::IOError(err.to_string()))?; + .map_err(|err| ShellError::IOError { + msg: err.to_string(), + })?; } file.flush()?; @@ -184,8 +188,9 @@ impl Command for Save { force, )?; - file.write_all(&bytes) - .map_err(|err| ShellError::IOError(err.to_string()))?; + file.write_all(&bytes).map_err(|err| ShellError::IOError { + msg: err.to_string(), + })?; file.flush()?; @@ -453,7 +458,9 @@ fn stream_to_file( if let Err(err) = writer.write(&buf) { *process_failed_p = true; - return Err(ShellError::IOError(err.to_string())); + return Err(ShellError::IOError { + msg: err.to_string(), + }); } Ok(()) }) diff --git a/crates/nu-command/src/filesystem/touch.rs b/crates/nu-command/src/filesystem/touch.rs index d08ba85c7..e5cbdacb6 100644 --- a/crates/nu-command/src/filesystem/touch.rs +++ b/crates/nu-command/src/filesystem/touch.rs @@ -136,12 +136,13 @@ impl Command for Touch { } if let Err(err) = OpenOptions::new().write(true).create(true).open(&item) { - return Err(ShellError::CreateNotPossible( - format!("Failed to create file: {err}"), - call.positional_nth(index) + return Err(ShellError::CreateNotPossible { + msg: format!("Failed to create file: {err}"), + span: call + .positional_nth(index) .expect("already checked positional") .span, - )); + }); }; if change_mtime { diff --git a/crates/nu-command/src/filesystem/watch.rs b/crates/nu-command/src/filesystem/watch.rs index 91fd30912..840192c51 100644 --- a/crates/nu-command/src/filesystem/watch.rs +++ b/crates/nu-command/src/filesystem/watch.rs @@ -83,10 +83,10 @@ impl Command for Watch { let path = match nu_path::canonicalize_with(path_no_whitespace, cwd) { Ok(p) => p, Err(_) => { - return Err(ShellError::DirectoryNotFound( - path_arg.span, - path_no_whitespace.to_string(), - )) + return Err(ShellError::DirectoryNotFound { + dir: path_no_whitespace.to_string(), + span: path_arg.span, + }) } }; @@ -153,15 +153,15 @@ impl Command for Watch { let mut debouncer = match new_debouncer(debounce_duration, None, tx) { Ok(d) => d, Err(e) => { - return Err(ShellError::IOError(format!( - "Failed to create watcher: {e}" - ))) + return Err(ShellError::IOError { + msg: format!("Failed to create watcher: {e}"), + }) } }; if let Err(e) = debouncer.watcher().watch(&path, recursive_mode) { - return Err(ShellError::IOError(format!( - "Failed to create watcher: {e}" - ))); + return Err(ShellError::IOError { + msg: format!("Failed to create watcher: {e}"), + }); } // need to cache to make sure that rename event works. debouncer.cache().add_root(&path, recursive_mode); @@ -275,14 +275,14 @@ impl Command for Watch { } } Ok(Err(_)) => { - return Err(ShellError::IOError( - "Unexpected errors when receiving events".into(), - )) + return Err(ShellError::IOError { + msg: "Unexpected errors when receiving events".into(), + }) } Err(RecvTimeoutError::Disconnected) => { - return Err(ShellError::IOError( - "Unexpected disconnect from file watcher".into(), - )); + return Err(ShellError::IOError { + msg: "Unexpected disconnect from file watcher".into(), + }); } Err(RecvTimeoutError::Timeout) => {} } diff --git a/crates/nu-command/src/network/http/client.rs b/crates/nu-command/src/network/http/client.rs index 9365f6261..d25dc1f99 100644 --- a/crates/nu-command/src/network/http/client.rs +++ b/crates/nu-command/src/network/http/client.rs @@ -209,9 +209,9 @@ pub fn send_request( } Value::List { vals, .. } if body_type == BodyType::Form => { if vals.len() % 2 != 0 { - return Err(ShellErrorOrRequestError::ShellError(ShellError::IOError( - "unsupported body input".into(), - ))); + return Err(ShellErrorOrRequestError::ShellError(ShellError::IOError { + msg: "unsupported body input".into(), + })); } let data = vals @@ -233,9 +233,9 @@ pub fn send_request( let data = value_to_json_value(&body)?; send_cancellable_request(&request_url, Box::new(|| request.send_json(data)), ctrl_c) } - _ => Err(ShellErrorOrRequestError::ShellError(ShellError::IOError( - "unsupported body input".into(), - ))), + _ => Err(ShellErrorOrRequestError::ShellError(ShellError::IOError { + msg: "unsupported body input".into(), + })), } } diff --git a/crates/nu-command/src/path/exists.rs b/crates/nu-command/src/path/exists.rs index 0b2acf9e7..8fcea168f 100644 --- a/crates/nu-command/src/path/exists.rs +++ b/crates/nu-command/src/path/exists.rs @@ -135,7 +135,13 @@ fn exists(path: &Path, span: Span, args: &Arguments) -> Value { match path.try_exists() { Ok(exists) => exists, Err(err) => { - return Value::error(ShellError::IOErrorSpanned(err.to_string(), span), span) + return Value::error( + ShellError::IOErrorSpanned { + msg: err.to_string(), + span, + }, + span, + ) } }, span, diff --git a/crates/nu-command/src/platform/clear.rs b/crates/nu-command/src/platform/clear.rs index ad33de0ca..abdd704cb 100644 --- a/crates/nu-command/src/platform/clear.rs +++ b/crates/nu-command/src/platform/clear.rs @@ -36,7 +36,10 @@ impl Command for Clear { CommandSys::new("cmd") .args(["/C", "cls"]) .status() - .map_err(|e| ShellError::IOErrorSpanned(e.to_string(), span))?; + .map_err(|e| ShellError::IOErrorSpanned { + msg: e.to_string(), + span, + })?; } else if cfg!(unix) { let mut cmd = CommandSys::new("/bin/sh"); @@ -46,7 +49,10 @@ impl Command for Clear { cmd.args(["-c", "clear"]) .status() - .map_err(|e| ShellError::IOErrorSpanned(e.to_string(), span))?; + .map_err(|e| ShellError::IOErrorSpanned { + msg: e.to_string(), + span, + })?; } Ok(Value::nothing(span).into_pipeline_data()) diff --git a/crates/nu-command/src/platform/input/input_.rs b/crates/nu-command/src/platform/input/input_.rs index 3ec4bf37f..b0a00b3ae 100644 --- a/crates/nu-command/src/platform/input/input_.rs +++ b/crates/nu-command/src/platform/input/input_.rs @@ -110,7 +110,9 @@ impl Command for Input { { if k.modifiers == KeyModifiers::CONTROL && c == 'c' { crossterm::terminal::disable_raw_mode()?; - return Err(ShellError::IOError("SIGINT".to_string())); + return Err(ShellError::IOError { + msg: "SIGINT".to_string(), + }); } continue; } diff --git a/crates/nu-command/src/platform/input/list.rs b/crates/nu-command/src/platform/input/list.rs index d6a7250c2..903743141 100644 --- a/crates/nu-command/src/platform/input/list.rs +++ b/crates/nu-command/src/platform/input/list.rs @@ -196,7 +196,9 @@ impl Command for InputList { .items(&options) .report(false) .interact_on_opt(&Term::stderr()) - .map_err(|err| ShellError::IOError(format!("{}: {}", INTERACT_ERROR, err)))?, + .map_err(|err| ShellError::IOError { + msg: format!("{}: {}", INTERACT_ERROR, err), + })?, ) } else if call.has_flag("fuzzy") { let fuzzy_select = FuzzySelect::new(); //::with_theme(&theme); @@ -211,7 +213,9 @@ impl Command for InputList { .default(0) .report(false) .interact_on_opt(&Term::stderr()) - .map_err(|err| ShellError::IOError(format!("{}: {}", INTERACT_ERROR, err)))?, + .map_err(|err| ShellError::IOError { + msg: format!("{}: {}", INTERACT_ERROR, err), + })?, ) } else { let select = Select::new(); //::with_theme(&theme); @@ -225,7 +229,9 @@ impl Command for InputList { .default(0) .report(false) .interact_on_opt(&Term::stderr()) - .map_err(|err| ShellError::IOError(format!("{}: {}", INTERACT_ERROR, err)))?, + .map_err(|err| ShellError::IOError { + msg: format!("{}: {}", INTERACT_ERROR, err), + })?, ) }; diff --git a/crates/nu-command/src/system/nu_check.rs b/crates/nu-command/src/system/nu_check.rs index bcd8e34dc..fc974c341 100644 --- a/crates/nu-command/src/system/nu_check.rs +++ b/crates/nu-command/src/system/nu_check.rs @@ -301,7 +301,9 @@ fn heuristic_parse_file( } } } else { - Err(ShellError::IOError("Can not read input".to_string())) + Err(ShellError::IOError { + msg: "Can not read input".to_string(), + }) } } else { Err(ShellError::NotFound { span: call.head }) @@ -402,7 +404,9 @@ fn parse_file_script( call.head, ) } else { - Err(ShellError::IOError("Can not read path".to_string())) + Err(ShellError::IOError { + msg: "Can not read path".to_string(), + }) } } else { Err(ShellError::NotFound { span: call.head }) @@ -425,7 +429,9 @@ fn parse_file_module( if let Ok(contents) = std::fs::read(path) { parse_module(working_set, Some(filename), &contents, is_debug, call.head) } else { - Err(ShellError::IOError("Can not read path".to_string())) + Err(ShellError::IOError { + msg: "Can not read path".to_string(), + }) } } else { Err(ShellError::NotFound { span: call.head }) diff --git a/crates/nu-engine/src/glob_from.rs b/crates/nu-engine/src/glob_from.rs index bc53aef1e..b6bacedd0 100644 --- a/crates/nu-engine/src/glob_from.rs +++ b/crates/nu-engine/src/glob_from.rs @@ -72,10 +72,10 @@ pub fn glob_from( let path = if let Ok(p) = canonicalize_with(path.clone(), cwd) { p } else { - return Err(ShellError::DirectoryNotFound( - pattern.span, - path.to_string_lossy().to_string(), - )); + return Err(ShellError::DirectoryNotFound { + dir: path.to_string_lossy().to_string(), + span: pattern.span, + }); }; (path.parent().map(|parent| parent.to_path_buf()), path) } diff --git a/crates/nu-explore/src/nu_common/command.rs b/crates/nu-explore/src/nu_common/command.rs index 53777d9b4..8367ea6a5 100644 --- a/crates/nu-explore/src/nu_common/command.rs +++ b/crates/nu-explore/src/nu_common/command.rs @@ -12,13 +12,17 @@ pub fn run_command_with_value( stack: &mut Stack, ) -> Result { if is_ignored_command(command) { - return Err(ShellError::IOError(String::from("the command is ignored"))); + return Err(ShellError::IOError { + msg: String::from("the command is ignored"), + }); } let pipeline = PipelineData::Value(input.clone(), None); let pipeline = run_nu_command(engine_state, stack, command, pipeline)?; if let PipelineData::Value(Value::Error { error, .. }, ..) = pipeline { - Err(ShellError::IOError(error.to_string())) + Err(ShellError::IOError { + msg: error.to_string(), + }) } else { Ok(pipeline) } @@ -63,7 +67,9 @@ fn eval_source2( ); if let Some(err) = working_set.parse_errors.first() { - return Err(ShellError::IOError(err.to_string())); + return Err(ShellError::IOError { + msg: err.to_string(), + }); } (output, working_set.render()) @@ -71,7 +77,9 @@ fn eval_source2( // We need to merge different info other wise things like PIPEs etc will not work. if let Err(err) = engine_state.merge_delta(delta) { - return Err(ShellError::IOError(err.to_string())); + return Err(ShellError::IOError { + msg: err.to_string(), + }); } // eval_block outputs all expressions except the last to STDOUT; diff --git a/crates/nu-protocol/src/eval_const.rs b/crates/nu-protocol/src/eval_const.rs index 2f6518c73..31e07eee9 100644 --- a/crates/nu-protocol/src/eval_const.rs +++ b/crates/nu-protocol/src/eval_const.rs @@ -32,7 +32,9 @@ pub fn create_nu_constant(engine_state: &EngineState, span: Span) -> Result Result Result Result Result Result Result Result for ShellError { fn from(input: std::io::Error) -> ShellError { - ShellError::IOError(format!("{input:?}")) + ShellError::IOError { + msg: format!("{input:?}"), + } } } impl std::convert::From> for ShellError { fn from(input: Box) -> ShellError { - ShellError::IOError(input.to_string()) + ShellError::IOError { + msg: input.to_string(), + } } } impl From> for ShellError { fn from(input: Box) -> ShellError { - ShellError::IOError(format!("{input:?}")) + ShellError::IOError { + msg: format!("{input:?}"), + } } } diff --git a/crates/nu-protocol/src/util.rs b/crates/nu-protocol/src/util.rs index 26383d1ce..13dca6e3a 100644 --- a/crates/nu-protocol/src/util.rs +++ b/crates/nu-protocol/src/util.rs @@ -30,7 +30,7 @@ impl Iterator for BufferedReader { Some(Ok(result)) } } - Err(e) => Some(Err(ShellError::IOError(e.to_string()))), + Err(e) => Some(Err(ShellError::IOError { msg: e.to_string() })), } } }