Learning Rust: recursive directories scan

code review
1

When I just started learning Rust, I wrote the below function:

fn recurse_dirs(path: &Path, parent: Option<&String>) -> io::Result<JsonStr> {
    let meta = path.metadata()?;
    let mut buf = JsonStr::with_capacity(256);
    if meta.is_dir() && path.file_name().unwrap().to_str() != Some(".git") {
        let dirs: Vec<_> = read_dir(path)?
            .filter_map(|f| match f {
                Ok(f)
                    if f.file_type().map(|t| t.is_dir()).unwrap_or_default()
                        && f.file_name().to_str() != Some(".git") =>
                {
                    Some(f)
                }
                _ => None,
            })
            .collect();
        let mut dirs_pick = dirs.iter().peekable();
        while let Some(entry) = dirs_pick.next() {
            buf.push('"');
            if let Some(parent) = parent {
                buf.push_str(parent);
                buf.push('/')
            }
            let file_name = entry
                .file_name()
                .into_string()
                .unwrap_or_default()
                .to_string();
            buf.push_str(&json_encode(&file_name));
            buf.push('"');
            let mut parent_str = String::new();
            if let Some(parent) = parent {
                parent_str.push_str(parent);
                parent_str.push('/')
            }
            parent_str.push_str(&file_name);
            let child_dirs = recurse_dirs(entry.path().as_path(), Some(&parent_str))?;
            if !child_dirs.is_empty() {
                buf.push(',');
                buf.push_str(&child_dirs)
            }
            if dirs_pick.peek().is_some() {
                buf.push(',')
            }
        }
    }
    Ok(buf)
}

And after couple years I decided to improve it, producing the following code:

fn recurse_dirs(path: &Path, parent: Option<&String>) -> io::Result<JsonStr> {
    Ok(dir_list(
        path,
        if let Some(parent) = parent {
            parent
        } else {
            ""
        },
        ".git",
    )
    .join(","))
}
fn dir_list(dir: &Path, anchor: &str, exclude: &str) -> Vec<String> {
    let dirs = if let Ok(dirs) = fs::read_dir(dir) {
        dirs.flatten()
            .filter_map(|entry| {
                if let Ok(ftype) = entry.file_type()
                    && ftype.is_dir()
                    && entry.file_name() != exclude
                {
                    Some(entry.file_name().display().to_string())
                } else {
                    None
                }
            })
            .collect::<Vec<_>>()
    } else {
        vec![]
    };
    let mut res = vec![];
    for name in dirs {
        let dir_name = if anchor.is_empty() {
            name.clone()
        } else {
            anchor.to_owned() + "/" + &name
        };
        res.push(enclose(&json_encode(&dir_name), "\"", "\""));
        res.extend(dir_list(&dir.join(&name), &dir_name, exclude));
    }
    res
}

But I think I made the code worse, because use AI a lot. How would you suggest to improve the code?

2

A couple of random notes:

  1. Posix declates nftw fun to traverse dirs, we used it on Linux it appeared to be faster the one we had in a standard lib of our language, I wonder if there is a crate written on top of it (nix probably?).
    nftw
    It might be faster than custom one.

  2. File systems aren't atomic (there was an attepmt to make atomic NTFS but it failed), so there is usually no need to check if something is dir before reading it: it might be changed right in the moment you check it. Instead, just read dir (or file) and process an error.

  3. The fact you want to skip .git dirs should better be coded in one place (array of strings-to-ignore should fast enough to be checked on each call, IMHO)

  4. JSON machinery better be separated from the iteration logic. Make your function accept lambda, and report your findings there.

  5. Empty vector could be replaced by None, not sure if it really matters, though.

PS: 2 and 3 seems to be solved in the latter one.
PPS: Dir walking might also raise some interesting questions e.g.: do you follow symlinks? If you do, how do you make sure you can't be caught in a loop? And do you follow mounted devices (cross file-system links) on **nix?

3

I tend to find using a stack of paths/entries is simplest, regardless of other considerations. eg:

let mut paths = vec![root_path];
while let Some(path) = paths.pop() {
    if entry.file_type().is_dir() {
        for entry in read_dir(path) {
            paths.push(entry.path());
        }
    } else {
        use file entry
    }
}

The specifics are pretty sensitive to what you're trying to do though.

4

It's a good point, however it won't come in a head of most reviewers (if any), so providing an explicit check for a directory is reasonable.

An interesting fact, that I had exact an opposite situation in other thread , I had to check for a directory to use Rust or native call:

let file = if path.is_dir() {
        open_directory(path)
    } else {
        File::open(path)
    }

As for my particular use case, I do not specifically care, because a result of the function is used for a suggestion of a directory, when a file gets created.

5

This depends on your need. Your code and the other code in the replies will not handle symlink that points to folder. You may want to visit symlink that also points to folder to not skip folder if it is behind symlink, or not, depending on your need. Then if you add that symlink handling, you have to also add mechanism to prevent infinite loop if that symlink points back to the previous folder