Add a way to inherit documentation <inheritdoc />

[gafter]

(moved from part of https://github.com/dotnet/roslyn/issues/67 by @dfkeenan, with part of the request tracked at https://github.com/dotnet/csharplang/issues/315)

Hi,

Please add <inheritdoc /> tag or similar:

I think it would be really handy if the compiler(s) when outputting the documentation XML file if it could use inherited documentation where available/requested. Though I am not sure what should be output if there is no documentation available.

P.S. A Diagnostic and Code Fix that generates document comments for me would also great :smiley:

[daveaglick]

Should <inheritdoc /> and namespace comments be tracked in separate issues?

Also, FWIW, I have an open pull request at https://github.com/dotnet/roslyn/pull/15494 that addresses namespace documentation comments as described in https://github.com/dotnet/roslyn/issues/15474 (which hasn't been closed or migrated yet).

[gafter]

@daveaglick Yes, having separate issues would be great. If you create a new one for one of these and link to it here, I'll edit the OP here.

[daveaglick]

@gafter :+1:

I created https://github.com/dotnet/csharplang/issues/315 which is just a copy of https://github.com/dotnet/roslyn/issues/15474 (so I guess that corresponding Roslyn issue can also be closed now)

[terrajobst]

Is's worth mentioning that this tag can be (and already is) supported by documentation generators. By making it a Roslyn feature it makes it first class by making IntelliSense work (e.g. hovering over calls in the class overriding the member shows the documentation). Also, it should be implied to reduce boilerplate without causing a compiler warning for missing documentation.

[sharwell]

I believe a good way to start here would be examining the features currently provided by SHFB for this documentation element, and then identifying ones which would be impractical from an implementation perspective or "not particularly needed" from an overall usage perspective.

:link: inheritdoc (Sandcastle XML Comments Guide)

Syntax

I believe it's reasonable to use the same form as defined by SHFB:

<inheritdoc [cref="member"] [select="xpath-filter-expr"] />

This element may be placed either as a top-level element or as an inline element in a documentation comment.

Compilation

The compiler is updated in the following ways to account for this element:

• The compiler MUST allow the use of an inheritdoc element as a top-level element in XML documentation comments
• The compiler MUST allow the use of an inheritdoc element as an inline element in XML documentation comments
• The compiler SHOULD report a warning if the inheritdoc element appears without a cref attribute, and no candidate for inheriting documentation exists
• The compiler MUST evaluate the cref attribute value in the same manner as it does for the see element, and include the resolved documentation ID as the value of the attribute in the output documentation file
• The compiler MAY report a warning if the select attribute is specified, but the value is not a syntactically valid XPath expression
• The compiler SHOULD preserve the placement and form of the inheritdoc element in the XML documentation file. It MUST NOT replace the inheritdoc element with inherited documentation when writing the documentation file. (Edit: This one is the subject of disagreement below; I plan to revisit after writing up a proposal for the Tools section.)

Candidate for inheritance

⚠️ Some items in this list are written as existence (Boolean), while others identify the actual candidate(s). This section should be cleaned up with the understanding that the compiler only needs to care about existence for the purpose of reporting a warning as described above.

• For types and interfaces
  • The type is derived from a class which is not System.ValueType, System.Enum, System.Delegate, or System.MulticastDelegate, OR
  • The type implements a named interface which is not itself
• For constructors
  • The containing type of the constructor is derived (directly or indirectly) from a type which contains a constructor with the same signature
• For explicit interface implementations
  • The candidate is the implemented interface member
• For overriding methods (methods marked with override
  • The candidate is the overridden method
• For implicit interface implementations
  • The candidates are the implemented interface members

Impacted warnings

The following warnings, which are specific to the Roslyn implementation of a compiler for C#, should be updated to account for this feature:

CS1573

Parameter 'parameter' has no matching param tag in the XML comment for 'parameter' (but other parameters do)

Care should be taken to not report this warning in the following scenario. It would be sufficient to disable this warning any time inheritdoc appears as a top-level documentation element.

class Base {
  /// <param name="x">Doc for x</param>
  protected void Method1(int x) { }
}

class Derived : Base {
  /// <inheritdoc cref="Base.Method1"/>
  /// <param name="y">Doc for y</param>
  protected void Method2(int x, int y) { }
}

CS1712

Type parameter has no matching typeparam tag in the XML comment (but other type parameters do)

This is similar to the previous case, but applies for type parameters. The example shows type parameters for a generic type, but it can also apply to generic methods.

/// <typeparam name="T">Doc for T</param>
class Base<T> {
}

/// <inheritdoc/>
/// <typeparam name="T2">Doc for T2</param>
class Derived<T, T2> : Base<T> {
}

Tools

This section describes the manner (semantics) in which inheritdoc elements should be evaluated.

Will be added in a later comment

[pascalberger]

The compiler SHOULD preserve the placement and form of the inheritdoc element in the XML documentation file. It MUST NOT replace the inheritdoc element with inherited documentation when writing the documentation file.

Why shouldn't inheritdoc be replaced in the XML documentation file? This would make the generated XML documentation files usable to a much broader set of tools IMHO

[daveaglick]

I agree with @pascalberger above about replacing in the XML documentation file. There are tools, including documentation generators, that use the XML doc file as a substitute for having access to the source. If the inheritdoc comments aren't replaced in the output XML file then those tools will have to continue to implement the inheritdoc logic and we're back to where we started.

If that's not feasible, perhaps the XmlDocumentationProvider could be extended to perform the substitution. At least that way a tool/generator could read the assembly with Roslyn, hooking up the XML comment file via the provider, and get consistent inheritdoc resolution.

[sharwell]

Why shouldn't inheritdoc be replaced in the XML documentation file?

Ignoring the "it's what we do now" issue, three immediate reasons:

1. The XML documentation files might not be available at compilation time, but added later. For example, suppose X.dll 1.0 depends on Y.dll 1.0, but the latter doesn't include XML documentation. A user later builds App.exe depending on X.dll 1.0 and explicitly updates Y.dll to 1.1, which now includes documentation. The inherited documentation should propagate to methods in X.dll where inheritdoc was specified.
2. XML documentation of a base type may change, and typically users want the changes to propagate without requiring the library be recompiled. (I know in all cases where I used inheritdoc in the past, this was my expectation.)
3. Duplicating the documentation unnecessarily bloats the output XML form.

At least that way a tool/generator could read the assembly with Roslyn, hooking up the XML comment file via the provider, and get consistent inheritdoc resolution.

I would expect Roslyn to provide an API which provides code element documentation with inheritdoc expanded.

[pascalberger]

While I see the advantages you mention, another downside, beside documentation generators mentioned by @daveaglick, of this approach is that if a library is created with inheritdoc tags in the documentation XML they won't be resolved for users of older versions of Visual Studio.

[sharwell]

While I see the advantages you mention, another downside, beside documentation generators mentioned by @daveaglick, of this approach is that if a library is created with inheritdoc tags in the documentation XML they won't be resolved for users of older versions of Visual Studio.

That's a very good point. I forgot I use SHFB as a preprocessor to resolve these before packaging NuGet from projects that use inheritdoc.

:link: https://github.com/tunnelvisionlabs/dotnet-threading/commit/a253a2b2e5ba0ef9614a978c89a32ad2d6d08804

[matkoch]

Given the potential pitfalls, I'm very much against adopting the select feature. When changing the documentation in some base class, there is no easy way to verify that none of the derived types won't break (i.e., gets their documentation changed unintentionally).

[sharwell]

Given the potential pitfalls, I'm very much against adopting the select feature. When changing the documentation in some base class, there is no easy way to verify that none of the derived types won't break (i.e., gets their documentation changed unintentionally).

Do you have examples? I've only very rarely needed select, but never run into specific problems with it. However, it's presence makes it a bit easier (IMO) to explain the behavior of inheritdoc as a nested element.

[matkoch]

@sharwell you could, for instance, select the second paragraph via select, and if you later insert a paragraph between the first and the second, the inherited paragraph gets changed without noticing that.

[daveaglick]

I'll also ++ for no select. In addition to the problems @matkoch mentioned with relative positioning of content, I think it also makes the feature more complex and confusing for very little actual gain.

[sharwell]

@matkoch I've never seen someone do that. Considering that it's hard to get users to write any documentation, much less put thought into the specific content resulting from what they write, I do not anticipate any notable number of problems arising from this feature. The only thing I've ever used it for IIRC was to remove a "note to implementers" that would otherwise have been inherited. It was a strange message to otherwise be included in the documentation for a sealed method, and I was happy to have a way to remove it.

[matkoch]

@sharwell it is even used as an example in the Sandcastle documentation (select="para[last()]"). In my opinion it shouldn't be a reasoning about whether someone is actually using this or not. It's about locality, the potential defects a change can introduce and the (understandable) missing tooling. If you add a new abstract method or change some signature, you'll get a compiler error, since derived types are affected. Changing inherited documentation however would become a task of manual verification.

I also agree with @daveaglick. Too much complexity for little gain.

[matkoch]

@sharwell A compromise could be to allow only a restricted set of select expressions. Like selection by ID.

[sharwell]

Given I have had use for select, I lean in favor of having the option. However, given how rarely I've needed it, it's certainly not essential.

A compromise could be to allow only a restricted set of select expressions. Like selection by ID.

This would be bad. Given we're already in the context of XML, XPath has a specific meaning and we don't have to redefine it. If we allow the attribute but don't use XPath, we have to redefine everything and can't use an off-the-shelf XPath implementation.

[matkoch]

@sharwell I didn't mean to introduce a new syntax, but rather to allow only a subset of xpath. I.e., no last() or selection by index.

[sharwell]

@pascalberger How about this? (I only modified the main affected element; other changes way be needed elsewhere in the text for consistency if this approach is adopted.)

The compiler SHOULD emit the documentation file with inheritdoc elements replaced by their inherited content. The compiler MAY support emitting the documentation file with inheritdoc elements not replaced; in this case the compiler SHOULD preserve the placement and form of the inheritdoc element in the XML documentation file, except with the cref attribute expanded to the documentation ID of the referenced member.

[sharwell]

Tools

:memo: This section defines the inheritance semantics of the inheritdoc element, as they would apply to the tools responsible for interpreting the meaning of inheritdoc. If the compiler expands inheritdoc during the production of a documentation file (e.g. https://github.com/dotnet/csharplang/issues/313#issuecomment-322247608), then the compiler would be considered a tool for the purposes of this section.

General

The expansion of an inheritdoc element produces a XML node set which replaces the inheritdoc element.

Inheritance rules

The inheritance rules determine the element(s) from which documentation is inherited. The behavior is unspecified if a cycle exists in these elements.

The search order is defined under the Top-Level Inheritance Rules section of inheritdoc. However, the rules for determining which elements to ignore are generalized to the following:

• The overloads element is never inherited when the select attribute is omitted. In other words, the default value for the select attribute is *[not(self::overloads)].
• If an element includes a cref attribute, it is only omitted if the matching existing element has a cref attribute that resolves to the same documentation ID is already included
• If an element includes a href attribute, it is only omitted if the matching existing element has an href attribute with an equivalent URI
• If an element includes a name, vref, and/or xref attribute, it is only omitted if the matching existing element has the corresponding attribute(s) with the same value(s)
• After observing the above, an element is omitted if an existing element (already inherited or a sibling of the inheritdoc element) has the same element name

Inline inheritdoc elements

When an inheritdoc element appears inline (as opposed to the top level), the base node from which the select query is evaluated changes to the parent element of the inheritdoc element. The path to the matching node is identified by all of the following:

• The element name
• The values of attributes of the element
• The index of the element

For example, the following inheritdoc elements are equivalent:

/// <summary>
/// <list type="number"><item></item></list>
/// <list type="bullet">
/// <item></item>
/// <item><inheritdoc/></item>
/// </list>
class WithoutSelectAttribute { }

/// <summary>
/// <list type="number"><item></item></list>
/// <list type="bullet">
/// <item></item>
/// <item><inheritdoc select="/summary[0]/list[@type='bullet'][0]/item[1]/*[not(self::overloads)]"/></item>
/// </list>
class WithSelectAttribute { }

[pascalberger]

The compiler SHOULD emit the documentation file with inheritdoc elements replaced by their inherited content. The compiler MAY support emitting the documentation file with inheritdoc elements not replaced; in this case the compiler SHOULD preserve the placement and form of the inheritdoc element in the XML documentation file, except with the cref attribute expanded to the documentation ID of the referenced member.

@sharwell Does this mean the compiler is free how it implements it (either the SHOULD or the MAY sentence)? Or that it's some kind of an option?

Otherwise LGTM

[sharwell]

Does this mean the compiler is free how it implements it (either the SHOULD or the MAY sentence)? Or that it's some kind of an option?

@pascalberger It's written with the intent of acknowledging users may want an option to control this, while also suggesting the preferred behavior in the event the compiler only implements one or the other.

[matkoch]

@sharwell I'm confused now given your recent post. Does that mean that you will keep the select feature?

[sharwell]

@matkoch I'm in favor of keeping select, but wanted to present the alternative in concrete wording for the sake of continued discussion. As mentioned in the alternative wording comment, other language including that seen in the Tools section would need to be updated to reflect the final supported behavior.

[matkoch]

Anyone got an idea how this following example should be handled?

public interface IFoo
{
  event EventHandler A;
  event EventHandler B;
}
public class Bar : IFoo
{
  /// <inheritdoc />
  public event EventHandler A, B;
}

Will it require a cref definition? And if yes, will we always allow to reference symbols, not contained in the actual inheritance chain ?

[sharwell]

@matkoch It would be treated exactly the same as if the comment were separately applied to Bar.A and Bar.B.

[dotMorten]

There's some chat between having the xml doc used for intellisense vs for generating documentation, supporting older clients etc. Perhaps the real solution here is to generate two? One for intellisense with the inheritdoc content evaluated, internals, privates, remarks etc stripped out*, , and one for generating doc with everything that you don't actually ship with the binary?

(*Stripping out privates, internals, remarks etc not used for intellisense can significantly reduce the payload for the intellisense doc)

[sharwell]

I've started work on an initial implementation and have some additional notes:

1. For consistency with <include>, I believe the select attribute in the proposal above should be renamed to path. @EWSoftware this is likely to cause a discrepancy with Sandcastle, but I believe this aspect of inheritdoc is "relatively" infrequently used and I'm hoping it will not be overly burdensome to update SHFB to support path (with select retained as a compatibility fallback).
   • The new behavior could result in an unexpected change in output for users who expected select to work with the C# compiler. We should consider either supporting select as an alias for path, reporting a warning for select, or aiding in the implementation of an analyzer/code fix that reports diagnostics for select and offers to convert it to path instead.
2. Given the compiler already uses XPath for <include> elements, it seems obvious to support the same for <inheritdoc>. Given that the compiler will validate the included XML content, it may be necessary to use filters in some cases to exclude content which would result in compilation warnings. (Whether that takes the form of a path attribute or a select attribute is a separate question.)
3. It's unclear (to me) if the new behavior should be tied to LangVersion or exposed as a compiler option in some manner. In the current prototype, it's simply implemented in the same manner as include elements are handled, meaning the replacement is always enabled during builds.

[EWSoftware]

@sharwell It's a minor change in two places in the SHFB tool to also check for path if select isn't found. I could also have the tool issue a warning if select is found stating that it's deprecated and to use path instead. Once this is ironed out and ready for implementation, let me know and I can make the appropriate changes.