build: carry proto comments into generated docs via --include_source_info#225
Merged
Conversation
…ed sets protoc was invoked without --include_source_info, so descriptor sets carried no SourceCodeInfo and proto comments never reached the generated Rust documentation (#222). Pass the flag, matching buf's default behavior, and document that precompiled descriptor sets need the flag on their own protoc invocation to get generated docs. Since reflection never needs source info, the set written by Config::emit_descriptor_set now has SourceCodeInfo stripped for the protoc and buf sources, keeping embedded descriptor bytes lean and proto comments out of shipped binaries. Precompiled sets are still written through byte-for-byte. Signed-off-by: Iain McGinniss <309153+iainmcgin@users.noreply.github.com>
Service and method comments were emitted into #[doc] attributes with no Markdown sanitization, so a proto comment containing a code fence, bracketed text, or a bare URL could break a downstream consumer's cargo doc (broken-intra-doc-links, invalid-html-tags, bare-urls under -D warnings) or be compiled as a doctest by cargo test --doc. buffa already sanitizes message and field comments; this ports its sanitizer (as of buffa v0.8.1) for the service/method path, with hardenings on top: fences without a language annotation default to text, unterminated fences are closed, and fence open/close detection follows CommonMark (matching tick counts, no info string on closers, 4+-space-indented markers are code, not fences). find_comment now also preserves blank lines and continuation indentation instead of flattening every comment into a single run-on block, so paragraph breaks and indented examples survive into the generated docs. Checked-in generated code is regenerated accordingly. Signed-off-by: Iain McGinniss <309153+iainmcgin@users.noreply.github.com>
Sanitizing the prose of a service or method comment is not enough: a code
fence in a proto comment is handed to rustdoc as-is, and rustdoc compiles
it as a doctest in the *consuming* crate, where the example has no imports
and names proto types rather than Rust ones.
Classifying which fences rustdoc considers Rust turns out to be a trap. An
explicit `rust` keeps a block Rust even beside an unknown word
(`rust,noplayground`), error-code tokens (`compile_fail,E0277`) and
`{class=...}` attributes keep it Rust too, and the verdict is
order-dependent. Of the attributes, only a bare `ignore` reliably stops
compilation: `no_run` still type-checks, `should_panic` runs,
`compile_fail` merely inverts the verdict, and `ignore-<target>` is read as
a target list that *replaces* a plain `ignore`, so the block still compiles
everywhere else.
So don't classify. Drop any `ignore-<target>` token and ensure a bare
`ignore` on every fence. The author's language annotation survives, so a
`rust` fence is still syntax-highlighted as `rust,ignore`; an unannotated
fence becomes `text`, since rustdoc highlights nothing but Rust and
guessing a language would buy no rendering benefit.
Signed-off-by: Iain McGinniss <309153+iainmcgin@users.noreply.github.com>
This was referenced Jul 12, 2026
rustdoc's markdown parser compiles a ~~~ block exactly like a ``` one, so a tilde fence in a proto comment was still reaching the consuming crate's cargo test --doc. Recognize both fence characters, and require a closer to match the opener's character and run length. Also tighten what counts as a marker at all: only 0-3 spaces may precede it (4+ spaces or a tab is indented code, and any other leading whitespace is prose), only spaces or tabs may trail a closer, and a backtick fence's info string may not contain a backtick. Reading a non-fence as a fence is not harmless — it "closes" a fence rustdoc never opened, leaving the run rustdoc does treat as an opener unguarded. Signed-off-by: Iain McGinniss <309153+iainmcgin@users.noreply.github.com>
azdagron
approved these changes
Jul 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #222.
connectrpc-buildinvokedprotocwith--include_importsbut not--include_source_info, so the resultingFileDescriptorSetcarried noSourceCodeInfoand proto comments never reached the generated Rust documentation. This adds the flag, matching buf's default behavior (and prost-build's), and updatesConfig::descriptor_setdocs to note that precompiled sets need--include_source_infoon the caller's ownprotocinvocation to get generated docs.One deliberate companion change: the set written by
Config::emit_descriptor_setnow hasSourceCodeInfostripped for the protoc and buf sources. Reflection never needs source info, and passing it through would have grown every embedded descriptor blob (source info is commonly a multiple of the semantic descriptor payload) and leaked proto comments into shipped binaries and reflection responses. Precompiled sets are still written through byte-for-byte, which doubles as the escape hatch for anyone who wants source info in the emitted set. The strip's decode→re-encode round trip preserves unknown fields (custom options), verified against buffa 0.8.1's__buffa_unknown_fieldsretention.The regression test compiles a proto with distinctive message and field comments through the real protoc pipeline, asserts both surface as
///doc comments, and asserts the emitted descriptor set is source-info-free while retaining the file and message. Verified the test fails without the flag. This is the first unit test in the crate that requiresprotocon PATH; CONTRIBUTING already lists protoc v27+ as a prerequisite and the CI Test job installs it.