Merge pull request #3315 from sharwell/handle-null

Fix handling of <include> on fields
DotNetAnalyzers · Mar 10, 2021 · fdb3d67 · fdb3d67
2 parents 24755d2 + bd3c7b8
commit fdb3d67
Show file tree

Hide file tree

Showing 10 changed files with 206 additions and 18 deletions.
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1608UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1608UnitTests.cs
@@ -185,6 +185,18 @@ public class ClassName
             await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
         }
 
+        [Fact]
+        [WorkItem(3150, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3150")]
+        public async Task TestClassWithIncludedMissingDocumentationAsync()
+        {
+            var testCode = @"
+/// <include file='MissingFile.xml' path='/ClassName/*' />
+public class ClassName
+{
+}";
+            await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
         [Fact]
         public async Task TestClassWithIncludedSummaryDocumentationAsync()
         {
@@ -210,6 +222,35 @@ public class ClassName
             await VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
         }
 
+        [Fact]
+        [WorkItem(3150, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3150")]
+        public async Task TestFieldWithIncludedSummaryDocumentationAsync()
+        {
+            var testCode = @"
+public class ClassName
+{
+    /// <include file='FieldWithSummary.xml' path='/FieldName/*' />
+    public int FieldName;
+}";
+            await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
+        [Fact]
+        [WorkItem(3150, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3150")]
+        public async Task TestFieldWithIncludedDefaultSummaryDocumentationAsync()
+        {
+            var testCode = @"
+public class ClassName
+{
+    /// <include file='FieldWithDefaultSummary.xml' path='/FieldName/*' />
+    public {|#0:int FieldName|};
+}";
+
+            DiagnosticResult expected = Diagnostic().WithLocation(0);
+
+            await VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
+        }
+
         private static Task VerifyCSharpDiagnosticAsync(string source, DiagnosticResult expected, CancellationToken cancellationToken)
             => VerifyCSharpDiagnosticAsync(source, new[] { expected }, cancellationToken);
 
@@ -232,6 +273,20 @@ private static Task VerifyCSharpDiagnosticAsync(string source, DiagnosticResult[
     Summary description for the ClassName class.
   </summary>
 </ClassName>
+";
+            string fieldContentWithSummary = @"<?xml version=""1.0"" encoding=""utf-8"" ?>
+<FieldName>
+  <summary>
+    Foo
+  </summary>
+</FieldName>
+";
+            string fieldContentWithDefaultSummary = @"<?xml version=""1.0"" encoding=""utf-8"" ?>
+<FieldName>
+  <summary>
+    Summary description for the ClassName class.
+  </summary>
+</FieldName>
 ";
 
             var test = new StyleCopDiagnosticVerifier<SA1608ElementDocumentationMustNotHaveDefaultSummary>.CSharpTest
@@ -242,6 +297,8 @@ private static Task VerifyCSharpDiagnosticAsync(string source, DiagnosticResult[
                     { "ClassWithoutSummary.xml", contentWithoutSummary },
                     { "ClassWithSummary.xml", contentWithSummary },
                     { "ClassWithDefaultSummary.xml", contentWithDefaultSummary },
+                    { "FieldWithSummary.xml", fieldContentWithSummary },
+                    { "FieldWithDefaultSummary.xml", fieldContentWithDefaultSummary },
                 },
             };
 

diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1611UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1611UnitTests.cs
@@ -312,6 +312,36 @@ public void TestMethod(string param1, string param2, string param3)
             await VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
         }
 
+        /// <summary>
+        /// Verifies that included documentation with missing documentation file produces no diagnostics.
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Fact]
+        [WorkItem(3150, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3150")]
+        public async Task VerifyIncludedMissingDocumentationAsync()
+        {
+            var testCode = @"
+/// <summary>
+/// Foo
+/// </summary>
+public class ClassName
+{
+    /// <include file='MissingFile.xml' path='/TestClass/TestMethod/*' />
+    public void TestMethod(string {|#0:param1|}, string {|#1:param2|}, string {|#2:param3|})
+    {
+    }
+}";
+
+            DiagnosticResult[] expected =
+            {
+                Diagnostic().WithLocation(0).WithArguments("param1"),
+                Diagnostic().WithLocation(1).WithArguments("param2"),
+                Diagnostic().WithLocation(2).WithArguments("param3"),
+            };
+
+            await VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
+        }
+
         /// <summary>
         /// Verifies that included documentation with missing elements documented produces the expected diagnostics.
         /// </summary>

diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1612UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1612UnitTests.cs
@@ -285,6 +285,22 @@ public class ClassName
             await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
         }
 
+        [Fact]
+        [WorkItem(3150, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3150")]
+        public async Task VerifyIncludedMissingFileIsNotReportedAsync()
+        {
+            var testCode = @"
+/// <summary>
+/// Foo
+/// </summary>
+public class ClassName
+{
+    /// <include file='MissingFile.xml' path='/ClassName/Method/*' />
+    public ClassName Method() { return null; }
+}";
+            await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
         [Fact]
         public async Task VerifyIncludedMemberWithValidParamsIsNotReportedAsync()
         {

diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1613UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1613UnitTests.cs
@@ -154,6 +154,22 @@ public class ClassName
             await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
         }
 
+        [Fact]
+        [WorkItem(3150, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3150")]
+        public async Task VerifyIncludedMissingFileAsync()
+        {
+            var testCode = @"
+/// <summary>
+/// Foo
+/// </summary>
+public class ClassName
+{
+    /// <include file='MissingFile.xml' path='/ClassName/Method/*' />
+    public ClassName Method(string foo, string bar) { return null; }
+}";
+            await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
         [Fact]
         public async Task VerifyMemberWithValidParamsAndIncludedDocumentationAsync()
         {

diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1614UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1614UnitTests.cs
@@ -154,6 +154,22 @@ public class ClassName
             await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
         }
 
+        [Fact]
+        [WorkItem(3150, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3150")]
+        public async Task VerifyIncludedMissingFileAsync()
+        {
+            var testCode = @"
+/// <summary>
+/// Foo
+/// </summary>
+public class ClassName
+{
+    /// <include file='MissingFile.xml' path='/ClassName/Method/*' />
+    public ClassName Method(string foo, string bar) { return null; }
+}";
+            await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
         [Fact]
         public async Task VerifyMemberIncludedDocumentationWithoutParamsAsync()
         {

diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1616UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1616UnitTests.cs
@@ -443,6 +443,22 @@ public class ClassName
             await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
         }
 
+        [Fact]
+        [WorkItem(3150, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3150")]
+        public async Task VerifyMemberIncludedMissingFileAsync()
+        {
+            var testCode = @"
+/// <summary>
+/// Foo
+/// </summary>
+public class ClassName
+{
+    /// <include file='MissingFile.xml' path='/ClassName/Method/*' />
+    public ClassName Method(string foo, string bar) { return null; }
+}";
+            await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
         [Fact]
         public async Task VerifyMemberIncludedDocumentationWithoutReturnsAsync()
         {

diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1625UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1625UnitTests.cs
@@ -321,6 +321,21 @@ public class TestClass2 {{ }}
             await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
         }
 
+        [Fact]
+        [WorkItem(3150, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3150")]
+        public async Task VerifyThatMissingIncludedDocumentationDoesNotReportADiagnosticAsync()
+        {
+            var testCode = $@"
+public class TestClass
+{{
+    /// <include file='MissingFile.xml' path='/TestClass/Test/*' />
+    public void Test() {{ }}
+}}
+public class TestClass2 {{ }}
+";
+            await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
         [Fact]
         public async Task VerifyThatTheAnalyzerDoesNotCrashOnIncludedInheritDocAsync()
         {

diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1629UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1629UnitTests.cs
@@ -315,6 +315,20 @@ public class TestClass
             await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
         }
 
+        [Fact]
+        [WorkItem(3150, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3150")]
+        public async Task TestIncludedMissingFileAsync()
+        {
+            var testCode = @"
+/// <include file='MissingFile.xml' path='/TestClass/*'/>
+public class TestClass
+{
+}
+";
+
+            await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
         [Fact]
         [WorkItem(2680, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2680")]
         public async Task TestReportingAfterEmptyElementAsync()

diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/StyleCop.Analyzers.Test.csproj b/StyleCop.Analyzers/StyleCop.Analyzers.Test/StyleCop.Analyzers.Test.csproj
@@ -18,7 +18,7 @@
 
   <ItemGroup>
     <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="1.3.2" />
-    <PackageReference Include="Microsoft.CodeAnalysis.CSharp.CodeFix.Testing.XUnit" Version="1.0.1-beta1.20623.3" />
+    <PackageReference Include="Microsoft.CodeAnalysis.CSharp.CodeFix.Testing.XUnit" Version="1.0.1-beta1.21159.2" />
     <PackageReference Include="Microsoft.VisualStudio.Composition" Version="16.1.8" />
     <PackageReference Include="System.ValueTuple" Version="4.5.0" />
     <PackageReference Include="xunit" Version="2.4.1" />

diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/ElementDocumentationBase.cs b/StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/ElementDocumentationBase.cs
@@ -234,29 +234,37 @@ private void HandleDeclaration(SyntaxNodeAnalysisContext context, StyleCopSettin
 
             if (hasIncludedDocumentation)
             {
-                var declaration = context.SemanticModel.GetDeclaredSymbol(node, context.CancellationToken);
-                var rawDocumentation = declaration?.GetDocumentationCommentXml(expandIncludes: true, cancellationToken: context.CancellationToken);
-                var completeDocumentation = XElement.Parse(rawDocumentation, LoadOptions.None);
+                var declaration = node switch
+                {
+                    BaseFieldDeclarationSyntax baseFieldDeclaration => baseFieldDeclaration.Declaration.Variables.FirstOrDefault() ?? node,
+                    _ => node,
+                };
 
-                if (this.inheritDocSuppressesWarnings &&
-                    completeDocumentation.Nodes().OfType<XElement>().Any(element => element.Name == XmlCommentHelper.InheritdocXmlTag))
+                var declaredSymbol = context.SemanticModel.GetDeclaredSymbol(declaration, context.CancellationToken);
+                if (declaredSymbol is not null)
                 {
-                    // Ignore nodes with an <inheritdoc/> tag in the included XML.
+                    var rawDocumentation = declaredSymbol?.GetDocumentationCommentXml(expandIncludes: true, cancellationToken: context.CancellationToken);
+                    var completeDocumentation = XElement.Parse(rawDocumentation ?? "<doc></doc>", LoadOptions.None);
+
+                    if (this.inheritDocSuppressesWarnings &&
+                        completeDocumentation.Nodes().OfType<XElement>().Any(element => element.Name == XmlCommentHelper.InheritdocXmlTag))
+                    {
+                        // Ignore nodes with an <inheritdoc/> tag in the included XML.
+                        return;
+                    }
+
+                    this.HandleCompleteDocumentation(context, needsComment, completeDocumentation, locations);
                     return;
                 }
-
-                this.HandleCompleteDocumentation(context, needsComment, completeDocumentation, locations);
             }
-            else
-            {
-                IEnumerable<XmlNodeSyntax> matchingXmlElements = string.IsNullOrEmpty(this.matchElementName)
-                    ? documentation.Content
-                        .Where(x => x is XmlElementSyntax || x is XmlEmptyElementSyntax)
-                        .Where(x => !string.Equals(x.GetName()?.ToString(), XmlCommentHelper.IncludeXmlTag, StringComparison.Ordinal))
-                    : documentation.Content.GetXmlElements(this.matchElementName);
 
-                this.HandleXmlElement(context, settings, needsComment, matchingXmlElements, locations);
-            }
+            IEnumerable<XmlNodeSyntax> matchingXmlElements = string.IsNullOrEmpty(this.matchElementName)
+                ? documentation.Content
+                    .Where(x => x is XmlElementSyntax || x is XmlEmptyElementSyntax)
+                    .Where(x => !string.Equals(x.GetName()?.ToString(), XmlCommentHelper.IncludeXmlTag, StringComparison.Ordinal))
+                : documentation.Content.GetXmlElements(this.matchElementName);
+
+            this.HandleXmlElement(context, settings, needsComment, matchingXmlElements, locations);
         }
     }
 }