From e01dbda156ad33149064277abac45aa1eceaf60c Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Mon, 16 Oct 2023 23:18:25 -0400 Subject: [PATCH] Clean up error display (#1699) --- src/error.rs | 407 +++++++++++++------------------------------------- tests/edit.rs | 2 +- tests/fmt.rs | 2 +- 3 files changed, 106 insertions(+), 305 deletions(-) diff --git a/src/error.rs b/src/error.rs index b5ff0f1..b14cc5a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -219,219 +219,88 @@ impl<'src> ColorDisplay for Error<'src> { fn fmt(&self, f: &mut Formatter, color: Color) -> fmt::Result { use Error::*; - write!( - f, - "{}: {}", - color.error().paint("error"), - color.message().prefix() - )?; + let error = color.error().paint("error"); + let message = color.message().prefix(); + write!(f, "{error}: {message}")?; match self { - ArgumentCountMismatch { - recipe, - found, - min, - max, - .. - } => { + ArgumentCountMismatch { recipe, found, min, max, .. } => { + let count = Count("argument", *found); if min == max { let expected = min; - write!( - f, - "Recipe `{recipe}` got {found} {} but {}takes {expected}", - Count("argument", *found), - if expected < found { "only " } else { "" } - )?; + let only = if expected < found { "only " } else { "" }; + write!(f, "Recipe `{recipe}` got {found} {count} but {only}takes {expected}")?; } else if found < min { - write!( - f, - "Recipe `{recipe}` got {found} {} but takes at least {min}", - Count("argument", *found) - )?; + write!(f, "Recipe `{recipe}` got {found} {count} but takes at least {min}")?; } else if found > max { - write!( - f, - "Recipe `{recipe}` got {found} {} but takes at most {max}", - Count("argument", *found) - )?; + write!(f, "Recipe `{recipe}` got {found} {count} but takes at most {max}")?; } } Backtick { output_error, .. } => match output_error { - OutputError::Code(code) => { - write!(f, "Backtick failed with exit code {code}")?; - } - OutputError::Signal(signal) => { - write!(f, "Backtick was terminated by signal {signal}")?; - } - OutputError::Unknown => { - write!(f, "Backtick failed for an unknown reason")?; - } - OutputError::Io(io_error) => { - match io_error.kind() { - io::ErrorKind::NotFound => write!( - f, - "Backtick could not be run because just could not find the shell:\n{io_error}" - ), - io::ErrorKind::PermissionDenied => write!( - f, - "Backtick could not be run because just could not run the shell:\n{io_error}" - ), - _ => write!( - f, - "Backtick could not be run because of an IO error while launching the shell:\n{io_error}" - ), - }?; - } - OutputError::Utf8(utf8_error) => { - write!( - f, - "Backtick succeeded but stdout was not utf8: {utf8_error}" - )?; - } - }, - ChooserInvoke { - shell_binary, - shell_arguments, - chooser, - io_error, - } => { - write!( - f, - "Chooser `{shell_binary} {shell_arguments} {}` invocation failed: {io_error}", - chooser.to_string_lossy(), - )?; + OutputError::Code(code) => write!(f, "Backtick failed with exit code {code}")?, + OutputError::Signal(signal) => write!(f, "Backtick was terminated by signal {signal}")?, + OutputError::Unknown => write!(f, "Backtick failed for an unknown reason")?, + OutputError::Io(io_error) => match io_error.kind() { + io::ErrorKind::NotFound => write!(f, "Backtick could not be run because just could not find the shell:\n{io_error}"), + io::ErrorKind::PermissionDenied => write!(f, "Backtick could not be run because just could not run the shell:\n{io_error}"), + _ => write!(f, "Backtick could not be run because of an IO error while launching the shell:\n{io_error}"), + }?, + OutputError::Utf8(utf8_error) => write!(f, "Backtick succeeded but stdout was not utf8: {utf8_error}")?, + } + ChooserInvoke { shell_binary, shell_arguments, chooser, io_error} => { + let chooser = chooser.to_string_lossy(); + write!(f, "Chooser `{shell_binary} {shell_arguments} {chooser}` invocation failed: {io_error}")?; } ChooserRead { chooser, io_error } => { - write!( - f, - "Failed to read output from chooser `{}`: {io_error}", - chooser.to_string_lossy() - )?; + let chooser = chooser.to_string_lossy(); + write!(f, "Failed to read output from chooser `{chooser}`: {io_error}")?; } ChooserStatus { chooser, status } => { - write!( - f, - "Chooser `{}` failed: {status}", - chooser.to_string_lossy() - )?; + let chooser = chooser.to_string_lossy(); + write!(f, "Chooser `{chooser}` failed: {status}")?; } ChooserWrite { chooser, io_error } => { - write!( - f, - "Failed to write to chooser `{}`: {io_error}", - chooser.to_string_lossy() - )?; + let chooser = chooser.to_string_lossy(); + write!(f, "Failed to write to chooser `{chooser}`: {io_error}")?; } CircularInclude { current, include } => { - write!( - f, - "Include `{}` in `{}` is a circular include", include.display(), current.display() - )?; - }, - Code { - recipe, - line_number, - code, - .. - } => { + let include = include.display(); + let current = current.display(); + write!(f, "Include `{include}` in `{current}` is a circular include")?; + } + Code { recipe, line_number, code, .. } => { if let Some(n) = line_number { - write!( - f, - "Recipe `{recipe}` failed on line {n} with exit code {code}" - )?; + write!(f, "Recipe `{recipe}` failed on line {n} with exit code {code}")?; } else { write!(f, "Recipe `{recipe}` failed with exit code {code}")?; } } - CommandInvoke { - binary, - arguments, - io_error, - } => { - write!( - f, - "Failed to invoke {}: {io_error}", - iter::once(binary) - .chain(arguments) - .map(|value| Enclosure::tick(value.to_string_lossy()).to_string()) - .collect::>() - .join(" "), - )?; + CommandInvoke { binary, arguments, io_error } => { + let cmd = format_cmd(binary, arguments); + write!(f, "Failed to invoke {cmd}: {io_error}")?; } - CommandStatus { - binary, - arguments, - status, - } => { - write!( - f, - "Command {} failed: {status}", - iter::once(binary) - .chain(arguments) - .map(|value| Enclosure::tick(value.to_string_lossy()).to_string()) - .collect::>() - .join(" "), - )?; + CommandStatus { binary, arguments, status} => { + let cmd = format_cmd(binary, arguments); + write!(f, "Command {cmd} failed: {status}")?; } Compile { compile_error } => Display::fmt(compile_error, f)?, Config { config_error } => Display::fmt(config_error, f)?, - Cygpath { - recipe, - output_error, - } => match output_error { - OutputError::Code(code) => { - write!( - f, - "Cygpath failed with exit code {code} while translating recipe `{recipe}` shebang interpreter \ - path" - )?; - } - OutputError::Signal(signal) => { - write!( - f, - "Cygpath terminated by signal {signal} while translating recipe `{recipe}` shebang interpreter \ - path" - )?; - } - OutputError::Unknown => { - write!( - f, - "Cygpath experienced an unknown failure while translating recipe `{recipe}` shebang \ - interpreter path" - )?; - } + Cygpath { recipe, output_error} => match output_error { + OutputError::Code(code) => write!(f, "Cygpath failed with exit code {code} while translating recipe `{recipe}` shebang interpreter path")?, + OutputError::Signal(signal) => write!(f, "Cygpath terminated by signal {signal} while translating recipe `{recipe}` shebang interpreter path")?, + OutputError::Unknown => write!(f, "Cygpath experienced an unknown failure while translating recipe `{recipe}` shebang interpreter path")?, OutputError::Io(io_error) => { match io_error.kind() { - io::ErrorKind::NotFound => write!( - f, - "Could not find `cygpath` executable to translate recipe `{recipe}` shebang interpreter \ - path:\n{io_error}" - ), - io::ErrorKind::PermissionDenied => write!( - f, - "Could not run `cygpath` executable to translate recipe `{recipe}` shebang interpreter \ - path:\n{io_error}" - ), + io::ErrorKind::NotFound => write!(f, "Could not find `cygpath` executable to translate recipe `{recipe}` shebang interpreter path:\n{io_error}"), + io::ErrorKind::PermissionDenied => write!(f, "Could not run `cygpath` executable to translate recipe `{recipe}` shebang interpreter path:\n{io_error}"), _ => write!(f, "Could not run `cygpath` executable:\n{io_error}"), }?; } - OutputError::Utf8(utf8_error) => { - write!( - f, - "Cygpath successfully translated recipe `{recipe}` shebang interpreter path, but output was \ - not utf8: {utf8_error}" - )?; - } - }, - DefaultRecipeRequiresArguments { - recipe, - min_arguments, - } => { - write!( - f, - "Recipe `{recipe}` cannot be used as default recipe since it requires at least {min_arguments} {}.", - Count("argument", *min_arguments), - )?; + OutputError::Utf8(utf8_error) => write!(f, "Cygpath successfully translated recipe `{recipe}` shebang interpreter path, but output was not utf8: {utf8_error}")?, + } + DefaultRecipeRequiresArguments { recipe, min_arguments} => { + let count = Count("argument", *min_arguments); + write!(f, "Recipe `{recipe}` cannot be used as default recipe since it requires at least {min_arguments} {count}.")?; } Dotenv { dotenv_error } => { write!(f, "Failed to load environment file: {dotenv_error}")?; @@ -440,21 +309,16 @@ impl<'src> ColorDisplay for Error<'src> { write!(f, "Failed to dump JSON to stdout: {serde_json_error}")?; } EditorInvoke { editor, io_error } => { - write!( - f, - "Editor `{}` invocation failed: {io_error}", - editor.to_string_lossy(), - )?; + let editor = editor.to_string_lossy(); + write!(f, "Editor `{editor}` invocation failed: {io_error}")?; } EditorStatus { editor, status } => { - write!(f, "Editor `{}` failed: {status}", editor.to_string_lossy(),)?; + let editor = editor.to_string_lossy(); + write!(f, "Editor `{editor}` failed: {status}")?; } - EvalUnknownVariable { - variable, - suggestion, - } => { + EvalUnknownVariable { variable, suggestion} => { write!(f, "Justfile does not contain variable `{variable}`.")?; - if let Some(suggestion) = *suggestion { + if let Some(suggestion) = suggestion { write!(f, "\n{suggestion}")?; } } @@ -462,154 +326,83 @@ impl<'src> ColorDisplay for Error<'src> { write!(f, "Formatted justfile differs from original.")?; } FunctionCall { function, message } => { - write!( - f, - "Call to function `{}` failed: {message}", - function.lexeme() - )?; + let function = function.lexeme(); + write!(f, "Call to function `{function}` failed: {message}")?; + } + IncludeMissingPath { file: justfile, line } => { + let line = line.ordinal(); + let justfile = justfile.display(); + write!(f, "!include directive on line {line} of `{justfile}` has no argument")?; } - IncludeMissingPath { - file: justfile, line - } => { - - write!( - f, - "!include directive on line {} of `{}` has no argument", - line.ordinal(), - justfile.display(), - )?; - - }, InitExists { justfile } => { write!(f, "Justfile `{}` already exists", justfile.display())?; } Internal { message } => { - write!( - f, - "Internal runtime error, this may indicate a bug in just: {message} \ - consider filing an issue: https://github.com/casey/just/issues/new" - )?; + write!(f, "Internal runtime error, this may indicate a bug in just: {message} \ + consider filing an issue: https://github.com/casey/just/issues/new")?; } InvalidDirective { line } => { write!(f, "Invalid directive: {line}")?; } Io { recipe, io_error } => { match io_error.kind() { - io::ErrorKind::NotFound => write!( - f, - "Recipe `{recipe}` could not be run because just could not find the shell: {io_error}" - ), - io::ErrorKind::PermissionDenied => write!( - f, - "Recipe `{recipe}` could not be run because just could not run the shell: {io_error}" - ), - _ => write!( - f, - "Recipe `{recipe}` could not be run because of an IO error while launching the shell: {io_error}" - ), + io::ErrorKind::NotFound => write!(f, "Recipe `{recipe}` could not be run because just could not find the shell: {io_error}"), + io::ErrorKind::PermissionDenied => write!(f, "Recipe `{recipe}` could not be run because just could not run the shell: {io_error}"), + _ => write!(f, "Recipe `{recipe}` could not be run because of an IO error while launching the shell: {io_error}"), }?; } Load { io_error, path } => { - write!( - f, - "Failed to read justfile at `{}`: {io_error}", - path.display() - )?; - } - NoChoosableRecipes => { - write!(f, "Justfile contains no choosable recipes.")?; - } - NoRecipes => { - write!(f, "Justfile contains no recipes.")?; - } - RegexCompile { source } => { - write!(f, "{source}")?; + let path = path.display(); + write!(f, "Failed to read justfile at `{path}`: {io_error}")?; } + NoChoosableRecipes => write!(f, "Justfile contains no choosable recipes.")?, + NoRecipes => write!(f, "Justfile contains no recipes.")?, + RegexCompile { source } => write!(f, "{source}")?, Search { search_error } => Display::fmt(search_error, f)?, - Shebang { - recipe, - command, - argument, - io_error, - } => { + Shebang { recipe, command, argument, io_error} => { if let Some(argument) = argument { - write!( - f, - "Recipe `{recipe}` with shebang `#!{command} {argument}` execution error: {io_error}", - )?; + write!(f, "Recipe `{recipe}` with shebang `#!{command} {argument}` execution error: {io_error}")?; } else { - write!( - f, - "Recipe `{recipe}` with shebang `#!{command}` execution error: {io_error}", - )?; + write!(f, "Recipe `{recipe}` with shebang `#!{command}` execution error: {io_error}")?; } } - Signal { - recipe, - line_number, - signal, - } => { + Signal { recipe, line_number, signal } => { if let Some(n) = line_number { - write!( - f, - "Recipe `{recipe}` was terminated on line {n} by signal {signal}", - )?; + write!(f, "Recipe `{recipe}` was terminated on line {n} by signal {signal}")?; } else { write!(f, "Recipe `{recipe}` was terminated by signal {signal}")?; } } - TmpdirIo { recipe, io_error } => write!( - f, - "Recipe `{recipe}` could not be run because of an IO error while trying to create a temporary \ - directory or write a file to that directory`:{io_error}", - )?, - Unknown { - recipe, - line_number, - } => { + TmpdirIo { recipe, io_error } => { + write!(f, "Recipe `{recipe}` could not be run because of an IO error while trying to create a temporary \ + directory or write a file to that directory`:{io_error}")?; + } + Unknown { recipe, line_number} => { if let Some(n) = line_number { - write!( - f, - "Recipe `{recipe}` failed on line {n} for an unknown reason", - )?; + write!(f, "Recipe `{recipe}` failed on line {n} for an unknown reason")?; } else { write!(f, "Recipe `{recipe}` failed for an unknown reason")?; } } UnknownOverrides { overrides } => { - write!( - f, - "{} {} overridden on the command line but not present in justfile", - Count("Variable", overrides.len()), - List::and_ticked(overrides), - )?; + let count = Count("Variable", overrides.len()); + let overrides = List::and_ticked(overrides); + write!(f, "{count} {overrides} overridden on the command line but not present in justfile")?; } - UnknownRecipes { - recipes, - suggestion, - } => { - write!( - f, - "Justfile does not contain {} {}.", - Count("recipe", recipes.len()), - List::or_ticked(recipes), - )?; - if let Some(suggestion) = *suggestion { + UnknownRecipes { recipes, suggestion } => { + let count = Count("recipe", recipes.len()); + let recipes = List::or_ticked(recipes); + write!(f, "Justfile does not contain {count} {recipes}.")?; + if let Some(suggestion) = suggestion { write!(f, "\n{suggestion}")?; } } Unstable { message } => { - write!( - f, - "{message} Invoke `just` with the `--unstable` flag to enable unstable features." - )?; + write!(f, "{message} Invoke `just` with the `--unstable` flag to enable unstable features.")?; } WriteJustfile { justfile, io_error } => { - write!( - f, - "Failed to write justfile to `{}`: {io_error}", - justfile.display() - )?; + let justfile = justfile.display(); + write!(f, "Failed to write justfile to `{justfile}`: {io_error}")?; } } @@ -634,3 +427,11 @@ impl<'src> ColorDisplay for Error<'src> { Ok(()) } } + +fn format_cmd(binary: &OsString, arguments: &Vec) -> String { + iter::once(binary) + .chain(arguments) + .map(|value| Enclosure::tick(value.to_string_lossy()).to_string()) + .collect::>() + .join(" ") +} diff --git a/tests/edit.rs b/tests/edit.rs index 38f59db..d89efdb 100644 --- a/tests/edit.rs +++ b/tests/edit.rs @@ -82,7 +82,7 @@ fn status_error() { assert!( Regex::new("^error: Editor `exit-2` failed: exit (code|status): 2\n$") .unwrap() - .is_match(str::from_utf8(&output.stderr).unwrap(),) + .is_match(str::from_utf8(&output.stderr).unwrap()) ); assert_eq!(output.status.code().unwrap(), 2); diff --git a/tests/fmt.rs b/tests/fmt.rs index 2a423d8..3afde9c 100644 --- a/tests/fmt.rs +++ b/tests/fmt.rs @@ -98,7 +98,7 @@ fn unstable_passed() { panic!("justfile failed with status: {}", output.status); } - assert_eq!(fs::read_to_string(&justfile).unwrap(), "x := 'hello'\n",); + assert_eq!(fs::read_to_string(&justfile).unwrap(), "x := 'hello'\n"); } #[test]