Use consistent tags in documentation comments

[bamarsha]

Most documentation comments in the compiler only use the <summary> tag. For short comments, this is fine, but in many cases the summary is several paragraphs long and would be easier to read if it used standard documentation tags to structure the information. This is especially important for exceptions - exceptions mentioned in the <summary> tag are easy to miss, whereas the special <exception> tag makes them stand out more.

I think we need to use some rules for our doc comments. My proposal is:

1. Limit <summary> to one paragraph. Anything longer than that should go in specific tags as appropriate, or the <remarks> tag if none of the other tags apply.
2. Always put exceptions in an <exception> tag.

Tasks

• Use exception tags in C# documentation comments (Use the XML exception tag in doc comments (C#) #732)
• Use exception tags in F# documentation comments (Use the XML exception tag in doc comments (F#) #766)
• Limit summaries to one paragraph
• Decide if other changes to the documentation tags are needed
• Enable C# documentation generation to validate doc comments

[cgranade]

Agreed entirely. Can I also request splitting parameter descriptions in to <param> elements so that they show up in intelliSense correctly?

[kevinhartman]

I'd be interested in working on this issue. Assuming I wouldn't be toe-stepping, my plan would be to start with exceptions, working out of this branch.

[bamarsha]

Hi @kevinhartman, that would be great! Let us know if you have any questions.

[kevinhartman]

Thanks @SamarSha!

I just submitted a PR to add the exception tag for C# doc comments. I didn't do this for the F# code, since most of it uses the implicit summary tag (omits the <summary> tag). Thoughts on using explicit XML tags for that portion of the codebase?

I was also wondering how strictly the <paramref> tag should be used in exception text. In most cases, I was worried that it'd make the documentation less readable when looking at source files. But perhaps if the XML is being used to generate API reference docs, it'd be pretty helpful!

[bamarsha]

Thanks @kevinhartman for the PR! I'll take a look soon.

For the F# comments, I think switching from an implicit summary tag to an explicit one when an exception tag (or other tag) is needed makes sense.

I think using <paramref> is fine too - while the Q# compiler currently doesn't have generated API docs, we're planning to do that at some point, so I think planning ahead and using proper tags makes sense even if they are slightly less readable in the source. Also, IDEs can help a little bit with automatically formatting doc comments while editing code (e.g. by displaying formatted versions in the hover text).

If you have any examples where you think using <paramref> would significantly decrease readability, or if it would be too annoying to write because the parameter name is used too often or something like that, though, let me know.

[kevinhartman]

Sounds great! I can create a separate PR to handle the F# code using that approach.

I'll go ahead and add the <paramref> tag where appropriate for this PR. There are only a handful of cases where things might get a little messy.

Perhaps in a case like this it might make sense to only add reference tags for the first mention of each parameter?

[bamarsha]

Perhaps in a case like this it might make sense to only add reference tags for the first mention of each parameter?

Yeah, that could be one option, although <paramref> has a couple of advantages that work best if every occurrence of a parameter name uses the tag (similar to nameof in C#):

1. When renaming a parameter, an IDE can automatically rename <paramref> tags too.
2. <paramref> tags can be checked automatically by a tool to make sure that the doc comment refers to actual parameter names.

[kevinhartman]

Hmm, that does sound useful. I suppose I'm just wishing C# doc comments weren't XML-based!

Somewhat related, I noticed something that might be worth considering: neither Visual Studio nor VS Code seem to render exception descriptions, at least in the floating window that pops up from the callsite:

Defined here:

At first this surprised me, but thinking more about it now, this would be alleviated through the use of proper application-specific exception design, where the exception class corresponds closely to a single cause, or provides its own interface to get the cause.

Thoughts?

And another question (thanks for your patience!): In many cases, I've noticed that exception descriptions can easily be written clearly without actually naming code references or parameters. When both approaches are possible, which should be preferred?

/// <exception cref="NotSupportedException">Thrown if the path to the output folder is malformed.</exception>
/// <exception cref="IOException">Thrown if <paramref name="outputFolder"/> is a file path.</exception>
public static void PublishResults(string outputFolder) { }

[bamarsha]

Hmm, that does sound useful. I suppose I'm just wishing C# doc comments weren't XML-based!

Yes, they do tend to be very verbose. :)

Somewhat related, I noticed something that might be worth considering: neither Visual Studio nor VS Code seem to render exception descriptions, at least in the floating window that pops up from the callsite:

Interesting, it looks like Rider does show the full exception description in the hover box, though.

At first this surprised me, but thinking more about it now, this would be alleviated through the use of proper application-specific exception design, where the exception class corresponds closely to a single cause, or provides its own interface to get the cause.

I think defining new exception types makes sense sometimes, but I wouldn't want to overuse them - I think it's fine to use the generic ArgumentException in most cases. Maybe for VS and VS Code you just need to go to the full doc comment to see the conditions for the exceptions?

In many cases, I've noticed that exception descriptions can easily be written clearly without actually naming code references or parameters. When both approaches are possible, which should be preferred?

I think I slightly prefer using <paramref>, since it's shorter (you don't have to repeat the parameter description), and if there are multiple parameters, it's easier to tell which one can cause the exception since you don't need to map the description back to the parameter name in your mind. What do you think?

[kevinhartman]

Interesting, it looks like Rider does show the full exception description in the hover box, though.

Wow, Rider looks very snazzy! I wonder if the VS ReSharper plugin also includes the exception description.

I think defining new exception types makes sense sometimes, but I wouldn't want to overuse them - I think it's fine to use the generic ArgumentException in most cases.

I agree. I think most of the ArgumentException usages I've seen are entirely reasonable. For that exception particularly, I believe it's conventionally only thrown when a developer calls a method incorrectly (i.e. the caller wouldn't plan to handle it, unless they were doing something weird).

Maybe for VS and VS Code you just need to go to the full doc comment to see the conditions for the exceptions?

That seem fine to me. Knowing that Rider is an option for the dev team makes me feel content.

I think I slightly prefer using , since it's shorter (you don't have to repeat the parameter description), and if there are multiple parameters, it's easier to tell which one can cause the exception since you don't need to map the description back to the parameter name in your mind. What do you think?

I think I agree. Originally, I was thinking that if devs had to look directly at the XML for exception descriptions, it might make sense to avoid as many inline tags as possible for readability. Perhaps it could be argued that a method with a parameter set not easily described is overly complex. But, this does of course happen, and I like the idea of super concise exception descriptions rendered in all of their glory with Rider :).