Extend <see cref=""/> to support referencing local methods and variables

[HellBrick]

(This is a continuation of dotnet/roslyn#21215: what I had thought of as a simple compiler bugfix/improvement turned out to be something bigger that requires changing the language specification, so here we are.)

Motivation

Generating documentation for public API is only one of the scenarios when XML comments are useful. Another one, that I'm going to focus on in this issue, is writing internal comments that describe implementation details of classes and methods. Even when a comment is not a part of the public documentation, it still can (and often does) benefit from referencing symbols (both public and private) by <see cref=""/>. You can consider something like this as a real-world example. Using <see cref=""/> instead of plain text comments provides a nice editor experience: the features like go to definition, rename, Ctril+Shift+Up/Down, etc. just work for this comments.

As of today, the language and the compiler already support <see cref=""/> referencing private class members. While not being very useful for generating public API documentation, it helps writing internal comments tremendously. However, the supports for this internal comment scenario ends there, and I'd like to suggest extending it further.

Suggestions

There are two correlating changes that I'd like to propose here.

1. Allow <see cref=""/> to reference local variables and methods.

Consider the following simple case:

namespace SeeCrefVersusLocalThings
{
	public class SomeClass
	{
		private readonly int _value = 42;

		/// <remarks>
		/// <see cref="LocalMethod"/> is NOT resolved
		/// <see cref="PrivateMethod"/> is resolved
		/// <see cref="localVar"/> is NOT resolved
		/// <see cref="_value"/> is resolved
		/// </remarks>
		public void OuterMethod()
		{
			int localVar = LocalMethod();

			int LocalMethod() => _value;
		}

		private int PrivateMethod() => _value;
	}
}

When confronted with this code today, the tooling works for the comment referencing PrivateMethod and _value, but not for LocalMethod and localVar. I believe it should work in all four cases: the compiler should resolve the symbol references to local symbols (both local variables and local methods) declared in the scope of the method the XML comment is placed at. At least as long as the reference is not ambiguous.

2. Officially allow XML comments to be placed in the middle of the method.

Normally, an XML comment is placed at a class member (or at a class declaration itself). There's no scoping issues there, since a class can't declare two members with the same name. Local symbols are different though, because the scoping allows this:

/// <remarks>
/// It's ambiguous which of the <see cref="value"/> variables this comment references.
/// </remarks>
public void OuterMethod( bool flag )
{
	if ( flag )
	{
		int value = 42;
		Console.WriteLine( value );
	}
	else
	{
		int value = 64;
		Console.WriteLine( value );
	}
}

However, in some cases it's beneficial to write a comment referencing a local symbol right in the scope it's declared. Not only this eliminates the potential scoping problems for the first suggestion, but placing a comment right next to the line it's describing often provides a better readability.

What's interesting here is this part already works, at least for the symbols that are supported by <see cref=""/> so far:

But, according to @sharwell, this is actually a bug and the language specification does not currently allow placing XML comments inside methods. In practice though, it's a very useful feature that integrates quite nicely with this whole pattern of using <see cref/> for writing internal comments, so I'm proposing to adjust the specification to allow this.

[jnm2]

I am super skeptical of XML documentation being able to see inside the black box, API-wise, that every method should be. Everything a caller should need to know about is exposed in the signature. Why should the method's caller have to know or care if the implementation has local variables or functions at all?

[svick]

If you had officially supported 2, why would you also need 1? XML comments outside a method are meant to describe how to use them method, not its implementation details. If you can document the implementation details using a comment inside the method and the IDE helps you with that, why would you also need support to do the same outside the method?

[HellBrick]

@jnm2

I am super skeptical of XML documentation being able to see inside the black box, API-wise, that every method should be. Everything a caller should need to know about is exposed in the signature. Why should the method's caller have to know or care if the implementation has local variables or functions at all?

First of all, this is not about the API documentation, this is about providing better implementation comments for the people working with the actual code (as compared to just calling the public API and getting docs light up in Intellisense). Second, the black box is already broken at the class level, since the class-level XML comments can reference the private members of the class that shouldn't really be exposed to the outside world either.

@svick

If you had officially supported 2, why would you also need 1? XML comments outside a method are meant to describe how to use them method, not its implementation details. If you can document the implementation details using a comment inside the method and the IDE helps you with that, why would you also need support to do the same outside the method?

As I showed by the last screenshot, 2 by itself already kind of works today. It's just not officially supported and shares the limitations of the "correctly placed" (according to the current specification) XML comments (meaning only class members can be referenced). We still lack the support for referencing local vars and methods, even from the in-method comments located in the scope that the referenced locals can be seen from. If we get this support, then referencing local symbols from the "correctly placed" method XML comments indeed isn't going to be necessary.

[jnm2]

@HellBrick

First of all, this is not about the API documentation, this is about providing better implementation comments for the people working with the actual code (as compared to just calling the public API and getting docs light up in Intellisense).

If that is what this is about: implementation comments belong inside the method, not outside, and either way XML documentation is overkill. Normal comments are perfectly suited.

Second, the black box is already broken at the class level, since the class-level XML comments can reference the private members of the class that shouldn't really be exposed to the outside world either.

So because we have a poor design, let's push it and see if we can make it really poor?

[sharwell]

I'd like to see the second option work. I wouldn't advocate at this time for completely arbitrary placement, since the current warnings alert developers to cases where documentation isn't showing up in the box output. However, placement within a method body seems like it wouldn't be mistaken as exposed documentation.

[sharwell]

Normal comments are perfectly suited.

This is simply incorrect. The inability to express semantics in the comments contained within a method severely limits the ability of an IDE to provide accurate refactorings, including the simplest cases like rename. XML documentation comments may be unnecessarily heavy-handed, but have merits considering familiarity and ease of implementation.

I'd love to see a lighter syntax that still allowed semantics to be fully expressed, but that seems like a separate issue.

[HellBrick]

@jnm2

If that is what this is about: implementation comments belong inside the method, not outside

I agree that allowing this functionality just inside the method would be the most appropriate thing do. And, quite similarly, referencing private members from "public" class-level XML comments is weird and probably shouldn't have been allowed in the first place. The following ship seems to have sailed though, so I'm not sure what decision would be the best right now in terms of producing a less confusing functionality. Personally, I would be fine with either solution, as long as we get the discussed scenario to work.

either way XML documentation is overkill. Normal comments are perfectly suited.

Normal comments are not enough, because they don't allow the tooling to work: no renaming, no peeking definition, no highlighting, etc. We actually don't need the full power of XML comments here, but <see cref=""/> is definitely required to provide a nice experience.

[HaloFour]

What does this have to do with the C# language, though? Sure, XML documentation itself is a part of the specification, but that certainly doesn't stop the use of the same syntax within a method, nor does it stop tooling from supporting it.

I'm fine with the idea of Visual Studio offering better validation and refactoring support for XML documentation comment syntax used in implementation scenarios, but I don't like the idea of that kind of support officially leaking into the language specification. Visual Studio already has support for refactoring comments when renaming variables, and the compilers don't care if you happen to use XML documentation syntax within a method.

[jnm2]

@sharwell @HellBrick The only thing that resonates with me is refactoring without breaking names. Beyond that, if a method is so big that there is even a use case for peeking at documentation of its internals, that's a code smell. Comments should only be explaining the "why." If the "how" isn't self-explanatory by reading the code, the code needs to be more thoughtfully written.

[sharwell]

@HaloFour Currently the language specification explicitly restricts the placement of these comments.

They must immediately precede a user-defined type (such as a class, delegate, or interface) or a member (such as a field, event, property, or method) that they annotate.

🔗 https://github.com/dotnet/csharplang/blob/master/spec/documentation-comments.md#introduction

@jnm2 writes:

... Comments should only be explaining the "why." If the "how" isn't self-explanatory by reading the code, the code needs to be more thoughtfully written.

This applies more to some industries than to others. It is not uncommon for "technically intricate" algorithms to contain documentation explaining decisions. Examples include, but are certainly not limited to, the description of assumptions made regarding the memory model in the implementation of high-performance lock-free or wait-free code. I do not believe it is reasonable to restrict the ability of developers working with such code to be expressive simply because other applications/algorithms can be well-interpreted at face value.

One (limited) workaround currently is writing developer commentary within a <remarks> section. However, there are clear advantages to having the ability to explain oneself at a more localized level (i.e. comments immediately preceding the code which is documented).

[jnm2]

@sharwell I was about to give the example of a mathematically intricate function as a (good) "why" not "how" comment... :'D There's clearly room for interpretation here!

[HaloFour]

@sharwell

Currently the language specification explicitly restricts the placement of these comments.

Currently, at worst, placing an XML documentation comment in an invalid area produces a warning (CS1587). That warning could always be suppressed. Beyond that the compiler doesn't care; it's just a comment. The IDE already offers syntax highlighting and autocomplete (once the comment is in place, it doesn't help in creating the initial skeleton of the comment) and refactoring support (just due to the fact that refactoring supports comments in general).

I'm all about comments within a method body, but I can't imagine why I would want to opt specifically into XML documentation for these cases. Any comments I write inline are generally succinct and specific to the very next line whereas documentation comments apply to the entire method or type. Nothing about the spec or the compiler would prevent the use of XML documentation tags like <see /> from being applied to these inline comments, either.

[tannergooding]

I'm all about comments within a method body, but I can't imagine why I would want to opt specifically into XML documentation for these cases.

I could see people placing things like exception and remarks in the actual method body, that way they are part of the documentation but are also placed where they are actually relevant, rather than just piled on at the top of the method (where one might forget to update if they change the code in the method).

/// <summary>Do the thing</summary>
/// <param name="obj">Parameter to the method.</param>
void MyMethod(object obj)
{
    if (obj is null)
    {
        /// <exception cref="ArgumentNullException"><paramref name="obj" /> is <c>null</c>.</exception>
        throw new ArgumentNullException(nameof(obj));
    }
}

or

/// <summary>Exits the run loop for the application.</summary>
void Exit()
{
    if (IsRunning == false)
    {
        /// <remark>This method does nothing if <see cref="IsRunning" /> is <c>false</c>.</remark>
        return;
    }

    // Exit the application
}

[HaloFour]

@tannergooding

I could see people placing things like exception and remarks in the actual method body, that way they are part of the documentation but are also placed where they are actually relevant, rather than just piled on at the top of the method (where one might forget to update if they change the code in the method).

Not a fan of that concept. When reading XML documentation in source I wouldn't want to have to skim through the method bodies in order to find additional tidbits regardless of their locality. The documentation is on the invocation of the method as a black box, not on the details of the implementation.

[tannergooding]

I tend to agree 😄. I was just pointing out something I could see people using it for.

[svick]

@sharwell

I'd love to see a lighter syntax that still allowed semantics to be fully expressed, but that seems like a separate issue.

I agree that a better syntax for documentation comments would be very useful. Unfortunately, it seems the C# team does not: dotnet/roslyn#85.

[sharwell]

@svick My main objection in that issue was not in regards to the problem being solved, but the specific proposed solution. If a switch is implemented that completely changes the syntax, I want to see a landmark move forward in the history of programming languages, not a marginal improvement that people could generally take or leave.