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.

Sign up for GitHub

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

Jump to bottom

Make SA1600 trigger on undocumented record primary ctor parameters #3783

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Make SA1600 trigger on undocumented record primary ctor parameters #3783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
61b8a13
Make SA1600 trigger on undocumented record primary ctor parameters
SapiensAnatis Jan 22, 2024
703b1dd
Respond to review comments
SapiensAnatis Jan 29, 2024
ad82a37
Clean up tests
SapiensAnatis Jan 29, 2024
bcdc55b
Fix formatting
SapiensAnatis Jan 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions ....Analyzers/StyleCop.Analyzers.Test.CSharp11/DocumentationRules/SA1600CSharp11UnitTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,70 @@

namespace StyleCop.Analyzers.Test.CSharp11.DocumentationRules
{
using Microsoft.CodeAnalysis.Testing;
using StyleCop.Analyzers.Test.CSharp10.DocumentationRules;
using static StyleCop.Analyzers.Test.Verifiers.StyleCopDiagnosticVerifier<
StyleCop.Analyzers.DocumentationRules.SA1600ElementsMustBeDocumented>;

public partial class SA1600CSharp11UnitTests : SA1600CSharp10UnitTests
{
/* These methods reflect a fix for a Roslyn issue from C# 9 where diagnostics were reported twice. */

protected override DiagnosticResult[] GetExpectedResultTestRecordPrimaryConstructorNoParameterDocumentation()
{
return new[]
{
// /0/Test0.cs(5,28): warning SA1600: Elements should be documented
Diagnostic().WithLocation(0),

// /0/Test0.cs(5,43): warning SA1600: Elements should be documented
Diagnostic().WithLocation(1),
};
}

protected override DiagnosticResult[] GetExpectedResultTestRecordPrimaryConstructorNoDocumentation()
{
return new[]
{
// /0/Test0.cs(2,15): warning SA1600: Elements should be documented
Diagnostic().WithLocation(0),

// /0/Test0.cs(2,28): warning SA1600: Elements should be documented
Diagnostic().WithLocation(1),
};
}

protected override DiagnosticResult[] GetExpectedResultTestRecordPrimaryConstructorPartialParameterDocumentation()
{
return new[]
{
// /0/Test0.cs(6,43): warning SA1600: Elements should be documented
Diagnostic().WithLocation(0),
};
}

protected override DiagnosticResult[] GetExpectedResultTestRecordPrimaryConstructorIncludePartialParameterDocumentation()
{
return new[]
{
// /0/Test0.cs(3,28): warning SA1600: Elements should be documented
Diagnostic().WithLocation(0),
};
}

protected override DiagnosticResult[] GetExpectedResultTestRecordPrimaryConstructorIncludeMissingParameterDocumentation()
{
return new[]
{
// /0/Test0.cs(3,28): warning SA1600: Elements should be documented
Diagnostic().WithLocation(0),

// /0/Test0.cs(3,43): warning SA1600: Elements should be documented
Diagnostic().WithLocation(1),

// /0/Test0.cs(3,56): warning SA1600: Elements should be documented
Diagnostic().WithLocation(2),
};
}
}
}
251 changes: 251 additions & 0 deletions ...op.Analyzers/StyleCop.Analyzers.Test.CSharp9/DocumentationRules/SA1600CSharp9UnitTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,194 @@

namespace StyleCop.Analyzers.Test.CSharp9.DocumentationRules
{
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Testing;
using StyleCop.Analyzers.Test.CSharp8.DocumentationRules;
using StyleCop.Analyzers.Test.Helpers;
using Xunit;
using static StyleCop.Analyzers.Test.Verifiers.StyleCopDiagnosticVerifier<
StyleCop.Analyzers.DocumentationRules.SA1600ElementsMustBeDocumented>;

public partial class SA1600CSharp9UnitTests : SA1600CSharp8UnitTests
{
[Theory]
[MemberData(nameof(CommonMemberData.RecordTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))]
[WorkItem(3780, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3780")]
public async Task TestRecordPrimaryConstructorNoParameterDocumentationAsync(string keyword)
{
string testCode = $@"
/// <summary>
/// Record.
/// </summary>
public {keyword} MyRecord(int {{|#0:Param1|}}, string {{|#1:Param2|}});";

DiagnosticResult[] expectedResults = this.GetExpectedResultTestRecordPrimaryConstructorNoParameterDocumentation();

await VerifyCSharpDiagnosticAsync(testCode, expectedResults, CancellationToken.None).ConfigureAwait(false);
}

[Theory]
[MemberData(nameof(CommonMemberData.RecordTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))]
[WorkItem(3780, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3780")]
public async Task TestRecordPrimaryConstructorNoDocumentationAsync(string keyword)
{
string testCode = $@"
public {keyword} {{|#0:MyRecord|}}(int {{|#1:Param1|}});";

DiagnosticResult[] expectedResults = this.GetExpectedResultTestRecordPrimaryConstructorNoDocumentation();

await VerifyCSharpDiagnosticAsync(testCode, expectedResults, CancellationToken.None).ConfigureAwait(false);
}

[Theory]
[MemberData(nameof(CommonMemberData.RecordTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))]
[WorkItem(3780, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3780")]
public async Task TestRecordPrimaryConstructorCompleteParameterDocumentationAsync(string keyword)
{
string testCode = $@"
/// <summary>
/// Record.
/// </summary>
/// <param name=""Param1"">Parameter one.</param>
/// <param name=""Param2"">Parameter two.</param>
public {keyword} MyRecord(int Param1, string Param2);";

await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
}

[Theory]
[MemberData(nameof(CommonMemberData.RecordTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))]
[WorkItem(3780, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3780")]
public async Task TestRecordPrimaryConstructorPartialParameterDocumentationAsync(string keyword)
{
string testCode = $@"
/// <summary>
/// Record.
/// </summary>
/// <param name=""Param1"">Parameter one.</param>
public {keyword} MyRecord(int Param1, string {{|#0:Param2|}});";

DiagnosticResult[] expectedResults = this.GetExpectedResultTestRecordPrimaryConstructorPartialParameterDocumentation();

await VerifyCSharpDiagnosticAsync(testCode, expectedResults, CancellationToken.None).ConfigureAwait(false);
}

[Theory]
[MemberData(nameof(CommonMemberData.RecordTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))]
[WorkItem(3780, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3780")]
public async Task TestRecordPrimaryConstructorInheritdocAsync(string keyword)
{
string testCode = $@"
/// <inheritdoc />
public {keyword} MyRecord(int Param1, string Param2);";

await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
}

[Theory]
[MemberData(nameof(CommonMemberData.RecordTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))]
[WorkItem(3780, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3780")]
public async Task TestRecordPrimaryConstructorIncludeParameterDocumentationAsync(string keyword)
{
string testCode = $@"
/// <include file='WithParameterDocumentation.xml' path='/TestType/*' />
public {keyword} MyRecord(int Param1, string Param2, bool Param3);";

await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
}

[Theory]
[MemberData(nameof(CommonMemberData.RecordTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))]
[WorkItem(3780, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3780")]
public async Task TestRecordPrimaryConstructorIncludeInheritdocAsync(string keyword)
{
string testCode = $@"
/// <include file='WithInheritdoc.xml' path='/TestType/*' />
public {keyword} MyRecord(int Param1, string Param2, bool Param3);";

await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
}

[Theory]
[MemberData(nameof(CommonMemberData.RecordTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))]
[WorkItem(3780, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3780")]
public async Task TestRecordPrimaryConstructorIncludePartialParameterDocumentationAsync(string keyword)
{
string testCode = $@"
/// <include file='WithPartialParameterDocumentation.xml' path='/TestType/*' />
public {keyword} MyRecord(int {{|#0:Param1|}}, string Param2, bool Param3);";

DiagnosticResult[] expectedResults = this.GetExpectedResultTestRecordPrimaryConstructorIncludePartialParameterDocumentation();

await VerifyCSharpDiagnosticAsync(testCode, expectedResults, CancellationToken.None).ConfigureAwait(false);
}

[Theory]
[MemberData(nameof(CommonMemberData.RecordTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))]
[WorkItem(3780, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3780")]
public async Task TestRecordPrimaryConstructorIncludeMissingParameterDocumentationAsync(string keyword)
{
string testCode = $@"
/// <include file='MissingParameterDocumentation.xml' path='/TestType/*' />
public {keyword} MyRecord(int {{|#0:Param1|}}, string {{|#1:Param2|}}, bool {{|#2:Param3|}});";

DiagnosticResult[] expectedResults = this.GetExpectedResultTestRecordPrimaryConstructorIncludeMissingParameterDocumentation();

await VerifyCSharpDiagnosticAsync(testCode, expectedResults, CancellationToken.None).ConfigureAwait(false);
}

protected static Task VerifyCSharpDiagnosticAsync(string source, DiagnosticResult[] expected, CancellationToken cancellationToken)
{
string typeWithoutParameterDocumentation = @"<?xml version=""1.0"" encoding=""utf-8"" ?>
<TestType>
<summary>
Foo
</summary>
</TestType>
";
string typeWithPartialParameterDocumentation = @"<?xml version=""1.0"" encoding=""utf-8"" ?>
<TestType>
<summary>
Foo
</summary>
<param name=""Param2"">Param 2.</param>
<param name=""Param3"">Param 3.</param>
</TestType>
";
string typeWithParameterDocumentation = @"<?xml version=""1.0"" encoding=""utf-8"" ?>
<TestType>
<summary>
Foo
</summary>
<param name=""Param1"">Param 1.</param>
<param name=""Param2"">Param 2.</param>
<param name=""Param3"">Param 3.</param>
</TestType>
";
string typeWithIncludedInheritdoc = @"<?xml version=""1.0"" encoding=""utf-8"" ?>
<TestType>
<inheritdoc />
</TestType>
";

var test = new CSharpTest
{
TestCode = source,
XmlReferences =
{
{ "MissingParameterDocumentation.xml", typeWithoutParameterDocumentation },
{ "WithParameterDocumentation.xml", typeWithParameterDocumentation },
{ "WithPartialParameterDocumentation.xml", typeWithPartialParameterDocumentation },
{ "WithInheritdoc.xml", typeWithIncludedInheritdoc },
},
};

test.ExpectedDiagnostics.AddRange(expected);
return test.RunAsync(cancellationToken);
}

protected override DiagnosticResult[] GetExpectedResultTestRegressionMethodGlobalNamespace(string code)
{
if (code == "public void {|#0:TestMember|}() { }")
Expand All @@ -26,5 +209,73 @@ protected override DiagnosticResult[] GetExpectedResultTestRegressionMethodGloba

return base.GetExpectedResultTestRegressionMethodGlobalNamespace(code);
}

/* The below result overrides are due to a Roslyn bug causing diagnostics to be reported twice, which was fixed in a later version. */

protected virtual DiagnosticResult[] GetExpectedResultTestRecordPrimaryConstructorNoParameterDocumentation()
{
return new[]
{
// /0/Test0.cs(5,28): warning SA1600: Elements should be documented
Diagnostic().WithLocation(0),
Diagnostic().WithLocation(0),

// /0/Test0.cs(5,43): warning SA1600: Elements should be documented
Diagnostic().WithLocation(1),
Diagnostic().WithLocation(1),
};
}

protected virtual DiagnosticResult[] GetExpectedResultTestRecordPrimaryConstructorNoDocumentation()
{
return new[]
{
// /0/Test0.cs(2,15): warning SA1600: Elements should be documented
Diagnostic().WithLocation(0),
Diagnostic().WithLocation(0),

// /0/Test0.cs(2,28): warning SA1600: Elements should be documented
Diagnostic().WithLocation(1),
Diagnostic().WithLocation(1),
};
}

protected virtual DiagnosticResult[] GetExpectedResultTestRecordPrimaryConstructorPartialParameterDocumentation()
{
return new[]
{
// /0/Test0.cs(6,43): warning SA1600: Elements should be documented
Diagnostic().WithLocation(0),
Diagnostic().WithLocation(0),
};
}

protected virtual DiagnosticResult[] GetExpectedResultTestRecordPrimaryConstructorIncludePartialParameterDocumentation()
{
return new[]
{
// /0/Test0.cs(3,28): warning SA1600: Elements should be documented
Diagnostic().WithLocation(0),
Diagnostic().WithLocation(0),
};
}

protected virtual DiagnosticResult[] GetExpectedResultTestRecordPrimaryConstructorIncludeMissingParameterDocumentation()
{
return new[]
{
// /0/Test0.cs(3,28): warning SA1600: Elements should be documented
Diagnostic().WithLocation(0),
Diagnostic().WithLocation(0),

// /0/Test0.cs(3,43): warning SA1600: Elements should be documented
Diagnostic().WithLocation(1),
Diagnostic().WithLocation(1),

// /0/Test0.cs(3,56): warning SA1600: Elements should be documented
Diagnostic().WithLocation(2),
Diagnostic().WithLocation(2),
};
}
}
}
34 changes: 31 additions & 3 deletions StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/SA1600ElementsMustBeDocumented.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@
namespace StyleCop.Analyzers.DocumentationRules
{
using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using System.Reflection.Metadata;
using System.Xml.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
Expand Down Expand Up @@ -143,11 +147,35 @@ public static void HandleBaseTypeDeclaration(SyntaxNodeAnalysisContext context,

Accessibility declaredAccessibility = declaration.GetDeclaredAccessibility(context.SemanticModel, context.CancellationToken);
Accessibility effectiveAccessibility = declaration.GetEffectiveAccessibility(context.SemanticModel, context.CancellationToken);
if (NeedsComment(settings.DocumentationRules, declaration.Kind(), declaration.Parent.Kind(), declaredAccessibility, effectiveAccessibility))
if (!NeedsComment(settings.DocumentationRules, declaration.Kind(), declaration.Parent.Kind(), declaredAccessibility, effectiveAccessibility))
{
if (!XmlCommentHelper.HasDocumentation(declaration))
return;
}

XmlComment xmlComment = context.GetParsedXmlComment(context.Node);

if (xmlComment.IsMissing)
{
context.ReportDiagnostic(Diagnostic.Create(Descriptor, declaration.Identifier.GetLocation()));
}

if (xmlComment.HasInheritdoc)
{
return;
}

if (context.Node is TypeDeclarationSyntax typeDeclaration && RecordDeclarationSyntaxWrapper.IsInstance(typeDeclaration))
{
RecordDeclarationSyntaxWrapper recordDeclaration = (RecordDeclarationSyntaxWrapper)typeDeclaration;

IEnumerable<ParameterSyntax> parameters = recordDeclaration.ParameterList?.Parameters ?? Enumerable.Empty<ParameterSyntax>();

foreach (var parameter in parameters)
{
context.ReportDiagnostic(Diagnostic.Create(Descriptor, declaration.Identifier.GetLocation()));
if (!xmlComment.DocumentedParameterNames.Contains(parameter.Identifier.ValueText))
{
context.ReportDiagnostic(Diagnostic.Create(Descriptor, parameter.Identifier.GetLocation()));
}
}
}
}
Expand Down
Loading