From 745c6adbf21f31aaf715e118989c6b0fbc505289 Mon Sep 17 00:00:00 2001 From: Hare Date: Sun, 30 Aug 2026 20:15:01 +0900 Subject: [PATCH] fix: apply Grep filters to direct files --- crates/fs-operation/src/lib.rs | 214 ++++++++++++++++----- crates/fs-operation/src/search.rs | 232 +++++++++++++++-------- crates/workdir/src/local.rs | 4 +- crates/worker-runtime/src/http_server.rs | 4 +- 4 files changed, 323 insertions(+), 131 deletions(-) diff --git a/crates/fs-operation/src/lib.rs b/crates/fs-operation/src/lib.rs index 21bbe2ab..fb2cd54f 100644 --- a/crates/fs-operation/src/lib.rs +++ b/crates/fs-operation/src/lib.rs @@ -157,6 +157,22 @@ mod tests { } } + fn grep_request(path: &str, pattern: &str) -> GrepRequest { + GrepRequest { + pattern: pattern.to_string(), + path: FsPath::new(path).unwrap(), + glob: None, + file_type: None, + case_insensitive: false, + before_context: 0, + after_context: 0, + multiline: false, + output_mode: GrepOutputMode::Content, + limit: 10, + offset: 0, + } + } + #[test] fn logical_paths_reject_absolute_parent_and_backslash_forms() { assert!(FsPath::new("src/lib.rs").is_ok()); @@ -289,25 +305,10 @@ mod tests { let root = temp.path().canonicalize().unwrap(); let readable = RootAccess(root.clone()); - let direct = run_grep( - &root, - selected, - GrepRequest { - pattern: "needle".to_string(), - path: FsPath::new("selected.txt").unwrap(), - glob: None, - file_type: None, - case_insensitive: false, - before_context: 1, - after_context: 1, - multiline: false, - output_mode: GrepOutputMode::Content, - limit: 10, - offset: 0, - }, - &readable, - ) - .unwrap(); + let mut request = grep_request("selected.txt", "needle"); + request.before_context = 1; + request.after_context = 1; + let direct = run_grep(&root, selected, request, &readable).unwrap(); assert_eq!(direct.match_count, 1); assert_eq!(direct.matched_files, 1); @@ -345,6 +346,115 @@ mod tests { assert_eq!(directory.matched_files, 2); } + #[test] + fn grep_direct_file_applies_glob_and_type_filters_for_every_output_mode() { + let temp = tempfile::tempdir().unwrap(); + let nested = temp.path().join("nested"); + std::fs::create_dir(&nested).unwrap(); + let selected = nested.join("selected.rs"); + std::fs::write(&selected, "needle one\nneedle two\n").unwrap(); + let root = temp.path().canonicalize().unwrap(); + let readable = RootAccess(root.clone()); + + for mode in [ + GrepOutputMode::Content, + GrepOutputMode::FilesWithMatches, + GrepOutputMode::Count, + ] { + for (glob, file_type) in [(Some("other/*.rs"), None), (None, Some("python"))] { + let mut request = grep_request("nested/selected.rs", "needle"); + request.output_mode = mode; + request.glob = glob.map(str::to_string); + request.file_type = file_type.map(str::to_string); + + let excluded = run_grep(&root, selected.clone(), request, &readable).unwrap(); + assert_eq!(excluded.output, "", "mode {mode:?}"); + assert_eq!(excluded.match_count, 0, "mode {mode:?}"); + assert_eq!(excluded.matched_files, 0, "mode {mode:?}"); + assert!(!excluded.truncated, "mode {mode:?}"); + } + + let mut request = grep_request("nested/selected.rs", "needle"); + request.output_mode = mode; + request.glob = Some("nested/*.rs".to_string()); + request.file_type = Some("rust".to_string()); + let matched = run_grep(&root, selected.clone(), request, &readable).unwrap(); + + match mode { + GrepOutputMode::Content => { + assert_eq!(matched.match_count, 2); + assert_eq!(matched.matched_files, 1); + assert!(matched.output.starts_with("nested/selected.rs\n")); + assert!(matched.output.contains("> 1 │ needle one")); + assert!(matched.output.contains("> 2 │ needle two")); + } + GrepOutputMode::FilesWithMatches => { + assert_eq!(matched.match_count, 1); + assert_eq!(matched.matched_files, 1); + assert_eq!(matched.output, "nested/selected.rs\n"); + } + GrepOutputMode::Count => { + assert_eq!(matched.match_count, 2); + assert_eq!(matched.matched_files, 1); + assert_eq!(matched.output, "nested/selected.rs:2\n"); + } + } + assert!(!matched.truncated, "mode {mode:?}"); + } + } + + #[test] + fn grep_direct_file_preserves_explicit_hidden_and_gitignored_behavior() { + let temp = tempfile::tempdir().unwrap(); + let hidden = temp.path().join(".hidden.rs"); + let ignored = temp.path().join("ignored.rs"); + std::fs::write(&hidden, "needle hidden\n").unwrap(); + std::fs::write(&ignored, "needle ignored\n").unwrap(); + std::fs::write(temp.path().join(".gitignore"), "ignored.rs\n").unwrap(); + let root = temp.path().canonicalize().unwrap(); + let readable = RootAccess(root.clone()); + + for (path, expected) in [ + (".hidden.rs", "needle hidden"), + ("ignored.rs", "needle ignored"), + ] { + let result = run_grep( + &root, + root.join(path), + grep_request(path, "needle"), + &readable, + ) + .unwrap(); + assert_eq!(result.match_count, 1, "path {path}"); + assert!(result.output.contains(expected), "path {path}"); + } + } + + #[test] + fn grep_direct_file_preserves_case_multiline_and_bounds() { + let temp = tempfile::tempdir().unwrap(); + let selected = temp.path().join("selected.txt"); + std::fs::write(&selected, "NEEDLE first\nstart\nfinish\nneedle last\n").unwrap(); + let root = temp.path().canonicalize().unwrap(); + let readable = RootAccess(root.clone()); + + let mut case_request = grep_request("selected.txt", "needle"); + case_request.case_insensitive = true; + case_request.offset = 1; + case_request.limit = 1; + let bounded = run_grep(&root, selected.clone(), case_request, &readable).unwrap(); + assert_eq!(bounded.match_count, 1); + assert!(!bounded.output.contains("NEEDLE first")); + assert!(bounded.output.contains("needle last")); + assert!(bounded.truncated); + + let mut multiline_request = grep_request("selected.txt", "start\\nfinish"); + multiline_request.multiline = true; + let multiline = run_grep(&root, selected, multiline_request, &readable).unwrap(); + assert_eq!(multiline.match_count, 1); + assert!(multiline.output.contains("start\nfinish")); + } + #[test] fn grep_returns_not_found_for_a_missing_direct_path() { let temp = tempfile::tempdir().unwrap(); @@ -355,19 +465,7 @@ mod tests { let error = run_grep( &root, missing.clone(), - GrepRequest { - pattern: "needle".to_string(), - path: FsPath::new("missing.txt").unwrap(), - glob: None, - file_type: None, - case_insensitive: false, - before_context: 0, - after_context: 0, - multiline: false, - output_mode: GrepOutputMode::Content, - limit: 10, - offset: 0, - }, + grep_request("missing.txt", "needle"), &readable, ) .unwrap_err(); @@ -384,22 +482,22 @@ mod tests { let root = temp.path().canonicalize().unwrap(); let readable = RootAccess(root.clone()); std::fs::create_dir(root.join("target-dir")).unwrap(); + std::fs::write(root.join("target-file.rs"), "needle file\n").unwrap(); + symlink(root.join("target-file.rs"), root.join("file-link.rs")).unwrap(); symlink(root.join("target-dir"), root.join("directory-link")).unwrap(); symlink(root.join("missing-target"), root.join("broken-link")).unwrap(); - let request = |path: &str| GrepRequest { - pattern: "needle".to_string(), - path: FsPath::new(path).unwrap(), - glob: None, - file_type: None, - case_insensitive: false, - before_context: 0, - after_context: 0, - multiline: false, - output_mode: GrepOutputMode::Content, - limit: 10, - offset: 0, - }; + let request = |path: &str| grep_request(path, "needle"); + + let file_result = run_grep( + &root, + root.join("file-link.rs"), + request("file-link.rs"), + &readable, + ) + .unwrap(); + assert_eq!(file_result.match_count, 1); + assert!(file_result.output.starts_with("file-link.rs\n")); let directory_error = run_grep( &root, @@ -427,6 +525,32 @@ mod tests { )); } + #[cfg(unix)] + #[test] + fn grep_rejects_a_direct_special_file_as_invalid_argument() { + use std::os::unix::net::UnixListener; + + let temp = tempfile::tempdir().unwrap(); + let socket = temp.path().join("grep.sock"); + let _listener = UnixListener::bind(&socket).unwrap(); + let root = temp.path().canonicalize().unwrap(); + let readable = RootAccess(root.clone()); + + let error = run_grep( + &root, + socket, + grep_request("grep.sock", "needle"), + &readable, + ) + .unwrap_err(); + + assert!(matches!( + error, + FsError::InvalidArgument(message) + if message.contains("must be a regular file or directory") + )); + } + #[test] fn grep_content_groups_lines_by_file_and_marks_matches() { let temp = tempfile::tempdir().unwrap(); diff --git a/crates/fs-operation/src/search.rs b/crates/fs-operation/src/search.rs index 5a61d18b..a6d9d336 100644 --- a/crates/fs-operation/src/search.rs +++ b/crates/fs-operation/src/search.rs @@ -7,8 +7,8 @@ use grep_regex::RegexMatcherBuilder; use grep_searcher::sinks::UTF8 as UTF8Sink; use grep_searcher::{BinaryDetection, Searcher, SearcherBuilder, Sink, SinkContext, SinkMatch}; use ignore::WalkBuilder; -use ignore::overrides::OverrideBuilder; -use ignore::types::TypesBuilder; +use ignore::overrides::{Override, OverrideBuilder}; +use ignore::types::{Types, TypesBuilder}; use crate::{FsError, GrepOutputMode, GrepRequest, GrepResult, direct_symlink}; @@ -126,6 +126,38 @@ fn logical_display(root: &Path, path: &Path) -> String { const DEFAULT_HEAD_LIMIT: usize = 250; +fn build_overrides(base: &Path, glob: Option<&str>) -> Result, FsError> { + let Some(glob) = glob else { + return Ok(None); + }; + let mut builder = OverrideBuilder::new(base); + builder + .add(glob) + .map_err(|error| FsError::InvalidGlob(error.to_string()))?; + builder + .build() + .map(Some) + .map_err(|error| FsError::InvalidGlob(error.to_string())) +} + +fn build_types(file_type: Option<&str>) -> Result, FsError> { + let Some(file_type) = file_type else { + return Ok(None); + }; + let mut builder = TypesBuilder::new(); + builder.add_defaults(); + builder.select(file_type); + builder + .build() + .map(Some) + .map_err(|error| FsError::InvalidArgument(format!("invalid type {file_type}: {error}"))) +} + +fn direct_file_selected(path: &Path, overrides: Option<&Override>, types: Option<&Types>) -> bool { + !overrides.is_some_and(|filter| filter.matched(path, false).is_ignore()) + && !types.is_some_and(|filter| filter.matched(path, false).is_ignore()) +} + struct GrepParams { pattern: String, path: Option, @@ -237,32 +269,9 @@ pub fn run_grep( }); } - let mut wb = WalkBuilder::new(&base); - wb.hidden(true) - .git_ignore(true) - .git_global(true) - .git_exclude(true) - .ignore(true) - .parents(true) - .follow_links(false); - - if let Some(t) = p.file_type.as_deref() { - let mut tb = TypesBuilder::new(); - tb.add_defaults(); - tb.select(t); - let types = tb - .build() - .map_err(|e| FsError::InvalidArgument(format!("invalid type {t}: {e}")))?; - wb.types(types); - } - if let Some(g) = p.glob.as_deref() { - let mut ob = OverrideBuilder::new(&base); - ob.add(g).map_err(|e| FsError::InvalidGlob(e.to_string()))?; - let ov = ob - .build() - .map_err(|e| FsError::InvalidGlob(e.to_string()))?; - wb.overrides(ov); - } + let filter_base = if base_meta.is_file() { root } else { &base }; + let types = build_types(p.file_type.as_deref())?; + let overrides = build_overrides(filter_base, p.glob.as_deref())?; let mode = p.output_mode.unwrap_or_default(); let head_limit = p.head_limit.unwrap_or(DEFAULT_HEAD_LIMIT); @@ -277,74 +286,133 @@ pub fn run_grep( lines: Vec::new(), truncated: false, }; + let mut matching_files_seen = 0; + let mut matches_seen = 0; - // Per-mode walker state. - let mut matching_files_seen: usize = 0; - let mut matches_seen: usize = 0; + if base_meta.is_file() { + if direct_file_selected(&base, overrides.as_ref(), types.as_ref()) { + scan_path( + &mut searcher, + &matcher, + &base, + mode, + &mut report, + &mut matching_files_seen, + &mut matches_seen, + offset, + head_limit, + )?; + } + return Ok(report.into_result(root)); + } - 'walker: for entry in wb.build().flatten() { - if !entry.file_type().map(|t| t.is_file()).unwrap_or(false) { + let mut walker = WalkBuilder::new(&base); + walker + .hidden(true) + .git_ignore(true) + .git_global(true) + .git_exclude(true) + .ignore(true) + .parents(true) + .follow_links(false); + if let Some(types) = types { + walker.types(types); + } + if let Some(overrides) = overrides { + walker.overrides(overrides); + } + + for entry in walker.build().flatten() { + if !entry + .file_type() + .map(|kind| kind.is_file()) + .unwrap_or(false) + { continue; } let path = entry.path(); if !access.is_readable(path) { continue; } - - match mode { - GrepOutputMode::FilesWithMatches => { - let hit = scan_any_match(&mut searcher, &matcher, path)?; - if !hit { - continue; - } - if matching_files_seen >= offset { - report.files.push(path.to_path_buf()); - if report.files.len() >= head_limit { - report.truncated = true; - break 'walker; - } - } - matching_files_seen += 1; - } - GrepOutputMode::Count => { - let count = scan_count(&mut searcher, &matcher, path)?; - if count == 0 { - continue; - } - if matching_files_seen >= offset { - report.counts.push((path.to_path_buf(), count)); - if report.counts.len() >= head_limit { - report.truncated = true; - break 'walker; - } - } - matching_files_seen += 1; - } - GrepOutputMode::Content => { - let before_count = matches_seen; - let mut sink = ContentSink { - path: path.to_path_buf(), - lines: &mut report.lines, - matches_seen: &mut matches_seen, - offset, - head_limit, - }; - searcher - .search_path(&matcher, path, &mut sink) - .map_err(|e| FsError::io(path, e))?; - // If we hit head_limit during this file, stop walking. - if matches_seen >= offset.saturating_add(head_limit) && matches_seen > before_count - { - report.truncated = true; - break 'walker; - } - } + if scan_path( + &mut searcher, + &matcher, + path, + mode, + &mut report, + &mut matching_files_seen, + &mut matches_seen, + offset, + head_limit, + )? { + break; } } Ok(report.into_result(root)) } +#[allow(clippy::too_many_arguments)] +fn scan_path( + searcher: &mut Searcher, + matcher: &grep_regex::RegexMatcher, + path: &Path, + mode: GrepOutputMode, + report: &mut GrepReport, + matching_files_seen: &mut usize, + matches_seen: &mut usize, + offset: usize, + head_limit: usize, +) -> Result { + match mode { + GrepOutputMode::FilesWithMatches => { + if !scan_any_match(searcher, matcher, path)? { + return Ok(false); + } + if *matching_files_seen >= offset { + report.files.push(path.to_path_buf()); + if report.files.len() >= head_limit { + report.truncated = true; + return Ok(true); + } + } + *matching_files_seen += 1; + } + GrepOutputMode::Count => { + let count = scan_count(searcher, matcher, path)?; + if count == 0 { + return Ok(false); + } + if *matching_files_seen >= offset { + report.counts.push((path.to_path_buf(), count)); + if report.counts.len() >= head_limit { + report.truncated = true; + return Ok(true); + } + } + *matching_files_seen += 1; + } + GrepOutputMode::Content => { + let before_count = *matches_seen; + let mut sink = ContentSink { + path: path.to_path_buf(), + lines: &mut report.lines, + matches_seen, + offset, + head_limit, + }; + searcher + .search_path(matcher, path, &mut sink) + .map_err(|error| FsError::io(path, error))?; + if *matches_seen >= offset.saturating_add(head_limit) && *matches_seen > before_count { + report.truncated = true; + return Ok(true); + } + } + } + Ok(false) +} + fn scan_any_match( searcher: &mut Searcher, matcher: &grep_regex::RegexMatcher, diff --git a/crates/workdir/src/local.rs b/crates/workdir/src/local.rs index ea500718..a897076f 100644 --- a/crates/workdir/src/local.rs +++ b/crates/workdir/src/local.rs @@ -1944,8 +1944,8 @@ mod tests { GrepRequest { pattern: "NEEDLE".into(), path: WorkdirPath::new("src/main.rs").unwrap(), - glob: None, - file_type: None, + glob: Some("src/*.rs".into()), + file_type: Some("rust".into()), case_insensitive: false, before_context: 0, after_context: 0, diff --git a/crates/worker-runtime/src/http_server.rs b/crates/worker-runtime/src/http_server.rs index a5fd8cb6..6bfd2100 100644 --- a/crates/worker-runtime/src/http_server.rs +++ b/crates/worker-runtime/src/http_server.rs @@ -2353,8 +2353,8 @@ mod tests { operation: WorkdirSessionOperation::Grep(GrepRequest { pattern: "hello".into(), path: WorkdirPath::new("hello.txt").unwrap(), - glob: None, - file_type: None, + glob: Some("*.txt".into()), + file_type: Some("txt".into()), case_insensitive: false, before_context: 0, after_context: 0,