From f01862409303883564fad437e2d612edb4964fae Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 8 Nov 2024 15:21:33 -0600 Subject: [PATCH 01/10] test(toml): Import common assertion --- src/cargo/util/toml/embedded.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index ae072c10e96..4b490ba84b2 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -267,6 +267,7 @@ fn split_source(input: &str) -> CargoResult> { #[cfg(test)] mod test_expand { + use snapbox::assert_data_eq; use snapbox::str; use super::*; @@ -284,7 +285,7 @@ mod test_expand { #[test] fn test_default() { - snapbox::assert_data_eq!( + assert_data_eq!( si!(r#"fn main() {}"#), str![[r#" [[bin]] @@ -312,7 +313,7 @@ strip = true #[test] fn test_dependencies() { - snapbox::assert_data_eq!( + assert_data_eq!( si!(r#"---cargo [dependencies] time="0.1.25" @@ -348,7 +349,7 @@ strip = true #[test] fn test_no_infostring() { - snapbox::assert_data_eq!( + assert_data_eq!( si!(r#"--- [dependencies] time="0.1.25" From d769cb5f77c3e4d5c3c7598979c16ffad36b0cf7 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 8 Nov 2024 15:14:25 -0600 Subject: [PATCH 02/10] test(toml): Clean up manifest expanding helper --- src/cargo/util/toml/embedded.rs | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 4b490ba84b2..9e0be303387 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -272,21 +272,20 @@ mod test_expand { use super::*; - macro_rules! si { - ($i:expr) => {{ - let shell = crate::Shell::from_write(Box::new(Vec::new())); - let cwd = std::env::current_dir().unwrap(); - let home = home::cargo_home_with_cwd(&cwd).unwrap(); - let gctx = GlobalContext::new(shell, cwd, home); - expand_manifest($i, std::path::Path::new("/home/me/test.rs"), &gctx) - .unwrap_or_else(|err| panic!("{}", err)) - }}; + #[track_caller] + fn expand(source: &str) -> String { + let shell = crate::Shell::from_write(Box::new(Vec::new())); + let cwd = std::env::current_dir().unwrap(); + let home = home::cargo_home_with_cwd(&cwd).unwrap(); + let gctx = GlobalContext::new(shell, cwd, home); + expand_manifest(source, std::path::Path::new("/home/me/test.rs"), &gctx) + .unwrap_or_else(|err| panic!("{}", err)) } #[test] fn test_default() { assert_data_eq!( - si!(r#"fn main() {}"#), + expand(r#"fn main() {}"#), str![[r#" [[bin]] name = "test-" @@ -314,12 +313,14 @@ strip = true #[test] fn test_dependencies() { assert_data_eq!( - si!(r#"---cargo + expand( + r#"---cargo [dependencies] time="0.1.25" --- fn main() {} -"#), +"# + ), str![[r#" [[bin]] name = "test-" @@ -350,12 +351,14 @@ strip = true #[test] fn test_no_infostring() { assert_data_eq!( - si!(r#"--- + expand( + r#"--- [dependencies] time="0.1.25" --- fn main() {} -"#), +"# + ), str![[r#" [[bin]] name = "test-" From 2ea0d973de251b307f57edadcca3a120a28e18a2 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 8 Nov 2024 15:20:04 -0600 Subject: [PATCH 03/10] test(toml): Separate manifest expansion from code fence tests --- src/cargo/util/toml/embedded.rs | 87 ++++++++++++++++++--------------- 1 file changed, 47 insertions(+), 40 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 9e0be303387..f818a460000 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -268,10 +268,55 @@ fn split_source(input: &str) -> CargoResult> { #[cfg(test)] mod test_expand { use snapbox::assert_data_eq; + use snapbox::prelude::*; use snapbox::str; use super::*; + #[track_caller] + fn assert_source(source: &str, expected: impl IntoData) { + use std::fmt::Write as _; + + let actual = match split_source(source) { + Ok(actual) => actual, + Err(err) => panic!("unexpected err: {err}"), + }; + + let mut rendered = String::new(); + write_optional_field(&mut rendered, "shebang", actual.shebang); + write_optional_field(&mut rendered, "info", actual.info); + write_optional_field(&mut rendered, "frontmatter", actual.frontmatter); + writeln!(&mut rendered, "content: {:?}", actual.content).unwrap(); + assert_data_eq!(rendered, expected.raw()); + } + + fn write_optional_field(writer: &mut dyn std::fmt::Write, field: &str, value: Option<&str>) { + if let Some(value) = value { + writeln!(writer, "{field}: {value:?}").unwrap(); + } else { + writeln!(writer, "{field}: None").unwrap(); + } + } + + #[test] + fn split_dependencies() { + assert_source( + r#"--- +[dependencies] +time="0.1.25" +--- +fn main() {} +"#, + str![[r#" +shebang: None +info: None +frontmatter: "[dependencies]\ntime=\"0.1.25\"\n" +content: "fn main() {}\n" + +"#]], + ); + } + #[track_caller] fn expand(source: &str) -> String { let shell = crate::Shell::from_write(Box::new(Vec::new())); @@ -283,7 +328,7 @@ mod test_expand { } #[test] - fn test_default() { + fn expand_default() { assert_data_eq!( expand(r#"fn main() {}"#), str![[r#" @@ -311,7 +356,7 @@ strip = true } #[test] - fn test_dependencies() { + fn expand_dependencies() { assert_data_eq!( expand( r#"---cargo @@ -344,44 +389,6 @@ strip = true [workspace] -"#]] - ); - } - - #[test] - fn test_no_infostring() { - assert_data_eq!( - expand( - r#"--- -[dependencies] -time="0.1.25" ---- -fn main() {} -"# - ), - str![[r#" -[[bin]] -name = "test-" -path = [..] - -[dependencies] -time = "0.1.25" - -[package] -autobenches = false -autobins = false -autoexamples = false -autolib = false -autotests = false -build = false -edition = "2021" -name = "test-" - -[profile.release] -strip = true - -[workspace] - "#]] ); } From c88c7891e0eaedf4855f315893a11984d5931695 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 6 Nov 2024 21:11:19 -0600 Subject: [PATCH 04/10] test(toml): Expand code fence tests --- src/cargo/util/toml/embedded.rs | 211 ++++++++++++++++++++++++++++++++ 1 file changed, 211 insertions(+) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index f818a460000..6bbd9083953 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -188,6 +188,7 @@ fn sanitize_name(name: &str) -> String { name } +#[derive(Debug)] struct Source<'s> { shebang: Option<&'s str>, info: Option<&'s str>, @@ -298,6 +299,32 @@ mod test_expand { } } + #[track_caller] + fn assert_err( + result: Result, + err: impl IntoData, + ) { + match result { + Ok(d) => panic!("unexpected Ok({d:#?})"), + Err(actual) => snapbox::assert_data_eq!(actual.to_string(), err.raw()), + } + } + + #[test] + fn split_default() { + assert_source( + r#"fn main() {} +"#, + str![[r#" +shebang: None +info: None +frontmatter: None +content: "fn main() {}\n" + +"#]], + ); + } + #[test] fn split_dependencies() { assert_source( @@ -317,6 +344,190 @@ content: "fn main() {}\n" ); } + #[test] + fn split_infostring() { + assert_source( + r#"---cargo +[dependencies] +time="0.1.25" +--- +fn main() {} +"#, + str![[r#" +shebang: None +info: "cargo" +frontmatter: "[dependencies]\ntime=\"0.1.25\"\n" +content: "fn main() {}\n" + +"#]], + ); + } + + #[test] + fn split_infostring_whitespace() { + assert_source( + r#"--- cargo +[dependencies] +time="0.1.25" +--- +fn main() {} +"#, + str![[r#" +shebang: None +info: " cargo" +frontmatter: "[dependencies]\ntime=\"0.1.25\"\n" +content: "fn main() {}\n" + +"#]], + ); + } + + #[test] + fn split_shebang() { + assert_source( + r#"#!/usr/bin/env cargo +--- +[dependencies] +time="0.1.25" +--- +fn main() {} +"#, + str![[r##" +shebang: "#!/usr/bin/env cargo" +info: None +frontmatter: "[dependencies]\ntime=\"0.1.25\"\n" +content: "fn main() {}\n" + +"##]], + ); + } + + #[test] + fn split_crlf() { + assert_source( + "#!/usr/bin/env cargo\r\n---\r\n[dependencies]\r\ntime=\"0.1.25\"\r\n---\r\nfn main() {}", + str![[r##" +shebang: "#!/usr/bin/env cargo\r" +info: "" +frontmatter: "[dependencies]\r\ntime=\"0.1.25\"\r\n" +content: "fn main() {}" + +"##]] + ); + } + + #[test] + fn split_leading_newlines() { + assert_source( + r#"#!/usr/bin/env cargo + + + +--- +[dependencies] +time="0.1.25" +--- + + +fn main() {} +"#, + str![[r##" +shebang: "#!/usr/bin/env cargo" +info: None +frontmatter: None +content: " \n\n\n---\n[dependencies]\ntime=\"0.1.25\"\n---\n\n\nfn main() {}\n" + +"##]], + ); + } + + #[test] + fn split_attribute() { + assert_source( + r#"#[allow(dead_code)] +--- +[dependencies] +time="0.1.25" +--- +fn main() {} +"#, + str![[r##" +shebang: None +info: None +frontmatter: None +content: "#[allow(dead_code)]\n---\n[dependencies]\ntime=\"0.1.25\"\n---\nfn main() {}\n" + +"##]], + ); + } + + #[test] + fn split_extra_dash() { + assert_source( + r#"#!/usr/bin/env cargo +---------- +[dependencies] +time="0.1.25" +---------- + +fn main() {}"#, + str![[r##" +shebang: "#!/usr/bin/env cargo" +info: None +frontmatter: "[dependencies]\ntime=\"0.1.25\"\n" +content: "\nfn main() {}" + +"##]], + ); + } + + #[test] + fn split_too_few_dashes() { + assert_err( + split_source( + r#"#!/usr/bin/env cargo +-- +[dependencies] +time="0.1.25" +-- +fn main() {} +"#, + ), + str!["found 2 `-` in rust frontmatter, expected at least 3"], + ); + } + + #[test] + fn split_mismatched_dashes() { + assert_err( + split_source( + r#"#!/usr/bin/env cargo +--- +[dependencies] +time="0.1.25" +---- +fn main() {} +"#, + ), + str!["unexpected trailing content on closing fence: `-`"], + ); + } + + #[test] + fn split_missing_close() { + assert_err( + split_source( + r#"#!/usr/bin/env cargo +--- +[dependencies] +time="0.1.25" +fn main() {} +"#, + ), + str!["no closing `---` found for frontmatter"], + ); + } + #[track_caller] fn expand(source: &str) -> String { let shell = crate::Shell::from_write(Box::new(Vec::new())); From 8c88f236fa8d5c80308b45455267738790242407 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 7 Nov 2024 09:01:43 -0600 Subject: [PATCH 05/10] refactor(toml): Remove 'tick' language We've switched to dashes --- src/cargo/util/toml/embedded.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 6bbd9083953..10d7e6521ac 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -225,21 +225,23 @@ fn split_source(input: &str) -> CargoResult> { } // Experiment: let us try which char works better - let tick_char = '-'; + let fence_char = '-'; - let tick_end = source + let fence_end = source .content .char_indices() - .find_map(|(i, c)| (c != tick_char).then_some(i)) + .find_map(|(i, c)| (c != fence_char).then_some(i)) .unwrap_or(source.content.len()); - let (fence_pattern, rest) = match tick_end { + let (fence_pattern, rest) = match fence_end { 0 => { return Ok(source); } 1 | 2 => { - anyhow::bail!("found {tick_end} `{tick_char}` in rust frontmatter, expected at least 3") + anyhow::bail!( + "found {fence_end} `{fence_char}` in rust frontmatter, expected at least 3" + ) } - _ => source.content.split_at(tick_end), + _ => source.content.split_at(fence_end), }; let (info, content) = rest.split_once("\n").unwrap_or((rest, "")); if !info.is_empty() { From 45c2ff80fc5869a7cda0cf4e3a6ea35693eca0e3 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 7 Nov 2024 09:02:50 -0600 Subject: [PATCH 06/10] refactor(toml): Remove references to multiple fence chars --- src/cargo/util/toml/embedded.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 10d7e6521ac..9620d35052f 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -224,13 +224,12 @@ fn split_source(input: &str) -> CargoResult> { source.content = content; } - // Experiment: let us try which char works better - let fence_char = '-'; + const FENCE_CHAR: char = '-'; let fence_end = source .content .char_indices() - .find_map(|(i, c)| (c != fence_char).then_some(i)) + .find_map(|(i, c)| (c != FENCE_CHAR).then_some(i)) .unwrap_or(source.content.len()); let (fence_pattern, rest) = match fence_end { 0 => { @@ -238,7 +237,7 @@ fn split_source(input: &str) -> CargoResult> { } 1 | 2 => { anyhow::bail!( - "found {fence_end} `{fence_char}` in rust frontmatter, expected at least 3" + "found {fence_end} `{FENCE_CHAR}` in rust frontmatter, expected at least 3" ) } _ => source.content.split_at(fence_end), From 51db4414d865a7a82a30d0a3764fb9a8134e7b72 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 7 Nov 2024 09:40:40 -0600 Subject: [PATCH 07/10] refactor(toml): Preserve the full newline for shebang The shebang is thrown away so this has no end-user impact --- src/cargo/util/toml/embedded.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 9620d35052f..e9cccd0e59c 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -216,10 +216,12 @@ fn split_source(input: &str) -> CargoResult> { } // No other choice than to consider this a shebang. - let (shebang, content) = source + let newline_end = source .content - .split_once('\n') - .unwrap_or((source.content, "")); + .find('\n') + .map(|pos| pos + 1) + .unwrap_or(source.content.len()); + let (shebang, content) = source.content.split_at(newline_end); source.shebang = Some(shebang); source.content = content; } @@ -394,7 +396,7 @@ time="0.1.25" fn main() {} "#, str![[r##" -shebang: "#!/usr/bin/env cargo" +shebang: "#!/usr/bin/env cargo\n" info: None frontmatter: "[dependencies]\ntime=\"0.1.25\"\n" content: "fn main() {}\n" @@ -408,7 +410,7 @@ content: "fn main() {}\n" assert_source( "#!/usr/bin/env cargo\r\n---\r\n[dependencies]\r\ntime=\"0.1.25\"\r\n---\r\nfn main() {}", str![[r##" -shebang: "#!/usr/bin/env cargo\r" +shebang: "#!/usr/bin/env cargo\r\n" info: "" frontmatter: "[dependencies]\r\ntime=\"0.1.25\"\r\n" content: "fn main() {}" @@ -433,7 +435,7 @@ time="0.1.25" fn main() {} "#, str![[r##" -shebang: "#!/usr/bin/env cargo" +shebang: "#!/usr/bin/env cargo\n" info: None frontmatter: None content: " \n\n\n---\n[dependencies]\ntime=\"0.1.25\"\n---\n\n\nfn main() {}\n" @@ -473,7 +475,7 @@ time="0.1.25" fn main() {}"#, str![[r##" -shebang: "#!/usr/bin/env cargo" +shebang: "#!/usr/bin/env cargo\n" info: None frontmatter: "[dependencies]\ntime=\"0.1.25\"\n" content: "\nfn main() {}" From 89186345c395c9a36354e7d467ee0a36c7eff2e8 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 7 Nov 2024 14:05:41 -0600 Subject: [PATCH 08/10] fix(toml): Don't error on infostrings with trailing whitespace --- src/cargo/util/toml/embedded.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index e9cccd0e59c..3ce56b7a92b 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -245,8 +245,9 @@ fn split_source(input: &str) -> CargoResult> { _ => source.content.split_at(fence_end), }; let (info, content) = rest.split_once("\n").unwrap_or((rest, "")); + let info = info.trim_end(); if !info.is_empty() { - source.info = Some(info.trim_end()); + source.info = Some(info); } source.content = content; @@ -411,7 +412,7 @@ content: "fn main() {}\n" "#!/usr/bin/env cargo\r\n---\r\n[dependencies]\r\ntime=\"0.1.25\"\r\n---\r\nfn main() {}", str![[r##" shebang: "#!/usr/bin/env cargo\r\n" -info: "" +info: None frontmatter: "[dependencies]\r\ntime=\"0.1.25\"\r\n" content: "fn main() {}" From c1248adbd4aa08e9fa6bc95f02659e884fa1a28a Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 7 Nov 2024 14:09:22 -0600 Subject: [PATCH 09/10] fix(toml): Don't error on leading whitespace in infostring According to the [RFC](https://rust-lang.github.io/rfcs/3503-frontmatter.html): > Opens with 3+ dashes followed by 0+ whitespace, > an optional term (one or more characters excluding whitespace and commas), > 0+ whitespace, and a newline --- src/cargo/util/toml/embedded.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 3ce56b7a92b..d25dcbfe52a 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -245,7 +245,7 @@ fn split_source(input: &str) -> CargoResult> { _ => source.content.split_at(fence_end), }; let (info, content) = rest.split_once("\n").unwrap_or((rest, "")); - let info = info.trim_end(); + let info = info.trim(); if !info.is_empty() { source.info = Some(info); } @@ -378,7 +378,7 @@ fn main() {} "#, str![[r#" shebang: None -info: " cargo" +info: "cargo" frontmatter: "[dependencies]\ntime=\"0.1.25\"\n" content: "fn main() {}\n" From 82836b0da5e00aa69e11a1ebf64d7224b0bcd225 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 7 Nov 2024 14:34:26 -0600 Subject: [PATCH 10/10] fix(toml): Strip blank linkes before frontmatter From the [RFC](https://rust-lang.github.io/rfcs/3503-frontmatter.html) > When parsing Rust source, after stripping the shebang (#!), rustc will strip the frontmatter: > > - May include 0+ blank lines (whitespace + newline) The question is what is a non-newline whitespace and what is a newline. Looking at the [Reference](https://doc.rust-lang.org/reference/whitespace.html), the answer is "unsure", particularly because the Rust language doesn't generally try to distinguish them. I kept things basic for now and we can revisit later. --- src/cargo/util/toml/embedded.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index d25dcbfe52a..6f569b94cf0 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -228,8 +228,17 @@ fn split_source(input: &str) -> CargoResult> { const FENCE_CHAR: char = '-'; - let fence_end = source - .content + let mut trimmed_content = source.content; + while !trimmed_content.is_empty() { + let c = trimmed_content; + let c = c.trim_start_matches([' ', '\t']); + let c = c.trim_start_matches(['\r', '\n']); + if c == trimmed_content { + break; + } + trimmed_content = c; + } + let fence_end = trimmed_content .char_indices() .find_map(|(i, c)| (c != FENCE_CHAR).then_some(i)) .unwrap_or(source.content.len()); @@ -242,7 +251,7 @@ fn split_source(input: &str) -> CargoResult> { "found {fence_end} `{FENCE_CHAR}` in rust frontmatter, expected at least 3" ) } - _ => source.content.split_at(fence_end), + _ => trimmed_content.split_at(fence_end), }; let (info, content) = rest.split_once("\n").unwrap_or((rest, "")); let info = info.trim(); @@ -438,8 +447,8 @@ fn main() {} str![[r##" shebang: "#!/usr/bin/env cargo\n" info: None -frontmatter: None -content: " \n\n\n---\n[dependencies]\ntime=\"0.1.25\"\n---\n\n\nfn main() {}\n" +frontmatter: "[dependencies]\ntime=\"0.1.25\"\n" +content: "\n\nfn main() {}\n" "##]], );