Add `Parameters` and `Returns` as part of the common headings in RFC 1574

[ckaran]

trafficstars

RFC 1574 gives the following list of updated common headings:

• Examples
• Panics
• Errors
• Safety
• Aborts
• Undefined Behavior

This list misses the formal parameters for function/method calls, as well as a description of what is returned. Can you please add Parameters and Returns as two more common headings?

[steveklabnik]

I personally am not a fan; I find projects that require these to have extremely substandard docs. If there’s something special that the name and type of a parameter or return value needs, including it in the main descriptions is far better.

On Feb 15, 2019, at 12:14 PM, Cem Karan [email protected] wrote:

RFC 1574 gives the following list of updated common headings:

Examples Panics Errors Safety Aborts Undefined Behavior This list misses the formal parameters for function/method calls, as well as a description of what is returned. Can you please add Parameters and Returns as two more common headings?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[ckaran]

@steveklabnik I think I understand what you're saying, but I disagree. Having each parameter individually called out and documented can force coders to really document what parameters do, and what arguments are expected; without this, coders tend to gloss over finer details sometimes.

[mark-i-m]

Also there is sometimes a gap between what’s obvious to the author and what’s obvious to a newbie.

[scottmcm]

I'm with @steveklabnik here. My experience is that with this possibility, someone adds a de-facto rule that it needs to be included, and you get useless repetitiveness from it.

For example, the other day I ended up here: https://msdn.microsoft.com/en-us/library/jj127058(v=vs.118).aspx

This is so common in C# that there's even a plugin to generate this automatically:

• Handling of verbs (GhostDoc assumes the first word of a method name is a verb): Add -> Adds, Do -> Does, Specify –> Specifies
• "Of the" reordering (using trigger words like "width", "height", "name", etc.): ColumnWidth -> Width of the column
• Combined with special handling of specific prefix words: MaximumColumnWidth -> "Maximum width of the column" instead of "Width of the maximum column"
• Automatic detection of acronyms consisting of consonants (Html -> HTML), combined with a list-based approach for other acronyms (Gui –> GUI)
• Use of a list of words where that should not be headed by a "the" (AddItem -> Adds the item, but BuildFromScratch -> Builds from scratch)

https://community.submain.com/blogs/tutorials/archive/2010/12/16/what-does-ghostdoc-lsquo-document-this-rsquo-do.aspx

I strongly believe that using the names of the parameters in the sentence describing what it does is far more useful, especially when you need to talk about how the parameters relate to each other. And in cases with a bunch of parameters that all could use standalone documentation, I think the better answer is to use a builder and document the methods on the builder.

[ckaran]

@scottmcm I see your point. The only valid documentation that I see in your example is for content, where there is a comment stating that it could be null.

That said, what @mark-i-m said is also valid, and also is part of my issue with leaving it out; the person that wrote the code has a clear understanding of what the code is supposed to do, and so assumes that everyone can figure it out from the type information. This only works when the types are strict (in the mathematical sense). However, in my experience it is common to sloppily reuse types everywhere. For example, while I really should define a new type for speed (which includes the units I'm using, etc.), I'm likely on a deadline, and figure f64 is good enough. The problem then is that my function may fail in some unexpected manner if someone passes in NaN, ±Infinity, or a negative value (in some cases, it may fail if the value is close to 0.0). As a good API developer, I should document all of this, and maybe even put in some assert!() macros to catch bad behavior at runtime. However, since I'm on a deadline, I might forget to put in the necessary documentation, which leaves everyone that needs to use the API scratching their heads. Having Parameters and Returns sections at least puts developers on notice about what they should do when documenting their APIs.

All that said, I really do see your point; obeying a stupid rule solely because someone up the food chain said 'you must do X' is the antithesis of what I'm hoping to accomplish here; I want enough documentation that I fully understand what is being accomplished without having to read through reams of code to figure it out, not add to some overworked developer's work load. So, is there a way to add clarity to RFC 1574 explaining what is needed?

Come to think of it, it may be useful to explain which headings are usually mutually exclusive; for example, would you ever have Panics and Aborts for the same function/method? And what precisely is expected in each section???

[ghost]

@ckaran

I disagree with you. My arguments are:

• For example, Panics should be required, because function signatures don't declare panics.
• Returns is good to have, but should not be required. For example: constructor like pub fn new() -> Self. The Self keyword is good enough, and Returns section is redundant.
• Similar to Returns, Parameters is good to have, but should not be required. The C# example above explains very well about that.

From my view, Returns and Parameters are not essential, and are not must-have.

May I ask if I could give you my humble advice? If you like designing good APIs, please try to avoid panics (like your statement about use of assert! to verify parameters). In my view, panics should be final-last resort. I always avoid it at all cost. To this day, all of my internal (Rust) libraries never have a single panic.

[ckaran]

@hkcorac I'm sorry if I gave the impression that Parameters and Returns would be required; I merely stated that they should be added to the list of common headings. I expect developers to apply common sense, and use the headings where they make sense. That said, having these as potential headings gives developers things to think about.

As for your comments about panics, you're right that panics should be avoided where possible. A tool that can help with this is rustig. But there are still cases where they are useful; in some cases, you don't have the resources (time/money/developers) to do things the right way, so you have to prioritize. On the expected path, you might make sure that you return Result, and on other paths you might accept using assert!() or panic(), with the knowledge that should you ever encounter those conditions, something is badly broken. A good example of this is a match on an integer; the last arm might be something like:

_ => panic!("If you're seeing this message, then some new constants were added, but the code wasn't updated")

Since you control all constants in your code, you might never expect to encounter that arm, but if you do encounter it, you want to panic immediately so you get a good backtrace to find the bad code.

[ghost]

@ckaran

Oh you're very nice. Please don't be sorry, your RFC is pefectly fine. It's just that I disagree with you :-D Now I understand it.

I faced some similar cases when I thought there could be no way some path of the code be run, and put unreachable! at that place. But later I found out that it was possible. So when time goes, with help of a clipboard manager, I use some similar code like return Err(...), instead of panics. Personally that rescued me several times.

I undersand your explanation too. But still hope you might be able to find some other workaround :-D

Have a nice day Sir,

[ckaran]

@hkcorac I agree that when you find code can panic that you should replace it with proper error results. It's just a case of not having enough time to do everything right! :cry:

[c-git]

I'm new to the rust ecosystem but I'm guessing by the lack of recent messages on this that it was declined?

If I may be permitted to add my 2 cents.

I strongly agree that it shouldn't always be required less you end up with "boilerplate" documentation like in the C++ example above, but sometimes it would be nice to be able to refer to the attributes in explaining what the function is for and ensure that the names used in the docs match the actual argument names.

If that were added it could lead to fragmentation where some ppl will use the lists approach. But I would argue that some ppl are already doing that because maybe the like me found that as the first option when the trying to google it. But then I tested typing the argument name incorrectly and found that it didn't give an error. Kept digging and ended up here.

[ckaran]

I think it's less of a rejection, and more of everyone getting busy and dropping this one. If it was outright rejected, then this issue would be closed.

[scottmcm]

Note that issues in this RFCs repo are mostly ignored. If someone wants to pick them up and attempt to push them forward that's great, but there's no regular triage of issues here. There's even talk of potentially making the whole issues section read-only, in favour of the rust issue tracker for things that are bugs, and leaving "it might be nice if ____ were added" to IRLO or zulip or blogs or ...

(This one in particular is also difficult as there's current no docs team, so procedurally it's hard to say who could even make a decision here.)

[daniel-brenot]

This RFC is important to me for tools such as N-API and other tools that generate bindings to other languages. A standard on how parameters and returns are documented means that tools can extract these comments in a more standard fashion, allowing for them to be transcribed to other languages. I'll adress some points here with my own opinions in a hope for it to push the conversation forward.

While I agree with the comment regarding the concept that requiring this could end up in substandard documentation for some projects, I think the benefit it adds outweighs this. If you want to describe the function as a library author as opposed to describing each parameter individually I wouldn't blame you, in fact in most libraries I write I would follow the same convention, however there are some cases where this is extremely important and I want to adress those as well.

Missing a comment or commenting an argument that doesn't exist should never yield a panic. That is inconsistent with the rest of the behavior in the language. If you want to validate this, I think using something like clippy for rules about comments being valid is much better.

Generally it would be nice to have this because then we could have clippy rules on a per file basis requiring all functions to have properly documented on argument types, return types, or both. This work wouldn't fall on the core team, as it's a linting option, but would require that we have some sort of standard.

And lastly to somewhat re-iterate something I brought up in my initial point, bindings to other languages. I feel that this is becomming more and more important with the prevelant use of webassembly and tools like py03, N-API and so on. Additionally, I believe this may be important when certifying code to state the bounds of input parameters explicitly. Someone more familiar with certification may be able to speak to this point better than I can, but it would be an important point to consider.

[opeik]

Personally, I'm not a fan; we have types to check constraints and markdown enabled doc comments. If a function is complex enough that each individual parameter needs elaboration that's code smell. Also, in my experience trawling through doxygen land, an unfortunate amount of documentation is @param foo The foo repeated ad nauseam.

[ckaran]

@opeik I agree with you about ...an unfortunate amount of documentation is @param foo The foo repeated ad nauseam. but that is a 'bad practice' issue, not a 'documenting parameters is bad' issue. The issue is that while ...we have types to check constraints..., developers sometimes don't have time to do things the right way, leading to the issues I mentioned earlier where we get sloppy with types. This is the reason I'm suggesting Parameters and Returns sections, so that if we don't have time to do it right, we can at least give our users (and maybe ourselves) a hint about what we're expecting to come in and go out.