From 241732da494f9029d212307ee7c068eeb4d66363 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 26 Sep 2026 19:36:45 +0200 Subject: [PATCH 1/6] find: match GNU's wording for expression errors Quote the predicate or operator as GNU 4.10.0 does, report an operator cut off by ')' and an unmatched ')' with GNU's messages, and name a binary operator as spelled. --- src/find/matchers/logical_matchers.rs | 10 +- src/find/matchers/mod.rs | 157 ++++++++++++++++---------- tests/test_find.rs | 8 +- 3 files changed, 104 insertions(+), 71 deletions(-) diff --git a/src/find/matchers/logical_matchers.rs b/src/find/matchers/logical_matchers.rs index a892e12c..35af99b3 100644 --- a/src/find/matchers/logical_matchers.rs +++ b/src/find/matchers/logical_matchers.rs @@ -252,16 +252,16 @@ impl ListMatcherBuilder { self.submatchers.last_mut().unwrap().new_or_condition(arg) } - pub fn check_new_and_condition(&mut self) -> Result<(), Box> { + pub fn check_new_and_condition(&mut self, arg: &str) -> Result<(), Box> { { let child_or_matcher = &self.submatchers.last().unwrap(); let grandchild_and_matcher = &child_or_matcher.submatchers.last().unwrap(); if grandchild_and_matcher.submatchers.is_empty() { - return Err(From::from( - "invalid expression; you have used a binary operator '-a' \ - with nothing before it.", - )); + return Err(From::from(format!( + "invalid expression; you have used a binary operator \ + '{arg}' with nothing before it." + ))); } } Ok(()) diff --git a/src/find/matchers/mod.rs b/src/find/matchers/mod.rs index a33d47c8..f0fb14a4 100644 --- a/src/find/matchers/mod.rs +++ b/src/find/matchers/mod.rs @@ -456,6 +456,22 @@ fn get_or_create_file(path: &str) -> Result> { Ok(file) } +/// The error for a predicate at `args[index]` that is missing its argument. +fn missing_argument_error(args: &[&str], index: usize) -> String { + format!("missing argument to `{}'", args[index]) +} + +/// The error for an operator at `args[index]` that has nothing to apply to, +/// either because the expression ends there or because a ')' closes it. +fn missing_operand_error(args: &[&str], index: usize) -> String { + let operator = args[index]; + if args.get(index + 1) == Some(&")") { + format!("expected an expression between '{operator}' and ')'") + } else { + format!("expected an expression after '{operator}'") + } +} + /// The main "translate command-line args into a matcher" function. Will call /// itself recursively if it encounters an opening bracket. A successful return /// consists of a tuple containing the new index into the args array to use (if @@ -482,14 +498,14 @@ fn build_matcher_tree( "-print0" => Some(Printer::new(PrintDelimiter::Null, None).into_box()), "-printf" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } i += 1; Some(Printf::new(args[i], None)?.into_box()) } "-fprint" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } i += 1; @@ -498,7 +514,7 @@ fn build_matcher_tree( } "-fprintf" => { if i + 2 >= args.len() { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } // Action: -fprintf file format @@ -512,7 +528,7 @@ fn build_matcher_tree( } "-fprint0" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } i += 1; @@ -522,7 +538,7 @@ fn build_matcher_tree( "-ls" => Some(Ls::new(None).into_box()), "-fls" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } i += 1; @@ -533,21 +549,21 @@ fn build_matcher_tree( "-false" => Some(FalseMatcher.into_box()), "-lname" | "-ilname" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } i += 1; Some(LinkNameMatcher::new(args[i], args[i - 1].starts_with("-i")).into_box()) } "-name" | "-iname" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } i += 1; Some(NameMatcher::new(args[i], args[i - 1].starts_with("-i")).into_box()) } "-path" | "-ipath" | "-wholename" | "-iwholename" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } i += 1; Some(PathMatcher::new(args[i], args[i - 1].starts_with("-i")).into_box()) @@ -555,7 +571,7 @@ fn build_matcher_tree( "-readable" => Some(AccessMatcher::Readable.into_box()), "-regextype" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } i += 1; regex_type = regex::RegexType::from_str(args[i])?; @@ -563,35 +579,35 @@ fn build_matcher_tree( } "-regex" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } i += 1; Some(RegexMatcher::new(regex_type, args[i], false)?.into_box()) } "-iregex" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } i += 1; Some(RegexMatcher::new(regex_type, args[i], true)?.into_box()) } "-type" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } i += 1; Some(TypeMatcher::new(args[i])?.into_box()) } "-xtype" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } i += 1; Some(XtypeMatcher::new(args[i])?.into_box()) } "-fstype" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } i += 1; Some(FileSystemMatcher::new(args[i].to_string()).into_box()) @@ -603,14 +619,14 @@ fn build_matcher_tree( } "-newer" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } i += 1; Some(NewerMatcher::new(args[i], config.follow)?.into_box()) } "-mtime" | "-atime" | "-ctime" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } let file_time_type = match args[i] { "-atime" => FileTimeType::Accessed, @@ -626,7 +642,7 @@ fn build_matcher_tree( } "-amin" | "-cmin" | "-mmin" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } let file_time_type = match args[i] { "-amin" => FileTimeType::Accessed, @@ -643,7 +659,7 @@ fn build_matcher_tree( } "-size" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } let (size, unit) = convert_arg_to_comparable_value_and_suffix(args[i], args[i + 1])?; @@ -667,7 +683,7 @@ fn build_matcher_tree( if arg_index < i + required_arg || arg_index == args.len() { // at the minimum we need the executable and the ';' // or the executable and the '{} +' - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } let expression = args[i]; if expression == "-execdir" { @@ -710,7 +726,7 @@ fn build_matcher_tree( } if arg_index < i + 2 || arg_index == args.len() { // Need at least the executable and the terminating ';'. - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } let expression = args[i]; if expression == "-okdir" { @@ -732,7 +748,7 @@ fn build_matcher_tree( #[cfg(unix)] "-inum" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } let inum = convert_arg_to_comparable_value(args[i], args[i + 1])?; i += 1; @@ -747,7 +763,7 @@ fn build_matcher_tree( #[cfg(unix)] "-links" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } let inum = convert_arg_to_comparable_value(args[i], args[i + 1])?; i += 1; @@ -759,7 +775,7 @@ fn build_matcher_tree( } "-samefile" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } i += 1; let path = args[i]; @@ -769,7 +785,7 @@ fn build_matcher_tree( } "-user" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } let user = args[i + 1]; @@ -789,7 +805,7 @@ fn build_matcher_tree( "-nouser" => Some(NoUserMatcher {}.into_box()), "-uid" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } // check if the argument is a number let uid = convert_arg_to_comparable_value(args[i], args[i + 1])?; @@ -798,7 +814,7 @@ fn build_matcher_tree( } "-group" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } let group = args[i + 1]; @@ -820,7 +836,7 @@ fn build_matcher_tree( "-nogroup" => Some(NoGroupMatcher {}.into_box()), "-gid" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } // check if the argument is a number let gid = convert_arg_to_comparable_value(args[i], args[i + 1])?; @@ -830,7 +846,7 @@ fn build_matcher_tree( "-executable" => Some(AccessMatcher::Executable.into_box()), "-perm" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } i += 1; Some(PermMatcher::new(args[i])?.into_box()) @@ -840,40 +856,28 @@ fn build_matcher_tree( "-writable" => Some(AccessMatcher::Writable.into_box()), "-not" | "!" => { if !are_more_expressions(args, i) { - return Err(From::from(format!( - "expected an expression after {}", - args[i] - ))); + return Err(missing_operand_error(args, i).into()); } invert_next_matcher = !invert_next_matcher; None } "-and" | "-a" => { if !are_more_expressions(args, i) { - return Err(From::from(format!( - "expected an expression after {}", - args[i] - ))); + return Err(missing_operand_error(args, i).into()); } - top_level_matcher.check_new_and_condition()?; + top_level_matcher.check_new_and_condition(args[i])?; None } "-or" | "-o" => { if !are_more_expressions(args, i) { - return Err(From::from(format!( - "expected an expression after {}", - args[i] - ))); + return Err(missing_operand_error(args, i).into()); } top_level_matcher.new_or_condition(args[i])?; None } "," => { if !are_more_expressions(args, i) { - return Err(From::from(format!( - "expected an expression after {}", - args[i] - ))); + return Err(missing_operand_error(args, i).into()); } top_level_matcher.new_list_condition()?; None @@ -885,9 +889,7 @@ fn build_matcher_tree( } ")" => { if !expecting_bracket { - return Err(From::from( - "invalid expression: expected expression before closing parentheses ')'.", - )); + return Err(From::from("you have too many ')'")); } let bracket = args[i - 1]; @@ -940,7 +942,7 @@ fn build_matcher_tree( } "-maxdepth" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } config.max_depth = convert_arg_to_number(args[i], args[i + 1])?; i += 1; @@ -948,7 +950,7 @@ fn build_matcher_tree( } "-mindepth" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } config.min_depth = convert_arg_to_number(args[i], args[i + 1])?; i += 1; @@ -964,7 +966,7 @@ fn build_matcher_tree( } "-files0-from" => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } let _ = config.files0_argument.insert(args[i + 1].to_string()); i += 1; @@ -975,7 +977,7 @@ fn build_matcher_tree( match parse_str_to_newer_args(args[i]) { Some((x_option, y_option)) => { if i >= args.len() - 1 { - return Err(From::from(format!("missing argument to {}", args[i]))); + return Err(missing_argument_error(args, i).into()); } #[cfg(target_os = "linux")] if x_option == "B" { @@ -1158,8 +1160,7 @@ mod tests { let mut config = Config::default(); if let Err(e) = build_top_level_matcher(&[arg], &mut config) { - assert!(e.to_string().contains("missing argument to")); - assert!(e.to_string().contains(arg)); + assert_eq!(e.to_string(), format!("missing argument to `{arg}'")); } else { panic!("parsing argument lists that end in -not should fail"); } @@ -1194,12 +1195,21 @@ mod tests { #[test] fn build_top_level_matcher_and_without_expr1() { - let mut config = Config::default(); + // The operator is named as it was spelled, not normalised to `-a`. + for arg in ["-and", "-a"] { + let mut config = Config::default(); - if let Err(e) = build_top_level_matcher(&["-a", "-true"], &mut config) { - assert!(e.to_string().contains("you have used a binary operator")); - } else { - panic!("parsing argument list that begins with -a should fail"); + if let Err(e) = build_top_level_matcher(&[arg, "-true"], &mut config) { + assert_eq!( + e.to_string(), + format!( + "invalid expression; you have used a binary operator \ + '{arg}' with nothing before it." + ) + ); + } else { + panic!("parsing argument list that begins with {arg} should fail"); + } } } @@ -1354,14 +1364,37 @@ mod tests { &["-type", "f", "(", "-name", "*.txt", ")", ")"], &mut config, ) { - assert!(e - .to_string() - .contains("expected expression before closing parentheses ')'")); + assert_eq!(e.to_string(), "you have too many ')'"); } else { panic!("parsing argument list with too many closing brackets should fail"); } } + #[test] + fn build_top_level_matcher_operator_without_operand() { + // An operator left dangling at the end names only itself; one cut off + // by a ')' names both, since the ')' is what ended the expression. + let cases: [(&[&str], &str); 4] = [ + (&["-false", ","], "expected an expression after ','"), + (&["-not"], "expected an expression after '-not'"), + ( + &["(", "-empty", "-or", ")"], + "expected an expression between '-or' and ')'", + ), + ( + &["(", "!", ")", "-print"], + "expected an expression between '!' and ')'", + ), + ]; + for (args, expected) in cases { + let mut config = Config::default(); + match build_top_level_matcher(args, &mut config) { + Err(e) => assert_eq!(e.to_string(), expected), + Ok(_) => panic!("{args:?} should fail to parse"), + } + } + } + #[test] fn build_top_level_matcher_can_use_bracket_as_arg() { let mut config = Config::default(); diff --git a/tests/test_find.rs b/tests/test_find.rs index 2a19a3ed..93efa223 100644 --- a/tests/test_find.rs +++ b/tests/test_find.rs @@ -434,7 +434,7 @@ fn files0_basic() { ucmd() .arg("-files0-from") .fails() - .stderr_contains("missing argument to -files0-from") + .stderr_contains("missing argument to `-files0-from'") .no_stdout(); } @@ -1340,12 +1340,12 @@ fn find_fprintf_missing_arguments() { ucmd() .args(&["-fprintf"]) .fails() - .stderr_contains("missing argument to -fprintf"); + .stderr_contains("missing argument to `-fprintf'"); ucmd() .args(&["-fprintf", "/tmp/find_fprintf_out"]) .fails() - .stderr_contains("missing argument to -fprintf"); + .stderr_contains("missing argument to `-fprintf'"); } #[test] @@ -1542,7 +1542,7 @@ fn find_ok_missing_semicolon() { .args(&["test_data/simple", "-ok", "echo", "{}"]) .pipe_in("") .fails() - .stderr_contains("missing argument to -ok") + .stderr_contains("missing argument to `-ok'") .no_stdout(); } From b15d29a1aee6169a805fd06bce94d056d7f17e4f Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Wed, 2 Sep 2026 09:11:55 +0200 Subject: [PATCH 2/6] find: carry the '(' index instead of a bool through the parser Lets an unclosed '(' be pointed at later. No behaviour change. --- src/find/matchers/mod.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/find/matchers/mod.rs b/src/find/matchers/mod.rs index f0fb14a4..0a3ce862 100644 --- a/src/find/matchers/mod.rs +++ b/src/find/matchers/mod.rs @@ -286,7 +286,7 @@ pub fn build_top_level_matcher( args: &[&str], config: &mut Config, ) -> Result, Box> { - let (_, top_level_matcher) = (build_matcher_tree(args, config, 0, false))?; + let (_, top_level_matcher) = (build_matcher_tree(args, config, 0, None))?; // if the matcher doesn't have any side-effects, then we default to printing if !top_level_matcher.has_side_effects() { @@ -476,11 +476,15 @@ fn missing_operand_error(args: &[&str], index: usize) -> String { /// itself recursively if it encounters an opening bracket. A successful return /// consists of a tuple containing the new index into the args array to use (if /// called recursively) and the resulting matcher. +/// +/// `open_bracket` is the index of the '(' this call is parsing the contents of, +/// or `None` at the top level. It doubles as the "a ')' is expected" flag and +/// lets an unclosed bracket be pointed at in the error. fn build_matcher_tree( args: &[&str], config: &mut Config, arg_index: usize, - mut expecting_bracket: bool, + mut open_bracket: Option, ) -> Result<(usize, Box), Box> { let mut top_level_matcher = ListMatcherBuilder::new(); @@ -883,12 +887,13 @@ fn build_matcher_tree( None } "(" => { - let (new_arg_index, sub_matcher) = build_matcher_tree(args, config, i + 1, true)?; + let (new_arg_index, sub_matcher) = + build_matcher_tree(args, config, i + 1, Some(i))?; i = new_arg_index; Some(sub_matcher) } ")" => { - if !expecting_bracket { + if open_bracket.is_none() { return Err(From::from("you have too many ')'")); } @@ -1014,7 +1019,7 @@ fn build_matcher_tree( i += 1; if config.help_requested || config.version_requested { // Ignore anything, even invalid expressions, after -help/-version - expecting_bracket = false; + open_bracket = None; break; } if let Some(submatcher) = possible_submatcher { @@ -1026,7 +1031,7 @@ fn build_matcher_tree( } } } - if expecting_bracket { + if open_bracket.is_some() { return Err(From::from( "invalid expression; I was expecting to find a ')' somewhere but \ did not see one.", From 79832411e59fd595d906c533d8d1346d34de6b1a Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 26 Sep 2026 19:38:12 +0200 Subject: [PATCH 3/6] find: record which argument each expression error refers to Add a `ParseError` carrying the argument index and a label. Its Display is the bare message, so output is unchanged. --- src/find/error.rs | 97 +++++++++++++++++++++++ src/find/matchers/mod.rs | 161 +++++++++++++++++++++++++++------------ src/find/mod.rs | 8 +- 3 files changed, 215 insertions(+), 51 deletions(-) create mode 100644 src/find/error.rs diff --git a/src/find/error.rs b/src/find/error.rs new file mode 100644 index 00000000..a2c7cc6f --- /dev/null +++ b/src/find/error.rs @@ -0,0 +1,97 @@ +// This file is part of the uutils findutils package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. + +//! Expression parse errors that remember which argument they refer to. +//! `Display` is the plain GNU message. + +use std::error::Error; + +/// A command-line expression error that can point at the argument that caused it. +#[derive(Debug, thiserror::Error)] +#[error("{message}")] +pub struct ParseError { + message: String, + /// Index of the offending argument, relative to the argument slice this + /// error was created in. [`ParseError::shift`] moves it along at each + /// parsing-layer boundary until it indexes the process's full argv. + arg_index: Option, + label: Option, +} + +impl ParseError { + pub fn new(message: impl Into) -> Self { + Self { + message: message.into(), + arg_index: None, + label: None, + } + } + + /// Records which argument the error refers to. + pub fn at(mut self, arg_index: usize) -> Self { + self.arg_index = Some(arg_index); + self + } + + /// Sets the text shown under the underlined argument. + pub fn with_label(mut self, label: impl Into) -> Self { + self.label = Some(label.into()); + self + } + + pub fn arg_index(&self) -> Option { + self.arg_index + } + + /// Adds `by` to the argument index of a `ParseError`, for callers that + /// parsed a sub-slice of argv. Other errors pass through untouched. + pub fn shift(err: Box, by: usize) -> Box { + match err.downcast::() { + Ok(mut parse_error) => { + if let Some(index) = parse_error.arg_index { + parse_error.arg_index = Some(index + by); + } + parse_error + } + Err(other) => other, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn display_is_the_bare_message() { + let error = ParseError::new("unknown predicate `-zap'") + .at(3) + .with_label("not a known predicate"); + assert_eq!(error.to_string(), "unknown predicate `-zap'"); + } + + #[test] + fn shift_moves_the_index_and_composes() { + let error: Box = Box::new(ParseError::new("oops").at(2)); + let error = ParseError::shift(ParseError::shift(error, 3), 1); + let shifted = error.downcast_ref::().unwrap(); + assert_eq!(shifted.arg_index(), Some(6)); + } + + #[test] + fn shift_leaves_indexless_and_foreign_errors_alone() { + let indexless: Box = Box::new(ParseError::new("oops")); + let indexless = ParseError::shift(indexless, 4); + assert_eq!( + indexless.downcast_ref::().unwrap().arg_index(), + None + ); + + let foreign: Box = From::from("plain message"); + let foreign = ParseError::shift(foreign, 4); + assert_eq!(foreign.to_string(), "plain message"); + assert!(foreign.downcast_ref::().is_none()); + } +} diff --git a/src/find/matchers/mod.rs b/src/find/matchers/mod.rs index 0a3ce862..299795e8 100644 --- a/src/find/matchers/mod.rs +++ b/src/find/matchers/mod.rs @@ -70,6 +70,7 @@ use std::{ time::SystemTime, }; +use super::error::ParseError; use super::{Config, Dependencies}; pub use entry::{FileType, WalkEntry, WalkError}; @@ -456,19 +457,35 @@ fn get_or_create_file(path: &str) -> Result> { Ok(file) } +/// Turns one of the `logical_matchers` "binary operator with nothing before it" +/// errors into a diagnostic pointing at the operator. +fn binary_operator_error(err: &dyn Error, arg_index: usize) -> ParseError { + ParseError::new(err.to_string()) + .at(arg_index) + .with_label("no expression before this operator") +} + /// The error for a predicate at `args[index]` that is missing its argument. -fn missing_argument_error(args: &[&str], index: usize) -> String { - format!("missing argument to `{}'", args[index]) +fn missing_argument_error(args: &[&str], index: usize) -> ParseError { + ParseError::new(format!("missing argument to `{}'", args[index])) + .at(index) + .with_label("this predicate needs an argument") } /// The error for an operator at `args[index]` that has nothing to apply to, /// either because the expression ends there or because a ')' closes it. -fn missing_operand_error(args: &[&str], index: usize) -> String { +fn missing_operand_error(args: &[&str], index: usize) -> ParseError { let operator = args[index]; if args.get(index + 1) == Some(&")") { - format!("expected an expression between '{operator}' and ')'") + ParseError::new(format!( + "expected an expression between '{operator}' and ')'" + )) + .at(index) + .with_label("nothing between this operator and the ')'") } else { - format!("expected an expression after '{operator}'") + ParseError::new(format!("expected an expression after '{operator}'")) + .at(index) + .with_label("nothing follows this operator") } } @@ -869,21 +886,27 @@ fn build_matcher_tree( if !are_more_expressions(args, i) { return Err(missing_operand_error(args, i).into()); } - top_level_matcher.check_new_and_condition(args[i])?; + top_level_matcher + .check_new_and_condition(args[i]) + .map_err(|e| binary_operator_error(e.as_ref(), i))?; None } "-or" | "-o" => { if !are_more_expressions(args, i) { return Err(missing_operand_error(args, i).into()); } - top_level_matcher.new_or_condition(args[i])?; + top_level_matcher + .new_or_condition(args[i]) + .map_err(|e| binary_operator_error(e.as_ref(), i))?; None } "," => { if !are_more_expressions(args, i) { return Err(missing_operand_error(args, i).into()); } - top_level_matcher.new_list_condition()?; + top_level_matcher + .new_list_condition() + .map_err(|e| binary_operator_error(e.as_ref(), i))?; None } "(" => { @@ -893,15 +916,21 @@ fn build_matcher_tree( Some(sub_matcher) } ")" => { - if open_bracket.is_none() { - return Err(From::from("you have too many ')'")); - } + let Some(open_bracket_index) = open_bracket else { + return Err(ParseError::new("you have too many ')'") + .at(i) + .with_label("no matching '(' before this") + .into()); + }; let bracket = args[i - 1]; if bracket == "(" { - return Err(From::from( + return Err(ParseError::new( "invalid expression; empty parentheses are not allowed.", - )); + ) + .at(open_bracket_index) + .with_label("nothing between these parentheses") + .into()); } return Ok((i, top_level_matcher.build())); @@ -979,40 +1008,38 @@ fn build_matcher_tree( } _ => { - match parse_str_to_newer_args(args[i]) { - Some((x_option, y_option)) => { - if i >= args.len() - 1 { - return Err(missing_argument_error(args, i).into()); - } - #[cfg(target_os = "linux")] - if x_option == "B" { - return Err(From::from("This system does not provide a way to find the birth time of a file.")); - } - if y_option == "t" { - let time = args[i + 1]; - let newer_time_type = NewerOptionType::from_str(x_option.as_str()); - // Convert args to unix timestamps. (expressed in numeric types) - let Some(comparable_time) = parse_date_str_to_timestamps(time) else { - return Err(From::from(format!( - "I cannot figure out how to interpret ‘{}’ as a date or time", - args[i + 1] - ))); - }; - i += 1; - Some(NewerTimeMatcher::new(newer_time_type, comparable_time).into_box()) - } else { - let file_path = args[i + 1]; - i += 1; - Some( - NewerOptionMatcher::new(&x_option, &y_option, file_path)? - .into_box(), - ) - } - } - // Match GNU find wording for unknown predicates. - None => { - return Err(From::from(format!("unknown predicate `{}'", args[i]))); - } + // Match GNU find wording for unknown predicates. + let Some((x_option, y_option)) = parse_str_to_newer_args(args[i]) else { + return Err(ParseError::new(format!("unknown predicate `{}'", args[i])) + .at(i) + .with_label("not a known predicate") + .into()); + }; + if i >= args.len() - 1 { + return Err(missing_argument_error(args, i).into()); + } + #[cfg(target_os = "linux")] + if x_option == "B" { + return Err(From::from( + "This system does not provide a way to find the birth time of a file.", + )); + } + if y_option == "t" { + let time = args[i + 1]; + let newer_time_type = NewerOptionType::from_str(x_option.as_str()); + // Convert args to unix timestamps. (expressed in numeric types) + let Some(comparable_time) = parse_date_str_to_timestamps(time) else { + return Err(From::from(format!( + "I cannot figure out how to interpret ‘{}’ as a date or time", + args[i + 1] + ))); + }; + i += 1; + Some(NewerTimeMatcher::new(newer_time_type, comparable_time).into_box()) + } else { + let file_path = args[i + 1]; + i += 1; + Some(NewerOptionMatcher::new(&x_option, &y_option, file_path)?.into_box()) } } }; @@ -1031,11 +1058,14 @@ fn build_matcher_tree( } } } - if open_bracket.is_some() { - return Err(From::from( + if let Some(open_bracket_index) = open_bracket { + return Err(ParseError::new( "invalid expression; I was expecting to find a ')' somewhere but \ did not see one.", - )); + ) + .at(open_bracket_index) + .with_label("this parenthesis is never closed") + .into()); } Ok((i, top_level_matcher.build())) } @@ -1937,6 +1967,37 @@ mod tests { } } + /// Parses `args`, expecting a failure, and returns the index of the + /// argument the resulting diagnostic points at. + fn failing_arg_index(args: &[&str]) -> Option { + let mut config = Config::default(); + let err = build_top_level_matcher(args, &mut config) + .err() + .expect("expression should have been rejected"); + err.downcast_ref::() + .expect("expression errors should carry an argument index") + .arg_index() + } + + #[test] + fn expression_errors_point_at_the_offending_argument() { + // The predicate that is missing its argument, not the end of the list. + assert_eq!(failing_arg_index(&["-true", "-a", "-name"]), Some(2)); + // The mistyped predicate. + assert_eq!(failing_arg_index(&["-true", "-o", "-nmae", "x"]), Some(2)); + // The operator with nothing after it. + assert_eq!(failing_arg_index(&["-true", "-o"]), Some(1)); + // The operator with nothing before it. + assert_eq!(failing_arg_index(&["-o", "-true"]), Some(0)); + assert_eq!(failing_arg_index(&[",", "-true"]), Some(0)); + // The '(' that is never closed, rather than the end of the list. + assert_eq!(failing_arg_index(&["-true", "(", "-false"]), Some(1)); + // The '(' of the empty pair, rather than its ')'. + assert_eq!(failing_arg_index(&["-true", "(", ")"]), Some(1)); + // The ')' that has no opener. + assert_eq!(failing_arg_index(&["-true", ")"]), Some(1)); + } + #[test] fn build_top_level_matcher_option_logical() { let mut config = Config::default(); diff --git a/src/find/mod.rs b/src/find/mod.rs index cbc49f7c..3007f50a 100644 --- a/src/find/mod.rs +++ b/src/find/mod.rs @@ -4,8 +4,10 @@ // license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. +pub mod error; pub mod matchers; +use error::ParseError; use matchers::{Follow, WalkEntry}; use std::cell::RefCell; use std::error::Error; @@ -239,7 +241,11 @@ fn parse_args(args: &[&str]) -> Result> { if i == paths_start { paths.push(".".to_string()); } - let matcher = matchers::build_top_level_matcher(&args[i..], &mut config)?; + // The matcher builder only sees the expression part of the command line, so + // any argument index it reports has to be moved back to where that part + // started. + let matcher = matchers::build_top_level_matcher(&args[i..], &mut config) + .map_err(|e| ParseError::shift(e, i))?; let mut files0_paths = None; if let Some(name) = &config.files0_argument { if paths.len() == 1 && paths[0] == "." { From 1a3a13bc578d86529172270ef394efbfee8ac9ec Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 26 Sep 2026 19:38:48 +0200 Subject: [PATCH 4/6] find: suggest a correction for a mistyped predicate The suggestion travels as help text, so the message stays GNU's. The parser now matches on a `Predicate` looked up in a single name table, which also supplies the suggestions: a table entry the parser does not handle fails the exhaustive match, and a variant missing from the table is flagged as never constructed. --- src/find/error.rs | 104 ++++++++++++- src/find/matchers/mod.rs | 329 +++++++++++++++++++++++++++++---------- 2 files changed, 351 insertions(+), 82 deletions(-) diff --git a/src/find/error.rs b/src/find/error.rs index a2c7cc6f..b5c10f88 100644 --- a/src/find/error.rs +++ b/src/find/error.rs @@ -18,6 +18,7 @@ pub struct ParseError { /// parsing-layer boundary until it indexes the process's full argv. arg_index: Option, label: Option, + help: Option, } impl ParseError { @@ -26,6 +27,7 @@ impl ParseError { message: message.into(), arg_index: None, label: None, + help: None, } } @@ -41,10 +43,20 @@ impl ParseError { self } + /// Sets a suggestion shown below the report. + pub fn with_help(mut self, help: impl Into) -> Self { + self.help = Some(help.into()); + self + } + pub fn arg_index(&self) -> Option { self.arg_index } + pub fn help(&self) -> Option<&str> { + self.help.as_deref() + } + /// Adds `by` to the argument index of a `ParseError`, for callers that /// parsed a sub-slice of argv. Other errors pass through untouched. pub fn shift(err: Box, by: usize) -> Box { @@ -60,6 +72,58 @@ impl ParseError { } } +/// Returns the entry of `candidates` closest to `input`, if one is close enough +/// to be a plausible typo. +pub fn closest_match<'a>( + input: &str, + candidates: impl IntoIterator, +) -> Option<&'a str> { + // Allow one edit for short predicates and more for longer ones, but never + // so many that unrelated names start matching. Two edits is the useful + // minimum for anything but the shortest names, since a swapped pair of + // letters already costs that much. + let input_length = input.chars().count(); + let max_distance = match input_length { + 0..=3 => 1, + 4..=7 => 2, + _ => 3, + }; + + candidates + .into_iter() + .map(|candidate| (edit_distance(input, candidate), candidate)) + // A suggestion is only worth making if more of what was typed survives + // it than not. Without this, every two-character candidate sits one + // edit from every other, and `-H` -- a real option a user is likely to + // misplace after the path operand -- would be "corrected" to whichever + // short predicate happens to come first in the candidate list. + .filter(|(distance, _)| *distance <= max_distance && distance * 2 < input_length) + .min_by_key(|(distance, candidate)| (*distance, candidate.len())) + .map(|(_, candidate)| candidate) +} + +/// Levenshtein distance between two strings, counting characters rather than +/// bytes. +fn edit_distance(left: &str, right: &str) -> usize { + let right_chars: Vec = right.chars().collect(); + // Distances from the empty prefix of `left` to each prefix of `right`. + let mut previous_row: Vec = (0..=right_chars.len()).collect(); + let mut current_row = vec![0; right_chars.len() + 1]; + + for (left_index, left_char) in left.chars().enumerate() { + current_row[0] = left_index + 1; + for (right_index, &right_char) in right_chars.iter().enumerate() { + let substitution_cost = usize::from(left_char != right_char); + current_row[right_index + 1] = (current_row[right_index] + 1) + .min(previous_row[right_index + 1] + 1) + .min(previous_row[right_index] + substitution_cost); + } + std::mem::swap(&mut previous_row, &mut current_row); + } + + previous_row[right_chars.len()] +} + #[cfg(test)] mod tests { use super::*; @@ -68,7 +132,8 @@ mod tests { fn display_is_the_bare_message() { let error = ParseError::new("unknown predicate `-zap'") .at(3) - .with_label("not a known predicate"); + .with_label("not a known predicate") + .with_help("did you mean `-zip'?"); assert_eq!(error.to_string(), "unknown predicate `-zap'"); } @@ -94,4 +159,41 @@ mod tests { assert_eq!(foreign.to_string(), "plain message"); assert!(foreign.downcast_ref::().is_none()); } + + #[test] + fn edit_distance_counts_single_edits() { + assert_eq!(edit_distance("", ""), 0); + assert_eq!(edit_distance("-name", "-name"), 0); + assert_eq!(edit_distance("-nmae", "-name"), 2); + assert_eq!(edit_distance("-nam", "-name"), 1); + assert_eq!(edit_distance("", "-name"), 5); + assert_eq!(edit_distance("-name", ""), 5); + } + + #[test] + fn closest_match_finds_plausible_typos() { + let candidates = ["-name", "-newer", "-nogroup", "-print"]; + assert_eq!(closest_match("-nmae", candidates), Some("-name")); + assert_eq!(closest_match("-printt", candidates), Some("-print")); + assert_eq!(closest_match("-nogruop", candidates), Some("-nogroup")); + } + + #[test] + fn closest_match_rejects_short_input() { + // `-H` is one edit from any other two-character name, so a suggestion + // here would be a coin toss dressed up as advice. + let candidates = ["-a", "-o", "-ls", "-name"]; + assert_eq!(closest_match("-H", candidates), None); + assert_eq!(closest_match("-P", candidates), None); + // Three characters leave a majority intact after a single edit, so a + // suggestion is fair game again. + assert_eq!(closest_match("-lz", candidates), Some("-ls")); + } + + #[test] + fn closest_match_rejects_distant_input() { + let candidates = ["-name", "-newer", "-print"]; + assert_eq!(closest_match("-zzzzzzzzzz", candidates), None); + assert_eq!(closest_match("-xyz", candidates), None); + } } diff --git a/src/find/matchers/mod.rs b/src/find/matchers/mod.rs index 299795e8..5a86af66 100644 --- a/src/find/matchers/mod.rs +++ b/src/find/matchers/mod.rs @@ -70,7 +70,7 @@ use std::{ time::SystemTime, }; -use super::error::ParseError; +use super::error::{self, ParseError}; use super::{Config, Dependencies}; pub use entry::{FileType, WalkEntry, WalkError}; @@ -457,6 +457,163 @@ fn get_or_create_file(path: &str) -> Result> { Ok(file) } +/// What a predicate or operator on the command line resolves to. The parser +/// matches on this, so it has to handle every entry of [`PREDICATES`]. +#[derive(Clone, Copy)] +enum Predicate { + Print, + Print0, + Printf, + Fprint, + Fprintf, + Fprint0, + Ls, + Fls, + True, + False, + Lname, + Name, + Path, + Readable, + RegexType, + Regex, + Iregex, + Type, + Xtype, + Fstype, + Delete, + Newer, + Time(FileTimeType), + Min(FileTimeType), + Size, + Empty, + Exec, + Prompt, + Inum, + Links, + SameFile, + User, + NoUser, + Uid, + Group, + NoGroup, + Gid, + Executable, + Perm, + Prune, + Quit, + Writable, + Not, + And, + Or, + Comma, + OpenParen, + CloseParen, + Follow, + DayStart, + NoLeaf, + Depth, + Mount, + Sorted, + MaxDepth, + MinDepth, + Help, + Version, + Files0From, + /// Not in [`PREDICATES`]: a `-newerXY` form, or an unknown predicate. + Other, +} + +/// Every name the parser accepts, which also serves as the list of corrections +/// for a mistyped one. A variant missing here is never constructed, and the +/// compiler warns about it. +const PREDICATES: &[(&str, Predicate)] = &[ + ("-print", Predicate::Print), + ("-print0", Predicate::Print0), + ("-printf", Predicate::Printf), + ("-fprint", Predicate::Fprint), + ("-fprintf", Predicate::Fprintf), + ("-fprint0", Predicate::Fprint0), + ("-ls", Predicate::Ls), + ("-fls", Predicate::Fls), + ("-true", Predicate::True), + ("-false", Predicate::False), + ("-lname", Predicate::Lname), + ("-ilname", Predicate::Lname), + ("-name", Predicate::Name), + ("-iname", Predicate::Name), + ("-path", Predicate::Path), + ("-ipath", Predicate::Path), + ("-wholename", Predicate::Path), + ("-iwholename", Predicate::Path), + ("-readable", Predicate::Readable), + ("-regextype", Predicate::RegexType), + ("-regex", Predicate::Regex), + ("-iregex", Predicate::Iregex), + ("-type", Predicate::Type), + ("-xtype", Predicate::Xtype), + ("-fstype", Predicate::Fstype), + ("-delete", Predicate::Delete), + ("-newer", Predicate::Newer), + ("-atime", Predicate::Time(FileTimeType::Accessed)), + ("-ctime", Predicate::Time(FileTimeType::Changed)), + ("-mtime", Predicate::Time(FileTimeType::Modified)), + ("-amin", Predicate::Min(FileTimeType::Accessed)), + ("-cmin", Predicate::Min(FileTimeType::Changed)), + ("-mmin", Predicate::Min(FileTimeType::Modified)), + ("-size", Predicate::Size), + ("-empty", Predicate::Empty), + ("-exec", Predicate::Exec), + ("-execdir", Predicate::Exec), + ("-ok", Predicate::Prompt), + ("-okdir", Predicate::Prompt), + ("-inum", Predicate::Inum), + ("-links", Predicate::Links), + ("-samefile", Predicate::SameFile), + ("-user", Predicate::User), + ("-nouser", Predicate::NoUser), + ("-uid", Predicate::Uid), + ("-group", Predicate::Group), + ("-nogroup", Predicate::NoGroup), + ("-gid", Predicate::Gid), + ("-executable", Predicate::Executable), + ("-perm", Predicate::Perm), + ("-prune", Predicate::Prune), + ("-quit", Predicate::Quit), + ("-writable", Predicate::Writable), + ("-not", Predicate::Not), + ("!", Predicate::Not), + ("-and", Predicate::And), + ("-a", Predicate::And), + ("-or", Predicate::Or), + ("-o", Predicate::Or), + (",", Predicate::Comma), + ("(", Predicate::OpenParen), + (")", Predicate::CloseParen), + ("-follow", Predicate::Follow), + ("-daystart", Predicate::DayStart), + ("-noleaf", Predicate::NoLeaf), + ("-d", Predicate::Depth), + ("-depth", Predicate::Depth), + ("-mount", Predicate::Mount), + ("-xdev", Predicate::Mount), + ("-sorted", Predicate::Sorted), + ("-maxdepth", Predicate::MaxDepth), + ("-mindepth", Predicate::MinDepth), + ("-help", Predicate::Help), + ("--help", Predicate::Help), + ("-version", Predicate::Version), + ("--version", Predicate::Version), + ("-files0-from", Predicate::Files0From), +]; + +fn lookup_predicate(arg: &str) -> Predicate { + PREDICATES + .iter() + .find(|(name, _)| *name == arg) + .map_or(Predicate::Other, |&(_, predicate)| predicate) +} + /// Turns one of the `logical_matchers` "binary operator with nothing before it" /// errors into a diagnostic pointing at the operator. fn binary_operator_error(err: &dyn Error, arg_index: usize) -> ParseError { @@ -514,17 +671,17 @@ fn build_matcher_tree( let mut i = arg_index; let mut invert_next_matcher = false; while i < args.len() { - let possible_submatcher = match args[i] { - "-print" => Some(Printer::new(PrintDelimiter::Newline, None).into_box()), - "-print0" => Some(Printer::new(PrintDelimiter::Null, None).into_box()), - "-printf" => { + let possible_submatcher = match lookup_predicate(args[i]) { + Predicate::Print => Some(Printer::new(PrintDelimiter::Newline, None).into_box()), + Predicate::Print0 => Some(Printer::new(PrintDelimiter::Null, None).into_box()), + Predicate::Printf => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } i += 1; Some(Printf::new(args[i], None)?.into_box()) } - "-fprint" => { + Predicate::Fprint => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } @@ -533,7 +690,7 @@ fn build_matcher_tree( let file = get_or_create_file(args[i])?; Some(Printer::new(PrintDelimiter::Newline, Some(file)).into_box()) } - "-fprintf" => { + Predicate::Fprintf => { if i + 2 >= args.len() { return Err(missing_argument_error(args, i).into()); } @@ -547,7 +704,7 @@ fn build_matcher_tree( i += 1; Some(Printf::new(args[i], Some((file, output_path)))?.into_box()) } - "-fprint0" => { + Predicate::Fprint0 => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } @@ -556,8 +713,8 @@ fn build_matcher_tree( let file = get_or_create_file(args[i])?; Some(Printer::new(PrintDelimiter::Null, Some(file)).into_box()) } - "-ls" => Some(Ls::new(None).into_box()), - "-fls" => { + Predicate::Ls => Some(Ls::new(None).into_box()), + Predicate::Fls => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } @@ -566,31 +723,31 @@ fn build_matcher_tree( let file = get_or_create_file(args[i])?; Some(Ls::new(Some(file)).into_box()) } - "-true" => Some(TrueMatcher.into_box()), - "-false" => Some(FalseMatcher.into_box()), - "-lname" | "-ilname" => { + Predicate::True => Some(TrueMatcher.into_box()), + Predicate::False => Some(FalseMatcher.into_box()), + Predicate::Lname => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } i += 1; Some(LinkNameMatcher::new(args[i], args[i - 1].starts_with("-i")).into_box()) } - "-name" | "-iname" => { + Predicate::Name => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } i += 1; Some(NameMatcher::new(args[i], args[i - 1].starts_with("-i")).into_box()) } - "-path" | "-ipath" | "-wholename" | "-iwholename" => { + Predicate::Path => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } i += 1; Some(PathMatcher::new(args[i], args[i - 1].starts_with("-i")).into_box()) } - "-readable" => Some(AccessMatcher::Readable.into_box()), - "-regextype" => { + Predicate::Readable => Some(AccessMatcher::Readable.into_box()), + Predicate::RegexType => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } @@ -598,79 +755,65 @@ fn build_matcher_tree( regex_type = regex::RegexType::from_str(args[i])?; Some(TrueMatcher.into_box()) } - "-regex" => { + Predicate::Regex => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } i += 1; Some(RegexMatcher::new(regex_type, args[i], false)?.into_box()) } - "-iregex" => { + Predicate::Iregex => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } i += 1; Some(RegexMatcher::new(regex_type, args[i], true)?.into_box()) } - "-type" => { + Predicate::Type => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } i += 1; Some(TypeMatcher::new(args[i])?.into_box()) } - "-xtype" => { + Predicate::Xtype => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } i += 1; Some(XtypeMatcher::new(args[i])?.into_box()) } - "-fstype" => { + Predicate::Fstype => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } i += 1; Some(FileSystemMatcher::new(args[i].to_string()).into_box()) } - "-delete" => { + Predicate::Delete => { // -delete implicitly requires -depth config.depth_first = true; Some(DeleteMatcher::new().into_box()) } - "-newer" => { + Predicate::Newer => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } i += 1; Some(NewerMatcher::new(args[i], config.follow)?.into_box()) } - "-mtime" | "-atime" | "-ctime" => { + Predicate::Time(file_time_type) => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } - let file_time_type = match args[i] { - "-atime" => FileTimeType::Accessed, - "-ctime" => FileTimeType::Changed, - "-mtime" => FileTimeType::Modified, - // This shouldn't be possible. We've already checked the value - // is one of those three values. - _ => unreachable!("Encountered unexpected value {}", args[i]), - }; let days = convert_arg_to_comparable_value(args[i], args[i + 1])?; i += 1; Some(FileTimeMatcher::new(file_time_type, days, config.today_start).into_box()) } - "-amin" | "-cmin" | "-mmin" => { + Predicate::Min(file_time_type) => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } - let file_time_type = match args[i] { - "-amin" => FileTimeType::Accessed, - "-cmin" => FileTimeType::Changed, - "-mmin" => FileTimeType::Modified, - _ => unreachable!("Encountered unexpected value {}", args[i]), - }; let minutes = convert_arg_to_comparable_value(args[i], args[i + 1])?; i += 1; Some( @@ -678,7 +821,7 @@ fn build_matcher_tree( .into_box(), ) } - "-size" => { + Predicate::Size => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } @@ -687,8 +830,8 @@ fn build_matcher_tree( i += 1; Some(SizeMatcher::new(size, &unit)?.into_box()) } - "-empty" => Some(EmptyMatcher::new().into_box()), - "-exec" | "-execdir" => { + Predicate::Empty => Some(EmptyMatcher::new().into_box()), + Predicate::Exec => { let mut arg_index = i + 1; while arg_index < args.len() && args[arg_index] != ";" @@ -737,7 +880,7 @@ fn build_matcher_tree( _ => unreachable!("Encountered unexpected value {}", args[arg_index]), } } - "-ok" | "-okdir" => { + Predicate::Prompt => { // -ok is like -exec ... ; but prompts before each invocation. // Only ';' is accepted: POSIX does not define -ok ... + and // GNU find rejects it (batch mode makes no sense with prompts). @@ -767,7 +910,7 @@ fn build_matcher_tree( ) } #[cfg(unix)] - "-inum" => { + Predicate::Inum => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } @@ -776,13 +919,13 @@ fn build_matcher_tree( Some(InodeMatcher::new(inum).into_box()) } #[cfg(not(unix))] - "-inum" => { + Predicate::Inum => { return Err(From::from( "Inode numbers are not available on this platform", )); } #[cfg(unix)] - "-links" => { + Predicate::Links => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } @@ -791,10 +934,10 @@ fn build_matcher_tree( Some(LinksMatcher::new(inum).into_box()) } #[cfg(not(unix))] - "-links" => { + Predicate::Links => { return Err(From::from("Link counts are not available on this platform")); } - "-samefile" => { + Predicate::SameFile => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } @@ -804,7 +947,7 @@ fn build_matcher_tree( .map_err(|e| format!("{path}: {e}"))?; Some(matcher.into_box()) } - "-user" => { + Predicate::User => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } @@ -823,8 +966,8 @@ fn build_matcher_tree( })?; Some(matcher.into_box()) } - "-nouser" => Some(NoUserMatcher {}.into_box()), - "-uid" => { + Predicate::NoUser => Some(NoUserMatcher {}.into_box()), + Predicate::Uid => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } @@ -833,7 +976,7 @@ fn build_matcher_tree( i += 1; Some(UserMatcher::from_comparable(uid).into_box()) } - "-group" => { + Predicate::Group => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } @@ -854,8 +997,8 @@ fn build_matcher_tree( })?; Some(matcher.into_box()) } - "-nogroup" => Some(NoGroupMatcher {}.into_box()), - "-gid" => { + Predicate::NoGroup => Some(NoGroupMatcher {}.into_box()), + Predicate::Gid => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } @@ -864,25 +1007,25 @@ fn build_matcher_tree( i += 1; Some(GroupMatcher::from_comparable(gid).into_box()) } - "-executable" => Some(AccessMatcher::Executable.into_box()), - "-perm" => { + Predicate::Executable => Some(AccessMatcher::Executable.into_box()), + Predicate::Perm => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } i += 1; Some(PermMatcher::new(args[i])?.into_box()) } - "-prune" => Some(PruneMatcher::new().into_box()), - "-quit" => Some(QuitMatcher.into_box()), - "-writable" => Some(AccessMatcher::Writable.into_box()), - "-not" | "!" => { + Predicate::Prune => Some(PruneMatcher::new().into_box()), + Predicate::Quit => Some(QuitMatcher.into_box()), + Predicate::Writable => Some(AccessMatcher::Writable.into_box()), + Predicate::Not => { if !are_more_expressions(args, i) { return Err(missing_operand_error(args, i).into()); } invert_next_matcher = !invert_next_matcher; None } - "-and" | "-a" => { + Predicate::And => { if !are_more_expressions(args, i) { return Err(missing_operand_error(args, i).into()); } @@ -891,7 +1034,7 @@ fn build_matcher_tree( .map_err(|e| binary_operator_error(e.as_ref(), i))?; None } - "-or" | "-o" => { + Predicate::Or => { if !are_more_expressions(args, i) { return Err(missing_operand_error(args, i).into()); } @@ -900,7 +1043,7 @@ fn build_matcher_tree( .map_err(|e| binary_operator_error(e.as_ref(), i))?; None } - "," => { + Predicate::Comma => { if !are_more_expressions(args, i) { return Err(missing_operand_error(args, i).into()); } @@ -909,13 +1052,13 @@ fn build_matcher_tree( .map_err(|e| binary_operator_error(e.as_ref(), i))?; None } - "(" => { + Predicate::OpenParen => { let (new_arg_index, sub_matcher) = build_matcher_tree(args, config, i + 1, Some(i))?; i = new_arg_index; Some(sub_matcher) } - ")" => { + Predicate::CloseParen => { let Some(open_bracket_index) = open_bracket else { return Err(ParseError::new("you have too many ')'") .at(i) @@ -935,7 +1078,7 @@ fn build_matcher_tree( return Ok((i, top_level_matcher.build())); } - "-follow" => { + Predicate::Follow => { // This option affects multiple matchers. // 1. It will use noleaf by default. (but -noleaf No change of behavior) // Unless -L or -H is specified: @@ -950,31 +1093,31 @@ fn build_matcher_tree( config.no_leaf_dirs = true; Some(TrueMatcher.into_box()) } - "-daystart" => { + Predicate::DayStart => { config.today_start = true; Some(TrueMatcher.into_box()) } - "-noleaf" => { + Predicate::NoLeaf => { // No change of behavior config.no_leaf_dirs = true; Some(TrueMatcher.into_box()) } - "-d" | "-depth" => { + Predicate::Depth => { // TODO add warning if it appears after actual testing criterion config.depth_first = true; Some(TrueMatcher.into_box()) } - "-mount" | "-xdev" => { + Predicate::Mount => { // TODO add warning if it appears after actual testing criterion config.same_file_system = true; Some(TrueMatcher.into_box()) } - "-sorted" => { + Predicate::Sorted => { // TODO add warning if it appears after actual testing criterion config.sorted_output = true; Some(TrueMatcher.into_box()) } - "-maxdepth" => { + Predicate::MaxDepth => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } @@ -982,7 +1125,7 @@ fn build_matcher_tree( i += 1; Some(TrueMatcher.into_box()) } - "-mindepth" => { + Predicate::MinDepth => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } @@ -990,15 +1133,15 @@ fn build_matcher_tree( i += 1; Some(TrueMatcher.into_box()) } - "-help" | "--help" => { + Predicate::Help => { config.help_requested = true; None } - "-version" | "--version" => { + Predicate::Version => { config.version_requested = true; None } - "-files0-from" => { + Predicate::Files0From => { if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); } @@ -1007,13 +1150,23 @@ fn build_matcher_tree( Some(TrueMatcher.into_box()) } - _ => { + Predicate::Other => { // Match GNU find wording for unknown predicates. let Some((x_option, y_option)) = parse_str_to_newer_args(args[i]) else { - return Err(ParseError::new(format!("unknown predicate `{}'", args[i])) + let mut err = ParseError::new(format!("unknown predicate `{}'", args[i])) .at(i) - .with_label("not a known predicate") - .into()); + .with_label("not a known predicate"); + if let Some(suggestion) = error::closest_match( + args[i], + PREDICATES + .iter() + .map(|(name, _)| *name) + // Handled as `-newerXY` but worth suggesting. + .chain(["-anewer", "-cnewer"]), + ) { + err = err.with_help(format!("did you mean `{suggestion}'?")); + } + return Err(err.into()); }; if i >= args.len() - 1 { return Err(missing_argument_error(args, i).into()); @@ -1998,6 +2151,20 @@ mod tests { assert_eq!(failing_arg_index(&["-true", ")"]), Some(1)); } + #[test] + fn unknown_predicates_suggest_a_close_match() { + let mut config = Config::default(); + let err = build_top_level_matcher(&["-nmae", "x"], &mut config) + .err() + .expect("typo should have been rejected"); + // The suggestion travels as help text, leaving the message GNU-compatible. + assert_eq!(err.to_string(), "unknown predicate `-nmae'"); + assert_eq!( + err.downcast_ref::().unwrap().help(), + Some("did you mean `-name'?") + ); + } + #[test] fn build_top_level_matcher_option_logical() { let mut config = Config::default(); From 1268322dbed959337fd2959c82f87f0b5156b7fe Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 27 Sep 2026 09:37:36 +0200 Subject: [PATCH 5/6] find: render expression errors under UUTILS_DIAG Underline the offending argument through `uucore::diagnostics` when stderr is a terminal or `UUTILS_DIAG=always`. The plain line still leads, and captured output stays the single GNU line. The report heading comes from uucore's `util_name()`, which keeps the `.exe` on Windows; the plain line keeps `program_name()` like every other message. --- Cargo.lock | 11 ++++++ Cargo.toml | 2 +- src/find/error.rs | 28 ++++++++++++++ src/find/mod.rs | 12 +++++- tests/test_find.rs | 95 ++++++++++++++++++++++++++++++++++++++++------ 5 files changed, 134 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b25e70fa..7f6a0b44 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -102,6 +102,16 @@ dependencies = [ "once_cell", ] +[[package]] +name = "ariadne" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8454c8a44ce2cb9cc7e7fae67fc6128465b343b92c6631e94beca3c8d1524ea5" +dependencies = [ + "unicode-width", + "yansi", +] + [[package]] name = "assert_cmd" version = "2.2.2" @@ -1569,6 +1579,7 @@ version = "0.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f24a5910ebbf2c0baf486a9125705f8e026a8f7741a3ea8cf5eca3c9257c9bdf" dependencies = [ + "ariadne", "bstr", "clap", "dns-lookup", diff --git a/Cargo.toml b/Cargo.toml index dd4992b7..05a6ab4d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,7 @@ faccess = "0.2.4" nix = { version = "0.31", features = ["fs", "user"] } onig = { version = "6.5", default-features = false } regex = "1.12" -uucore = { version = "0.12.0", features = ["entries", "fs", "fsext", "mode"] } +uucore = { version = "0.12.0", features = ["diagnostics", "entries", "fs", "fsext", "mode"] } walkdir = "2.5" itertools = "0.15.0" thiserror = "2.0.12" diff --git a/src/find/error.rs b/src/find/error.rs index b5c10f88..33ea08a7 100644 --- a/src/find/error.rs +++ b/src/find/error.rs @@ -8,6 +8,8 @@ use std::error::Error; +use uucore::diagnostics::Snapshot; + /// A command-line expression error that can point at the argument that caused it. #[derive(Debug, thiserror::Error)] #[error("{message}")] @@ -70,6 +72,25 @@ impl ParseError { Err(other) => other, } } + + /// Draws the error under the offending argument on stderr, headed by the + /// plain message line. Returns `false`, having printed nothing, when the + /// error names no argument. + pub fn render(&self, argv: &[&str]) -> bool { + let Some(arg_index) = self.arg_index else { + return false; + }; + // uucore translates the "Help:" label from its own embedded strings; + // without a localizer it prints the raw message id instead. Done here + // so that only a drawn report pays for it. + let _ = uucore::locale::setup_localization("find"); + Snapshot::with_program(argv).render( + arg_index, + &self.message, + self.label.as_deref(), + self.help.as_deref(), + ) + } } /// Returns the entry of `candidates` closest to `input`, if one is close enough @@ -137,6 +158,13 @@ mod tests { assert_eq!(error.to_string(), "unknown predicate `-zap'"); } + #[test] + fn render_without_an_index_draws_nothing() { + // Nothing to point at: the caller has to print the plain line. + let error = ParseError::new("something went wrong"); + assert!(!error.render(&["find", "/srv"])); + } + #[test] fn shift_moves_the_index_and_composes() { let error: Box = Box::new(ParseError::new("oops").at(2)); diff --git a/src/find/mod.rs b/src/find/mod.rs index 3007f50a..b3c1982f 100644 --- a/src/find/mod.rs +++ b/src/find/mod.rs @@ -524,7 +524,17 @@ pub fn find_main(args: &[&str], deps: &dyn Dependencies) -> i32 { match do_find(&args[1..], deps) { Ok(ret) => ret, Err(e) => { - writeln!(&mut stderr(), "{}: {e}", program_name()).unwrap(); + // `do_find` was handed argv without the program name. + let e = ParseError::shift(e, 1); + // Only at a terminal or with `UUTILS_DIAG=always`, so captured + // output (the GNU and bfs suites) keeps the single GNU line. + let rendered = uucore::diagnostics::enabled() + && e.downcast_ref::() + .is_some_and(|parse_error| parse_error.render(args)); + // The report heads itself with the same line. + if !rendered { + writeln!(&mut stderr(), "{}: {e}", program_name()).unwrap(); + } 1 } } diff --git a/tests/test_find.rs b/tests/test_find.rs index 93efa223..95df061f 100644 --- a/tests/test_find.rs +++ b/tests/test_find.rs @@ -24,6 +24,13 @@ use common::test_helpers::fix_up_slashes; mod common; +/// How `find` names itself in errors. +const PROGRAM: &str = "find"; + +/// How uucore heads an expression report: argv[0]'s base name, which keeps the +/// `.exe` on Windows. +const REPORT_PROGRAM: &str = if cfg!(windows) { "find.exe" } else { "find" }; + /// Returns a UCommand for `find` with the working directory set to the /// repository root, so that tests using relative `test_data/` paths work. fn ucmd() -> uutests::util::UCommand { @@ -179,6 +186,67 @@ fn mindepth_exceeds_maxdepth_outputs_nothing() { .no_stdout(); } +#[test] +fn expression_diagnostics_stay_out_of_the_way_of_scripts() { + // A test harness captures stderr, so it is never a terminal and the error + // must stay the single GNU-compatible line -- which is what the GNU and + // bfs compatibility suites assert on. `never` holds even at a terminal; + // any value other than `always` reads as unset rather than as "on". + for value in [None, Some(""), Some("never"), Some("NEVER"), Some("1")] { + let mut command = ucmd(); + command.args(&["-true", "-o", "-nmae", "x"]); + if let Some(value) = value { + command.env("UUTILS_DIAG", value); + } + let output = command.fails().no_stdout().stderr_str().to_owned(); + assert_eq!( + output.trim_end(), + format!("{PROGRAM}: unknown predicate `-nmae'") + ); + } +} + +#[test] +fn expression_diagnostics_underline_the_bad_argument() { + // Asking for a report has to produce one, including at a redirected + // stderr like this test's. + for value in ["always", "ALWAYS"] { + let output = ucmd() + .args(&["-true", "-o", "-nmae", "x"]) + .env("UUTILS_DIAG", value) + .fails() + .no_stdout() + .stderr_str() + .to_owned(); + + // The plain message still leads, so nothing that matched before stops + // matching. + assert!(output.starts_with(&format!("{REPORT_PROGRAM}: unknown predicate `-nmae'\n"))); + assert!(output.contains("-true -o -nmae x")); + assert!(output.contains("not a known predicate")); + assert!(output.contains("did you mean `-name'?")); + // Piped output must not be coloured. + assert!(!output.contains('\u{1b}')); + } +} + +#[test] +fn expression_diagnostics_do_not_guess_at_two_letter_arguments() { + // `-H` is a real option, valid only before the paths; put it after them and + // it reaches the expression parser. Every two-character predicate is one + // edit away from it, so any suggestion here would be arbitrary. + let output = ucmd() + .args(&[".", "-H"]) + .env("UUTILS_DIAG", "always") + .fails() + .no_stdout() + .stderr_str() + .to_owned(); + + assert!(output.starts_with(&format!("{REPORT_PROGRAM}: unknown predicate `-H'\n"))); + assert!(!output.contains("did you mean")); +} + #[test] fn multiple_matcher_failure() { ucmd() @@ -779,7 +847,7 @@ fn find_printf_width_too_large() { "%70000s\\n", ]) .fails() - .stderr_contains("find: Format width too large"); + .stderr_contains(format!("{PROGRAM}: Format width too large")); ucmd() .args(&[ "./test_data/simple", @@ -789,7 +857,7 @@ fn find_printf_width_too_large() { "%99999999999999999999s\\n", ]) .fails() - .stderr_contains("find: Invalid format width"); + .stderr_contains(format!("{PROGRAM}: Invalid format width")); } #[test] @@ -807,7 +875,7 @@ fn find_printf_multibyte_char_after_directive() { ucmd() .args(&["./test_data/simple", "-maxdepth", "0", "-printf", "%A€"]) .fails() - .stderr_contains("find: Invalid time specifier"); + .stderr_contains(format!("{PROGRAM}: Invalid time specifier")); } #[cfg(unix)] @@ -1107,18 +1175,21 @@ fn find_newer_xy() { .no_stderr(); } - ucmd().args(&[".", arg, "invalid"]).fails().stderr_only( - "find: I cannot figure out how to interpret ‘invalid’ as a date or time\n", - ); + ucmd() + .args(&[".", arg, "invalid"]) + .fails() + .stderr_only(format!( + "{PROGRAM}: I cannot figure out how to interpret ‘invalid’ as a date or time\n" + )); } #[cfg(target_os = "linux")] ucmd() .args(&[".", "-newerBt", "jan 01, 2000"]) .fails() - .stderr_only( - "find: This system does not provide a way to find the birth time of a file.\n", - ); + .stderr_only(format!( + "{PROGRAM}: This system does not provide a way to find the birth time of a file.\n" + )); } #[test] @@ -1140,9 +1211,9 @@ fn find_age_range() { ucmd() .args(&["test_data/simple", arg, time_string]) .fails() - .stderr_contains( - "find: Expected a decimal integer (with optional + or - prefix) argument to", - ) + .stderr_contains(format!( + "{PROGRAM}: Expected a decimal integer (with optional + or - prefix) argument to" + )) .no_stdout(); } } From 4c15fdb5a7be0759bed2ef0f6b9cdb0e1359b4d7 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 27 Sep 2026 14:25:31 +0200 Subject: [PATCH 6/6] docs: document the expression diagnostics --- README.md | 3 + docs/src/extensions.md | 226 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 229 insertions(+) diff --git a/README.md b/README.md index a5747a81..495cccf0 100644 --- a/README.md +++ b/README.md @@ -9,6 +9,9 @@ Rust implementation of [GNU findutils](https://www.gnu.org/software/findutils/): `xargs`, `find`, `locate` and `updatedb`. The goal is to be a full drop-in replacement of the original commands. +At a terminal, `find` underlines the argument at fault in expression errors; see +[Extensions](docs/src/extensions.md#rich-expression-diagnostics). + ## Run the GNU testsuite on rust/findutils: ``` diff --git a/docs/src/extensions.md b/docs/src/extensions.md index fcc2f08d..d07e81a7 100644 --- a/docs/src/extensions.md +++ b/docs/src/extensions.md @@ -44,3 +44,229 @@ default. This is most useful when you want reproducible output: comparing two trees, generating a manifest, or writing a test whose expected output is a fixed list of lines. + +## Rich expression diagnostics + +`find` expressions get long, and a lone error line does not say *where* in the +expression the problem is. Down a pipe — a script, a test suite — that lone +line is all you get, because that is what such callers parse: + +```console +$ find /srv/www -type f -a \( -name '*.php' -o -nmae '*.inc' \) -print 2>&1 | cat +find: unknown predicate `-nmae' +``` + +At a terminal, where the reader is a person, `find` underlines the argument at +fault in place, on the command line you actually typed: + +```console +$ find /srv/www -type f -a \( -name '*.php' -o -name '*.phtml' -o -nmae '*.inc' \) -print +find: unknown predicate `-nmae' + ╭─[ find:1:60 ] + │ + 1 │ find /srv/www -type f -a ( -name *.php -o -name *.phtml -o -nmae *.inc ) -print + │ ──┬── + │ ╰──── not a known predicate + │ + │ Help: did you mean `-name'? +───╯ +``` + +The first line is the message `find` has always printed, so anything that matched +it before still matches. Everything below it is added context. + +### What gets a diagnostic + +Errors in the *structure* of the expression, and in the predicates themselves. +The unknown-predicate case is shown above; the rest follow, all with +`UUTILS_DIAG=always` set in the environment. + +A predicate left without its argument: + +```console +$ find /home -xdev -type f -size +1G -a -mtime -7 -printf +find: missing argument to `-printf' + ╭─[ find:1:49 ] + │ + 1 │ find /home -xdev -type f -size +1G -a -mtime -7 -printf + │ ───┬─── + │ ╰───── this predicate needs an argument +───╯ +``` + +An operator with nothing *after* it, typically a half-finished edit: + +```console +$ find /var/spool -type f -a \( -user postfix -o -group mail \) -a -mtime +7 -o +find: expected an expression after '-o' + ╭─[ find:1:74 ] + │ + 1 │ find /var/spool -type f -a ( -user postfix -o -group mail ) -a -mtime +7 -o + │ ─┬ + │ ╰── nothing follows this operator +───╯ +``` + +The same slip inside a group, where it is the `)` that cuts the operator off: + +```console +$ find /var/log -type f \( -name '*.gz' -o -name '*.old' -o \) -print +find: expected an expression between '-o' and ')' + ╭─[ find:1:53 ] + │ + 1 │ find /var/log -type f ( -name *.gz -o -name *.old -o ) -print + │ ─┬ + │ ╰── nothing between this operator and the ')' +───╯ +``` + +An operator with nothing *before* it. Note this underlines the leading token, +where the previous example underlined the trailing one — two mistakes that read +identically in the plain message now look different: + +```console +$ find . -o -type f -name '*.tmp' -print +find: invalid expression; you have used a binary operator '-o' with nothing before it. + ╭─[ find:1:8 ] + │ + 1 │ find . -o -type f -name *.tmp -print + │ ─┬ + │ ╰── no expression before this operator +───╯ +``` + +A `(` that is never closed. There are three `(` on this line; the diagnostic +picks the unbalanced one rather than pointing at the end of the command: + +```console +$ find . \( -type d -a \( -name .git -o -name target \) -prune \) -o \( -type f -print +find: invalid expression; I was expecting to find a ')' somewhere but did not see one. + ╭─[ find:1:64 ] + │ + 1 │ find . ( -type d -a ( -name .git -o -name target ) -prune ) -o ( -type f -print + │ ┬ + │ ╰── this parenthesis is never closed +───╯ +``` + +An empty group: + +```console +$ find . -type f \( \) -o -name '*.bak' -print +find: invalid expression; empty parentheses are not allowed. + ╭─[ find:1:16 ] + │ + 1 │ find . -type f ( ) -o -name *.bak -print + │ ┬ + │ ╰── nothing between these parentheses +───╯ +``` + +And a `)` with no opener: + +```console +$ find /etc \( -name '*.conf' -a -newer /etc/fstab \) \) -o -name '*.cfg' -print +find: you have too many ')' + ╭─[ find:1:49 ] + │ + 1 │ find /etc ( -name *.conf -a -newer /etc/fstab ) ) -o -name *.cfg -print + │ ┬ + │ ╰── no matching '(' before this +───╯ +``` + +Errors in the *value* of an argument — a bad `-size` suffix, `-perm` mode, +`-type` list, `-printf` format or `-newerXY` date — keep the plain single-line +message for now. + +### Comparison with GNU + +GNU `find` reports the same errors, but only ever as a single line. On a typo +buried in a group, that is all you get: + +```console +$ find /srv/www -type f -a \( -name '*.php' -o -name '*.phtml' -o -nmae '*.inc' \) -print +find: unknown predicate `-nmae' +``` + +```console +$ UUTILS_DIAG=always find /srv/www -type f -a \( -name '*.php' -o -name '*.phtml' -o -nmae '*.inc' \) -print +find: unknown predicate `-nmae' + ╭─[ find:1:60 ] + │ + 1 │ find /srv/www -type f -a ( -name *.php -o -name *.phtml -o -nmae *.inc ) -print + │ ──┬── + │ ╰──── not a known predicate + │ + │ Help: did you mean `-name'? +───╯ +``` + +The difference is starkest where the message names no argument at all. GNU tells +you a `)` is missing, but not which `(` is unbalanced — on a line with three of +them, that is the whole question: + +```console +$ find . \( -type d -a \( -name .git -o -name target \) -prune \) -o \( -type f -print +find: invalid expression; I was expecting to find a ')' somewhere but did not see one. +``` + +```console +$ UUTILS_DIAG=always find . \( -type d -a \( -name .git -o -name target \) -prune \) -o \( -type f -print +find: invalid expression; I was expecting to find a ')' somewhere but did not see one. + ╭─[ find:1:64 ] + │ + 1 │ find . ( -type d -a ( -name .git -o -name target ) -prune ) -o ( -type f -print + │ ┬ + │ ╰── this parenthesis is never closed +───╯ +``` + +In both cases the first line is identical to what GNU 4.10.0 prints, and the exit +code is `1` either way. The report only ever *adds* the block below. + +`UUTILS_DIAG` overrides the terminal check in either direction: set it to +`never` to always get the single line, or to `always` to get the report even +when standard error is redirected. Values are matched case-insensitively; +anything else — including an empty value — counts as unset and leaves the +terminal check in charge. It is the same variable, with the same values, that +the uutils coreutils read. + +Every message is byte-for-byte identical to GNU's under `LC_ALL=C`: + +```text +unknown predicate `-nmae' +missing argument to `-printf' +expected an expression after '-o' +expected an expression between '-o' and ')' +invalid expression; you have used a binary operator '-o' with nothing before it. +invalid expression; empty parentheses are not allowed. +invalid expression; I was expecting to find a ')' somewhere but did not see one. +you have too many ')' +``` + +### Notes + +- **The rendered line is a reconstruction, not an echo.** It shows the arguments + `find` was handed, after the shell had its say: `\(` and `'*.php'` arrive as + `(` and `*.php` and are shown that way. Only an argument holding whitespace, or + an empty one, is quoted, so that the underline still lines up with a word. That + is what makes the underline trustworthy even when the shell rewrote what you + typed. +- **Suggestions are edit-distance based**, with a threshold that scales with the + length of the name (one edit up to 3 characters, two up to 7, three beyond), + and a second rule that an edit may never rewrite half of what was typed. + Transpositions and dropped letters are caught — `-nam`, `-pritn`, `-exce`, + `-mindpeth` — while genuinely unrelated input gets no suggestion rather than a + misleading one. The half rule is what keeps a misplaced `-H` or `-P` from + being "corrected" to an unrelated two-character predicate it happens to sit + one edit away from. +- **Colour follows [`NO_COLOR`](https://no-color.org)** and is used only when + standard error is a terminal. A report forced on with `UUTILS_DIAG=always` + still goes wherever standard error points, so it stays plain when that is a + file. +- **On Windows the report heading reads `find.exe:`**, where the plain line + reads `find:`: the heading comes from uucore, which keeps the `.exe`. +- **Nothing changes for a non-terminal standard error**, which is how the GNU + and bfs compatibility testsuites — and every script — run `find`. Standard + error stays byte-for-byte what it was.