File system regex searcher

Question

This is my first Rust project (I'm primarily a C guy). I want to know if my code is properly idiomatic (Rusty?). Also, are there any inefficiencies?

The code defines an iterator (Searcher) that takes a regex, a starting directory, and an optional maximum search depth. It generates locations of regex matches within the file system.

use std::fs;
use std::io;
use std::io::BufRead;
use std::iter;
use std::path;
use std::vec::Vec;

use regex;

pub struct Location {
    pub file: path::PathBuf,
    pub line: usize,
    pub text: String,
}

struct FileScanner<'a> {
    path: path::PathBuf,
    pattern: &'a regex::Regex,
    lines: iter::Enumerate<io::Lines<io::BufReader<fs::File>>>,
}

fn is_line_printable(line: &str) -> bool {
    line.chars()
        .all(|c| c.is_ascii_graphic() || c.is_whitespace())
}

impl<'a> FileScanner<'a> {
    fn build(path: path::PathBuf, pattern: &'a regex::Regex) -> Option<Self> {
        let handle = match fs::File::open(&path) {
            Ok(h) => h,
            Err(_) => return None,
        };
        let reader = io::BufReader::new(handle);
        Some(FileScanner {
            path,
            pattern,
            lines: reader.lines().into_iter().enumerate(),
        })
    }
}

impl<'a> Iterator for FileScanner<'a> {
    type Item = Location;

    fn next(&mut self) -> Option<Location> {
        loop {
            let (index, line) = match self.lines.next() {
                Some((i, Ok(l))) => (i, l),
                _ => return None,
            };
            if !is_line_printable(&line) {
                return None;
            }

            let pattern_match = match self.pattern.find(&line) {
                Some(m) => m,
                None => continue,
            };
            let start = pattern_match.start();
            let end = pattern_match.end();
            return Some(Location {
                file: self.path.to_path_buf(),
                line: index + 1,
                text: String::from(&line[start..end]),
            });
        }
    }
}

pub struct Searcher<'a> {
    pattern: &'a regex::Regex,
    max_depth: Option<u8>,
    readers: Vec<fs::ReadDir>,
    current_scanner: Option<FileScanner<'a>>,
}

impl<'a> Searcher<'a> {
    pub fn build(
        pattern: &'a regex::Regex,
        directory: &'a path::Path,
        depth: Option<u8>,
    ) -> Result<Self, String> {
        match depth {
            Some(0) => return Err(String::from("Depth cannot be 0")),
            _ => (),
        };

        let reader = match fs::read_dir(directory) {
            Ok(r) => r,
            Err(error) => return Err(error.to_string()),
        };
        let readers = vec![reader];

        Ok(Searcher {
            pattern,
            max_depth: depth,
            readers,
            current_scanner: None,
        })
    }

    fn push_directory(&mut self, directory: path::PathBuf) {
        match self.max_depth {
            Some(depth) if usize::from(depth) == self.readers.len() => return,
            _ => (),
        }

        let reader = match fs::read_dir(directory) {
            Ok(r) => r,
            Err(_) => return,
        };
        self.readers.push(reader);
    }

    fn next_match_from_file(&mut self) -> Option<Location> {
        let scanner = match &mut self.current_scanner {
            Some(s) => s,
            None => return None,
        };

        let location = scanner.next();
        if location.is_none() {
            self.current_scanner = None;
        }
        location
    }
}

impl<'a> Iterator for Searcher<'a> {
    type Item = Location;

    fn next(&mut self) -> Option<Location> {
        let location = self.next_match_from_file();
        if location.is_some() {
            return location;
        }

        while self.readers.len() > 0 {
            let current_reader = self.readers.last_mut().unwrap();
            let entry = match current_reader.next() {
                Some(Ok(ent)) => ent,
                Some(Err(_)) | None => {
                    self.readers.pop();
                    continue;
                }
            };

            let path = entry.path();
            if path.is_dir() {
                self.push_directory(path);
                continue;
            }

            self.current_scanner = FileScanner::build(path, self.pattern);
            let location = self.next_match_from_file();
            if location.is_some() {
                return location;
            }
        }

        None
    }
}

Answer 1

You did a good job, your code already leverages on main rust features, but there are some suggestions to make your code more idiomatic and efficient:

Idiomatic improvements

1. Use ? for error propagation

Instead of manual match expressions:

// Current approach
let reader = match fs::read_dir(directory) {
    Ok(r) => r,
    Err(error) => return Err(error.to_string()),
};

// More idiomatic
let reader = fs::read_dir(directory).map_err(|e| e.to_string())?;

2. Use if let for simpler pattern matching (it's cleaner)

// Instead of
match depth {
    Some(0) => return Err(String::from("Depth cannot be 0")),
    _ => (),
};

// More idiomatic
if let Some(0) = depth {
    return Err("Depth cannot be 0".to_string());
}

3. Avoid unnecessary empty match arms

// Instead of
match self.max_depth {
    Some(depth) if usize::from(depth) == self.readers.len() => return,
    _ => (),
}

// Better
if let Some(depth) = self.max_depth {
    if usize::from(depth) == self.readers.len() {
        return;
    }
}

Performance considerations

1. Binary file detection: Your is_line_printable function reads every character of every line. Consider checking a sample of bytes at the beginning of the file instead.

2. Path cloning: You're creating a new PathBuf for each match with self.path.to_path_buf(). This is sometimes unavoidable (unfortunately) but worth noting.

3. File filtering: Add extension filtering to avoid scanning binary files altogether.

4. Memory usage: The current approach holds file handles open until matches are found, which is generally fine but could be improved.

Here's an improved version of your code:

use std::fs;
use std::io::{self, BufRead};
use std::iter;
use std::path::{Path, PathBuf};
use std::vec::Vec;

use regex::Regex;

pub struct Location {
    pub file: PathBuf,
    pub line: usize,
    pub line_content: String, // store whole line for context
    pub match_text: String,   // store matched text separately
    pub match_span: (usize, usize), // start and end positions
}

struct FileScanner<'a> {
    path: PathBuf,
    pattern: &'a Regex,
    lines: iter::Enumerate<io::Lines<io::BufReader<fs::File>>>,
}

// improved binary detection - (check first few bytes)
fn is_likely_binary(path: &Path) -> io::Result<bool> {
    let file = fs::File::open(path)?;
    let mut buffer = [0; 512]; // sample first 512 bytes
    let bytes_read = file.take(512).read(&mut buffer)?;
    
    // check for null bytes or high concentration of non-ASCII chars
    let non_text_chars = buffer[..bytes_read]
        .iter()
        .filter(|&&b| b == 0 || b > 127)
        .count();
        
    Ok(non_text_chars > bytes_read / 4) // > 25% non-text chars indicates binary
}

impl<'a> FileScanner<'a> {
    fn new(path: PathBuf, pattern: &'a Regex) -> io::Result<Self> {
        let handle = fs::File::open(&path)?;
        let reader = io::BufReader::new(handle);
        
        Ok(FileScanner {
            path,
            pattern,
            lines: reader.lines().enumerate(),
        })
    }
    
    fn build(path: PathBuf, pattern: &'a Regex) -> Option<Self> {
        if is_likely_binary(&path).unwrap_or(true) {
            return None; // skip
        }
        
        Self::new(path).ok()
    }
}

impl<'a> Iterator for FileScanner<'a> {
    type Item = Location;

    fn next(&mut self) -> Option<Location> {
        while let Some((index, line_result)) = self.lines.next() {
            let line = match line_result {
                Ok(l) => l,
                Err(_) => continue, // skip lines with IO errors instead of aborting
            };

            if let Some(pattern_match) = self.pattern.find(&line) {
                let start = pattern_match.start();
                let end = pattern_match.end();
                
                return Some(Location {
                    file: self.path.clone(), // clone is necessary here
                    line: index + 1,
                    line_content: line.clone(),
                    match_text: line[start..end].to_string(),
                    match_span: (start, end),
                });
            }
        }
        None
    }
}

pub struct Searcher<'a> {
    pattern: &'a Regex,
    max_depth: Option<u8>,
    readers: Vec<fs::ReadDir>,
    current_scanner: Option<FileScanner<'a>>,
    allowed_extensions: Option<Vec<String>>, // add extension-based filtering
}

impl<'a> Searcher<'a> {
    pub fn new(
        pattern: &'a Regex,
        directory: &Path,
        depth: Option<u8>,
        extensions: Option<Vec<String>>,
    ) -> Result<Self, String> {
        if let Some(0) = depth {
            return Err("Depth cannot be 0".to_string());
        }

        let reader = fs::read_dir(directory).map_err(|e| e.to_string())?;
        
        Ok(Searcher {
            pattern,
            max_depth: depth,
            readers: vec![reader],
            current_scanner: None,
            allowed_extensions: extensions,
        })
    }
    
    pub fn build(
        pattern: &'a Regex,
        directory: &Path,
        depth: Option<u8>,
    ) -> Result<Self, String> {
        Self::new(pattern, directory, depth, None)
    }

    fn should_scan_file(&self, path: &Path) -> bool {
        if let Some(ref extensions) = self.allowed_extensions {
            if let Some(ext) = path.extension().and_then(|e| e.to_str()) {
                return extensions.iter().any(|e| e == ext);
            }
            return false; // no extension
        }
        true // no extension filtering
    }

    fn push_directory(&mut self, directory: PathBuf) {
        // early return if we've reached max depth
        if let Some(depth) = self.max_depth {
            if usize::from(depth) == self.readers.len() {
                return;
            }
        }

        if let Ok(reader) = fs::read_dir(directory) {
            self.readers.push(reader);
        }
    }

    fn next_match_from_file(&mut self) -> Option<Location> {
        let location = self.current_scanner.as_mut()?.next();
        if location.is_none() {
            self.current_scanner = None;
        }
        location
    }
}

impl<'a> Iterator for Searcher<'a> {
    type Item = Location;

    fn next(&mut self) -> Option<Location> {
        if let Some(location) = self.next_match_from_file() {
            return Some(location);
        }

        while let Some(reader) = self.readers.last_mut() {
            // here we search through all directories
            match reader.next() {
                Some(Ok(entry)) => {
                    let path = entry.path();
                    
                    if path.is_dir() {
                        self.push_directory(path);
                    } else if self.should_scan_file(&path) {
                        self.current_scanner = FileScanner::build(path, self.pattern);
                        if let Some(location) = self.next_match_from_file() {
                            return Some(location);
                        }
                    }
                }
                _ => {
                    // if it reaches here it means there's an error
                    self.readers.pop();
                }
            }
        }

        None
    }
}