From 92a8bea26f1459f3c9c97c900fac89e620d72803 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Wed, 8 Apr 2026 23:28:50 -0400 Subject: [PATCH 1/4] refactor(patch): extract tests into dedicated module --- src/patch/mod.rs | 2 + src/patch/parse.rs | 382 --------------------------------------------- src/patch/tests.rs | 379 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 381 insertions(+), 382 deletions(-) create mode 100644 src/patch/tests.rs diff --git a/src/patch/mod.rs b/src/patch/mod.rs index 468824a5..3a182506 100644 --- a/src/patch/mod.rs +++ b/src/patch/mod.rs @@ -3,6 +3,8 @@ mod format; mod parse; #[cfg(feature = "color")] mod style; +#[cfg(test)] +mod tests; pub use error::ParsePatchError; pub use format::PatchFormatter; diff --git a/src/patch/parse.rs b/src/patch/parse.rs index 996b36d5..711a411b 100644 --- a/src/patch/parse.rs +++ b/src/patch/parse.rs @@ -264,385 +264,3 @@ fn strip_newline(s: &T) -> Result<&T> { Err(ParsePatchErrorKind::MissingNewline.into()) } } - -#[cfg(test)] -mod tests { - use super::{parse, parse_bytes, ParsePatchErrorKind}; - - #[test] - fn test_escaped_filenames() { - // No escaped characters - let s = "\ ---- original -+++ modified -@@ -1,0 +1,1 @@ -+Oathbringer -"; - parse(s).unwrap(); - parse_bytes(s.as_ref()).unwrap(); - - // unescaped characters fail parsing - let s = "\ ---- ori\"ginal -+++ modified -@@ -1,0 +1,1 @@ -+Oathbringer -"; - assert_eq!( - parse(s).unwrap_err(), - ParsePatchErrorKind::InvalidCharInUnquotedFilename.into(), - ); - parse_bytes(s.as_ref()).unwrap_err(); - - // quoted with invalid escaped characters - let s = "\ ---- \"ori\\\"g\rinal\" -+++ modified -@@ -1,0 +1,1 @@ -+Oathbringer -"; - assert_eq!( - parse(s).unwrap_err(), - ParsePatchErrorKind::InvalidUnescapedChar.into(), - ); - parse_bytes(s.as_ref()).unwrap_err(); - - // quoted with escaped characters - let s = r#"\ ---- "ori\"g\tinal" -+++ "mo\000\t\r\n\\dified" -@@ -1,0 +1,1 @@ -+Oathbringer -"#; - let p = parse(s).unwrap(); - assert_eq!(p.original(), Some("ori\"g\tinal")); - assert_eq!(p.modified(), Some("mo\0\t\r\n\\dified")); - let b = parse_bytes(s.as_ref()).unwrap(); - assert_eq!(b.original(), Some(&b"ori\"g\tinal"[..])); - assert_eq!(b.modified(), Some(&b"mo\0\t\r\n\\dified"[..])); - } - - // Git uses named escapes \a (BEL), \b (BS), \f (FF), \v (VT) in - // quoted filenames. Both `git apply` and GNU patch decode them. - // - // Observed with git 2.53.0: - // $ printf 'x' > "$(printf 'f\x07')" && git add -A - // $ git diff --cached --name-only - // "f\a" - // - // Observed with GNU patch 2.7.1: - // $ patch -p0 < test.patch # with +++ "bel\a" - // patching file bel - // - #[test] - fn escaped_filename_named_escapes() { - let cases: &[(&str, u8)] = &[ - ("\\a", b'\x07'), - ("\\b", b'\x08'), - ("\\f", b'\x0c'), - ("\\v", b'\x0b'), - ]; - for (esc, expected_byte) in cases { - let s = format!( - "\ ---- \"orig{esc}\" -+++ \"mod{esc}\" -@@ -1,0 +1,1 @@ -+content -" - ); - let p = parse(&s).unwrap(); - let expected_orig = format!("orig{}", *expected_byte as char); - let expected_mod = format!("mod{}", *expected_byte as char); - assert_eq!(p.original(), Some(expected_orig.as_str())); - assert_eq!(p.modified(), Some(expected_mod.as_str())); - } - } - - // Git uses 3-digit octal escapes (\000–\377) for bytes without a - // named escape. Both `git apply` and GNU patch decode them. - // - // Observed with git 2.53.0: - // $ printf 'x' > "$(printf 'f\033')" && git add -A - // $ git diff --cached | grep '+++' - // +++ "b/f\033" - // - // Observed with GNU patch 2.7.1: - // $ patch -p1 < test.patch # with +++ "b/tl\033" - // patching file tl - // - // Found via llvm/llvm-project full-history replay - // (commits 17af06ba..229c95ab, 6c031780..0683a1e5). - #[test] - fn escaped_filename_octal() { - // \033 = ESC (0x1B) - let s = r#"\ ---- "orig\033" -+++ "mod\033" -@@ -1,0 +1,1 @@ -+content -"#; - let p = parse(s).unwrap(); - assert_eq!(p.original(), Some("orig\x1b")); - assert_eq!(p.modified(), Some("mod\x1b")); - - // \000 = NUL - let s = r#"\ ---- "orig\000" -+++ "mod\000" -@@ -1,0 +1,1 @@ -+content -"#; - let p = parse(s).unwrap(); - assert_eq!(p.original(), Some("orig\x00")); - assert_eq!(p.modified(), Some("mod\x00")); - - // \177 = DEL (0x7F) - let s = r#"\ ---- "orig\177" -+++ "mod\177" -@@ -1,0 +1,1 @@ -+content -"#; - let p = parse(s).unwrap(); - assert_eq!(p.original(), Some("orig\x7f")); - assert_eq!(p.modified(), Some("mod\x7f")); - - // \377 = 0xFF - let s = r#"\ ---- "orig\377" -+++ "mod\377" -@@ -1,0 +1,1 @@ -+content -"#; - let b = parse_bytes(s.as_ref()).unwrap(); - assert_eq!(b.original(), Some(&b"orig\xff"[..])); - assert_eq!(b.modified(), Some(&b"mod\xff"[..])); - - // Truncated octal (only 2 digits) → error - let s = r#"\ ---- "orig\03" -+++ "mod\03" -@@ -1,0 +1,1 @@ -+content -"#; - assert_eq!( - parse(s).unwrap_err(), - ParsePatchErrorKind::InvalidEscapedChar.into(), - ); - - // Non-octal digit in second position → error - let s = r#"\ ---- "orig\08x" -+++ "mod\08x" -@@ -1,0 +1,1 @@ -+content -"#; - assert_eq!( - parse(s).unwrap_err(), - ParsePatchErrorKind::InvalidEscapedChar.into(), - ); - - // First octal digit > 3 → error (would overflow a byte) - let s = r#"\ ---- "orig\477" -+++ "mod\477" -@@ -1,0 +1,1 @@ -+content -"#; - assert_eq!( - parse(s).unwrap_err(), - ParsePatchErrorKind::InvalidEscapedChar.into(), - ); - - // \101 = 'A' (0x41), first octal digit 1 - let s = r#"\ ---- "orig\101" -+++ "mod\101" -@@ -1,0 +1,1 @@ -+content -"#; - let p = parse(s).unwrap(); - assert_eq!(p.original(), Some("origA")); - assert_eq!(p.modified(), Some("modA")); - - // \277 = 0xBF, first octal digit 2 - let s = r#"\ ---- "orig\277" -+++ "mod\277" -@@ -1,0 +1,1 @@ -+content -"#; - let b = parse_bytes(s.as_ref()).unwrap(); - assert_eq!(b.original(), Some(&b"orig\xbf"[..])); - assert_eq!(b.modified(), Some(&b"mod\xbf"[..])); - } - - // Verify that formatting a parsed patch with escaped filenames - // produces output that re-parses to the same patch. This covers - // both the `Display` (str) and `to_bytes` ([u8]) paths. - #[test] - fn escaped_filename_roundtrip_named() { - // Named escapes: \a \b \t \n \v \f \r \\ \" - let s = r#"\ ---- "a\a\b\t\n\v\f\r\\\"" -+++ "b\a\b\t\n\v\f\r\\\"" -@@ -1,1 +1,1 @@ --old -+new -"#; - let p = parse(s).unwrap(); - - // str roundtrip via Display - let formatted = p.to_string(); - let p2 = parse(&formatted).unwrap(); - assert_eq!(p.original(), p2.original()); - assert_eq!(p.modified(), p2.modified()); - - // bytes roundtrip via to_bytes - let b = parse_bytes(s.as_ref()).unwrap(); - let bytes = b.to_bytes(); - let b2 = parse_bytes(&bytes).unwrap(); - assert_eq!(b.original(), b2.original()); - assert_eq!(b.modified(), b2.modified()); - } - - #[test] - fn escaped_filename_roundtrip_octal() { - // Octal escapes for control chars without named escapes - // and for high bytes (> 0x7f). - let s = r#"\ ---- "a\001\002\037\177" -+++ "b\001\002\037\177" -@@ -1,1 +1,1 @@ --old -+new -"#; - let p = parse(s).unwrap(); - let formatted = p.to_string(); - let p2 = parse(&formatted).unwrap(); - assert_eq!(p.original(), p2.original()); - assert_eq!(p.modified(), p2.modified()); - - // Bytes roundtrip with a high byte (\377 = 0xFF). - let s = r#"\ ---- "a\377" -+++ "b\377" -@@ -1,1 +1,1 @@ --old -+new -"#; - let b = parse_bytes(s.as_ref()).unwrap(); - let bytes = b.to_bytes(); - let b2 = parse_bytes(&bytes).unwrap(); - assert_eq!(b.original(), b2.original()); - assert_eq!(b.modified(), b2.modified()); - } - - // Filenames without special characters should not be quoted. - #[test] - fn plain_filename_roundtrip() { - let s = "\ ---- a/normal.txt -+++ b/normal.txt -@@ -1,1 +1,1 @@ --old -+new -"; - let p = parse(s).unwrap(); - let formatted = p.to_string(); - assert!(!formatted.contains('"')); - let p2 = parse(&formatted).unwrap(); - assert_eq!(p.original(), p2.original()); - assert_eq!(p.modified(), p2.modified()); - } - - #[test] - fn test_missing_filename_header() { - // Missing Both '---' and '+++' lines - let patch = r#" -@@ -1,11 +1,12 @@ - diesel::table! { - users1 (id) { -- id -> Nullable, -+ id -> Integer, - } - } - - diesel::table! { -- users2 (id) { -- id -> Nullable, -+ users2 (myid) { -+ #[sql_name = "id"] -+ myid -> Integer, - } - } -"#; - - parse(patch).unwrap(); - - // Missing '---' - let s = "\ -+++ modified -@@ -1,0 +1,1 @@ -+Oathbringer -"; - parse(s).unwrap(); - - // Missing '+++' - let s = "\ ---- original -@@ -1,0 +1,1 @@ -+Oathbringer -"; - parse(s).unwrap(); - - // Headers out of order - let s = "\ -+++ modified ---- original -@@ -1,0 +1,1 @@ -+Oathbringer -"; - parse(s).unwrap(); - - // multiple headers should fail to parse - let s = "\ ---- original ---- modified -@@ -1,0 +1,1 @@ -+Oathbringer -"; - assert_eq!( - parse(s).unwrap_err(), - ParsePatchErrorKind::MultipleOriginalHeaders.into(), - ); - } - - #[test] - fn adjacent_hunks_correctly_parse() { - let s = "\ ---- original -+++ modified -@@ -110,7 +110,7 @@ - -- - - I am afraid, however, that all I have known - that my story - will be forgotten. - I am afraid for the world that is to come. --Afraid that my plans will fail. Afraid of a doom worse than the Deepness. -+Afraid that Alendi will fail. Afraid of a doom brought by the Deepness. - - Alendi was never the Hero of Ages. -@@ -117,7 +117,7 @@ - At best, I have amplified his virtues, creating a Hero where there was none. - --At worst, I fear that all we believe may have been corrupted. -+At worst, I fear that I have corrupted all we believe. - - -- - Alendi must not reach the Well of Ascension. He must not take the power for himself. - -"; - parse(s).unwrap(); - } -} diff --git a/src/patch/tests.rs b/src/patch/tests.rs new file mode 100644 index 00000000..31b18a91 --- /dev/null +++ b/src/patch/tests.rs @@ -0,0 +1,379 @@ +use super::parse::{parse, parse_bytes}; +use super::error::ParsePatchErrorKind; + +#[test] +fn test_escaped_filenames() { + // No escaped characters + let s = "\ +--- original ++++ modified +@@ -1,0 +1,1 @@ ++Oathbringer +"; + parse(s).unwrap(); + parse_bytes(s.as_ref()).unwrap(); + + // unescaped characters fail parsing + let s = "\ +--- ori\"ginal ++++ modified +@@ -1,0 +1,1 @@ ++Oathbringer +"; + assert_eq!( + parse(s).unwrap_err(), + ParsePatchErrorKind::InvalidCharInUnquotedFilename.into(), + ); + parse_bytes(s.as_ref()).unwrap_err(); + + // quoted with invalid escaped characters + let s = "\ +--- \"ori\\\"g\rinal\" ++++ modified +@@ -1,0 +1,1 @@ ++Oathbringer +"; + assert_eq!( + parse(s).unwrap_err(), + ParsePatchErrorKind::InvalidUnescapedChar.into(), + ); + parse_bytes(s.as_ref()).unwrap_err(); + + // quoted with escaped characters + let s = r#"\ +--- "ori\"g\tinal" ++++ "mo\000\t\r\n\\dified" +@@ -1,0 +1,1 @@ ++Oathbringer +"#; + let p = parse(s).unwrap(); + assert_eq!(p.original(), Some("ori\"g\tinal")); + assert_eq!(p.modified(), Some("mo\0\t\r\n\\dified")); + let b = parse_bytes(s.as_ref()).unwrap(); + assert_eq!(b.original(), Some(&b"ori\"g\tinal"[..])); + assert_eq!(b.modified(), Some(&b"mo\0\t\r\n\\dified"[..])); +} + +// Git uses named escapes \a (BEL), \b (BS), \f (FF), \v (VT) in +// quoted filenames. Both `git apply` and GNU patch decode them. +// +// Observed with git 2.53.0: +// $ printf 'x' > "$(printf 'f\x07')" && git add -A +// $ git diff --cached --name-only +// "f\a" +// +// Observed with GNU patch 2.7.1: +// $ patch -p0 < test.patch # with +++ "bel\a" +// patching file bel +// +#[test] +fn escaped_filename_named_escapes() { + let cases: &[(&str, u8)] = &[ + ("\\a", b'\x07'), + ("\\b", b'\x08'), + ("\\f", b'\x0c'), + ("\\v", b'\x0b'), + ]; + for (esc, expected_byte) in cases { + let s = format!( + "\ +--- \"orig{esc}\" ++++ \"mod{esc}\" +@@ -1,0 +1,1 @@ ++content +" + ); + let p = parse(&s).unwrap(); + let expected_orig = format!("orig{}", *expected_byte as char); + let expected_mod = format!("mod{}", *expected_byte as char); + assert_eq!(p.original(), Some(expected_orig.as_str())); + assert_eq!(p.modified(), Some(expected_mod.as_str())); + } +} + +// Git uses 3-digit octal escapes (\000–\377) for bytes without a +// named escape. Both `git apply` and GNU patch decode them. +// +// Observed with git 2.53.0: +// $ printf 'x' > "$(printf 'f\033')" && git add -A +// $ git diff --cached | grep '+++' +// +++ "b/f\033" +// +// Observed with GNU patch 2.7.1: +// $ patch -p1 < test.patch # with +++ "b/tl\033" +// patching file tl +// +// Found via llvm/llvm-project full-history replay +// (commits 17af06ba..229c95ab, 6c031780..0683a1e5). +#[test] +fn escaped_filename_octal() { + // \033 = ESC (0x1B) + let s = r#"\ +--- "orig\033" ++++ "mod\033" +@@ -1,0 +1,1 @@ ++content +"#; + let p = parse(s).unwrap(); + assert_eq!(p.original(), Some("orig\x1b")); + assert_eq!(p.modified(), Some("mod\x1b")); + + // \000 = NUL + let s = r#"\ +--- "orig\000" ++++ "mod\000" +@@ -1,0 +1,1 @@ ++content +"#; + let p = parse(s).unwrap(); + assert_eq!(p.original(), Some("orig\x00")); + assert_eq!(p.modified(), Some("mod\x00")); + + // \177 = DEL (0x7F) + let s = r#"\ +--- "orig\177" ++++ "mod\177" +@@ -1,0 +1,1 @@ ++content +"#; + let p = parse(s).unwrap(); + assert_eq!(p.original(), Some("orig\x7f")); + assert_eq!(p.modified(), Some("mod\x7f")); + + // \377 = 0xFF + let s = r#"\ +--- "orig\377" ++++ "mod\377" +@@ -1,0 +1,1 @@ ++content +"#; + let b = parse_bytes(s.as_ref()).unwrap(); + assert_eq!(b.original(), Some(&b"orig\xff"[..])); + assert_eq!(b.modified(), Some(&b"mod\xff"[..])); + + // First octal digit > 3 → error (would overflow a byte) + let s = r#"\ +--- "orig\477" ++++ "mod\477" +@@ -1,0 +1,1 @@ ++content +"#; + assert_eq!( + parse(s).unwrap_err(), + ParsePatchErrorKind::InvalidEscapedChar.into(), + ); + + // \101 = 'A' (0x41), first octal digit 1 + let s = r#"\ +--- "orig\101" ++++ "mod\101" +@@ -1,0 +1,1 @@ ++content +"#; + let p = parse(s).unwrap(); + assert_eq!(p.original(), Some("origA")); + assert_eq!(p.modified(), Some("modA")); + + // \277 = 0xBF, first octal digit 2 + let s = r#"\ +--- "orig\277" ++++ "mod\277" +@@ -1,0 +1,1 @@ ++content +"#; + let b = parse_bytes(s.as_ref()).unwrap(); + assert_eq!(b.original(), Some(&b"orig\xbf"[..])); + assert_eq!(b.modified(), Some(&b"mod\xbf"[..])); + + // Truncated octal (only 2 digits) → error + let s = r#"\ +--- "orig\03" ++++ "mod\03" +@@ -1,0 +1,1 @@ ++content +"#; + assert_eq!( + parse(s).unwrap_err(), + ParsePatchErrorKind::InvalidEscapedChar.into(), + ); + + // Non-octal digit in second position → error + let s = r#"\ +--- "orig\08x" ++++ "mod\08x" +@@ -1,0 +1,1 @@ ++content +"#; + assert_eq!( + parse(s).unwrap_err(), + ParsePatchErrorKind::InvalidEscapedChar.into(), + ); +} + +#[test] +fn test_missing_filename_header() { + // Missing Both '---' and '+++' lines + let patch = r#" +@@ -1,11 +1,12 @@ + diesel::table! { + users1 (id) { +- id -> Nullable, ++ id -> Integer, + } + } + + diesel::table! { +- users2 (id) { +- id -> Nullable, ++ users2 (myid) { ++ #[sql_name = "id"] ++ myid -> Integer, + } + } +"#; + + parse(patch).unwrap(); + + // Missing '---' + let s = "\ ++++ modified +@@ -1,0 +1,1 @@ ++Oathbringer +"; + parse(s).unwrap(); + + // Missing '+++' + let s = "\ +--- original +@@ -1,0 +1,1 @@ ++Oathbringer +"; + parse(s).unwrap(); + + // Headers out of order + let s = "\ ++++ modified +--- original +@@ -1,0 +1,1 @@ ++Oathbringer +"; + parse(s).unwrap(); + + // multiple headers should fail to parse + let s = "\ +--- original +--- modified +@@ -1,0 +1,1 @@ ++Oathbringer +"; + assert_eq!( + parse(s).unwrap_err(), + ParsePatchErrorKind::MultipleOriginalHeaders.into(), + ); +} + +#[test] +fn adjacent_hunks_correctly_parse() { + let s = "\ +--- original ++++ modified +@@ -110,7 +110,7 @@ + -- + + I am afraid, however, that all I have known - that my story - will be forgotten. + I am afraid for the world that is to come. +-Afraid that my plans will fail. Afraid of a doom worse than the Deepness. ++Afraid that Alendi will fail. Afraid of a doom brought by the Deepness. + + Alendi was never the Hero of Ages. +@@ -117,7 +117,7 @@ + At best, I have amplified his virtues, creating a Hero where there was none. + +-At worst, I fear that all we believe may have been corrupted. ++At worst, I fear that I have corrupted all we believe. + + -- + Alendi must not reach the Well of Ascension. He must not take the power for himself. + +"; + parse(s).unwrap(); +} + +// Verify that formatting a parsed patch with escaped filenames +// produces output that re-parses to the same patch. This covers +// both the `Display` (str) and `to_bytes` ([u8]) paths. +#[test] +fn escaped_filename_roundtrip_named() { + // Named escapes: \a \b \t \n \v \f \r \\ \" + let s = r#"\ +--- "a\a\b\t\n\v\f\r\\\"" ++++ "b\a\b\t\n\v\f\r\\\"" +@@ -1,1 +1,1 @@ +-old ++new +"#; + let p = parse(s).unwrap(); + + // str roundtrip via Display + let formatted = p.to_string(); + let p2 = parse(&formatted).unwrap(); + assert_eq!(p.original(), p2.original()); + assert_eq!(p.modified(), p2.modified()); + + // bytes roundtrip via to_bytes + let b = parse_bytes(s.as_ref()).unwrap(); + let bytes = b.to_bytes(); + let b2 = parse_bytes(&bytes).unwrap(); + assert_eq!(b.original(), b2.original()); + assert_eq!(b.modified(), b2.modified()); +} + +#[test] +fn escaped_filename_roundtrip_octal() { + // Octal escapes for control chars without named escapes + // and for high bytes (> 0x7f). + let s = r#"\ +--- "a\001\002\037\177" ++++ "b\001\002\037\177" +@@ -1,1 +1,1 @@ +-old ++new +"#; + let p = parse(s).unwrap(); + let formatted = p.to_string(); + let p2 = parse(&formatted).unwrap(); + assert_eq!(p.original(), p2.original()); + assert_eq!(p.modified(), p2.modified()); + + // Bytes roundtrip with a high byte (\377 = 0xFF). + let s = r#"\ +--- "a\377" ++++ "b\377" +@@ -1,1 +1,1 @@ +-old ++new +"#; + let b = parse_bytes(s.as_ref()).unwrap(); + let bytes = b.to_bytes(); + let b2 = parse_bytes(&bytes).unwrap(); + assert_eq!(b.original(), b2.original()); + assert_eq!(b.modified(), b2.modified()); +} + +// Filenames without special characters should not be quoted. +#[test] +fn plain_filename_roundtrip() { + let s = "\ +--- a/normal.txt ++++ b/normal.txt +@@ -1,1 +1,1 @@ +-old ++new +"; + let p = parse(s).unwrap(); + let formatted = p.to_string(); + assert!(!formatted.contains('"')); + let p2 = parse(&formatted).unwrap(); + assert_eq!(p.original(), p2.original()); + assert_eq!(p.modified(), p2.modified()); +} From 90966819bb597aaef6f89fa43960528ef3bbea33 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Wed, 8 Apr 2026 23:32:14 -0400 Subject: [PATCH 2/4] test(error): error display assertions Use snapbox for snapshot testing for Display. This would be useful for span-aware errors. --- Cargo.toml | 3 +++ src/patch/tests.rs | 54 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index a7f9f879..cae78dcc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,6 +18,9 @@ color = ["dep:anstyle"] [dependencies] anstyle = { version = "1.0.13", optional = true } +[dev-dependencies] +snapbox = "0.6.24" + [[example]] name = "patch_formatter" required-features = ["color"] diff --git a/src/patch/tests.rs b/src/patch/tests.rs index 31b18a91..278d01a4 100644 --- a/src/patch/tests.rs +++ b/src/patch/tests.rs @@ -1,5 +1,5 @@ -use super::parse::{parse, parse_bytes}; use super::error::ParsePatchErrorKind; +use super::parse::{parse, parse_bytes}; #[test] fn test_escaped_filenames() { @@ -377,3 +377,55 @@ fn plain_filename_roundtrip() { assert_eq!(p.original(), p2.original()); assert_eq!(p.modified(), p2.modified()); } + +mod error_display { + use crate::patch::error::ParsePatchErrorKind; + use crate::Patch; + use snapbox::assert_data_eq; + use snapbox::str; + + #[test] + fn invalid_hunk_header() { + let content = "\ +--- a/file.rs ++++ b/file.rs +@@ invalid @@ +-old ++new +"; + let err = Patch::from_str(content).unwrap_err(); + assert_data_eq!( + err.to_string(), + str!["error parsing patch: unable to parse hunk header"] + ); + } + + #[test] + fn hunk_mismatch() { + let content = "\ +--- a/file.rs ++++ b/file.rs +@@ -1,2 +1,2 @@ +-only one line ++only one line +"; + let err = Patch::from_str(content).unwrap_err(); + assert_data_eq!( + err.to_string(), + str!["error parsing patch: hunk header does not match hunk"] + ); + } + + #[test] + fn kind_preserved() { + let content = "\ +--- a/file.rs ++++ b/file.rs +@@ invalid @@ +-old ++new +"; + let err = Patch::from_str(content).unwrap_err(); + assert_eq!(err.kind, ParsePatchErrorKind::InvalidHunkHeader); + } +} From be57c485630532e2c1782f35d35bac3a4dec6f54 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Wed, 8 Apr 2026 23:33:44 -0400 Subject: [PATCH 3/4] refactor(patch): track byte offset in Parser Add an `offset` field accumulating byte length of every line consumed. This lays groundwork for attaching byte-offset spans to parse errors. --- src/patch/parse.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/patch/parse.rs b/src/patch/parse.rs index 711a411b..4ae45b27 100644 --- a/src/patch/parse.rs +++ b/src/patch/parse.rs @@ -14,12 +14,14 @@ type Result = std::result::Result; struct Parser<'a, T: Text + ?Sized> { lines: std::iter::Peekable>, + offset: usize, } impl<'a, T: Text + ?Sized> Parser<'a, T> { fn new(input: &'a T) -> Self { Self { lines: LineIter::new(input).peekable(), + offset: 0, } } @@ -32,8 +34,10 @@ impl<'a, T: Text + ?Sized> Parser<'a, T> { .lines .next() .ok_or(ParsePatchErrorKind::UnexpectedEof)?; + self.offset += line.len(); Ok(line) } + } pub fn parse(input: &str) -> Result> { From a8c0e6d911974f023da8596b1964ea59996ff804 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Wed, 8 Apr 2026 23:36:28 -0400 Subject: [PATCH 4/4] feat(error): offset-based span tracking New methods `error()` and `error_at()` attach the current offset so callers get location info in error messages. --- src/patch/error.rs | 24 ++++++++++++++++++++++-- src/patch/parse.rs | 38 +++++++++++++++++++++++++++----------- src/patch/tests.rs | 24 ++++++++++++------------ 3 files changed, 61 insertions(+), 25 deletions(-) diff --git a/src/patch/error.rs b/src/patch/error.rs index 13db8f42..70828629 100644 --- a/src/patch/error.rs +++ b/src/patch/error.rs @@ -1,6 +1,7 @@ //! Error types for patch parsing. use std::fmt; +use std::ops::Range; /// An error returned when parsing a `Patch` using [`Patch::from_str`] fails. /// @@ -8,11 +9,30 @@ use std::fmt; #[derive(Debug, Clone, PartialEq, Eq)] pub struct ParsePatchError { pub(crate) kind: ParsePatchErrorKind, + span: Option>, +} + +impl ParsePatchError { + /// Creates a new error with the given kind and span. + pub(crate) fn new(kind: ParsePatchErrorKind, span: Range) -> Self { + Self { + kind, + span: Some(span), + } + } } impl fmt::Display for ParsePatchError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "error parsing patch: {}", self.kind) + if let Some(span) = &self.span { + write!( + f, + "error parsing patch at byte {}: {}", + span.start, self.kind + ) + } else { + write!(f, "error parsing patch: {}", self.kind) + } } } @@ -20,7 +40,7 @@ impl std::error::Error for ParsePatchError {} impl From for ParsePatchError { fn from(kind: ParsePatchErrorKind) -> Self { - Self { kind } + Self { kind, span: None } } } diff --git a/src/patch/parse.rs b/src/patch/parse.rs index 4ae45b27..1176222b 100644 --- a/src/patch/parse.rs +++ b/src/patch/parse.rs @@ -29,15 +29,28 @@ impl<'a, T: Text + ?Sized> Parser<'a, T> { self.lines.peek() } + fn offset(&self) -> usize { + self.offset + } + fn next(&mut self) -> Result<&'a T> { let line = self .lines .next() - .ok_or(ParsePatchErrorKind::UnexpectedEof)?; + .ok_or_else(|| self.error(ParsePatchErrorKind::UnexpectedEof))?; self.offset += line.len(); Ok(line) } + /// Creates an error with the current offset as span. + fn error(&self, kind: ParsePatchErrorKind) -> ParsePatchError { + ParsePatchError::new(kind, self.offset..self.offset) + } + + /// Creates an error with a specific offset as span. + fn error_at(&self, kind: ParsePatchErrorKind, offset: usize) -> ParsePatchError { + ParsePatchError::new(kind, offset..offset) + } } pub fn parse(input: &str) -> Result> { @@ -80,12 +93,12 @@ fn patch_header<'a, T: Text + ToOwned + ?Sized>( while let Some(line) = parser.peek() { if line.starts_with("--- ") { if filename1.is_some() { - return Err(ParsePatchErrorKind::MultipleOriginalHeaders.into()); + return Err(parser.error(ParsePatchErrorKind::MultipleOriginalHeaders)); } filename1 = Some(parse_filename("--- ", parser.next()?)?); } else if line.starts_with("+++ ") { if filename2.is_some() { - return Err(ParsePatchErrorKind::MultipleModifiedHeaders.into()); + return Err(parser.error(ParsePatchErrorKind::MultipleModifiedHeaders)); } filename2 = Some(parse_filename("+++ ", parser.next()?)?); } else { @@ -149,20 +162,23 @@ fn hunks<'a, T: Text + ?Sized>(parser: &mut Parser<'a, T>) -> Result(parser: &mut Parser<'a, T>) -> Result> { - let (range1, range2, function_context) = hunk_header(parser.next()?)?; + let hunk_start = parser.offset(); + let header_line = parser.next()?; + let (range1, range2, function_context) = + hunk_header(header_line).map_err(|e| parser.error_at(e.kind, hunk_start))?; let lines = hunk_lines(parser)?; // check counts of lines to see if they match the ranges in the hunk header let (len1, len2) = super::hunk_lines_count(&lines); if len1 != range1.len || len2 != range2.len { - return Err(ParsePatchErrorKind::HunkMismatch.into()); + return Err(parser.error_at(ParsePatchErrorKind::HunkMismatch, hunk_start)); } Ok(Hunk::new(range1, range2, function_context, lines)) @@ -217,25 +233,25 @@ fn hunk_lines<'a, T: Text + ?Sized>(parser: &mut Parser<'a, T>) -> Result { no_newline_context = true; @@ -251,7 +267,7 @@ fn hunk_lines<'a, T: Text + ?Sized>(parser: &mut Parser<'a, T>) -> Result