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

[ckaran]

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 [e-mail] 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: