Merge pull request #1233 from Noryoko/fix-1231

Fix SA1514 directive trivia handling
DotNetAnalyzers · Aug 17, 2015 · 047b5ab · 047b5ab
2 parents 72002b5 + d59d4a6
commit 047b5ab
Show file tree

Hide file tree

Showing 2 changed files with 128 additions and 14 deletions.
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/LayoutRules/SA1514UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/LayoutRules/SA1514UnitTests.cs
@@ -829,33 +829,143 @@ public void Method2()
 }
 ";
 
-            var fixedCode = @"
+            await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
+        /// <summary>
+        /// Verifies that preprocessor directives before a documentation header are properly handled.
+        /// This is regression for https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/1231
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Fact]
+        public async Task TestDirectivesPrecedingDocumentationAsync()
+        {
+            var testCode = @"
 public class TestClass
 {
-    #pragma warning disable RS1012
-
+#if true
     /// <summary>
     /// ...
     /// </summary>
     public void Method1()
     {
     }
+#else
+    /// <summary>
+    /// ...
+    /// </summary>
+    public void Method2()
+    {
+    }
+#endif
+    /// <summary>
+    /// ...
+    /// </summary>
+    public void Method3()
+    {
+    }
 
-    #pragma checksum ""test0.cs"" ""{00000000-0000-0000-0000-000000000000}"" ""{01234567}"" // comment
+#region SomeRegion
+    /// <summary>
+    /// ...
+    /// </summary>
+    public void Method4()
+    {
+    }
+#endregion SomeRegion
+    /// <summary>
+    /// ...
+    /// </summary>
+    public void Method5()
+    {
+    }
+
+#region AnotherRegion // comment
+    /// <summary>
+    /// ...
+    /// </summary>
+    public void Method6()
+    {
+    }
+#endregion AnotherRegion // comment
+    /// <summary>
+    /// ...
+    /// </summary>
+    public void Method7()
+    {
+    }
+}
+";
 
+            var fixedCode = @"
+public class TestClass
+{
+#if true
+    /// <summary>
+    /// ...
+    /// </summary>
+    public void Method1()
+    {
+    }
+#else
     /// <summary>
     /// ...
     /// </summary>
     public void Method2()
     {
     }
+#endif
+
+    /// <summary>
+    /// ...
+    /// </summary>
+    public void Method3()
+    {
+    }
+
+#region SomeRegion
+
+    /// <summary>
+    /// ...
+    /// </summary>
+    public void Method4()
+    {
+    }
+#endregion SomeRegion
+
+    /// <summary>
+    /// ...
+    /// </summary>
+    public void Method5()
+    {
+    }
+
+#region AnotherRegion // comment
+
+    /// <summary>
+    /// ...
+    /// </summary>
+    public void Method6()
+    {
+    }
+#endregion AnotherRegion // comment
+
+    /// <summary>
+    /// ...
+    /// </summary>
+    public void Method7()
+    {
+    }
 }
 ";
 
             DiagnosticResult[] expected =
             {
-                this.CSharpDiagnostic().WithLocation(5, 5),
-                this.CSharpDiagnostic().WithLocation(13, 5)
+                this.CSharpDiagnostic().WithLocation(19, 5),
+                this.CSharpDiagnostic().WithLocation(27, 5),
+                this.CSharpDiagnostic().WithLocation(34, 5),
+                this.CSharpDiagnostic().WithLocation(42, 5),
+                this.CSharpDiagnostic().WithLocation(49, 5)
             };
 
             await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);

diff --git a/...yleCop.Analyzers/LayoutRules/SA1514ElementDocumentationHeaderMustBePrecededByBlankLine.cs b/...yleCop.Analyzers/LayoutRules/SA1514ElementDocumentationHeaderMustBePrecededByBlankLine.cs
@@ -113,22 +113,26 @@ private static void HandleDeclaration(SyntaxNodeAnalysisContext context)
             var done = false;
             for (var i = documentationHeaderIndex - 1; !done && (i >= 0); i--)
             {
-                switch (triviaList[i].Kind())
+                var trivia = triviaList[i];
+                if (trivia.IsDirective
+                    && !trivia.IsKind(SyntaxKind.EndIfDirectiveTrivia)
+                    && !trivia.IsKind(SyntaxKind.RegionDirectiveTrivia)
+                    && !trivia.IsKind(SyntaxKind.EndRegionDirectiveTrivia))
+                {
+                    return;
+                }
+
+                switch (trivia.Kind())
                 {
                 case SyntaxKind.WhitespaceTrivia:
                     break;
                 case SyntaxKind.EndOfLineTrivia:
                     eolCount++;
                     break;
-                case SyntaxKind.IfDirectiveTrivia:
-                case SyntaxKind.ElseDirectiveTrivia:
-                case SyntaxKind.ElifDirectiveTrivia:
-                    // if the documentation header is inside a directive trivia, no leading blank line is needed
-                    return;
 
                 case SyntaxKind.EndIfDirectiveTrivia:
-                case SyntaxKind.PragmaWarningDirectiveTrivia:
-                case SyntaxKind.PragmaChecksumDirectiveTrivia:
+                case SyntaxKind.RegionDirectiveTrivia:
+                case SyntaxKind.EndRegionDirectiveTrivia:
                     eolCount++;
                     done = true;
                     break;