-
Notifications
You must be signed in to change notification settings - Fork 13k
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
rustdoc support for per-parameter documentation #57525
Comments
I am not aware of any sort of "standard" for documenting arguments. We don't generally do this in the standard library. |
@steveklabnik I was under wrong impression then, thanks for correcting me. There is still value, I think, to support properly documented function arguments. |
Perhaps it is possible to reuse intra rustdoc links syntax for this. We could include the arguments of a function in its rustdoc scope. Using implied reference links,
Similar to paramref in C# xml documentation. |
Although this would not be per-argument documentation, it would fix the de-syncing docs issue while being a relatively lightweight addition to rustdoc. |
It could interesting. However a big problem remains: what should it look like, both in input and ouput? (markdown side and html side.) |
I'm a fan of trying to get per-argument doc comments. Having a finer-grain set of documentation than "the entire function" provides a hook for people to write more docs. The thing we'll have to solve first is to get doc comments to actually work on function arguments: /// function docs
fn asdf<
/// typaram docs
Asdf: Display
>(
/// function arg docs
// ~^ ERROR: expected pattern, found `/// function arg docs`
thing: Asdf
) -> String {
format!("{}", thing)
} Using an unsugared As for how to display it, that's another matter. For bare functions, it's not a stretch that we can add headings to that page (it's currently empty except for the main docs). For associated functions or trait functions, it gets hairier. We currently don't have any precedent for adding headings underneath a generated item (like a function), so anything we add will be new ground. However, it's possible to just print the "whole function" docs, then add headings for anything we have docs for (i.e. don't add argument/typaram listings for functions that haven't added fine-grain docs for them). At least, that's my initial idea, and the one i would try if/when we get the docs to load in the first place. |
One thing i would be interested in seeing (and this may become a lang-team question) is whether/how we can add an attribute to the return type of a function, to possibly document that separately. |
The feature request as proposed by @kvark is brilliant! I think it fits very nicely with rust's pragmatic view of language design.
Bottom line for me is that good docs are critical and this seems to encourage both creation and consumption. |
My two cents: I don't mind this feature, but I just wanted to say that I love the fact that documenting every argument individually is not done frequently in the rust docs. In my experience with other languages where documenting every argument with a line of explanation is encouraged or even enforced by a linter, that leads to really pointless documentation since often it is completely obvious from the name of the argument what it is for. I very much prefer a few sentences explaining the function and reference to the argument names using |
we could use definition lists for that:
and with css this could be aligned like this (without bullet points and with correct alignment)
|
I think @mathijshenquet idea of using implied reference links is a good one and using That's my 2 cents anyway and I hope to see this added soon. |
We could of course just use the Whatever syntax we use, personally I think the No. 1 reason to implement some kind of per-argument docs system that rustdoc understands is that we could add diagnostics so that they never go stale if an argument is removed. I just ran into this and opened a PR to fix it (#80103) but that wouldn't have ever happened if rustdoc gave an error when documentation refers to an argument that doesn't exist. |
@jyn514 Why did you apply the |
Rather than adding special syntax, I would rather just accept doc-comments on the parameters themselves: /// Supplies a new frame to WebRender.
///
/// Non-blocking, it notifies a worker process which processes the display list.
///
/// Note: Scrolling doesn't require an own Frame.
pub fn set_display_list(
/// Target Document ID.
&mut self,
/// The unique Frame ID, monotonically increasing.
epoch: Epoch,
/// The background color of this pipeline.
background: Option<ColorF>,
/// The size of the viewport for this frame.
viewport_size: LayoutSize,
/// * `pipeline_id`: The ID of the pipeline that is supplying this display list.
/// * `content_size`: The total screen space size of this display list's display items.
/// * `display_list`: The root Display list used in this frame.
(pipeline_id, content_size, display_list): (PipelineId, LayoutSize, BuiltDisplayList),
/// If a previous frame exists which matches this pipeline
/// id, this setting determines if frame state (such as scrolling
/// position) should be preserved for this new display list.
preserve_frame_state: bool,
) Then there's no need for special HTML or markdown syntax, it's very clear what text applies to which parameters just by looking at it. I added
I agree that this shouldn't be the encouraged form of documentation, but I think it would be nice to at least make it possible to write (for the same reason that |
This isn't an issue if the syntax is on the argument itself - you'd know when you remove the argument to remove the documentation, because it's right there staring you in the face. |
7060: INT: Generate documentation stub r=mchernyavsky a=0x7CA changelog: Added an editor intention that generates a documentation stub as demonstrated in: https://doc.rust-lang.org/beta/rust-by-example/meta/doc.html and rust-lang/rust#57525 ![oKw9t3X](https://user-images.githubusercontent.com/12113428/113785446-7103c500-9737-11eb-963b-0deda81a1052.gif) I'm not sure if the current implementation is robust enough, as the RustCommenter lacks the functionality that was used for IntelliJ Javadocs and Pycharm docstring implementations. So this implementation has to manually create the comments by inserting strings. I was also not able to parse the function context like in intellij to see whether a function was being declared or just being referenced, or already had a doc, so I had to implement it. Feedback is very welcome Co-authored-by: 0x7CA <12113428+0x7CA@users.noreply.github.com>
This is also what Dart does. I think it would be quite nice to have. |
Would love this. It is already called out as a future possibility in RFC 2565: Attributes in formal function parameter position |
The next step here is for someone to make a lang team MCP or RFC. It is not something rustdoc can do on its own. |
The useful part about this for me in languages that support it, like C# and Java, is that you can create a linter which detects when you mention a function parameter that does not exist. This helps keep documentation up-to-date when functions change. |
Summary: Looks like function parameter docs are not supported in Rust: rust-lang/rust#57525 Move them up so that they are displayed as in our docs, would be nice to move it back once it is supported. Differential Revision: D44629092 fbshipit-source-id: 6124f70f2e31ddc9d268e5aedbc674a42f994add
What if instead I prefer
Many people do comments like that |
this can work with the format presented in the original post that OP doesn't like (it is, in fact, many people's preferred way of doing things) |
This suggestion covers your first case, I think in a more rustic way: pub fn f(
a: u32, //! Parameter A
b: u32, //! Parameter B
) { ... } As for your second case, that's the first time I've seen that, what language or ecosystem is that common in? Other than the obscurity, I think there is value in consistency within the language and between users of Rust. By that I mean there should be one good way of doing things. |
@tigregalis what suggestion do you have for argument comments that require multiple lines? |
You go above the line you're commenting, as in the original post, and in perfect alignment with existing Rust documentation syntax. To be clear the suggestion is that the I think something that's obvious but no one has specifically mentioned here, are the parallels to documenting a struct. You can easily imagine this: /// Supplies a new frame to WebRender.
///
/// Non-blocking, it notifies a worker process which processes the display list.
///
/// Note: Scrolling doesn't require an own Frame.
pub struct DisplayListSetter {
/// Unique Frame ID, monotonically increasing.
epoch: Epoch,
/// Background color of this pipeline.
background: Option<ColorF>,
/// Size of the viewport for this frame.
viewport_size: LayoutSize,
/// The ID of the pipeline that is supplying this display list,
/// the total screen space size of this display list's display items,
/// and the root Display list used in this frame.
(pipeline_id, content_size, display_list): (PipelineId, LayoutSize, BuiltDisplayList),
/// If a previous frame exists which matches this pipeline id,
/// this setting determines if frame state (such as scrolling position)
/// should be preserved for this new display list.
preserve_frame_state: bool,
} Being roughly equivalent to this: /// Supplies a new frame to WebRender.
///
/// Non-blocking, it notifies a worker process which processes the display list.
///
/// Note: Scrolling doesn't require an own Frame.
pub fn set_display_list(
&mut self,
/// Unique Frame ID, monotonically increasing.
epoch: Epoch,
/// Background color of this pipeline.
background: Option<ColorF>,
/// Size of the viewport for this frame.
viewport_size: LayoutSize,
/// The ID of the pipeline that is supplying this display list,
/// the total screen space size of this display list's display items,
/// and the root Display list used in this frame.
(pipeline_id, content_size, display_list): (PipelineId, LayoutSize, BuiltDisplayList),
/// If a previous frame exists which matches this pipeline id,
/// this setting determines if frame state (such as scrolling position)
/// should be preserved for this new display list.
preserve_frame_state: bool,
) {...} |
Note that Rust also supports inner ( fn foo(
arg1: u32, /*! First arg
Second line
Third line */
) { ... } Though personally I would strongly prefer to use multiline |
@tigregalis I also think that's a good model, but there are quite some corner cases, such as when there are generics on the parameters (including for impl Traits, trait items and Self references). For Self, in the struct it would refer to the new struct not to the original one, and this is specially important when some trait bounds types must be defined to Self. Finally, the function may also have generics and bounds, which would also make the struct being generic, have the same bounds and so on. For example: /// Doc.
pub fn f(
/// Doc.
a: Self
) {...} /// Doc.
pub struct FSetter {
/// Doc.
a: Self, // <- should not refer to `FSetter`
} |
|
Do I understand correctly that the best way forward seems to be an RFC for attributes on function arguments? If so, I'd want to write one (having never written one but needing this for other reasons). note: only recently re-discovered this comment, completely forgot about it. i'll still try to do it though |
(minor nit: fwiw I guess the title should be about parameters instead, not arguments? arguments would be the actual value passed at runtime i think) |
Just ran into this myself. Does anybody know which things would have to be modified in order to implement the initial proposal? (or has it already been implemented in a nightly?) |
That's correct (it's not that that's "the best way" so much as it's a necessity for any change to the surface syntax of the language, which this would amount to).
Make sure you first read carefully and follow the instructions in the Rust RFC Book and review the discussion here (especially @scottmcm's comment above: amongst other things, they're a member of the lang team that must approve the RFC). |
Please feel free to delete this comment if this is not the right place to talk about this. I have released the roxygen crate which exposes an attribute macro to document function parameters and generic arguments. It can maybe act as a polyfill until this lands upstream. |
This is entirely self-inclicted verbosity, the following would work even better than your single sentence since it visually highlights the difference between the "0=top and 1=bottom" labels due to the fact that the names are vertically aligned:
|
Your example is worse, You'd need something like this for the nirvana of self-documentation and instant visual recognition , but Rust blocks this beauty fn draw_rect ( // draw a rectangle accepting coords for 4 edges
←, →,
↑ ,↓,
// or if you think in corners
// draw a rectangle accepting coords for 2 corners
x↖, y↖,
x↘, y↘,
) {} |
apparently github is hell bent on mangling that comment, so here it is again:
sure you can write pathological code in any setting, but the above example shows you that you can write code here which is almost entirely self-documenting. this was what eugene was replying to here:
Well, since you gave no argument to why it's worse, I'll just say this: "It's not", and I will also not provide any arguments. Great! Both sides have provided no arguments. |
I gave an argument, it's right there in your quote: "more readable than going full words" in the same way |
"It's not because going full words is more readable than going single letters" |
All that's being asked for here is the possibility of documenting function parameters in a consistent way. I don't think anybody's arguing for making this required if it were to be added. Some things might be better documented as part of the overall function documentation, and this feature doesn't change that. In fact, I'd very much like to have this feature, but I'd like to suggest that the missing_docs lint should not trigger on undocumented function parameters by default. There could be another lint for that if people want it. |
@eugenesvk This is lovely! In a "never in production" sort of way XD. @cheater I'll provide an argument from a very specific subdomain: mathematics and algorithms. It's very common that people want to program a companion to a paper using the same variable names as the variables in the paper. It makes it easier to read the paper inline with the code. And like it or not, math and algorithm papers use a lot of single letter variable notation. I think @maia-s is exactly right. This is not a requirement, and the lint should be by default "allow". But you can manually set it to "warn" if you want, or do it case-by-case. |
(fyi: even though i'm still interested in tackling this, realistically i'll only get around to this in a few months? so if anyone else might be faster, feel free to pick this up and write an RFC!!) |
@ryanpeach why are you arguing with me? I'm literally one of the first few people who supported adding this. |
It appears to me that this pattern of documenting function parameter has become an informal standard:
It's quite sub-optimal currently for a number of reasons:
So not only it's not exactly most convenient, it's also easy to get the docs de-synchronized from the code (notice the
document_id
above ^). It would be much better if we could do this instead:Does that sound remotely possible? With an assumption that the latter case would produce near-identical documentation to the former.
The text was updated successfully, but these errors were encountered: