From d7c62ed05c1021603fb03b55cf23fc121fec92f2 Mon Sep 17 00:00:00 2001 From: Jelle Helsen Date: Thu, 10 Aug 2023 21:20:15 +0200 Subject: [PATCH 1/2] Fix clippy warning on 'format!' strings Should fix #223 --- src/find/matchers/delete.rs | 2 +- src/find/matchers/logical_matchers.rs | 3 +-- src/find/matchers/mod.rs | 11 ++++------- src/find/matchers/printf.rs | 20 ++++++++++---------- src/find/matchers/size.rs | 5 ++--- src/find/matchers/type_matcher.rs | 6 ++---- src/find/mod.rs | 4 ++-- src/xargs/mod.rs | 20 ++++++++++---------- 8 files changed, 32 insertions(+), 39 deletions(-) diff --git a/src/find/matchers/delete.rs b/src/find/matchers/delete.rs index ca13f55e..7869215b 100644 --- a/src/find/matchers/delete.rs +++ b/src/find/matchers/delete.rs @@ -46,7 +46,7 @@ impl Matcher for DeleteMatcher { match self.delete(path, file_info.file_type()) { Ok(_) => true, Err(e) => { - writeln!(&mut stderr(), "Failed to delete {}: {}", path_str, e).unwrap(); + writeln!(&mut stderr(), "Failed to delete {path_str}: {e}").unwrap(); false } } diff --git a/src/find/matchers/logical_matchers.rs b/src/find/matchers/logical_matchers.rs index 6fc5ea3d..6b77be5d 100644 --- a/src/find/matchers/logical_matchers.rs +++ b/src/find/matchers/logical_matchers.rs @@ -155,8 +155,7 @@ impl OrMatcherBuilder { if self.submatchers.last().unwrap().submatchers.is_empty() { return Err(From::from(format!( "invalid expression; you have used a binary operator \ - '{}' with nothing before it.", - arg + '{arg}' with nothing before it." ))); } self.submatchers.push(AndMatcherBuilder::new()); diff --git a/src/find/matchers/mod.rs b/src/find/matchers/mod.rs index d46a12f3..25a760b0 100644 --- a/src/find/matchers/mod.rs +++ b/src/find/matchers/mod.rs @@ -203,9 +203,8 @@ fn convert_arg_to_number( match value_as_string.parse::() { Ok(val) => Ok(val), _ => Err(From::from(format!( - "Expected a positive decimal integer argument to {}, but got \ - `{}'", - option_name, value_as_string + "Expected a positive decimal integer argument to {option_name}, but got \ + `{value_as_string}'" ))), } } @@ -226,8 +225,7 @@ fn convert_arg_to_comparable_value( } Err(From::from(format!( "Expected a decimal integer (with optional + or - prefix) argument \ - to {}, but got `{}'", - option_name, value_as_string + to {option_name}, but got `{value_as_string}'" ))) } @@ -250,8 +248,7 @@ fn convert_arg_to_comparable_value_and_suffix( } Err(From::from(format!( "Expected a decimal integer (with optional + or - prefix) and \ - (optional suffix) argument to {}, but got `{}'", - option_name, value_as_string + (optional suffix) argument to {option_name}, but got `{value_as_string}'" ))) } diff --git a/src/find/matchers/printf.rs b/src/find/matchers/printf.rs index 71d9bf45..484c3963 100644 --- a/src/find/matchers/printf.rs +++ b/src/find/matchers/printf.rs @@ -166,7 +166,7 @@ impl FormatStringParser<'_> { let octal = self.advance_by(OCTAL_LEN).unwrap(); return match char::from_u32(code) { Some(c) => Ok(FormatComponent::Literal(c.to_string())), - None => Err(format!("Invalid character value: \\{}", octal).into()), + None => Err(format!("Invalid character value: \\{octal}").into()), }; } } @@ -186,7 +186,7 @@ impl FormatStringParser<'_> { 'v' => "\x0B", '0' => "\0", '\\' => "\\", - c => return Err(format!("Invalid escape sequence: \\{}", c).into()), + c => return Err(format!("Invalid escape sequence: \\{c}").into()), }; Ok(FormatComponent::Literal(c.to_string())) @@ -220,10 +220,10 @@ impl FormatStringParser<'_> { // We can't store the parsed items inside TimeFormat, because the items // take a reference to the full format string, but we still try to parse // it here so that errors get caught early. - let format = format!("%{}", c); + let format = format!("%{c}"); match StrftimeItems::new(&format).next() { None | Some(chrono::format::Item::Error) => { - Err(format!("Invalid time specifier: %{}{}", first, c).into()) + Err(format!("Invalid time specifier: %{first}{c}").into()) } Some(_item) => Ok(TimeFormat::Strftime(format)), } @@ -318,7 +318,7 @@ impl FormatStringParser<'_> { let component = match self.advance_one().unwrap() { '\\' => self.parse_escape_sequence()?, '%' => self.parse_format_specifier()?, - _ => panic!("Stopped at unexpected character: {}", self.string), + _ => panic!("{}", "Stopped at unexpected character: {self.string}"), }; components.push(component); } @@ -620,7 +620,7 @@ impl Matcher for Printf { for component in &self.format.components { match component { - FormatComponent::Literal(literal) => write!(out, "{}", literal).unwrap(), + FormatComponent::Literal(literal) => write!(out, "{literal}").unwrap(), FormatComponent::Flush => out.flush().unwrap(), FormatComponent::Directive { directive, @@ -631,14 +631,14 @@ impl Matcher for Printf { if let Some(width) = width { match justify { Justify::Left => { - write!(out, "{: { - write!(out, "{:>width$}", content, width = width).unwrap(); + write!(out, "{content:>width$}").unwrap(); } } } else { - write!(out, "{}", content).unwrap(); + write!(out, "{content}").unwrap(); } } Err(e) => { @@ -1102,7 +1102,7 @@ mod tests { let matcher = Printf::new("%u %U %g %G").unwrap(); assert!(matcher.matches(&file_info, &mut deps.new_matcher_io())); assert_eq!( - format!("{} {} {} {}", user, uid, group, gid), + format!("{user} {uid} {group} {gid}"), deps.get_output_as_string() ); } diff --git a/src/find/matchers/size.rs b/src/find/matchers/size.rs index 06b5bb81..9832fa6e 100644 --- a/src/find/matchers/size.rs +++ b/src/find/matchers/size.rs @@ -33,9 +33,8 @@ impl FromStr for Unit { "G" => Self::GibiByte, _ => { return Err(From::from(format!( - "Invalid suffix {} for -size. Only allowed \ - values are , b, c, w, k, M or G", - s + "Invalid suffix {s} for -size. Only allowed \ + values are , b, c, w, k, M or G" ))); } }) diff --git a/src/find/matchers/type_matcher.rs b/src/find/matchers/type_matcher.rs index 1da95697..e489a012 100644 --- a/src/find/matchers/type_matcher.rs +++ b/src/find/matchers/type_matcher.rs @@ -32,14 +32,12 @@ impl TypeMatcher { // D: door (Solaris) "D" => { return Err(From::from(format!( - "Type argument {} not supported yet", - type_string + "Type argument {type_string} not supported yet" ))) } _ => { return Err(From::from(format!( - "Unrecognised type argument {}", - type_string + "Unrecognised type argument {type_string}" ))) } }; diff --git a/src/find/mod.rs b/src/find/mod.rs index bdccd0e1..14cef297 100644 --- a/src/find/mod.rs +++ b/src/find/mod.rs @@ -149,7 +149,7 @@ fn process_dir<'a>( let mut it = walkdir.into_iter(); while let Some(result) = it.next() { match result { - Err(err) => writeln!(&mut stderr(), "Error: {}: {}", dir, err).unwrap(), + Err(err) => writeln!(&mut stderr(), "Error: {dir}: {err}").unwrap(), Ok(entry) => { let mut matcher_io = matchers::MatcherIO::new(deps); if matcher.matches(&entry, &mut matcher_io) { @@ -255,7 +255,7 @@ pub fn find_main<'a>(args: &[&str], deps: &'a dyn Dependencies<'a>) -> i32 { match do_find(&args[1..], deps) { Ok(_) => 0, Err(e) => { - writeln!(&mut stderr(), "Error: {}", e).unwrap(); + writeln!(&mut stderr(), "Error: {e}").unwrap(); 1 } } diff --git a/src/xargs/mod.rs b/src/xargs/mod.rs index d0f203e2..b86b1993 100644 --- a/src/xargs/mod.rs +++ b/src/xargs/mod.rs @@ -318,9 +318,9 @@ impl Display for CommandExecutionError { match self { CommandExecutionError::UrgentlyFailed => write!(f, "Command exited with code 255"), CommandExecutionError::Killed { signal } => { - write!(f, "Command was killed with signal {}", signal) + write!(f, "Command was killed with signal {signal}") } - CommandExecutionError::CannotRun(err) => write!(f, "Command could not be run: {}", err), + CommandExecutionError::CannotRun(err) => write!(f, "Command could not be run: {err}"), CommandExecutionError::NotFound => write!(f, "Command not found"), CommandExecutionError::Unknown => write!(f, "Unknown error running command"), } @@ -407,7 +407,7 @@ impl CommandBuilder<'_> { command.stdin(Stdio::null()); } if self.options.verbose { - eprintln!("{:?}", command); + eprintln!("{command:?}"); } match &self.options.action { @@ -511,7 +511,7 @@ where if let Some(Escape::Quote(q)) = &escape { return Err(io::Error::new( io::ErrorKind::InvalidInput, - format!("Unterminated quote: {}", q), + format!("Unterminated quote: {q}"), )); } else if i == 0 { return Ok(None); @@ -622,9 +622,9 @@ impl Display for XargsError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { XargsError::ArgumentTooLarge => write!(f, "Argument too large"), - XargsError::CommandExecution(e) => write!(f, "{}", e), - XargsError::Io(e) => write!(f, "{}", e), - XargsError::Untyped(s) => write!(f, "{}", s), + XargsError::CommandExecution(e) => write!(f, "{e}"), + XargsError::Io(e) => write!(f, "{e}"), + XargsError::Untyped(s) => write!(f, "{s}"), } } } @@ -707,7 +707,7 @@ fn parse_delimiter(s: &str) -> Result { "v" => Ok(b'\x0B'), "0" => Ok(b'\0'), "\\" => Ok(b'\\'), - _ => Err(format!("Invalid escape sequence: {}", s)), + _ => Err(format!("Invalid escape sequence: {s}")), } } else { let bytes = s.as_bytes(); @@ -722,7 +722,7 @@ fn parse_delimiter(s: &str) -> Result { fn validate_positive_usize(s: String) -> Result<(), String> { match s.parse::() { Ok(v) if v > 0 => Ok(()), - Ok(v) => Err(format!("Value must be > 0, not: {}", v)), + Ok(v) => Err(format!("Value must be > 0, not: {v}")), Err(e) => Err(e.to_string()), } } @@ -922,7 +922,7 @@ pub fn xargs_main(args: &[&str]) -> i32 { Ok(CommandResult::Success) => 0, Ok(CommandResult::Failure) => 123, Err(e) => { - eprintln!("Error: {}", e); + eprintln!("Error: {e}"); if let XargsError::CommandExecution(cx) = e { match cx { CommandExecutionError::UrgentlyFailed => 124, From 6068512549aef5a9e16e72e1db9333472f8b6de1 Mon Sep 17 00:00:00 2001 From: Jelle Helsen Date: Sun, 13 Aug 2023 21:27:46 +0200 Subject: [PATCH 2/2] added some tests to improve coverage --- tests/xargs_tests.rs | 60 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/xargs_tests.rs b/tests/xargs_tests.rs index 90374f03..45059ea6 100644 --- a/tests/xargs_tests.rs +++ b/tests/xargs_tests.rs @@ -324,3 +324,63 @@ fn xargs_exec_not_found() { .stderr(predicate::str::contains("Error:")) .stdout(predicate::str::is_empty()); } + +#[test] +fn xargs_exec_verbose() { + Command::cargo_bin("xargs") + .expect("found binary") + .args([ + "-n2", + "--verbose", + &path_to_testing_commandline(), + "-", + "--print_stdin", + "--no_print_cwd", + ]) + .write_stdin("a b c\nd") + .assert() + .success() + .stderr(predicate::str::contains("testing-commandline")) + .stdout(predicate::str::diff( + "stdin=\nargs=\n--print_stdin\n--no_print_cwd\na\nb\n\ + stdin=\nargs=\n--print_stdin\n--no_print_cwd\nc\nd\n", + )); +} + +#[test] +fn xargs_unterminated_quote() { + Command::cargo_bin("xargs") + .expect("found binary") + .args([ + "-n2", + &path_to_testing_commandline(), + "-", + "--print_stdin", + "--no_print_cwd", + ]) + .write_stdin("a \"b c\nd") + .assert() + .failure() + .code(1) + .stderr(predicate::str::contains("Error: Unterminated quote:")) + .stdout(predicate::str::is_empty()); +} + +#[test] +fn xargs_zero_lines() { + Command::cargo_bin("xargs") + .expect("found binary") + .args([ + "-L0", + &path_to_testing_commandline(), + "-", + "--print_stdin", + "--no_print_cwd", + ]) + .write_stdin("a \"b c\nd") + .assert() + .failure() + .code(1) + .stderr(predicate::str::contains("Value must be > 0, not: 0")) + .stdout(predicate::str::is_empty()); +}