Ability to reference a method group in XML comment

[sharwell]

tl;dr

This is a proposal to extend the syntax supported by XML documentation comments to allow documentation comments to reference a group of overloaded methods, and allow these references to be verified at compile time.

Summary

Currently the following works. It compiles without warning and the documentation comment contains a reference to the Foo() method.

/// <summary>
/// Reference to a method group with one item: <see cref="Foo"/>
/// </summary>
void Foo() { }

However, the following does not work:

/// <summary>
/// Reference to a method group with two items: <see cref="Foo"/>
/// </summary>
void Foo() { }
void Foo(int x) { }

The specific warning produced is:

warning CS0419: Ambiguous reference in cref attribute: 'Foo'. Assuming 'TypeName.Foo()', but could have also matched other overloads including 'TypeName.Foo(int)'.

To reference a method group, the following syntax is required:

/// <summary>
/// Reference to a method group with two items:
/// <see cref="O:Full.Declaring.Namespace.TypeName.Foo"/>
/// </summary>
void Foo() { }
void Foo(int x) { }

This is problematic for the following reasons:

1. The syntax is not validated during the build. Errors made while typing are not reported until if/when Sandcastle Help File Builder processes the comments.
2. The syntax is extremely verbose.
3. The syntax only works if there are more than one method with the same name. (This limitation needs to be resolved by tooling, and is not addressed by this proposal specifically.)
4. There is no editor support for this syntax, including the following features:
   • The reference has no syntax highlighting
   • The reference will not be located by Find All References
   • Go To Definition does not work on the reference
   • QuickInfo does not work on the reference

I propose the following modification to the way parameterless method references are resolved:

1. If no argument list is provided and the method group contains exactly one method, compile the comment as a direct reference to that method. This is how the compiler already behaves for this case.
2. If no parameter list is provided and the method group contains more than one method, compile the comment as a reference to the overloads of the method, with the O: form listed above.

Details

Processing of documentation comments during the compilation process behaves as a source-to-source transformation. The C# Language Specification is not clear with respect to the syntax and/or limitations of referencing code elements from a cref attribute. However, the structure of the compiled output is more clear.

Changes to input form

There are no changes to the lexical structure of the input or to symbol resolution when building the semantic model. When generating documentation comment IDs for the purpose of writing resolved references to the output, a new special case is provided for references which do not specify parameters or type parameters. In this case, when the reference resolves to more than one candidate symbol, all of which are methods, the compiler will no longer report CS0419.

Open questions:

• Is it possible to resolve multiple candidates in otherwise-valid code where one or more references resolve to non-method symbols?
• Does resolution of documentation comment references treat extension methods as extension methods, or only as static methods? If the former, are the results mixed with non-extension methods? If extension methods are included we likely need to limit resolution to overloaded members of the object type or report a warning if only extension members exist and they appear in more than one type.

Changes to output form

The ID string format defined in the language specification is amended to include the following member kind:

Character	Description
O	Overloaded method group (containing one or more methods)

In addition, the description of the format for methods and properties is modified to read as follows:

For single methods and properties with arguments, the argument list follows, enclosed in parentheses. For groups of more than one method, and for single methods without arguments, the parentheses and arguments are omitted. The arguments are separated by commas. The encoding of each argument is the same as a CLI signature, as follows:

📝 Originally copied from dotnet/roslyn#45, then modified to formalize the proposal.

[sharwell]

@EWSoftware What does SHFB do in a situation where all of the following occur:

1. A method contains two overloads, one of which is public and the other is private
2. The user creates a comment with <see cref="O:MethodGroup"/>
3. The user generates documentation with private members excluded

In this situation, no overloads page would be generated for the method group, and I would expect the rendered reference to point directly to the public method, which is the only one included in the final documentation.

Do you foresee any problems with SHFB handling O: references in cases where there are 1 or more methods in the referenced group?

[EWSoftware]

As currently written, an overload link will only link to an overload page. If the overload target ID doesn't exist in the reflection data, it will generate a non-link and issue an "unknown reference link" warning. That would be the case above if private methods are excluded. If private methods are included or both are public, the normal overload page would be generated and it would work as expected.

That should be easy enough to change so that it works either way. All it should need to do is when the target isn't found and the ID starts with "Overload:", change it to "M:" and see if there's a key that starts with that ID. If so, use it, otherwise, treat it as an unknown reference link as before.

As currently written, the presentation style transforms convert the "O:" to "Overload:" to match the topic ID generated by the document model transformation hence the full word above. I think it was probably spelled out to make it clear it wasn't a standard prefix and to avoid any possible conflict. It can look for either prefix in case that were to ever change in future presentation styles.

[sharwell]

@EWSoftware Thank you for the awesome details. I was a bit concerned originally regarding a change like my proposal breaking documentation rendering tools (or forcing us to treat method visibility in a manner unique to this feature), but it sounds like that the problems would actually be minor and resolvable.

@gafter If I was interested in formalizing this proposal as a modification to the language specification, what would be a good way to proceed?

[gafter]

@sharwell If the goal is to "get it done", I think the first step is to write up a semi-formal description of what exactly you want to change, and how, with plenty of motivational discussion. A working prototype would help, but it isn't absolutely required. Then the second step is to find someone on the @dotnet/csharplangdesign (LDM) team who would be willing to act as an advocate for the proposal (i.e. advocate prioritizing it over other things we could do) to the LDM. Once a champion is identified, he would create a champion issue, and the LDM will triage it.

[sharwell]

@gafter Aside from the fact that I would find this feature useful myself, I asked due to my interest in the following:

1. I don't want the proposal to fail simply because I failed to adequately define it. From the perspective of the C# Language Specification, I believe I have now addressed this concern by editing the initial proposal.
2. I can use this as a learning experience and apply it to other proposals either myself or by helping those with less experience.
3. I can practice with the prototyping process without worrying that my limited understanding of some of the compiler internals will necessarily cause the process to stall.

[EWSoftware]

@sharwell I've updated the ResolveReferenceLinksComponent in SHFB to try and resolve missing overload IDs to an equivalent non-overload method ID if the overload ID is not found so it'll be in the next release. I figured it's a worthwhile change since it'll work with the hand-crafted IDs people are currently using.

By the way, here's something I'd forgotten. If you specify autoUpgrade="true" on a see or seealso link, SHFB will automatically convert it to a link to the overloads page if the method has overloads. The cref attribute still needs to point to one of the overloads though so less convenient than just specifying the member name by itself.

[drewnoakes]

This would be a useful feature, in my experience. Adding an overload can break docs, but more often it just allows more succinct documentation when you can refer to a group of overloads rather than having to list them all out explicitly (further exposing yourself to breaks following changes).

[RussKie]

Nice one

[mtaylorfsmb]

This request seems stale but needs to be resolved. To be honest there is no way to fix this compiler warning "properly" without using a suppression pragma right now. Any other approach causes other problems.

• The obvious approach of using an overload only works when you have an explicit overload in mind. If you have related methods however then the overload list is the correct link. Furthermore in many cases a method starts with a single definition and therefore the cref is fine without annotation. But as soon as you add an overload later it suddenly causes warnings on all the previously valid crefs. This is a maintenance issue for large code bases.

• If you use the O: prefix then you lose out on refactoring support. Without the prefix if you look at the references of an identifier or rename the identifier then it'll pick up the crefs as well. However if you include the prefix then you no longer see the cref as a reference and rename won't catch it. Therefore that solution, while correct, breaks refactoring.

Additionally, and it may just be a setting I have with an analyzer or something, crefs marked with a prefix generate a warning about avoiding the use of prefixes. I don't see this problem right now in the codebase I'm working on but it is the reason we switched to the pragma instead of using the prefix.

So at this point the only solution that works for me that doesn't break refactoring is using a pragma to disable the warning. Would love to see this fixed soon.

[carlossanlop]

@DustinCampbell any chance this could move forward?

In the .NET Libraries team, we want the triple slash comments in source code to be considered the source of truth for documentation. Currently, the source of truth for us is MS Docs (full proposal).

I am working on this tool that backports MS Docs descriptions into source code xml comments.

MS Docs supports expressing method overloads in see crefs by appending %2A in the method's DocId and excluding the parenthesis and parameters. It looks like: <see cref="M:Namespace.Class.Method%2A" /> (if xml) or <xref:Namespace.Class.Method%2A> (if remarks mardkown). Example.

My backporting tool detects these strings and removes %2A suffix.

Unfortunately, this is causing a build error (since we turn on the mandatory triple slash comments for public APIs).

I have two workarounds:

• Use <see langword="Namespace.Class.Method" /> to express an overload without a build failure, instead of see cref, causing us to lose the validation that ensures an API with that DocId exists. The problem with this approach is that when we build and generate the xmls, then send them back to MS Docs, the links won't be clickable (they need to be see cref).
• Keep using see cref and prefix it with O:, then suppress the error CA1200: Avoid using cref tags with a prefix for the whole assembly.

[jerviscui]

mark.
I want to know the final answer.

You guys are great!