Skip to content

Commit 4a5cccc

Browse files
nbarbettininbarbettini
committed
Update SA1613 to handle included documentation (fixes #1906)
1 parent 025a3fd commit 4a5cccc

5 files changed

Lines changed: 227 additions & 34 deletions

File tree

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

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ namespace StyleCop.Analyzers.Test.DocumentationRules
77
using System.Threading;
88
using System.Threading.Tasks;
99
using Analyzers.DocumentationRules;
10+
using Helpers;
11+
using Microsoft.CodeAnalysis;
1012
using Microsoft.CodeAnalysis.Diagnostics;
1113
using TestHelper;
1214
using Xunit;
@@ -21,6 +23,7 @@ public static IEnumerable<object[]> Declarations
2123
get
2224
{
2325
yield return new[] { " public ClassName Method(string foo, string bar) { return null; }" };
26+
yield return new[] { " public delegate ClassName Method(string foo, string bar);" };
2427
yield return new[] { " public ClassName this[string foo, string bar] { get { return null; } set { } }" };
2528
}
2629
}
@@ -137,6 +140,146 @@ public class ClassName
137140
await this.VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), expected, CancellationToken.None).ConfigureAwait(false);
138141
}
139142

143+
[Fact]
144+
public async Task VerifyMemberWithoutParamsAndIncludedDocumentationAsync()
145+
{
146+
var testCode = @"
147+
/// <summary>
148+
/// Foo
149+
/// </summary>
150+
public class ClassName
151+
{
152+
/// <include file='MissingParamDocumentation.xml' path='/ClassName/Method/*' />
153+
public ClassName Method(string foo, string bar) { return null; }
154+
}";
155+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
156+
}
157+
158+
[Fact]
159+
public async Task VerifyMemberWithValidParamsAndIncludedDocumentationAsync()
160+
{
161+
var testCode = @"
162+
/// <summary>
163+
/// Foo
164+
/// </summary>
165+
public class ClassName
166+
{
167+
/// <include file='WithParamDocumentation.xml' path='/ClassName/Method/*' />
168+
public ClassName Method(string foo, string bar) { return null; }
169+
}";
170+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
171+
}
172+
173+
[Fact]
174+
public async Task VerifyMemberWithInvalidParamsAndIncludedDocumentationAsync()
175+
{
176+
var testCode = @"
177+
/// <summary>
178+
/// Foo
179+
/// </summary>
180+
public class ClassName
181+
{
182+
/// <include file='WithInvalidParamDocumentation.xml' path='/ClassName/Method/*' />
183+
public ClassName Method(string foo, string bar) { return null; }
184+
}";
185+
186+
var expected = new[]
187+
{
188+
this.CSharpDiagnostic().WithLocation(8, 22),
189+
this.CSharpDiagnostic().WithLocation(8, 22),
190+
this.CSharpDiagnostic().WithLocation(8, 22),
191+
this.CSharpDiagnostic().WithLocation(8, 22)
192+
};
193+
194+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
195+
}
196+
197+
[Fact]
198+
public async Task VerifyMemberWithInvalidParamsAndInheritDocAndIncludedDocumentationAsync()
199+
{
200+
var testCode = @"
201+
/// <summary>
202+
/// Foo
203+
/// </summary>
204+
public class ClassName
205+
{
206+
/// <include file='WithInheritedDocumentation.xml' path='/ClassName/Method/*' />
207+
public ClassName Method(string foo, string bar) { return null; }
208+
}";
209+
210+
var expected = new[]
211+
{
212+
this.CSharpDiagnostic().WithLocation(8, 22),
213+
this.CSharpDiagnostic().WithLocation(8, 22),
214+
this.CSharpDiagnostic().WithLocation(8, 22),
215+
this.CSharpDiagnostic().WithLocation(8, 22)
216+
};
217+
218+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
219+
}
220+
221+
/// <inheritdoc/>
222+
protected override Project CreateProject(string[] sources, string language = "C#", string[] filenames = null)
223+
{
224+
var resolver = new TestXmlReferenceResolver();
225+
226+
string contentWithoutParamDocumentation = @"<?xml version=""1.0"" encoding=""utf-8"" ?>
227+
<ClassName>
228+
<Method>
229+
<summary>
230+
Foo
231+
</summary>
232+
</Method>
233+
</ClassName>
234+
";
235+
resolver.XmlReferences.Add("MissingParamDocumentation.xml", contentWithoutParamDocumentation);
236+
237+
string contentWithParamDocumentation = @"<?xml version=""1.0"" encoding=""utf-8"" ?>
238+
<ClassName>
239+
<Method>
240+
<summary>
241+
Foo
242+
</summary>
243+
<param name=""foo"">Param 1</param>
244+
<param name=""bar"">Param 2</param>
245+
</Method>
246+
</ClassName>
247+
";
248+
resolver.XmlReferences.Add("WithParamDocumentation.xml", contentWithParamDocumentation);
249+
250+
string contentWithInvalidParamDocumentation = @"<?xml version=""1.0"" encoding=""utf-8"" ?>
251+
<ClassName>
252+
<Method>
253+
<summary>
254+
Foo
255+
</summary>
256+
<param>Test</param>
257+
<param/>
258+
<param name="""">Test</param>
259+
<param name="" "">Test</param>
260+
</Method>
261+
</ClassName>
262+
";
263+
resolver.XmlReferences.Add("WithInvalidParamDocumentation.xml", contentWithInvalidParamDocumentation);
264+
265+
string contentWithInheritedDocumentation = @"<?xml version=""1.0"" encoding=""utf-8"" ?>
266+
<ClassName>
267+
<Method>
268+
<inheritdoc />
269+
<param>Test</param>
270+
<param/>
271+
<param name="""">Test</param>
272+
<param name="" "">Test</param>
273+
</Method>
274+
</ClassName>
275+
";
276+
resolver.XmlReferences.Add("WithInheritedDocumentation.xml", contentWithInheritedDocumentation);
277+
278+
Project project = base.CreateProject(sources, language, filenames);
279+
project = project.WithCompilationOptions(project.CompilationOptions.WithXmlReferenceResolver(resolver));
280+
return project;
281+
}
282+
140283
protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
141284
{
142285
yield return new SA1613ElementParameterDocumentationMustDeclareParameterName();

‎StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/ElementDocumentationParameterBase.cs‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ namespace StyleCop.Analyzers.DocumentationRules
1818
/// </summary>
1919
internal abstract class ElementDocumentationParameterBase : DiagnosticAnalyzer
2020
{
21+
private readonly bool inheritDocSuppressesWarnings;
22+
2123
private readonly Action<CompilationStartAnalysisContext> compilationStartAction;
2224
private readonly Action<SyntaxNodeAnalysisContext> methodDeclarationAction;
2325
private readonly Action<SyntaxNodeAnalysisContext> constructorDeclarationAction;
@@ -26,8 +28,10 @@ internal abstract class ElementDocumentationParameterBase : DiagnosticAnalyzer
2628
private readonly Action<SyntaxNodeAnalysisContext> operatorDeclarationAction;
2729
private readonly Action<SyntaxNodeAnalysisContext> conversionOperatorDeclarationAction;
2830

29-
protected ElementDocumentationParameterBase()
31+
protected ElementDocumentationParameterBase(bool inheritDocSuppressesWarnings)
3032
{
33+
this.inheritDocSuppressesWarnings = inheritDocSuppressesWarnings;
34+
3135
this.compilationStartAction = this.HandleCompilationStart;
3236
this.methodDeclarationAction = this.HandleMethodDeclaration;
3337
this.constructorDeclarationAction = this.HandleConstructorDeclaration;
@@ -137,7 +141,8 @@ private void HandleDeclaration(SyntaxNodeAnalysisContext context, SyntaxNode nod
137141
return;
138142
}
139143

140-
if (documentation.Content.GetFirstXmlElement(XmlCommentHelper.InheritdocXmlTag) != null)
144+
if (this.inheritDocSuppressesWarnings
145+
&& documentation.Content.GetFirstXmlElement(XmlCommentHelper.InheritdocXmlTag) != null)
141146
{
142147
// Ignore nodes with an <inheritdoc/> tag.
143148
return;
@@ -153,7 +158,9 @@ private void HandleDeclaration(SyntaxNodeAnalysisContext context, SyntaxNode nod
153158
var declaration = context.SemanticModel.GetDeclaredSymbol(node, context.CancellationToken);
154159
var rawDocumentation = declaration?.GetDocumentationCommentXml(expandIncludes: true, cancellationToken: context.CancellationToken);
155160
completeDocumentation = XElement.Parse(rawDocumentation, LoadOptions.None);
156-
if (completeDocumentation.Nodes().OfType<XElement>().Any(element => element.Name == XmlCommentHelper.InheritdocXmlTag))
161+
162+
if (this.inheritDocSuppressesWarnings &&
163+
completeDocumentation.Nodes().OfType<XElement>().Any(element => element.Name == XmlCommentHelper.InheritdocXmlTag))
157164
{
158165
// Ignore nodes with an <inheritdoc/> tag in the included XML.
159166
return;

‎StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/SA1611ElementParametersMustBeDocumented.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@ internal class SA1611ElementParametersMustBeDocumented : ElementDocumentationPar
4343
private static readonly DiagnosticDescriptor Descriptor =
4444
new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.DocumentationRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink);
4545

46+
public SA1611ElementParametersMustBeDocumented()
47+
: base(inheritDocSuppressesWarnings: true)
48+
{
49+
}
50+
4651
/// <inheritdoc/>
4752
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } =
4853
ImmutableArray.Create(Descriptor);

‎StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/SA1612ElementParameterDocumentationMustMatchElementParameters.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ internal class SA1612ElementParameterDocumentationMustMatchElementParameters : E
4848
private static readonly DiagnosticDescriptor OrderDescriptor =
4949
new DiagnosticDescriptor(DiagnosticId, Title, ParamWrongOrderMessageFormat, AnalyzerCategory.DocumentationRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink);
5050

51+
public SA1612ElementParameterDocumentationMustMatchElementParameters()
52+
: base(inheritDocSuppressesWarnings: true)
53+
{
54+
}
55+
5156
/// <inheritdoc/>
5257
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } =
5358
ImmutableArray.Create(MissingParameterDescriptor);

‎StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/SA1613ElementParameterDocumentationMustDeclareParameterName.cs‎

Lines changed: 64 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@
44
namespace StyleCop.Analyzers.DocumentationRules
55
{
66
using System;
7+
using System.Collections.Generic;
78
using System.Collections.Immutable;
9+
using System.Linq;
10+
using System.Xml.Linq;
811
using Helpers;
912
using Microsoft.CodeAnalysis;
10-
using Microsoft.CodeAnalysis.CSharp;
1113
using Microsoft.CodeAnalysis.CSharp.Syntax;
1214
using Microsoft.CodeAnalysis.Diagnostics;
1315

@@ -26,7 +28,7 @@ namespace StyleCop.Analyzers.DocumentationRules
2628
/// which is missing a <c>name</c> attribute, or which contains an empty <c>name</c> attribute.</para>
2729
/// </remarks>
2830
[DiagnosticAnalyzer(LanguageNames.CSharp)]
29-
internal class SA1613ElementParameterDocumentationMustDeclareParameterName : DiagnosticAnalyzer
31+
internal class SA1613ElementParameterDocumentationMustDeclareParameterName : ElementDocumentationParameterBase
3032
{
3133
/// <summary>
3234
/// The ID for diagnostics produced by the
@@ -41,55 +43,86 @@ internal class SA1613ElementParameterDocumentationMustDeclareParameterName : Dia
4143
private static readonly DiagnosticDescriptor Descriptor =
4244
new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.DocumentationRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink);
4345

44-
private static readonly Action<CompilationStartAnalysisContext> CompilationStartAction = HandleCompilationStart;
45-
private static readonly Action<SyntaxNodeAnalysisContext> XmlElementAction = HandleXmlElement;
46-
private static readonly Action<SyntaxNodeAnalysisContext> XmlEmptyElementAction = HandleXmlEmptyElement;
46+
/// <summary>
47+
/// Initializes a new instance of the <see cref="SA1613ElementParameterDocumentationMustDeclareParameterName"/> class.
48+
/// </summary>
49+
/// <remarks>The presence of a &lt;inheritdoc/&gt; tag should NOT suppress warnings from this diagnostic. See DotNetAnalyzers/StyleCopAnalyzers#631</remarks>
50+
public SA1613ElementParameterDocumentationMustDeclareParameterName()
51+
: base(inheritDocSuppressesWarnings: false)
52+
{
53+
}
4754

4855
/// <inheritdoc/>
4956
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } =
5057
ImmutableArray.Create(Descriptor);
5158

5259
/// <inheritdoc/>
53-
public override void Initialize(AnalysisContext context)
60+
protected override void HandleXmlElement(SyntaxNodeAnalysisContext context, IEnumerable<XmlNodeSyntax> syntaxList, XElement completeDocumentation, params Location[] diagnosticLocations)
5461
{
55-
context.RegisterCompilationStartAction(CompilationStartAction);
56-
}
62+
bool includeElementPresent = completeDocumentation != null;
63+
if (includeElementPresent)
64+
{
65+
var xmlParameterNames = completeDocumentation.Nodes()
66+
.OfType<XElement>()
67+
.Where(e => e.Name == XmlCommentHelper.ParamXmlTag)
68+
.Select(x =>
69+
{
70+
var name = x.Attributes().FirstOrDefault(a => a.Name == "name")?.Value;
5771

58-
private static void HandleCompilationStart(CompilationStartAnalysisContext context)
59-
{
60-
context.RegisterSyntaxNodeActionHonorExclusions(XmlElementAction, SyntaxKind.XmlElement);
61-
context.RegisterSyntaxNodeActionHonorExclusions(XmlEmptyElementAction, SyntaxKind.XmlEmptyElement);
62-
}
72+
return new Tuple<string, Location>(name, null);
73+
})
74+
.ToImmutableArray();
6375

64-
private static void HandleXmlElement(SyntaxNodeAnalysisContext context)
65-
{
66-
XmlElementSyntax emptyElement = context.Node as XmlElementSyntax;
76+
VerifyParameters(context, xmlParameterNames, diagnosticLocations.First());
77+
}
78+
else if (syntaxList != null)
79+
{
80+
var xmlParameterNames = syntaxList
81+
.Where(x => string.Equals(GetName(x)?.ToString(), XmlCommentHelper.ParamXmlTag))
82+
.Select(x =>
83+
{
84+
var nameAttribute = XmlCommentHelper.GetFirstAttributeOrDefault<XmlNameAttributeSyntax>(x);
85+
var location = x.GetLocation();
86+
87+
if (nameAttribute != null)
88+
{
89+
location = nameAttribute.GetLocation();
90+
}
6791

68-
var name = emptyElement?.StartTag?.Name;
92+
return new Tuple<string, Location>(nameAttribute?.Identifier?.Identifier.ValueText, location);
93+
})
94+
.ToImmutableArray();
6995

70-
HandleElement(context, emptyElement, name, emptyElement?.StartTag?.GetLocation());
96+
VerifyParameters(context, xmlParameterNames, diagnosticLocations.First());
97+
}
7198
}
7299

73-
private static void HandleXmlEmptyElement(SyntaxNodeAnalysisContext context)
100+
private static void VerifyParameters(SyntaxNodeAnalysisContext context, ImmutableArray<Tuple<string, Location>> documentationParameters, Location identifierLocation)
74101
{
75-
XmlEmptyElementSyntax emptyElement = context.Node as XmlEmptyElementSyntax;
102+
var index = 0;
76103

77-
var name = emptyElement?.Name;
104+
foreach (var documentedParameter in documentationParameters)
105+
{
106+
if (string.IsNullOrWhiteSpace(documentedParameter.Item1))
107+
{
108+
context.ReportDiagnostic(Diagnostic.Create(Descriptor, documentedParameter.Item2 ?? identifierLocation));
109+
}
78110

79-
HandleElement(context, emptyElement, name, emptyElement?.GetLocation());
111+
index++;
112+
}
80113
}
81114

82-
private static void HandleElement(SyntaxNodeAnalysisContext context, XmlNodeSyntax element, XmlNameSyntax name, Location alternativeDiagnosticLocation)
115+
private static XmlNameSyntax GetName(XmlNodeSyntax element)
83116
{
84-
if (string.Equals(name.ToString(), XmlCommentHelper.ParamXmlTag))
85-
{
86-
var nameAttribute = XmlCommentHelper.GetFirstAttributeOrDefault<XmlNameAttributeSyntax>(element);
117+
return (element as XmlElementSyntax)?.StartTag?.Name
118+
?? (element as XmlEmptyElementSyntax)?.Name;
119+
}
87120

88-
if (string.IsNullOrWhiteSpace(nameAttribute?.Identifier?.Identifier.ValueText))
89-
{
90-
context.ReportDiagnostic(Diagnostic.Create(Descriptor, nameAttribute?.GetLocation() ?? alternativeDiagnosticLocation));
91-
}
92-
}
121+
private static SyntaxToken GetIdentifier(SyntaxNode node)
122+
{
123+
return (node as MethodDeclarationSyntax)?.Identifier
124+
?? (node as IndexerDeclarationSyntax)?.ThisKeyword
125+
?? (node as DelegateDeclarationSyntax).Identifier;
93126
}
94127
}
95128
}

0 commit comments

Comments
 (0)