Implement : Chained Type matcher arguments (#527)

* code dump

* fix : new checks + working stage

* minor fixes + duplicates check

* fix : add check for incorrect chained commands

* add : Chained Argument matching for -xtype

* add: test

* cargo fmt

* Add : more tests + old test fix

* refactor : move common code to function

* fix : convert match to if/else

* Improve : Testcase for xtype

* cargo clippy fix

* update : testcase

* fix : error handling

* fix : use single type

* add: basic tests on binary level

* Add : -xtype tests

* refactor : use single type without Option<>

* fix : use better search logic

* refactor : use HashSet<> instead of Vec<>

* remove : unnecessary hashmap usage

* Update src/find/matchers/type_matcher.rs

Co-authored-by: Tavian Barnes <tavianator@tavianator.com>

* change: variable name

* remove: type and replace with HashSet<FileType>

* remove: trimming

---------

Co-authored-by: Tavian Barnes <tavianator@tavianator.com>

src/find/matchers/entry.rs

@@ -24,7 +24,7 @@ enum Entry {
 }
 
 /// File types.
-#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)]
 pub enum FileType {
     Unknown,
     Fifo,

src/find/matchers/type_matcher.rs

@@ -4,16 +4,16 @@
 // license that can be found in the LICENSE file or at
 // https://opensource.org/licenses/MIT.
 
-use std::error::Error;
-
 use super::{FileType, Follow, Matcher, MatcherIO, WalkEntry};
+use std::collections::HashSet;
+use std::error::Error;
 
 /// This matcher checks the type of the file.
 pub struct TypeMatcher {
-    file_type: FileType,
+    file_type: HashSet<FileType>,
 }
 
-fn parse(type_string: &str) -> Result<FileType, Box<dyn Error>> {
+fn parse(type_string: &str, mode: &str) -> Result<FileType, Box<dyn Error>> {
     let file_type = match type_string {
         "f" => FileType::Regular,
         "d" => FileType::Directory,
@@ -24,8 +24,20 @@ fn parse(type_string: &str) -> Result<FileType, Box<dyn Error>> {
         "s" => FileType::Socket,
         // D: door (Solaris)
         "D" => {
+            #[cfg(not(target_os = "solaris"))]
+            {
+                return Err(From::from(format!("{mode} D is not supported because Solaris doors are not supported on the platform find was compiled on.")));
+            }
+            #[cfg(target_os = "solaris")]
+            {
+                return Err(From::from(format!(
+                    "Type argument {type_string} not supported yet"
+                )));
+            }
+        }
+        "" => {
             return Err(From::from(format!(
-                "Type argument {type_string} not supported yet"
+                "Arguments to {mode} should contain at least one letter"
             )))
         }
         _ => {
@@ -39,29 +51,32 @@ fn parse(type_string: &str) -> Result<FileType, Box<dyn Error>> {
 
 impl TypeMatcher {
     pub fn new(type_string: &str) -> Result<Self, Box<dyn Error>> {
-        let file_type = parse(type_string)?;
-        Ok(Self { file_type })
+        let main_file_type = type_creator(type_string, "-type")?;
+        Ok(Self {
+            file_type: main_file_type,
+        })
     }
 }
 
 impl Matcher for TypeMatcher {
     fn matches(&self, file_info: &WalkEntry, _: &mut MatcherIO) -> bool {
-        file_info.file_type() == self.file_type
+        self.file_type.contains(&file_info.file_type())
     }
 }
 
 /// Like [TypeMatcher], but toggles whether symlinks are followed.
 pub struct XtypeMatcher {
-    file_type: FileType,
+    file_type: HashSet<FileType>,
 }
 
 impl XtypeMatcher {
     pub fn new(type_string: &str) -> Result<Self, Box<dyn Error>> {
-        let file_type = parse(type_string)?;
-        Ok(Self { file_type })
+        let main_file_type = type_creator(type_string, "-xtype")?;
+        Ok(Self {
+            file_type: main_file_type,
+        })
     }
 }
-
 impl Matcher for XtypeMatcher {
     fn matches(&self, file_info: &WalkEntry, _: &mut MatcherIO) -> bool {
         let follow = if file_info.follow() {
@@ -72,18 +87,47 @@ impl Matcher for XtypeMatcher {
 
         let file_type = follow
             .metadata(file_info)
-            .map(|m| m.file_type())
-            .map(FileType::from);
+            .map(|m| m.file_type().into())
+            .or_else(|e| {
+                if e.is_loop() {
+                    Ok(FileType::Symlink)
+                } else {
+                    Err(e)
+                }
+            })
+            .unwrap_or(FileType::Unknown);
 
-        match file_type {
-            Ok(file_type) if file_type == self.file_type => true,
-            // Since GNU find 4.10, ELOOP will match -xtype l
-            Err(e) if self.file_type.is_symlink() && e.is_loop() => true,
-            _ => false,
-        }
+        self.file_type.contains(&file_type)
     }
 }
 
+fn type_creator(type_string: &str, mode: &str) -> Result<HashSet<FileType>, Box<dyn Error>> {
+    let mut file_types = std::collections::HashSet::new();
+
+    if type_string.contains(',') {
+        for part in type_string.split(',') {
+            if part.is_empty() {
+                return Err(From::from(format!("find: Last file type in list argument to {mode} is missing, i.e., list is ending on: ','")));
+            }
+            let file_type = parse(part, mode)?;
+            if !file_types.insert(file_type) {
+                return Err(From::from(format!(
+                    "Duplicate file type '{part}' in the argument list to {mode}"
+                )));
+            }
+        }
+    } else {
+        if type_string.len() > 1 {
+            return Err(From::from(format!(
+                "Must separate multiple arguments to {mode} using: ','"
+            )));
+        }
+        file_types.insert(parse(type_string, mode)?);
+    }
+
+    Ok(file_types)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -238,4 +282,58 @@ mod tests {
         let deps = FakeDependencies::new();
         assert!(matcher.matches(&entry, &mut deps.new_matcher_io()));
     }
+
+    #[test]
+    fn chained_arguments_type() {
+        assert!(TypeMatcher::new("").is_err());
+        assert!(TypeMatcher::new("f,f").is_err());
+        assert!(TypeMatcher::new("f,").is_err());
+        assert!(TypeMatcher::new("x,y").is_err());
+        assert!(TypeMatcher::new("fd").is_err());
+
+        assert!(XtypeMatcher::new("").is_err());
+        assert!(XtypeMatcher::new("f,f").is_err());
+        assert!(XtypeMatcher::new("f,").is_err());
+        assert!(XtypeMatcher::new("x,y").is_err());
+        assert!(XtypeMatcher::new("fd").is_err());
+    }
+
+    #[test]
+    fn type_matcher_multiple_valid_types() {
+        let deps = FakeDependencies::new();
+        let file = get_dir_entry_for("test_data/simple", "abbbc");
+        let dir = get_dir_entry_for("test_data", "simple");
+        let symlink = get_dir_entry_for("test_data/links", "link-f");
+
+        let matcher = TypeMatcher::new("f,d").unwrap();
+        assert!(matcher.matches(&file, &mut deps.new_matcher_io()));
+        assert!(matcher.matches(&dir, &mut deps.new_matcher_io()));
+        assert!(!matcher.matches(&symlink, &mut deps.new_matcher_io()));
+
+        let matcher = TypeMatcher::new("l,d").unwrap();
+        assert!(!matcher.matches(&file, &mut deps.new_matcher_io()));
+        assert!(matcher.matches(&dir, &mut deps.new_matcher_io()));
+        assert!(matcher.matches(&symlink, &mut deps.new_matcher_io()));
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn xtype_matcher_mixed_types_with_symlinks() {
+        let deps = FakeDependencies::new();
+
+        // Regular file through symlink
+        let entry = get_dir_entry_follow("test_data/links", "link-f", Follow::Always);
+        let matcher = XtypeMatcher::new("f,l").unwrap();
+        assert!(matcher.matches(&entry, &mut deps.new_matcher_io()));
+
+        // Broken symlink
+        let broken_entry = get_dir_entry_for("test_data/links", "link-missing");
+        assert!(matcher.matches(&broken_entry, &mut deps.new_matcher_io()));
+
+        //looping symlink
+        let matcher2 = XtypeMatcher::new("l").unwrap();
+        let looping_entry = get_dir_entry_for("test_data/links", "link-loop");
+        assert!(matcher.matches(&looping_entry, &mut deps.new_matcher_io()));
+        assert!(matcher2.matches(&looping_entry, &mut deps.new_matcher_io()));
+    }
 }

tests/find_cmd_tests.rs

@@ -74,6 +74,112 @@ fn two_matchers_one_matches() {
         .stdout(predicate::str::is_empty());
 }
 
+#[test]
+fn multiple_matcher_success() {
+    Command::cargo_bin("find")
+        .expect("found binary")
+        .args(["-type", "f,d,l", "-name", "abbbc"])
+        .assert()
+        .success()
+        .stderr(predicate::str::is_empty())
+        .stdout(predicate::str::contains("abbbc"));
+
+    Command::cargo_bin("find")
+        .expect("found binary")
+        .args(["-xtype", "f,d,l", "-name", "abbbc"])
+        .assert()
+        .success()
+        .stderr(predicate::str::is_empty())
+        .stdout(predicate::str::contains("abbbc"));
+}
+
+#[test]
+fn multiple_matcher_failure() {
+    Command::cargo_bin("find")
+        .expect("found binary")
+        .args(["-type", "fd", "-name", "abbb"])
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains("Must separate multiple arguments"))
+        .stdout(predicate::str::is_empty());
+
+    Command::cargo_bin("find")
+        .expect("found binary")
+        .args(["-type", "f,", "-name", "abbb"])
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains("list is ending on: ','"))
+        .stdout(predicate::str::is_empty());
+
+    Command::cargo_bin("find")
+        .expect("found binary")
+        .args(["-type", "f,f", "-name", "abbb"])
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains("Duplicate file type"))
+        .stdout(predicate::str::is_empty());
+
+    Command::cargo_bin("find")
+        .expect("found binary")
+        .args(["-type", "", "-name", "abbb"])
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains(
+            "should contain at least one letter",
+        ))
+        .stdout(predicate::str::is_empty());
+
+    Command::cargo_bin("find")
+        .expect("found binary")
+        .args(["-type", "x,y", "-name", "abbb"])
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains("Unrecognised type argument"))
+        .stdout(predicate::str::is_empty());
+    // x-type tests below
+    Command::cargo_bin("find")
+        .expect("found binary")
+        .args(["-xtype", "fd", "-name", "abbb"])
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains("Must separate multiple arguments"))
+        .stdout(predicate::str::is_empty());
+
+    Command::cargo_bin("find")
+        .expect("found binary")
+        .args(["-xtype", "f,", "-name", "abbb"])
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains("list is ending on: ','"))
+        .stdout(predicate::str::is_empty());
+
+    Command::cargo_bin("find")
+        .expect("found binary")
+        .args(["-xtype", "f,f", "-name", "abbb"])
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains("Duplicate file type"))
+        .stdout(predicate::str::is_empty());
+
+    Command::cargo_bin("find")
+        .expect("found binary")
+        .args(["-xtype", "", "-name", "abbb"])
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains(
+            "should contain at least one letter",
+        ))
+        .stdout(predicate::str::is_empty());
+
+    Command::cargo_bin("find")
+        .expect("found binary")
+        .args(["-xtype", "x,y", "-name", "abbb"])
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains("Unrecognised type argument"))
+        .stdout(predicate::str::is_empty());
+}
+
 #[serial(working_dir)]
 #[test]
 fn files0_empty_file() {