Skip to content

feat: add docstrings to generated rust code - #665

Open
ksil wants to merge 5 commits into
capnproto:masterfrom
ksil:ksil/generate-docstrings
Open

ksil wants to merge 5 commits into
capnproto:masterfrom
ksil:ksil/generate-docstrings

Conversation

@ksil

@ksil ksil commented Jun 8, 2026

Copy link
Copy Markdown

First-time contributor to the repo here! Please feel free to be picky and let me know if there are any cultural or stylistic things to change!

This is a (hopefully) small change to read the source info from the schema and use it to generate rust doc strings, which should address #165 and #582.

Struct-level comments are added to the module as well as Owned, Reader, Builder, and Pipeline structs. This is potentially overkill, but I thought it was kind of nice for tooltips in IDEs and at various levels of the generated cargo docs. Happy to debate it, though.

Field-level comments are added to getters and setters in Builders and Readers. File-level comments are added at the top of the generated rust file (and are not included in cargo docs because //!-type comments can easily break builds). Enum value comments are added before every value. Interface comments are added for both Client and Server methods. Constants feature comments before their definitions. Annotation comments are added to the module blocks.

Also, I added .idea to the list of gitignored things for those that use RustRover like me.

@ksil

ksil commented Jun 9, 2026

Copy link
Copy Markdown
Author

@dwrensha assuming you're the right person to review this, but let me know if I should loop in others! I don't think I have permission to add reviewers or launch CI workflows

@dwrensha

Copy link
Copy Markdown
Member

Thanks for the PR. My main hesitation here is that this seems like a lot of duplication and could bloat the size of the generated files. Sometimes documentation for individual fields can span multiple paragraphs. (See e.g. this sandstorm schema.) If each such comment gets duplicated three times, it seems like things could get out of hand.

@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 6.12245% with 138 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.99%. Comparing base (ab342b3) to head (a396442).
⚠️ Report is 332 commits behind head on master.

Files with missing lines Patch % Lines
capnpc/src/codegen.rs 6.12% 138 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master     #665       +/-   ##
===========================================
+ Coverage   51.64%   74.99%   +23.35%     
===========================================
  Files          69       68        -1     
  Lines       33735    21057    -12678     
===========================================
- Hits        17422    15792     -1630     
+ Misses      16313     5265    -11048     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ksil

ksil commented Jun 16, 2026

Copy link
Copy Markdown
Author

Thanks for the PR. My main hesitation here is that this seems like a lot of duplication and could bloat the size of the generated files. Sometimes documentation for individual fields can span multiple paragraphs. (See e.g. this sandstorm schema.) If each such comment gets duplicated three times, it seems like things could get out of hand.

I just removed the structure comments on Owned, Reader, Builder, and Pipeline, so now they're just attached to the associated Rust module. Not sure what to do about fields, though, because they are nice to see in the IDE on both the getters and setters and aren't tied to a module. What do you think?

@ksil

ksil commented Jun 25, 2026

Copy link
Copy Markdown
Author

Hi @dwrensha, let me know if we can discuss anything or if there's anything else I can change! Appreciate the help.

@ksil

ksil commented Jul 9, 2026

Copy link
Copy Markdown
Author

Just following up here @dwrensha if there's anything I can do to move this PR along! Apologies for bothering you about it; just wanted to try to contribute a useful change to Rust capnproto here that several of us at my company would love to see

@dwrensha

Copy link
Copy Markdown
Member

The 4x duplication for field doc comments is what makes me hesitate here. I wonder whether it would make sense to hoist the field comments into the module-level struct comment, so that they appear exactly once. And perhaps getters and setters could have comments that somehow link to the module-level comment.

@ksil

ksil commented Jul 12, 2026

Copy link
Copy Markdown
Author

It's pretty common practice across other IDL code generators (like Protobuf in Java and Thrift) to intentionally duplicate field comments across every accessor to prioritize developer experience in IDEs. This duplication only lives in the autogenerated Rust files, which people generally should not check into repos and should not reference directly. These are also comments, which negligibly affect compiler speed compared to other AST elements. Bloat in non-autogenerated files would be a different story. I opt for prioritizing IDE hover support over DRYness in autogenerated build files. Indirection to other module-level comments will most likely make the IDE experience more challenging than necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants