-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a way to inherit documentation <inheritdoc /> #313
Comments
Should Also, FWIW, I have an open pull request at dotnet/roslyn#15494 that addresses namespace documentation comments as described in dotnet/roslyn#15474 (which hasn't been closed or migrated yet). |
@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. |
@gafter 👍 I created #315 which is just a copy of dotnet/roslyn#15474 (so I guess that corresponding Roslyn issue can also be closed now) |
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. |
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. 🔗 inheritdoc (Sandcastle XML Comments Guide) SyntaxI 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. CompilationThe compiler is updated in the following ways to account for this element:
Candidate for inheritance
Impacted warningsThe following warnings, which are specific to the Roslyn implementation of a compiler for C#, should be updated to account for this feature: CS1573
Care should be taken to not report this warning in the following scenario. It would be sufficient to disable this warning any time 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
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> {
} ToolsThis section describes the manner (semantics) in which Will be added in a later comment |
Why shouldn't |
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 If that's not feasible, perhaps the |
Ignoring the "it's what we do now" issue, three immediate reasons:
I would expect Roslyn to provide an API which provides code element documentation with |
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 |
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. |
Given the potential pitfalls, I'm very much against adopting the |
Do you have examples? I've only very rarely needed |
@sharwell you could, for instance, select the second paragraph via |
I'll also ++ for no |
@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. |
@sharwell it is even used as an example in the Sandcastle documentation ( I also agree with @daveaglick. Too much complexity for little gain. |
@sharwell A compromise could be to allow only a restricted set of select expressions. Like selection by ID. |
Given I have had use for
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. |
@sharwell I didn't mean to introduce a new syntax, but rather to allow only a subset of xpath. I.e., no |
@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.)
|
Tools📝 This section defines the inheritance semantics of the GeneralThe expansion of an Inheritance rulesThe 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
Inline
|
@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 |
@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. |
@sharwell I'm confused now given your recent post. Does that mean that you will keep the |
@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. |
Anyone got an idea how this following example should be handled?
Will it require a |
Yep, starting from Visual Studio 16.4 |
Is there a reason why |
|
@BillWagner or @sharwell, do we have this documented somewhere? |
Unofficially it's documented in the Sandcastle XML Comments guide. It's also documented in the Microsoft Docs C# programming guide but isn't as thorough and doesn't mention the |
@333fred You mentioned: it is a compiler feature. But it is in the comment and describing the some documentation, wouldn't this typically belong to the mentioned chapter in the manual? This chapter is called "Documentation comments" and it states at the first line: "C# provides a mechanism for programmers to document their code using a special comment syntax that contains XML text. " |
The manual you linked is the C# language specification, defined as ECMA-334. |
I linked to https://github.com/dotnet/csharplang/blob/master/spec/documentation-comments.md in my first comment (#313 (comment)) and in my second comment (#313 (comment)) I made reference of the quoted which is in the mentioned link as well as in the Annex D of the: so I would expect that the I'm definitely not saying it is part of the c# language but I don't think |
If it is part of the spec, it is part of the language. |
I don't really see a difference between |
@albert-github The |
@sharwell This shines a bit of new light on the problem.
|
The correct place to expand the documentation would probably be the XML documentation chapter on this, i.e. the following document on GitHub: https://github.com/dotnet/docs/blob/master/docs/csharp/programming-guide/xmldoc/inheritdoc.md. |
@poke Looks like promising, though I don't see any reference that this is a repository intended for preparing for standardization by e.g. ECMA / ISO / ... |
This repository is about additions to the C# language. Eventually, such changes will make their way into the ecma spec, as they are part of the language. |
@333fred Nearly contradicting to previous comments, but as I understand it now the |
I'm not sure if this is the right place; Sorry if it's not. I've noticed that Eg.: /// <summary>a</summary>
public void A() {}
/// <inheritdoc cref="A()"/>
/// <summary>b</summary>
/// <returns>foo</returns>
public void B() {} The output docs for
When I expected:
|
tagging @sharwell . however, it's not a case where one set of docs is brought in, then overwritten by other docs. Rather, the set of docs brought in is just brought in verbatim with no processing done on it. |
Is it intended that For example, a constructor might take values to populate the class properties, and the parameters of that constructor will have the same description as the original property. For example: class MyEntity
{
/// <summary>
/// The code of the item
/// </summary>
public string Code
{
get;
set;
}
/// <summary>
/// The description of the item
/// </summary>
public string Description
{
get;
set;
}
/// <summary>
/// Creates a new entity
/// </summary>
/// <param name="code"><inheritdoc cref="MyEntity.Code"/>/param>
/// <param name="description"><inheritdoc cref="MyEntity.Description"/>/param>
public MyEntity(string code, string description)
{
}
} Currently this example inserts blank values in place of the |
It was intended that In your case you are using Personally, I don't like how it functions based on x-paths by default, but it was intended. |
sorry I bumped the comment button earlier than intended... Here is an example that will fix your issue @geoffbon class MyEntity
{
/// <summary>
/// The code of the item
/// </summary>
public string Code
{
get;
set;
}
/// <summary>
/// The description of the item
/// </summary>
public string Description
{
get;
set;
}
/// <summary>
/// Creates a new entity
/// </summary>
/// <param name="code"><inheritdoc cref="Code" path="/summary"/></param>
/// <param name="description"><inheritdoc cref="Description" path="/summary"/></param>
public MyEntity(string code, string description)
{
}
} If you explicitly provide an x-path (via the |
This is outstanding, thanks so much @ZacharyPatten! This makes a lot more sense to me now. |
vs 17.4 has added the inherit doc feature, but what is the shortcut? |
@Varorbc what sort of shortcut are you looking for? If you want IntelliSense to inherit the full documentation for an interface member or abstract method, it already does that if you simply omit the documentation comment entirely. |
* Add tests to identify the current behavior (all tests are green). * Fix: respect [ConfigurationKeyName] override for property names This initially broke the integration test, which loads reference assemblies from the 'refs' subdirectory. But Microsoft.Extensions.Configuration.Abstractions.dll (which contains ConfigurationKeyNameAttribute) isn't copied there during build, which is why I changed the test to not use the refs subdirectory. * Ignore leading/trailing whitespace and line breaks in doc-comments, because they have no semantic meaning. IDEs auto-break lines when rendering the text, depending on screen space. However, do convert <br/> and <para/> tags into visible line breaks. While this doesn't have much effect on Aspire itself, it matters greatly to my team because we have configured Resharper to auto-format doc-comments, which takes the configured maximum line length into account. So it merges and breaks lines in doc-comments all the time. The changes on Aspire .json files are the following, which can be seen after replacing all \n with a single space in the diff. - ` ,` becomes `,` - `( 'SomeType' )` becomes `('SomeType')` - `Some. Other` becomes `Some. Other` - `Some. Other` becomes `Some.\n\nOther` (because of <para> usage) - `"Some ."` becomes `"Some."` * Fix crash on property of type byte[]. The ConfigurationBinder supports an array of numbers, as well as a base64-encoded string. See dotnet/runtime#43150. * Allow settings to appear top-level * Add support for using <inheritdoc /> in code and external assemblies. Because roslyn provides no public API to expand inherited doc-comments (see dotnet/csharplang#313), I'm using the internal Microsoft.CodeAnalysis.Shared.Extensions.ISymbolExtensions.GetDocumentationComment method. The method is made accessible by using reflection. This method behaves a bit odd though: If there's no doc-comment on a member, it internally assumes that the member contains "<doc><inheritdoc/></doc>" (which is completely invalid) and feeds that to itself. As a consequence, the method may return something wrapped in <doc>, instead of the expected <member> element. I didn't want to write a test for this undocumented behavior, but it explains the fallback to <doc> when <member> does not exist. * Refactor recursion to support collections and recursive/circular types, allow all types as component, preserve existing JSON structure on empty nodes. Note this commit includes a fix in the RuntimeSource directory to prevent a StackOverflowException. * Fix broken build after rebase on latest main * Review feedback: use Queue instead of ImmutableQueue * Preserve blank lines in doc-comments * Remove IgnoresAccessChecksToGenerator usage and just use normal reflection to reduce dependencies. * Remove unnecessary #ifs * Add explaining comments for node backup strategy * Fix normalization of whitespace characters, such as tabs * Add explaining comments for line break handling in doc-comments * Rename GenerateComponent to GeneratePathSegment * Provide compiled sample usage instead of appsettings.json comment * Replace minimal assertions with full JSON comparison * PR feedback - rename variables to remove "component" --------- Co-authored-by: Eric Erhardt <[email protected]>
(moved from part of dotnet/roslyn#67 by @dfkeenan, with part of the request tracked at #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 😃
The text was updated successfully, but these errors were encountered: