From 0f964d3bdfc0ec19f2d00a2b8f1b669c5a327c92 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Sat, 16 Sep 2023 13:23:00 -0600 Subject: [PATCH 1/6] refactor(embedded): Allow detecting missing doc comment --- src/cargo/util/toml/embedded.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 395430c1b58..31277ed4f59 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -16,7 +16,7 @@ pub fn expand_manifest( config: &Config, ) -> CargoResult { let comment = match extract_comment(content) { - Ok(comment) => Some(comment), + Ok(comment) => comment, Err(err) => { tracing::trace!("failed to extract doc comment: {err}"); None @@ -160,7 +160,7 @@ fn sanitize_name(name: &str) -> String { } /// Locates a "code block manifest" in Rust source. -fn extract_comment(input: &str) -> CargoResult { +fn extract_comment(input: &str) -> CargoResult> { let mut doc_fragments = Vec::new(); let file = syn::parse_file(input)?; // HACK: `syn` doesn't tell us what kind of comment was used, so infer it from how many @@ -181,7 +181,7 @@ fn extract_comment(input: &str) -> CargoResult { } } if doc_fragments.is_empty() { - anyhow::bail!("no doc-comment found"); + return Ok(None); } unindent_doc_fragments(&mut doc_fragments); @@ -190,7 +190,7 @@ fn extract_comment(input: &str) -> CargoResult { add_doc_fragment(&mut doc_comment, frag); } - Ok(doc_comment) + Ok(Some(doc_comment)) } /// A `#[doc]` @@ -561,29 +561,30 @@ mod test_comment { macro_rules! ec { ($s:expr) => { - extract_comment($s).unwrap_or_else(|err| panic!("{}", err)) + extract_comment($s) + .unwrap_or_else(|err| panic!("{}", err)) + .unwrap() }; } #[test] fn test_no_comment() { - snapbox::assert_eq( - "no doc-comment found", + assert_eq!( + None, extract_comment( r#" fn main () { } "#, ) - .unwrap_err() - .to_string(), + .unwrap() ); } #[test] fn test_no_comment_she_bang() { - snapbox::assert_eq( - "no doc-comment found", + assert_eq!( + None, extract_comment( r#"#!/usr/bin/env cargo-eval @@ -591,8 +592,7 @@ fn main () { } "#, ) - .unwrap_err() - .to_string(), + .unwrap() ); } From 6e90f0ef6eb0d981f7a07a668705cc4515d5d01e Mon Sep 17 00:00:00 2001 From: Ed Page Date: Sat, 16 Sep 2023 17:10:00 -0500 Subject: [PATCH 2/6] fix(embedded): report parse errors Before we were delegating to rustc for errors but that doesn't help with commands like `cargo metadata`. --- src/cargo/util/toml/embedded.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 31277ed4f59..78a695da27a 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -15,14 +15,7 @@ pub fn expand_manifest( path: &std::path::Path, config: &Config, ) -> CargoResult { - let comment = match extract_comment(content) { - Ok(comment) => comment, - Err(err) => { - tracing::trace!("failed to extract doc comment: {err}"); - None - } - } - .unwrap_or_default(); + let comment = extract_comment(content)?.unwrap_or_default(); let manifest = match extract_manifest(&comment)? { Some(manifest) => Some(manifest), None => { From 4638ef9d2bec7ba13f8fd08abf65f4b534bf63e6 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 18 Sep 2023 14:47:49 -0500 Subject: [PATCH 3/6] refactor(embedded): Clarify a variable name --- src/cargo/util/toml/embedded.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 78a695da27a..65b7059ab76 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -59,10 +59,11 @@ fn expand_manifest_( anyhow::bail!("`package.{key}` is not allowed in embedded manifests") } } - let file_name = path + let bin_path = path .file_name() .ok_or_else(|| anyhow::format_err!("no file name"))? - .to_string_lossy(); + .to_string_lossy() + .into_owned(); let file_stem = path .file_stem() .ok_or_else(|| anyhow::format_err!("no file name"))? @@ -96,10 +97,7 @@ fn expand_manifest_( let mut bin = toml::Table::new(); bin.insert("name".to_owned(), toml::Value::String(bin_name)); - bin.insert( - "path".to_owned(), - toml::Value::String(file_name.into_owned()), - ); + bin.insert("path".to_owned(), toml::Value::String(bin_path)); manifest.insert( "bin".to_owned(), toml::Value::Array(vec![toml::Value::Table(bin)]), From ba869d36edf03c84dea98bc3f9bc7734dd84f86e Mon Sep 17 00:00:00 2001 From: Ed Page Date: Sat, 16 Sep 2023 17:35:46 -0600 Subject: [PATCH 4/6] feat(embedded): Hack in code fence support This is to allow us to get feedback on the design proposed [on zulip](https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Embedding.20cargo.20manifests.20in.20rust.20source/near/391427092) to verify we want to make an RFC for this syntax. --- src/cargo/util/toml/embedded.rs | 160 ++++++++++++++++++++++++++++---- 1 file changed, 142 insertions(+), 18 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 65b7059ab76..482268923fc 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -15,19 +15,72 @@ pub fn expand_manifest( path: &std::path::Path, config: &Config, ) -> CargoResult { - let comment = extract_comment(content)?.unwrap_or_default(); - let manifest = match extract_manifest(&comment)? { - Some(manifest) => Some(manifest), - None => { - tracing::trace!("failed to extract manifest"); - None + let source = split_source(content)?; + if let Some(frontmatter) = source.frontmatter { + match source.info { + Some("cargo") => {} + None => { + anyhow::bail!("frontmatter is missing an infostring; specify `cargo` for embedding a manifest"); + } + Some(other) => { + if let Some(remainder) = other.strip_prefix("cargo,") { + anyhow::bail!("cargo does not support frontmatter infostring attributes like `{remainder}` at this time") + } else { + anyhow::bail!("frontmatter infostring `{other}` is unsupported by cargo; specify `cargo` for embedding a manifest") + } + } + } + + // HACK: until rustc has native support for this syntax, we have to remove it from the + // source file + use std::fmt::Write as _; + let hash = crate::util::hex::short_hash(&path.to_string_lossy()); + let mut rel_path = std::path::PathBuf::new(); + rel_path.push("target"); + rel_path.push(&hash[0..2]); + rel_path.push(&hash[2..]); + let target_dir = config.home().join(rel_path); + let hacked_path = target_dir + .join( + path.file_name() + .expect("always a name for embedded manifests"), + ) + .into_path_unlocked(); + let mut hacked_source = String::new(); + if let Some(shebang) = source.shebang { + writeln!(hacked_source, "{shebang}")?; + } + writeln!(hacked_source)?; // open + for _ in 0..frontmatter.lines().count() { + writeln!(hacked_source)?; } + writeln!(hacked_source)?; // close + writeln!(hacked_source, "{}", source.content)?; + if let Some(parent) = hacked_path.parent() { + cargo_util::paths::create_dir_all(parent)?; + } + cargo_util::paths::write_if_changed(&hacked_path, hacked_source)?; + + let manifest = expand_manifest_(&frontmatter, &hacked_path, config) + .with_context(|| format!("failed to parse manifest at {}", path.display()))?; + let manifest = toml::to_string_pretty(&manifest)?; + Ok(manifest) + } else { + // Legacy doc-comment support; here only for transitional purposes + let comment = extract_comment(content)?.unwrap_or_default(); + let manifest = match extract_manifest(&comment)? { + Some(manifest) => Some(manifest), + None => { + tracing::trace!("failed to extract manifest"); + None + } + } + .unwrap_or_default(); + let manifest = expand_manifest_(&manifest, path, config) + .with_context(|| format!("failed to parse manifest at {}", path.display()))?; + let manifest = toml::to_string_pretty(&manifest)?; + Ok(manifest) } - .unwrap_or_default(); - let manifest = expand_manifest_(&manifest, path, config) - .with_context(|| format!("failed to parse manifest at {}", path.display()))?; - let manifest = toml::to_string_pretty(&manifest)?; - Ok(manifest) } fn expand_manifest_( @@ -59,11 +112,8 @@ fn expand_manifest_( anyhow::bail!("`package.{key}` is not allowed in embedded manifests") } } - let bin_path = path - .file_name() - .ok_or_else(|| anyhow::format_err!("no file name"))? - .to_string_lossy() - .into_owned(); + // HACK: Using an absolute path while `hacked_path` is in use + let bin_path = path.to_string_lossy().into_owned(); let file_stem = path .file_stem() .ok_or_else(|| anyhow::format_err!("no file name"))? @@ -150,6 +200,80 @@ fn sanitize_name(name: &str) -> String { name } +struct Source<'s> { + shebang: Option<&'s str>, + info: Option<&'s str>, + frontmatter: Option<&'s str>, + content: &'s str, +} + +fn split_source(input: &str) -> CargoResult> { + let mut source = Source { + shebang: None, + info: None, + frontmatter: None, + content: input, + }; + + // See rust-lang/rust's compiler/rustc_lexer/src/lib.rs's `strip_shebang` + // Shebang must start with `#!` literally, without any preceding whitespace. + // For simplicity we consider any line starting with `#!` a shebang, + // regardless of restrictions put on shebangs by specific platforms. + if let Some(rest) = source.content.strip_prefix("#!") { + // Ok, this is a shebang but if the next non-whitespace token is `[`, + // then it may be valid Rust code, so consider it Rust code. + if rest.trim_start().starts_with('[') { + return Ok(source); + } + + // No other choice than to consider this a shebang. + let (shebang, content) = source + .content + .split_once('\n') + .unwrap_or((source.content, "")); + source.shebang = Some(shebang); + source.content = content; + } + + let tick_end = source + .content + .char_indices() + .find_map(|(i, c)| (c != '`').then_some(i)) + .unwrap_or(source.content.len()); + let (fence_pattern, rest) = match tick_end { + 0 => { + return Ok(source); + } + 1 | 2 => { + anyhow::bail!("found {tick_end} backticks in rust frontmatter, expected at least 3") + } + _ => source.content.split_at(tick_end), + }; + let (info, content) = rest.split_once("\n").unwrap_or((rest, "")); + if !info.is_empty() { + source.info = Some(info.trim_end()); + } + source.content = content; + + let Some((frontmatter, content)) = source.content.split_once(fence_pattern) else { + anyhow::bail!("no closing `{fence_pattern}` found for frontmatter"); + }; + source.frontmatter = Some(frontmatter); + source.content = content; + + let (line, content) = source + .content + .split_once("\n") + .unwrap_or((source.content, "")); + let line = line.trim(); + if !line.is_empty() { + anyhow::bail!("unexpected trailing content on closing fence: `{line}`"); + } + source.content = content; + + Ok(source) +} + /// Locates a "code block manifest" in Rust source. fn extract_comment(input: &str) -> CargoResult> { let mut doc_fragments = Vec::new(); @@ -487,7 +611,7 @@ mod test_expand { snapbox::assert_eq( r#"[[bin]] name = "test-" -path = "test.rs" +path = "/home/me/test.rs" [package] autobenches = false @@ -514,7 +638,7 @@ strip = true snapbox::assert_eq( r#"[[bin]] name = "test-" -path = "test.rs" +path = "/home/me/test.rs" [dependencies] time = "0.1.25" From a365a69130ba6bc68eb304744a66cddaa0a4052c Mon Sep 17 00:00:00 2001 From: Ed Page Date: Sat, 16 Sep 2023 17:38:48 -0600 Subject: [PATCH 5/6] test(embedded): Migrate to new syntax --- tests/testsuite/script.rs | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/tests/testsuite/script.rs b/tests/testsuite/script.rs index 75cafc2b2f6..dbfa4f2d0ff 100644 --- a/tests/testsuite/script.rs +++ b/tests/testsuite/script.rs @@ -208,11 +208,10 @@ error: running `echo.rs` requires `-Zscript` #[cargo_test] fn clean_output_with_edition() { let script = r#"#!/usr/bin/env cargo - -//! ```cargo -//! [package] -//! edition = "2018" -//! ``` +```cargo +[package] +edition = "2018" +``` fn main() { println!("Hello world!"); @@ -240,10 +239,9 @@ fn main() { #[cargo_test] fn warning_without_edition() { let script = r#"#!/usr/bin/env cargo - -//! ```cargo -//! [package] -//! ``` +```cargo +[package] +``` fn main() { println!("Hello world!"); @@ -625,11 +623,10 @@ fn missing_script_rs() { fn test_name_same_as_dependency() { Package::new("script", "1.0.0").publish(); let script = r#"#!/usr/bin/env cargo - -//! ```cargo -//! [dependencies] -//! script = "1.0.0" -//! ``` +```cargo +[dependencies] +script = "1.0.0" +``` fn main() { println!("Hello world!"); @@ -662,11 +659,10 @@ fn main() { #[cargo_test] fn test_path_dep() { let script = r#"#!/usr/bin/env cargo - -//! ```cargo -//! [dependencies] -//! bar.path = "./bar" -//! ``` +```cargo +[dependencies] +bar.path = "./bar" +``` fn main() { println!("Hello world!"); From b3bc67679e6942908fffb5654ee475f5db0167ba Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 18 Sep 2023 15:10:07 -0500 Subject: [PATCH 6/6] docs(unstable): Update script documentation --- src/doc/src/reference/unstable.md | 31 ++++++++----------------------- 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index 456cb22f4cf..4661ec0a91e 100644 --- a/src/doc/src/reference/unstable.md +++ b/src/doc/src/reference/unstable.md @@ -1191,13 +1191,12 @@ fn main() {} ``` A user may optionally specify a manifest in a `cargo` code fence in a module-level comment, like: -```rust +````rust #!/usr/bin/env -S cargo +nightly -Zscript - -//! ```cargo -//! [dependencies] -//! clap = { version = "4.2", features = ["derive"] } -//! ``` +```cargo +[dependencies] +clap = { version = "4.2", features = ["derive"] } +``` use clap::Parser; @@ -1212,7 +1211,7 @@ fn main() { let args = Args::parse(); println!("{:?}", args); } -``` +```` ### Single-file packages @@ -1225,22 +1224,8 @@ Single-file packages may be selected via `--manifest-path`, like `cargo test --manifest-path foo.rs`. Unlike `Cargo.toml`, these files cannot be auto-discovered. A single-file package may contain an embedded manifest. An embedded manifest -is stored using `TOML` in a markdown code-fence with `cargo` at the start of the -infostring inside a target-level doc-comment. It is an error to have multiple -`cargo` code fences in the target-level doc-comment. We can relax this later, -either merging the code fences or ignoring later code fences. - -Supported forms of embedded manifest are: -``````rust -//! ```cargo -//! ``` -`````` -``````rust -/*! - * ```cargo - * ``` - */ -`````` +is stored using `TOML` in rust "frontmatter", a markdown code-fence with `cargo` +at the start of the infostring at the top of the file. Inferred / defaulted manifest fields: - `package.name = `