From 1dfd98a5708bc9e77e8328a6c04f04d321ad9a8d Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 20 Jul 2025 11:12:27 +0100 Subject: [PATCH 1/3] Refactor io rewrite helpers and add error tests --- src/io.rs | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/src/io.rs b/src/io.rs index e4c71c7a..9f6cd105 100644 --- a/src/io.rs +++ b/src/io.rs @@ -4,26 +4,35 @@ use std::{fs, path::Path}; use crate::process::{process_stream, process_stream_no_wrap}; -/// Rewrite a file in place with wrapped tables. +/// Read `path`, process the contents with `f`, and write the result back. +/// +/// This helper encapsulates the common pattern used by [`rewrite`] and +/// [`rewrite_no_wrap`]. /// /// # Errors /// Returns an error if reading or writing the file fails. -pub fn rewrite(path: &Path) -> std::io::Result<()> { +fn rewrite_with(path: &Path, f: F) -> std::io::Result<()> +where + F: Fn(&[String]) -> Vec, +{ let text = fs::read_to_string(path)?; let lines: Vec = text.lines().map(str::to_string).collect(); - let fixed = process_stream(&lines); + let fixed = f(&lines); fs::write(path, fixed.join("\n") + "\n") } +/// Rewrite a file in place with wrapped tables. +/// +/// # Errors +/// Returns an error if reading or writing the file fails. +pub fn rewrite(path: &Path) -> std::io::Result<()> { rewrite_with(path, process_stream) } + /// Rewrite a file in place without wrapping text. /// /// # Errors /// Returns an error if reading or writing the file fails. pub fn rewrite_no_wrap(path: &Path) -> std::io::Result<()> { - let text = fs::read_to_string(path)?; - let lines: Vec = text.lines().map(str::to_string).collect(); - let fixed = process_stream_no_wrap(&lines); - fs::write(path, fixed.join("\n") + "\n") + rewrite_with(path, process_stream_no_wrap) } #[cfg(test)] @@ -51,4 +60,19 @@ mod tests { let out = fs::read_to_string(&file).unwrap(); assert_eq!(out, "| A | B |\n| 1 | 2 |\n"); } + + #[test] + fn rewrite_missing_file() { + let dir = tempdir().unwrap(); + let file = dir.path().join("missing.md"); + let err = rewrite(&file).expect_err("expected error for missing file"); + assert_eq!(err.kind(), std::io::ErrorKind::NotFound); + } + + #[test] + fn rewrite_permission_denied() { + let file = Path::new("/proc/1/attr/current"); + let err = rewrite(file).expect_err("expected permission denied error"); + assert_eq!(err.kind(), std::io::ErrorKind::PermissionDenied); + } } From 870c91eb26d2533afa007436c0926350139ae0ce Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 20 Jul 2025 12:19:59 +0100 Subject: [PATCH 2/3] Handle IO errors and update tests --- Cargo.lock | 1 + Cargo.toml | 1 + src/io.rs | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 70 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d1676e23..b29d29fd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -403,6 +403,7 @@ dependencies = [ "assert_cmd", "clap", "html5ever", + "libc", "markup5ever_rcdom", "once_cell", "regex", diff --git a/Cargo.toml b/Cargo.toml index 19afabb3..ed31b4a6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,7 @@ unicode-width = ">=0.1, <0.2" rstest = "0.18" assert_cmd = "2" tempfile = "3" +libc = "0.2" [lints.clippy] pedantic = "warn" diff --git a/src/io.rs b/src/io.rs index 9f6cd105..6f8a017b 100644 --- a/src/io.rs +++ b/src/io.rs @@ -18,7 +18,12 @@ where let text = fs::read_to_string(path)?; let lines: Vec = text.lines().map(str::to_string).collect(); let fixed = f(&lines); - fs::write(path, fixed.join("\n") + "\n") + let output = if fixed.is_empty() { + String::new() + } else { + fixed.join("\n") + "\n" + }; + fs::write(path, output) } /// Rewrite a file in place with wrapped tables. @@ -37,6 +42,12 @@ pub fn rewrite_no_wrap(path: &Path) -> std::io::Result<()> { #[cfg(test)] mod tests { + use std::fs::Permissions; + #[cfg(unix)] + use std::os::unix::fs::PermissionsExt; + + #[cfg(unix)] + use libc; use tempfile::tempdir; use super::*; @@ -71,8 +82,61 @@ mod tests { #[test] fn rewrite_permission_denied() { - let file = Path::new("/proc/1/attr/current"); - let err = rewrite(file).expect_err("expected permission denied error"); - assert_eq!(err.kind(), std::io::ErrorKind::PermissionDenied); + let dir = tempdir().unwrap(); + let file = dir.path().join("deny.md"); + fs::write(&file, "data").unwrap(); + fs::set_permissions(&file, Permissions::from_mode(0o444)).unwrap(); + let result = rewrite(&file); + #[cfg(unix)] + if unsafe { libc::geteuid() } == 0 { + assert!(result.is_ok()); + } else { + let err = result.expect_err("expected permission denied error"); + assert_eq!(err.kind(), std::io::ErrorKind::PermissionDenied); + } + #[cfg(not(unix))] + { + let err = result.expect_err("expected permission denied error"); + assert_eq!(err.kind(), std::io::ErrorKind::PermissionDenied); + } + } + + #[test] + fn rewrite_no_wrap_missing_file() { + let dir = tempdir().unwrap(); + let file = dir.path().join("missing.md"); + let err = rewrite_no_wrap(&file).expect_err("expected error for missing file"); + assert_eq!(err.kind(), std::io::ErrorKind::NotFound); + } + + #[test] + fn rewrite_no_wrap_permission_denied() { + let dir = tempdir().unwrap(); + let file = dir.path().join("deny.md"); + fs::write(&file, "data").unwrap(); + fs::set_permissions(&file, Permissions::from_mode(0o444)).unwrap(); + let result = rewrite_no_wrap(&file); + #[cfg(unix)] + if unsafe { libc::geteuid() } == 0 { + assert!(result.is_ok()); + } else { + let err = result.expect_err("expected permission denied error"); + assert_eq!(err.kind(), std::io::ErrorKind::PermissionDenied); + } + #[cfg(not(unix))] + { + let err = result.expect_err("expected permission denied error"); + assert_eq!(err.kind(), std::io::ErrorKind::PermissionDenied); + } + } + + #[test] + fn rewrite_empty_file_no_extra_newline() { + let dir = tempdir().unwrap(); + let file = dir.path().join("empty.md"); + fs::write(&file, "").unwrap(); + rewrite(&file).unwrap(); + let contents = fs::read_to_string(&file).unwrap(); + assert!(contents.is_empty()); } } From 87905850d926bf3df9768f47dbe89af2ca14c75a Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 20 Jul 2025 14:08:04 +0100 Subject: [PATCH 3/3] Refactor tests for io error cases --- Cargo.toml | 2 +- src/io.rs | 53 ++++++++++++++++++++--------------------------------- 2 files changed, 21 insertions(+), 34 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ed31b4a6..e025ec42 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,7 +17,7 @@ unicode-width = ">=0.1, <0.2" rstest = "0.18" assert_cmd = "2" tempfile = "3" -libc = "0.2" +libc = ">=0.2, <0.3" [lints.clippy] pedantic = "warn" diff --git a/src/io.rs b/src/io.rs index 6f8a017b..cb30bea4 100644 --- a/src/io.rs +++ b/src/io.rs @@ -42,12 +42,13 @@ pub fn rewrite_no_wrap(path: &Path) -> std::io::Result<()> { #[cfg(test)] mod tests { - use std::fs::Permissions; #[cfg(unix)] use std::os::unix::fs::PermissionsExt; + use std::{fs::Permissions, path::Path}; #[cfg(unix)] use libc; + use rstest::rstest; use tempfile::tempdir; use super::*; @@ -72,23 +73,16 @@ mod tests { assert_eq!(out, "| A | B |\n| 1 | 2 |\n"); } - #[test] - fn rewrite_missing_file() { - let dir = tempdir().unwrap(); - let file = dir.path().join("missing.md"); - let err = rewrite(&file).expect_err("expected error for missing file"); - assert_eq!(err.kind(), std::io::ErrorKind::NotFound); + #[cfg(unix)] + fn can_write_as_root() -> bool { + // SAFETY: `geteuid()` has no side effects and is safe to call in tests. + let uid = unsafe { libc::geteuid() }; + uid == 0 } - #[test] - fn rewrite_permission_denied() { - let dir = tempdir().unwrap(); - let file = dir.path().join("deny.md"); - fs::write(&file, "data").unwrap(); - fs::set_permissions(&file, Permissions::from_mode(0o444)).unwrap(); - let result = rewrite(&file); + fn assert_permission_error_or_root_success(result: std::io::Result<()>) { #[cfg(unix)] - if unsafe { libc::geteuid() } == 0 { + if can_write_as_root() { assert!(result.is_ok()); } else { let err = result.expect_err("expected permission denied error"); @@ -101,33 +95,26 @@ mod tests { } } - #[test] - fn rewrite_no_wrap_missing_file() { + #[rstest] + #[case(rewrite)] + #[case(rewrite_no_wrap)] + fn missing_file_error(#[case] rewrite_fn: fn(&Path) -> std::io::Result<()>) { let dir = tempdir().unwrap(); let file = dir.path().join("missing.md"); - let err = rewrite_no_wrap(&file).expect_err("expected error for missing file"); + let err = rewrite_fn(&file).expect_err("expected error for missing file"); assert_eq!(err.kind(), std::io::ErrorKind::NotFound); } - #[test] - fn rewrite_no_wrap_permission_denied() { + #[rstest] + #[case(rewrite)] + #[case(rewrite_no_wrap)] + fn permission_denied_error(#[case] rewrite_fn: fn(&Path) -> std::io::Result<()>) { let dir = tempdir().unwrap(); let file = dir.path().join("deny.md"); fs::write(&file, "data").unwrap(); fs::set_permissions(&file, Permissions::from_mode(0o444)).unwrap(); - let result = rewrite_no_wrap(&file); - #[cfg(unix)] - if unsafe { libc::geteuid() } == 0 { - assert!(result.is_ok()); - } else { - let err = result.expect_err("expected permission denied error"); - assert_eq!(err.kind(), std::io::ErrorKind::PermissionDenied); - } - #[cfg(not(unix))] - { - let err = result.expect_err("expected permission denied error"); - assert_eq!(err.kind(), std::io::ErrorKind::PermissionDenied); - } + let result = rewrite_fn(&file); + assert_permission_error_or_root_success(result); } #[test]