Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct codeblocks to not break cargo test --doc #633

Merged
merged 8 commits into from
May 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/continuous-integration-workflow.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ jobs:
matrix:
toolchain:
- stable
- 1.51.0
- 1.53.0
os:
- ubuntu-latest
- macos-latest
Expand Down
5 changes: 5 additions & 0 deletions prost-build/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ edition = "2018"
[features]
default = []
vendored = []
# When MSRV moves to 1.60, these can change to dep:
cleanup-markdown = ["pulldown-cmark", "pulldown-cmark-to-cmark"]

[dependencies]
bytes = { version = "1", default-features = false }
Expand All @@ -28,6 +30,9 @@ prost-types = { version = "0.10.0", path = "../prost-types", default-features =
tempfile = "3"
lazy_static = "1.4.0"
regex = { version = "1.5.5", default-features = false, features = ["std", "unicode-bool"] }
# These two must be kept in sync
pulldown-cmark = { version = "0.9.1", optional = true, default-features = false }
pulldown-cmark-to-cmark = { version = "10.0.1", optional = true }

[build-dependencies]
which = { version = "4", default-features = false }
Expand Down
111 changes: 103 additions & 8 deletions prost-build/src/ast.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use lazy_static::lazy_static;
use prost_types::source_code_info::Location;
#[cfg(feature = "cleanup-markdown")]
use pulldown_cmark::{CodeBlockKind, Event, Options, Parser, Tag};
use regex::Regex;

/// Comments on a Protobuf item.
Expand All @@ -17,13 +19,59 @@ pub struct Comments {

impl Comments {
pub(crate) fn from_location(location: &Location) -> Comments {
#[cfg(not(feature = "cleanup-markdown"))]
fn get_lines<S>(comments: S) -> Vec<String>
where
S: AsRef<str>,
{
comments.as_ref().lines().map(str::to_owned).collect()
}

#[cfg(feature = "cleanup-markdown")]
fn get_lines<S>(comments: S) -> Vec<String>
kinnison marked this conversation as resolved.
Show resolved Hide resolved
where
S: AsRef<str>,
{
let comments = comments.as_ref();
let mut buffer = String::with_capacity(comments.len() + 256);
let opts = pulldown_cmark_to_cmark::Options {
code_block_token_count: 3,
..Default::default()
};
match pulldown_cmark_to_cmark::cmark_with_options(
Parser::new_ext(comments, Options::all() - Options::ENABLE_SMART_PUNCTUATION).map(
|event| {
fn map_codeblock(kind: CodeBlockKind) -> CodeBlockKind {
match kind {
CodeBlockKind::Fenced(s) => {
if &*s == "rust" {
CodeBlockKind::Fenced("compile_fail".into())
} else {
CodeBlockKind::Fenced(format!("text,{}", s).into())
}
}
CodeBlockKind::Indented => CodeBlockKind::Fenced("text".into()),
}
}
match event {
Event::Start(Tag::CodeBlock(kind)) => {
Event::Start(Tag::CodeBlock(map_codeblock(kind)))
}
Event::End(Tag::CodeBlock(kind)) => {
Event::End(Tag::CodeBlock(map_codeblock(kind)))
}
e => e,
}
},
),
&mut buffer,
opts,
) {
Ok(_) => buffer.lines().map(str::to_owned).collect(),
Err(_) => comments.lines().map(str::to_owned).collect(),
}
}

let leading_detached = location
.leading_detached_comments
.iter()
Expand Down Expand Up @@ -101,6 +149,9 @@ impl Comments {

let mut s = RULE_URL.replace_all(line, r"<$0>").to_string();
s = RULE_BRACKETS.replace_all(&s, r"\$1$2\$3").to_string();
if !s.is_empty() {
s.insert(0, ' ');
}
s
}
}
Expand Down Expand Up @@ -163,22 +214,22 @@ mod tests {
TestCases {
name: "valid_http",
input: "See https://www.rust-lang.org/".to_string(),
expected: "///See <https://www.rust-lang.org/>\n".to_string(),
expected: "/// See <https://www.rust-lang.org/>\n".to_string(),
},
TestCases {
name: "valid_https",
input: "See https://www.rust-lang.org/".to_string(),
expected: "///See <https://www.rust-lang.org/>\n".to_string(),
expected: "/// See <https://www.rust-lang.org/>\n".to_string(),
},
TestCases {
name: "valid_https_parenthesis",
input: "See (https://www.rust-lang.org/)".to_string(),
expected: "///See (<https://www.rust-lang.org/>)\n".to_string(),
expected: "/// See (<https://www.rust-lang.org/>)\n".to_string(),
},
TestCases {
name: "invalid",
input: "See note://abc".to_string(),
expected: "///See note://abc\n".to_string(),
expected: "/// See note://abc\n".to_string(),
},
];
for t in tests {
Expand Down Expand Up @@ -207,22 +258,22 @@ mod tests {
TestCases {
name: "valid_brackets",
input: "foo [bar] baz".to_string(),
expected: "///foo \\[bar\\] baz\n".to_string(),
expected: "/// foo \\[bar\\] baz\n".to_string(),
},
TestCases {
name: "invalid_start_bracket",
input: "foo [= baz".to_string(),
expected: "///foo [= baz\n".to_string(),
expected: "/// foo [= baz\n".to_string(),
},
TestCases {
name: "invalid_end_bracket",
input: "foo =] baz".to_string(),
expected: "///foo =] baz\n".to_string(),
expected: "/// foo =] baz\n".to_string(),
},
TestCases {
name: "invalid_bracket_combination",
input: "[0, 9)".to_string(),
expected: "///[0, 9)\n".to_string(),
expected: "/// [0, 9)\n".to_string(),
},
];
for t in tests {
Expand All @@ -238,4 +289,48 @@ mod tests {
assert_eq!(t.expected, actual, "failed {}", t.name);
}
}

#[test]
fn test_codeblocks() {
struct TestCase {
name: &'static str,
input: &'static str,
#[allow(unused)]
cleanedup_expected: Vec<&'static str>,
}

let tests = vec![
TestCase {
name: "unlabelled_block",
input: " thingy\n",
cleanedup_expected: vec!["", "```text", "thingy", "```"],
},
TestCase {
name: "rust_block",
input: "```rust\nfoo.bar()\n```\n",
cleanedup_expected: vec!["", "```compile_fail", "foo.bar()", "```"],
},
TestCase {
name: "js_block",
input: "```javascript\nfoo.bar()\n```\n",
cleanedup_expected: vec!["", "```text,javascript", "foo.bar()", "```"],
},
];

for t in tests {
let loc = Location {
path: vec![],
span: vec![],
leading_comments: Some(t.input.into()),
trailing_comments: None,
leading_detached_comments: vec![],
};
let comments = Comments::from_location(&loc);
#[cfg(feature = "cleanup-markdown")]
let expected = t.cleanedup_expected;
#[cfg(not(feature = "cleanup-markdown"))]
let expected: Vec<&str> = t.input.lines().collect();
assert_eq!(expected, comments.leading, "failed {}", t.name);
}
}
}
12 changes: 12 additions & 0 deletions prost-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,18 @@
//! That's it! Run `cargo doc` to see documentation for the generated code. The full
//! example project can be found on [GitHub](https://github.com/danburkert/snazzy).
//!
//! ### Cleaning up Markdown in code docs
//!
//! If you are using protobuf files from third parties, where the author of the protobuf
//! is not treating comments as Markdown, or is, but has codeblocks in their docs,
//! then you may need to clean up the documentation in order that `cargo test --doc`
//! will not fail spuriously, and that `cargo doc` doesn't attempt to render the
//! codeblocks as Rust code.
//!
//! To do this, in your `Cargo.toml`, add `features = ["cleanup-markdown"]` to the inclusion
//! of the `prost-build` crate and when your code is generated, the code docs will automatically
//! be cleaned up a bit.
//!
//! ## Sourcing `protoc`
//!
//! `prost-build` depends on the Protocol Buffers compiler, `protoc`, to parse `.proto` files into
Expand Down
6 changes: 3 additions & 3 deletions prost-types/src/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ pub mod code_generator_response {
/// The file name, relative to the output directory. The name must not
/// contain "." or ".." components and must be relative, not be absolute (so,
/// the file cannot lie outside the output directory). "/" must be used as
/// the path separator, not "\".
/// the path separator, not "".
///
/// If the name is omitted, the content will be appended to the previous
/// file. This allows the generator to break large files into small chunks,
Expand All @@ -87,7 +87,7 @@ pub mod code_generator_response {
/// produced by another code generator. The original generator may provide
/// insertion points by placing special annotations in the file that look
/// like:
/// @@protoc_insertion_point(NAME)
/// @@protoc_insertion_point(NAME)
/// The annotation can have arbitrary text before and after it on the line,
/// which allows it to be placed in a comment. NAME should be replaced with
/// an identifier naming the point -- this is what other generators will use
Expand All @@ -99,7 +99,7 @@ pub mod code_generator_response {
///
/// For example, the C++ code generator places the following line in the
/// .pb.h files that it generates:
/// // @@protoc_insertion_point(namespace_scope)
/// // @@protoc_insertion_point(namespace_scope)
/// This line appears within the scope of the file's package namespace, but
/// outside of any particular class. Another plugin can then specify the
/// insertion_point "namespace_scope" to generate additional classes or
Expand Down
Loading