Skip to content

Commit 578d7f7

Browse files
nbarbettininbarbettini
committed
Update SA1616 to handle included documentation (fixes #1908)
1 parent 44683a8 commit 578d7f7

3 files changed

Lines changed: 227 additions & 30 deletions

File tree

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/DocumentationRules/SA1615SA1616CodeFixProvider.cs‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,13 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
7575
DocumentationCommentTriviaSyntax documentationComment =
7676
methodDeclarationSyntax?.GetDocumentationCommentTriviaSyntax()
7777
?? delegateDeclarationSyntax?.GetDocumentationCommentTriviaSyntax();
78-
if (documentationComment == null)
78+
bool canIgnoreDocumentation =
79+
documentationComment == null
80+
|| documentationComment.Content
81+
.Where(x => x is XmlElementSyntax || x is XmlEmptyElementSyntax)
82+
.All(x => string.Equals(GetName(x)?.ToString(), XmlCommentHelper.IncludeXmlTag));
83+
84+
if (canIgnoreDocumentation)
7985
{
8086
return document;
8187
}
@@ -224,6 +230,12 @@ private static bool IsAsynchronousTestMethod(SemanticModel semanticModel, Method
224230
return false;
225231
}
226232

233+
private static XmlNameSyntax GetName(XmlNodeSyntax element)
234+
{
235+
return (element as XmlElementSyntax)?.StartTag?.Name
236+
?? (element as XmlEmptyElementSyntax)?.Name;
237+
}
238+
227239
private static SyntaxTrivia GetLastDocumentationCommentExteriorTrivia(SyntaxNode node)
228240
{
229241
return node

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

Lines changed: 189 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.CodeFixes;
1113
using Microsoft.CodeAnalysis.Diagnostics;
1214
using TestHelper;
@@ -449,6 +451,193 @@ internal sealed class ##Attribute : System.Attribute { }
449451
await this.VerifyCSharpFixAsync(testCode.Replace("$$", declaration).Replace("##", testAttribute), fixedCode.Replace("$$", declaration).Replace("##", testAttribute), cancellationToken: CancellationToken.None).ConfigureAwait(false);
450452
}
451453

454+
[Fact]
455+
public async Task VerifyMemberIncludedDocumentationNoDocumentationAsync()
456+
{
457+
var testCode = @"
458+
/// <summary>
459+
/// Foo
460+
/// </summary>
461+
public class ClassName
462+
{
463+
/// <include file='NoDocumentation.xml' path='/ClassName/Method/*' />
464+
public ClassName Method(string foo, string bar) { return null; }
465+
}";
466+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
467+
}
468+
469+
[Fact]
470+
public async Task VerifyMemberIncludedDocumentationWithoutReturnsAsync()
471+
{
472+
var testCode = @"
473+
/// <summary>
474+
/// Foo
475+
/// </summary>
476+
public class ClassName
477+
{
478+
/// <include file='WithoutReturns.xml' path='/ClassName/Method/*' />
479+
public ClassName Method(string foo, string bar) { return null; }
480+
}";
481+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
482+
}
483+
484+
[Fact]
485+
public async Task VerifyMemberIncludedDocumentationWithValidReturnsAsync()
486+
{
487+
var testCode = @"
488+
/// <summary>
489+
/// Foo
490+
/// </summary>
491+
public class ClassName
492+
{
493+
/// <include file='WithReturns.xml' path='/ClassName/Method/*' />
494+
public ClassName Method(string foo, string bar) { return null; }
495+
}";
496+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
497+
}
498+
499+
[Fact]
500+
public async Task VerifyMemberIncludedDocumentationWithEmptyReturnsAsync()
501+
{
502+
var testCode = @"
503+
/// <summary>
504+
/// Foo
505+
/// </summary>
506+
public class ClassName
507+
{
508+
/// <include file='WithEmptyReturns.xml' path='/ClassName/Method/*' />
509+
public ClassName Method(string foo, string bar) { return null; }
510+
}";
511+
512+
var expected = this.CSharpDiagnostic().WithLocation(8, 22);
513+
514+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
515+
516+
// The code fix does not alter this case.
517+
await this.VerifyCSharpFixAsync(testCode, testCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
518+
}
519+
520+
[Fact]
521+
public async Task VerifyMemberIncludedDocumentationWithEmptyReturns2Async()
522+
{
523+
var testCode = @"
524+
/// <summary>
525+
/// Foo
526+
/// </summary>
527+
public class ClassName
528+
{
529+
/// <include file='WithEmptyReturns2.xml' path='/ClassName/Method/*' />
530+
public ClassName Method(string foo, string bar) { return null; }
531+
}";
532+
533+
var expected = this.CSharpDiagnostic().WithLocation(8, 22);
534+
535+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
536+
537+
// The code fix does not alter this case.
538+
await this.VerifyCSharpFixAsync(testCode, testCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
539+
}
540+
541+
[Fact]
542+
public async Task VerifyMemberIncludedDocumentationWithEmptyReturns3Async()
543+
{
544+
var testCode = @"
545+
/// <summary>
546+
/// Foo
547+
/// </summary>
548+
public class ClassName
549+
{
550+
/// <include file='WithEmptyReturns3.xml' path='/ClassName/Method/*' />
551+
public ClassName Method(string foo, string bar) { return null; }
552+
}";
553+
554+
var expected = this.CSharpDiagnostic().WithLocation(8, 22);
555+
556+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
557+
558+
// The code fix does not alter this case.
559+
await this.VerifyCSharpFixAsync(testCode, testCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
560+
}
561+
562+
/// <inheritdoc/>
563+
protected override Project ApplyCompilationOptions(Project project)
564+
{
565+
var resolver = new TestXmlReferenceResolver();
566+
567+
string contentWithoutDocumentation = @"<?xml version=""1.0"" encoding=""utf-8"" ?>
568+
<ClassName>
569+
<Method>
570+
</Method>
571+
</ClassName>
572+
";
573+
resolver.XmlReferences.Add("NoDocumentation.xml", contentWithoutDocumentation);
574+
575+
string contentWithoutReturns = @"<?xml version=""1.0"" encoding=""utf-8"" ?>
576+
<ClassName>
577+
<Method>
578+
<summary>
579+
Foo
580+
</summary>
581+
</Method>
582+
</ClassName>
583+
";
584+
resolver.XmlReferences.Add("WithoutReturns.xml", contentWithoutReturns);
585+
586+
string contentWithReturns = @"<?xml version=""1.0"" encoding=""utf-8"" ?>
587+
<ClassName>
588+
<Method>
589+
<summary>
590+
Foo
591+
</summary>
592+
<returns>Test</returns>
593+
</Method>
594+
</ClassName>
595+
";
596+
resolver.XmlReferences.Add("WithReturns.xml", contentWithReturns);
597+
598+
string contentWithEmptyReturns = @"<?xml version=""1.0"" encoding=""utf-8"" ?>
599+
<ClassName>
600+
<Method>
601+
<summary>
602+
Foo
603+
</summary>
604+
<returns>
605+
606+
</returns>
607+
</Method>
608+
</ClassName>
609+
";
610+
resolver.XmlReferences.Add("WithEmptyReturns.xml", contentWithEmptyReturns);
611+
612+
string contentWithEmptyReturns2 = @"<?xml version=""1.0"" encoding=""utf-8"" ?>
613+
<ClassName>
614+
<Method>
615+
<summary>
616+
Foo
617+
</summary>
618+
<returns />
619+
</Method>
620+
</ClassName>
621+
";
622+
resolver.XmlReferences.Add("WithEmptyReturns2.xml", contentWithEmptyReturns2);
623+
624+
string contentWithEmptyReturns3 = @"<?xml version=""1.0"" encoding=""utf-8"" ?>
625+
<ClassName>
626+
<Method>
627+
<summary>
628+
Foo
629+
</summary>
630+
<returns></returns>
631+
</Method>
632+
</ClassName>
633+
";
634+
resolver.XmlReferences.Add("WithEmptyReturns3.xml", contentWithEmptyReturns3);
635+
636+
project = base.ApplyCompilationOptions(project);
637+
project = project.WithCompilationOptions(project.CompilationOptions.WithXmlReferenceResolver(resolver));
638+
return project;
639+
}
640+
452641
protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
453642
{
454643
yield return new SA1616ElementReturnValueDocumentationMustHaveText();

‎StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/SA1616ElementReturnValueDocumentationMustHaveText.cs‎

Lines changed: 25 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@
33

44
namespace StyleCop.Analyzers.DocumentationRules
55
{
6-
using System;
6+
using System.Collections.Generic;
77
using System.Collections.Immutable;
8+
using System.Linq;
9+
using System.Xml.Linq;
810
using Helpers;
911
using Microsoft.CodeAnalysis;
10-
using Microsoft.CodeAnalysis.CSharp;
1112
using Microsoft.CodeAnalysis.CSharp.Syntax;
1213
using Microsoft.CodeAnalysis.Diagnostics;
1314

@@ -25,7 +26,7 @@ namespace StyleCop.Analyzers.DocumentationRules
2526
/// </remarks>
2627
[DiagnosticAnalyzer(LanguageNames.CSharp)]
2728
[NoCodeFix("Cannot generate documentation")]
28-
internal class SA1616ElementReturnValueDocumentationMustHaveText : DiagnosticAnalyzer
29+
internal class SA1616ElementReturnValueDocumentationMustHaveText : ElementDocumentationBase
2930
{
3031
/// <summary>
3132
/// The ID for diagnostics produced by the <see cref="SA1616ElementReturnValueDocumentationMustHaveText"/>
@@ -40,46 +41,41 @@ internal class SA1616ElementReturnValueDocumentationMustHaveText : DiagnosticAna
4041
private static readonly DiagnosticDescriptor Descriptor =
4142
new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.DocumentationRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink);
4243

43-
private static readonly Action<CompilationStartAnalysisContext> CompilationStartAction = HandleCompilationStart;
44-
private static readonly Action<SyntaxNodeAnalysisContext> XmlElementAction = HandleXmlElement;
45-
private static readonly Action<SyntaxNodeAnalysisContext> XmlEmptyElementAction = HandleXmlEmptyElement;
44+
public SA1616ElementReturnValueDocumentationMustHaveText()
45+
: base(matchElementName: XmlCommentHelper.ReturnsXmlTag, inheritDocSuppressesWarnings: true)
46+
{
47+
}
4648

4749
/// <inheritdoc/>
4850
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } =
4951
ImmutableArray.Create(Descriptor);
5052

5153
/// <inheritdoc/>
52-
public override void Initialize(AnalysisContext context)
53-
{
54-
context.RegisterCompilationStartAction(CompilationStartAction);
55-
}
56-
57-
private static void HandleCompilationStart(CompilationStartAnalysisContext context)
54+
protected override void HandleXmlElement(SyntaxNodeAnalysisContext context, IEnumerable<XmlNodeSyntax> syntaxList, params Location[] diagnosticLocations)
5855
{
59-
context.RegisterSyntaxNodeActionHonorExclusions(XmlElementAction, SyntaxKind.XmlElement);
60-
context.RegisterSyntaxNodeActionHonorExclusions(XmlEmptyElementAction, SyntaxKind.XmlEmptyElement);
61-
}
62-
63-
private static void HandleXmlElement(SyntaxNodeAnalysisContext context)
64-
{
65-
XmlElementSyntax emptyElement = context.Node as XmlElementSyntax;
66-
67-
var name = emptyElement?.StartTag?.Name;
68-
69-
if (string.Equals(name.ToString(), XmlCommentHelper.ReturnsXmlTag) && XmlCommentHelper.IsConsideredEmpty(emptyElement))
56+
foreach (var syntax in syntaxList)
7057
{
71-
context.ReportDiagnostic(Diagnostic.Create(Descriptor, emptyElement.GetLocation()));
58+
bool isEmpty = syntax is XmlEmptyElementSyntax || XmlCommentHelper.IsConsideredEmpty(syntax);
59+
if (isEmpty)
60+
{
61+
context.ReportDiagnostic(Diagnostic.Create(Descriptor, syntax.GetLocation()));
62+
}
7263
}
7364
}
7465

75-
private static void HandleXmlEmptyElement(SyntaxNodeAnalysisContext context)
66+
/// <inheritdoc/>
67+
protected override void HandleCompleteDocumentation(SyntaxNodeAnalysisContext context, XElement completeDocumentation, params Location[] diagnosticLocations)
7668
{
77-
XmlEmptyElementSyntax emptyElement = context.Node as XmlEmptyElementSyntax;
69+
var returnsNodes = completeDocumentation.Nodes()
70+
.OfType<XElement>()
71+
.Where(n => n.Name == XmlCommentHelper.ReturnsXmlTag);
7872

79-
if (string.Equals(emptyElement?.Name.ToString(), XmlCommentHelper.ReturnsXmlTag))
73+
foreach (var node in returnsNodes)
8074
{
81-
// <returns .../> is empty.
82-
context.ReportDiagnostic(Diagnostic.Create(Descriptor, emptyElement.GetLocation()));
75+
if (XmlCommentHelper.IsConsideredEmpty(node))
76+
{
77+
context.ReportDiagnostic(Diagnostic.Create(Descriptor, diagnosticLocations.First()));
78+
}
8379
}
8480
}
8581
}

0 commit comments

Comments
 (0)