XML Documents for Positional Records

[danstur]

According to #44571 it should be possible with Visual Studio 16.9 to specify xml documents for auto generated properties based on the primary constructor.

When using VS 16.9.3 I can't seem to get this to work. Am I not using the correct syntax or is there still a problem?

Using the following definitions:

/// <summary>
/// Class description.
/// </summary>
/// <param name="Baz">Foo-Baz description</param>
public sealed record Foo(string Baz)
{
}

public sealed class Bar
{
	/// <summary>
	/// Bar-Baz description
	/// </summary>
	public string Baz { get; init; }

	public Bar(string baz)
	{
		Baz = baz;
	}
}

I get the following results:

The generated documentation file also does not include the information:

<?xml version="1.0"?>
<doc>
    <assembly>
        <name>Test</name>
    </assembly>
    <members>
        <member name="T:Test.Foo">
            <summary>
            Class description.
            </summary>
            <param name="Baz">Foo-Baz description</param>
        </member>
        <member name="M:Test.Foo.#ctor(System.String)">
            <summary>
            Class description.
            </summary>
            <param name="Baz">Foo-Baz description</param>
        </member>
        <member name="P:Test.Bar.Baz">
            <summary>
            Bar-Baz description
            </summary>
        </member>
    </members>
</doc>

PS: I originally wrote this as a comment, but since the other issue is already closed and I didn't get any feedback in the last five days, I thought it might be more sensible to create a new issue to track this.

[Youssef1313]

@jcouv Can you take a look?

[Youssef1313]

@danstur Reading through the issue, there are couple of points:

• The doc comment on compiler side is tied to the parameter, not the property (this currently works as expected).
• IDE may choose to adjust inheritance logic so that properties show some information from associated parameter (this doesn't work currently, @sharwell @CyrusNajmabadi What do you think about this?)
• Completion should work (it currently doesn't) - Being done in Handle records in documentation comments in IDE side #52737

[danstur]

@Youssef1313 Thanks for the explanation. So basically the linked issue was more of an internal plumbing point that requires additional work to make these improvements available.

Not listing the documentation for the properties in the xml doc is a pity, because it means all projects using those files (e.g. Swashbuckle) will have to be adapted to handle records explicitly - and I think this might not even be completely possible without also having access to the source/binaries.

Is it worth creating an issue about this or is this unlikely to be changed for some reason? And if this could be changed, would a pull request be accepted?

[Youssef1313]

Not listing the documentation for the properties in the xml doc is a pity

As far as I understand, this is how doc comments for records was decided to work. The param XML comments is tied only to the constructor parameter, not the corresponding properties. Tagging @jcouv to confirm.

[danstur]

Not listing the documentation for the properties in the xml doc is a pity

As far as I understand, this is how doc comments for records was decided to work. The param XML comments is tied only to the constructor parameter, not the corresponding properties. Tagging @jcouv to confirm.

Yes I understand that this is working as intended currently. I'm wondering if there would be a chance that this behavior might be changed in the future for the reasons listed and if so if external pull requests would be accepted.
It seems like a reasonable change with little risk involved, but obviously the bar for changes is pretty high here.

In any case thanks for #52737 , it's always great to get such quick feedback and improvement when reporting things to a project 👍

[Youssef1313]

I'm wondering if there would be a chance that this behavior might be changed in the future for the reasons listed and if so if external pull requests would be accepted.

Since I'm not working for Microsoft, I have no idea if this would be acceptable to the team.

[jcouv]

We'd intentionally taken a minimalistic approach for first pass (#44571 (comment)), so the feedback is important.

My understanding of the proposal: extract contents of <params> and apply them to auto-prop

I understand how that would be convenient, but what makes me hesitate:

1. You're explicitly documenting a parameter (with <param>) but now means something else (both parameter and property).
2. The wording for documenting a parameter may not be applicable to a property. Maybe that's not a big concern until we allow primary constructor bodies.
3. From an implementation perspective, I don't know how much manipulation of xml docs the compiler can or should do. The current behavior does not require special manipulation of xml, but the proposal means we'd extract some contents and use them in a different context.
4. Do references mean the same thing in those two different context? Is the meaning of <see ... or <seealso ... the same when applied to a primary constructor parameter or a property?
5. We have much less experience with xml docs behavior than core C# semantics, so the above questions are clouded with some uncertainty, and we worry about other unknowns....

I would hold off on a PR until we can agree on desired design.

Tagging @sharwell @AlekseyTs @RikkiGibson @BillWagner @jaredpar for thoughts

[CyrusNajmabadi]

I do think we need some sort of story here at the compiler-only layer. People need to doc these things, and they need those docs to appear in tools beyond the IDE.

That said, we can also likely just do a spot fix on the ide side so these show up.

[danstur]

I would hold off on a PR until we can agree on desired design.

Oh yes I was more thinking about whether creating a proposal would be worthwhile, not jumping head first into the implementation.

I could summarize the use cases of our internal tools as well as for things like Swashbuckle ASP.NET Core if some more detailed practical examples would be helpful. Otherwise I'll just wait and see what the experts in the field have to say.

(There's also an argument to be made that records are not necessarily the right choice for external API layers and that the workaround to explicitly specify properties might be acceptable anyhow since it also offers more flexibility and options.
Fixing the IDE behavior would definitely be the biggest win for us and would allow us to use records in most places just fine.)

[InspiringCode]

This issue is really preventing us from using records sometimes. I mean, we have to be able to document our properties somehow...

[jcouv]

From design discussion on 9/14/2021, we'll have the <param name="...">…ABC…</param> element also produce <summary>…ABC…</summary> documentation for the synthesized property (ie. not if the positional member was explicitly declared).

[buvinghausen]

Following...

[RikkiGibson]

Let's make sure we prioritize this in 17.1.

[VBAndCs]

I Expect that the xml comment will be added in the xml file for both params and properties. Is this correct?
We sometimes parse these XML docs in our code, and there are old tools that expect to read these comments related to properties not just params.

[RikkiGibson]

@VBAndCs In the currently shipped compiler, this doesn't happen, unfortunately. We intend to change this soon per #52663 (comment).

I see the number of thumbs up on the issue and I can see that this is a significant inconvenience for people, so I am going to make every effort to get this done soon.

[RikkiGibson]

This should ship in 17.2.

[IanKemp]

This should ship in 17.2.

Any idea when 17.2 will ship? As this fix went in beginning of Feb but it is now nearly end of March...

[RikkiGibson]

It's currently available in public preview. It will probably be a few more months before it releases as a stable version.

[MartinJohns]

@RikkiGibson Can you also say of what dotnet Version this fix will be part of?

[RikkiGibson]

That would be 6.0.300. See https://docs.microsoft.com/en-us/dotnet/core/porting/versioning-sdk-msbuild-vs.