Skip to content

Commit 1403715

Browse files
sharwellsharwell
committed
Update SA1605 to suppress messages when documentation for an element is optional
Fixes #2450
1 parent 362d6ba commit 1403715

4 files changed

Lines changed: 69 additions & 11 deletions

File tree

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1605UnitTests.cs‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,18 @@ namespace StyleCop.Analyzers.Test.DocumentationRules
1818
/// </summary>
1919
public class SA1605UnitTests : DiagnosticVerifier
2020
{
21+
private const string TestSettings = @"
22+
{
23+
""settings"": {
24+
""documentationRules"": {
25+
""documentPrivateElements"": true
26+
}
27+
}
28+
}
29+
";
30+
31+
private string currentTestSettings = TestSettings;
32+
2133
[Theory]
2234
[InlineData("class")]
2335
[InlineData("struct")]
@@ -224,6 +236,32 @@ public void Test() { }
224236
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
225237
}
226238

239+
[Fact]
240+
[WorkItem(2450, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2450")]
241+
public async Task TestIncludedNotRequiredDocumentationWithoutSummaryAsync()
242+
{
243+
var testCode = @"
244+
/// <include file='ClassWithoutSummary.xml' path='/ClassName/*'/>
245+
public partial class ClassName
246+
{
247+
///
248+
public void Test() { }
249+
}";
250+
251+
// The situation is allowed if 'documentExposedElements' false
252+
this.currentTestSettings = @"
253+
{
254+
""settings"": {
255+
""documentationRules"": {
256+
""documentExposedElements"": false
257+
}
258+
}
259+
}
260+
";
261+
262+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
263+
}
264+
227265
[Fact]
228266
public async Task TestIncludedDocumentationWithInheritdocAsync()
229267
{
@@ -307,6 +345,11 @@ protected override Project ApplyCompilationOptions(Project project)
307345
return project;
308346
}
309347

348+
protected override string GetSettings()
349+
{
350+
return this.currentTestSettings ?? base.GetSettings();
351+
}
352+
310353
protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
311354
{
312355
yield return new SA1605PartialElementDocumentationMustHaveSummary();

‎StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/PartialElementDocumentationSummaryBase.cs‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ namespace StyleCop.Analyzers.DocumentationRules
1313
using Microsoft.CodeAnalysis.CSharp.Syntax;
1414
using Microsoft.CodeAnalysis.Diagnostics;
1515
using StyleCop.Analyzers.Helpers;
16+
using StyleCop.Analyzers.Settings.ObjectModel;
1617

1718
/// <summary>
1819
/// This is the base class for analyzers which examine the <c>&lt;summary&gt;</c> or <c>&lt;content&gt;</c> text of
@@ -23,8 +24,8 @@ internal abstract class PartialElementDocumentationSummaryBase : DiagnosticAnaly
2324
private static readonly XElement EmptyElement = new XElement("empty");
2425

2526
private readonly Action<CompilationStartAnalysisContext> compilationStartAction;
26-
private readonly Action<SyntaxNodeAnalysisContext> typeDeclarationAction;
27-
private readonly Action<SyntaxNodeAnalysisContext> methodDeclarationAction;
27+
private readonly Action<SyntaxNodeAnalysisContext, StyleCopSettings> typeDeclarationAction;
28+
private readonly Action<SyntaxNodeAnalysisContext, StyleCopSettings> methodDeclarationAction;
2829

2930
protected PartialElementDocumentationSummaryBase()
3031
{
@@ -46,14 +47,16 @@ public override void Initialize(AnalysisContext context)
4647
/// Analyzes the top-level <c>&lt;summary&gt;</c> or <c>&lt;content&gt;</c> element of a documentation comment.
4748
/// </summary>
4849
/// <param name="context">The current analysis context.</param>
50+
/// <param name="needsComment"><see langword="true"/> if the current documentation settings indicate that the
51+
/// element should be documented; otherwise, <see langword="false"/>.</param>
4952
/// <param name="syntax">The <see cref="XmlElementSyntax"/> or <see cref="XmlEmptyElementSyntax"/> of the node
5053
/// to examine.</param>
5154
/// <param name="completeDocumentation">The complete documentation for the declared symbol, with any
5255
/// <c>&lt;include&gt;</c> elements expanded. If the XML documentation comment included a <c>&lt;summary&gt;</c>
5356
/// element, this value will be <see langword="null"/>, even if the XML documentation comment also included an
5457
/// <c>&lt;include&gt;</c> element.</param>
5558
/// <param name="diagnosticLocations">The location(s) where diagnostics, if any, should be reported.</param>
56-
protected abstract void HandleXmlElement(SyntaxNodeAnalysisContext context, XmlNodeSyntax syntax, XElement completeDocumentation, params Location[] diagnosticLocations);
59+
protected abstract void HandleXmlElement(SyntaxNodeAnalysisContext context, bool needsComment, XmlNodeSyntax syntax, XElement completeDocumentation, params Location[] diagnosticLocations);
5760

5861
private static bool IsPartialMethodDefinition(SyntaxNode node)
5962
{
@@ -68,7 +71,7 @@ private static bool IsPartialMethodDefinition(SyntaxNode node)
6871
&& (methodDeclaration.Body == null);
6972
}
7073

71-
private void HandleTypeDeclaration(SyntaxNodeAnalysisContext context)
74+
private void HandleTypeDeclaration(SyntaxNodeAnalysisContext context, StyleCopSettings settings)
7275
{
7376
// We handle TypeDeclarationSyntax instead of BaseTypeDeclarationSyntax because enums are not allowed to be
7477
// partial.
@@ -84,10 +87,13 @@ private void HandleTypeDeclaration(SyntaxNodeAnalysisContext context)
8487
return;
8588
}
8689

87-
this.HandleDeclaration(context, node, node.Identifier.GetLocation());
90+
Accessibility declaredAccessibility = node.GetDeclaredAccessibility(context.SemanticModel, context.CancellationToken);
91+
Accessibility effectiveAccessibility = node.GetEffectiveAccessibility(context.SemanticModel, context.CancellationToken);
92+
bool needsComment = SA1600ElementsMustBeDocumented.NeedsComment(settings.DocumentationRules, node.Kind(), node.Parent.Kind(), declaredAccessibility, effectiveAccessibility);
93+
this.HandleDeclaration(context, needsComment, node, node.Identifier.GetLocation());
8894
}
8995

90-
private void HandleMethodDeclaration(SyntaxNodeAnalysisContext context)
96+
private void HandleMethodDeclaration(SyntaxNodeAnalysisContext context, StyleCopSettings settings)
9197
{
9298
var node = (MethodDeclarationSyntax)context.Node;
9399
if (node.Identifier.IsMissing)
@@ -101,10 +107,13 @@ private void HandleMethodDeclaration(SyntaxNodeAnalysisContext context)
101107
return;
102108
}
103109

104-
this.HandleDeclaration(context, node, node.Identifier.GetLocation());
110+
Accessibility declaredAccessibility = node.GetDeclaredAccessibility(context.SemanticModel, context.CancellationToken);
111+
Accessibility effectiveAccessibility = node.GetEffectiveAccessibility(context.SemanticModel, context.CancellationToken);
112+
bool needsComment = SA1600ElementsMustBeDocumented.NeedsComment(settings.DocumentationRules, node.Kind(), node.Parent.Kind(), declaredAccessibility, effectiveAccessibility);
113+
this.HandleDeclaration(context, needsComment, node, node.Identifier.GetLocation());
105114
}
106115

107-
private void HandleDeclaration(SyntaxNodeAnalysisContext context, SyntaxNode node, params Location[] locations)
116+
private void HandleDeclaration(SyntaxNodeAnalysisContext context, bool needsComment, SyntaxNode node, params Location[] locations)
108117
{
109118
var documentation = node.GetDocumentationCommentTriviaSyntax();
110119
if (documentation == null)
@@ -153,7 +162,7 @@ private void HandleDeclaration(SyntaxNodeAnalysisContext context, SyntaxNode nod
153162
}
154163
}
155164

156-
this.HandleXmlElement(context, relevantXmlElement, completeDocumentation, locations);
165+
this.HandleXmlElement(context, needsComment, relevantXmlElement, completeDocumentation, locations);
157166
}
158167

159168
private string ExpandDocumentation(Compilation compilation, DocumentationCommentTriviaSyntax documentCommentTrivia, XmlNodeSyntax includeTag)

‎StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/SA1605PartialElementDocumentationMustHaveSummary.cs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,14 @@ internal class SA1605PartialElementDocumentationMustHaveSummary : PartialElement
8888
ImmutableArray.Create(Descriptor);
8989

9090
/// <inheritdoc/>
91-
protected override void HandleXmlElement(SyntaxNodeAnalysisContext context, XmlNodeSyntax syntax, XElement completeDocumentation, Location[] diagnosticLocations)
91+
protected override void HandleXmlElement(SyntaxNodeAnalysisContext context, bool needsComment, XmlNodeSyntax syntax, XElement completeDocumentation, Location[] diagnosticLocations)
9292
{
93+
if (!needsComment)
94+
{
95+
// A missing summary is allowed for this element.
96+
return;
97+
}
98+
9399
if (completeDocumentation != null)
94100
{
95101
var hasSummaryTag = completeDocumentation.Nodes().OfType<XElement>().Any(element => element.Name == XmlCommentHelper.SummaryXmlTag);

‎StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/SA1607PartialElementDocumentationMustHaveSummaryText.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ internal class SA1607PartialElementDocumentationMustHaveSummaryText : PartialEle
8989
ImmutableArray.Create(Descriptor);
9090

9191
/// <inheritdoc/>
92-
protected override void HandleXmlElement(SyntaxNodeAnalysisContext context, XmlNodeSyntax syntax, XElement completeDocumentation, Location[] diagnosticLocations)
92+
protected override void HandleXmlElement(SyntaxNodeAnalysisContext context, bool needsComment, XmlNodeSyntax syntax, XElement completeDocumentation, Location[] diagnosticLocations)
9393
{
9494
if (completeDocumentation != null)
9595
{

0 commit comments

Comments
 (0)