Skip to content
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

CS1591 incorrectly reported for records defined using the short syntax #47144

Closed
WilStead opened this issue Aug 26, 2020 · 3 comments
Closed
Labels
Area-Compilers Concept-Design Debt Engineering Debt, Design Debt, or poor product code quality Feature - Records Records
Milestone

Comments

@WilStead
Copy link

Version Used: Version 16.8.0 Preview 2.0

Steps to Reproduce:

  1. Enable XML documentation
<PropertyGroup>
  <GenerateDocumentationFile>true</GenerateDocumentationFile>
</PropertyGroup>
  1. (Taken directly from the ASP.NET Core .NET 5 Preview 8 blog post)
public record Person([Required] string Name, [Range(0, 150)] int Age);

Expected Behavior: No warnings

Actual Behavior:
CS1591 reported for both parameters (properties?).

This might actually be a valid warning, except that there is no mechanism to add such documentation. For example:

public record Person
(
    /// <summary>
    /// The person's name.
    /// </summary>
    [Required] string Name
);

The above produces CS1587 instead: XML comment is not placed on a valid language element.

I am also unaware of any tag which might be added to the record itself to document the parameters. For example:

/// <summary>
/// A person.
/// </summary>
/// <property cref="Name">
/// The person's name.
/// </property>
public record Person([Required] string Name);

This hypothetical syntax is invalid (produces CS1574 and fails to remove CS1591), but I could imagine something like it being used to document the auto-generated properties in a way that satisfies CS1591.

@RikkiGibson
Copy link
Contributor

This could require a language design decision: is a doc comment directly on the parameter valid, or is a new kind of doc comment on the record declaration required. FYI @jcouv @333fred

When I tried adopting records in a personal project it felt natural to me to put the doc comment directly on the parameter, since I was basically refactoring auto-property declarations into record parameters.

@sharwell sharwell added Concept-Design Debt Engineering Debt, Design Debt, or poor product code quality and removed Bug labels Aug 26, 2020
@jcouv jcouv added this to the 16.8 milestone Aug 26, 2020
@jcouv
Copy link
Member

jcouv commented Aug 27, 2020

xml doc work for records is tracked in #44571
That includes a summary of the plan (we'll allow <param> on the record type to carry over to the primary constructor parameters).

@333fred
Copy link
Member

333fred commented Aug 27, 2020

Duplicate of #44571. I'll add a note to that issue to make sure we consider this warning, but we should have 1 issue tracking this.

@333fred 333fred closed this as completed Aug 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Concept-Design Debt Engineering Debt, Design Debt, or poor product code quality Feature - Records Records
Projects
None yet
Development

No branches or pull requests

5 participants