diff --git a/src/ops/archive/sevenz.rs b/src/ops/archive/sevenz.rs index 554a695..eb4eaea 100644 --- a/src/ops/archive/sevenz.rs +++ b/src/ops/archive/sevenz.rs @@ -1,5 +1,5 @@ use std::cell::Cell; -use std::fs::{self}; +use std::fs; use std::io; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, Ordering}; diff --git a/src/ops/archive/tar.rs b/src/ops/archive/tar.rs index 7bcc1ca..a221ba7 100644 --- a/src/ops/archive/tar.rs +++ b/src/ops/archive/tar.rs @@ -14,6 +14,10 @@ use super::{ use crate::debug_log; use crate::ops::helpers::cleanup_file as cleanup_temp_file; +/// Upper bound on entries written into a created archive. Intentionally equal +/// to `super::MAX_LIST_ENTRIES` (100_000): the create and list caps are kept in +/// lockstep so a user-created archive always fits a listing. Mirrored in +/// `zip::MAX_CREATE_ENTRIES`; both must track the list cap. const MAX_CREATE_ENTRIES: usize = 100_000; /// Write adapter that aborts once cumulative output exceeds diff --git a/src/ops/archive/zip.rs b/src/ops/archive/zip.rs index cde0409..c3682c2 100644 --- a/src/ops/archive/zip.rs +++ b/src/ops/archive/zip.rs @@ -38,6 +38,12 @@ fn map_zip_err(e: zip::result::ZipError) -> ArchiveError { } } +/// Maps a zip crate `CompressionMethod` to a display string. +/// +/// `_ => "Unknown"` is forced by the `zip` crate's API: `CompressionMethod` is +/// `#[non_exhaustive]`, so it cannot be exhaustively matched. New methods added +/// by a future crate release land here as `"Unknown"` until this match is +/// updated — there is no string accessor on the type to forward to. fn compression_method_name(method: CompressionMethod) -> &'static str { match method { CompressionMethod::Stored => "Stored", @@ -169,7 +175,7 @@ fn extract_zip_entries( } pub fn extract_zip( - file: std::fs::File, + file: File, dest: &Path, progress: &Sender, cancel: &AtomicBool, diff --git a/src/ops/batch/tests.rs b/src/ops/batch/tests.rs index 598293c..c30824e 100644 --- a/src/ops/batch/tests.rs +++ b/src/ops/batch/tests.rs @@ -738,7 +738,6 @@ fn batch_move_cancel_reports_canceled() { cancel.store(true, Ordering::Relaxed); }; let sources = vec![src_dir.path().join("a.txt")]; - let _sizes = helpers::path_sizes(&sources, None); let action_label = "Move"; let report = execute_batch_generic( diff --git a/src/ops/chunk_copy.rs b/src/ops/chunk_copy.rs index 9133f62..18b9662 100644 --- a/src/ops/chunk_copy.rs +++ b/src/ops/chunk_copy.rs @@ -3,8 +3,9 @@ use crate::debug_log; use std::ffi::OsString; use std::fs::{self, File}; use std::io::{self, Read, Write}; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::mpsc::Sender; use std::time::{Duration, Instant}; /// 256 KiB — balances syscall overhead against memory use. Above 512 KiB @@ -23,7 +24,7 @@ const PROGRESS_CHECK_BYTES: usize = 64 * 1024; pub fn copy_with_progress( src: &Path, dest: &Path, - progress_tx: &std::sync::mpsc::Sender, + progress_tx: &Sender, cancel: &AtomicBool, overwrite: bool, ) -> io::Result { @@ -135,7 +136,7 @@ fn open_regular_file(src: &Path) -> io::Result { /// `File::create_new` refuses to clobber an existing entry, so a stale temp (or a /// racing sibling copy) that happens to reuse the same name is retried a few /// times rather than aborting the whole copy with `AlreadyExists`. -fn create_temp_file(dest: &Path) -> io::Result<(std::path::PathBuf, File)> { +fn create_temp_file(dest: &Path) -> io::Result<(PathBuf, File)> { const MAX_ATTEMPTS: u32 = 8; let mut last_err = None; for _ in 0..MAX_ATTEMPTS { @@ -159,7 +160,7 @@ fn copy_to_temp( dest_file: File, temp_dest: &Path, metadata: &fs::Metadata, - progress_tx: &std::sync::mpsc::Sender, + progress_tx: &Sender, cancel: &AtomicBool, ) -> io::Result { let mut reader = src_file; @@ -288,7 +289,7 @@ fn publish_temp( fs::rename(temp_dest, dest) } -fn temp_path_for(dest: &Path) -> std::path::PathBuf { +fn temp_path_for(dest: &Path) -> PathBuf { let mut name = dest .file_name() .map(|name| name.to_os_string()) diff --git a/src/ops/file_ops/common.rs b/src/ops/file_ops/common.rs index d5511fb..450f88f 100644 --- a/src/ops/file_ops/common.rs +++ b/src/ops/file_ops/common.rs @@ -22,7 +22,12 @@ pub(super) fn same_inode(_a: &fs::Metadata, _b: &fs::Metadata) -> bool { false } +/// Windows needs the raw file_attributes bit because `Metadata::is_dir()` +/// follows reparse points (returns true for directory symlinks/junctions), +/// which must be removed via `remove_dir` not `remove_file`. On Unix this is +/// unused (symlinks are never `is_dir()`), so it's gated to avoid dead code. #[cfg(not(windows))] +#[allow(dead_code)] pub(super) fn is_dir_meta(meta: &fs::Metadata) -> bool { meta.is_dir() } @@ -36,13 +41,7 @@ pub(super) fn is_dir_meta(meta: &fs::Metadata) -> bool { /// Mandatory cancel check for callers with a required cancel token. pub(super) fn check_canceled(cancel: &AtomicBool) -> io::Result<()> { - if cancel.load(Ordering::Relaxed) { - return Err(io::Error::new( - io::ErrorKind::Interrupted, - "operation canceled", - )); - } - Ok(()) + check_optional_canceled(Some(cancel)) } /// Optional cancel check — used by functions that may or may not have a cancel token. @@ -259,7 +258,9 @@ pub(super) fn remove_any(path: &Path) -> io::Result<()> { return remove_dir_all_idempotent(path); } // Windows-only: directory symlinks/junctions have is_symlink() + is_dir_meta(). - // On Unix this branch is unreachable — symlink_metadata symlinks are !is_dir(). + // On Unix this branch is unreachable — symlink_metadata symlinks are !is_dir(), + // and !is_dir() symlinks fall through to remove_file below. + #[cfg(windows)] if meta.is_symlink() && is_dir_meta(&meta) { return match fs::remove_dir(path) { Ok(()) => Ok(()), diff --git a/src/ops/file_ops/move_ops.rs b/src/ops/file_ops/move_ops.rs index ec6bd3a..72de5e9 100644 --- a/src/ops/file_ops/move_ops.rs +++ b/src/ops/file_ops/move_ops.rs @@ -116,6 +116,11 @@ impl MoveKind { /// change atomically. /// - On case-sensitive filesystems, `dest` metadata lookup fails (target does /// not exist), so the function proceeds as a normal move. +/// +/// Test-only convenience wrapper around `move_entry_impl` that synthesizes a +/// dummy progress channel and an always-clear cancel token. Gated `#[cfg(test)]` +/// so the only public entry point for callers is `move_entry_with_progress`; +/// `pub` visibility is required for the integration tests that link against it. #[cfg(test)] pub fn move_entry(src: &Path, dest: &Path, overwrite: bool) -> io::Result<()> { let cancel = AtomicBool::new(false); diff --git a/src/ops/file_ops/temp.rs b/src/ops/file_ops/temp.rs index 08ba15b..4a1bea1 100644 --- a/src/ops/file_ops/temp.rs +++ b/src/ops/file_ops/temp.rs @@ -12,6 +12,8 @@ use super::common::{MSG_DEST_EXISTS, remove_any}; const DEFAULT_NAME: &str = "copy"; const TEMP_NAME_MAX_ATTEMPTS: u32 = 128; +#[derive(Debug)] +#[must_use = "the temp directory is cleaned up on drop unless committed"] pub(crate) struct TempDirGuard { path: PathBuf, committed: bool, diff --git a/src/ops/search.rs b/src/ops/search.rs index e5c7fc3..797de11 100644 --- a/src/ops/search.rs +++ b/src/ops/search.rs @@ -2,15 +2,10 @@ //! //! Full file search by name pattern or content. -#[path = "search/content.rs"] mod content; -#[path = "search/model.rs"] mod model; -#[path = "search/name.rs"] mod name; -#[path = "search/pattern.rs"] mod pattern; -#[path = "search/walk.rs"] mod walk; pub use content::search_content; diff --git a/src/ops/search/content.rs b/src/ops/search/content.rs index 3429c7b..2587f89 100644 --- a/src/ops/search/content.rs +++ b/src/ops/search/content.rs @@ -3,7 +3,7 @@ use std::fs::File; use std::io::{BufRead, BufReader, Read}; use std::path::Path; use std::sync::Arc; -use std::sync::atomic::AtomicBool; +use std::sync::atomic::{AtomicBool, Ordering}; use memchr::{memchr, memmem}; @@ -16,7 +16,6 @@ use crate::ops::search::{ MAX_CONTENT_FILE_BYTES, MAX_CONTENT_LINE_BYTES, MAX_CONTENT_RESULTS, MAX_SEARCH_DEPTH, MAX_SEARCH_ITEMS, SearchError, SearchErrorKind, SearchOutcome, TruncationReason, }; -use std::sync::atomic::Ordering; /// A content-search hit: the file it was found in, the 1-based line number, and /// the matched line text. The path is an [`Arc`] so a file with many matches diff --git a/src/ops/search/name.rs b/src/ops/search/name.rs index 2ec2611..615fb29 100644 --- a/src/ops/search/name.rs +++ b/src/ops/search/name.rs @@ -2,7 +2,7 @@ use std::collections::HashSet; use std::fs::Metadata; use std::io; use std::path::Path; -use std::sync::atomic::AtomicBool; +use std::sync::atomic::{AtomicBool, Ordering}; use crate::app::types::FileEntry; use crate::fs::reader::{file_info_from_metadata, get_file_info}; @@ -13,7 +13,6 @@ use crate::ops::search::walk::{ use crate::ops::search::{ MAX_SEARCH_DEPTH, MAX_SEARCH_ITEMS, SearchError, SearchErrorKind, SearchOutcome, }; -use std::sync::atomic::Ordering; /// Initial capacity for the visited inode set. Most directories contain well under /// 256 entries; this avoids reallocations for typical workloads while staying small. @@ -112,7 +111,10 @@ fn search_files_recursive( // For a plain directory `entry.metadata()` (lstat) is enough and is // reused for FileEntry + cycle detection. For a symlink we follow // once via `fs::metadata` and only recurse when the target is a dir. - let plain_dir = recursive && file_type.is_dir() && !file_type.is_symlink(); + // `FileType::is_dir()` returns false for symlinks on std::fs, so the + // bare `is_dir()` check already excludes symlinked dirs — the redundant + // `!is_symlink()` that guarded it has been removed. + let plain_dir = recursive && file_type.is_dir(); let dir_meta: Option> = plain_dir.then(|| entry.metadata()); // Whether this entry needs its path allocated (for a match result or diff --git a/src/ops/search/pattern.rs b/src/ops/search/pattern.rs index d60b826..f93e25b 100644 --- a/src/ops/search/pattern.rs +++ b/src/ops/search/pattern.rs @@ -163,9 +163,11 @@ impl WildcardAffix { // Only insensitive matching consults the char slices; don't pay for them // on case-sensitive patterns. let to_chars = |s: &Option| { - insensitive - .then(|| s.as_ref().map(|x| x.chars().collect::>())) - .flatten() + if insensitive { + s.as_ref().map(|x| x.chars().collect::>()) + } else { + None + } }; let prefix_chars = to_chars(&prefix); let suffix_chars = to_chars(&suffix);