Merge pull request #1599 from vweijsters/fix-1595

Author: sharwell

StyleCop.Analyzers/StyleCop.Analyzers.Test/LayoutRules/SA1516UnitTests.cs

@@ -523,6 +523,26 @@ public event System.EventHandler FooProperty
             await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
         }
 
+        /// <summary>
+        /// Verifies that private fields with attributes are handled properly.
+        /// This is a regression test for #1595
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Fact]
+        public async Task VerifyThatPrivateFieldsAreHandledProperlyAsync()
+        {
+            string testCode = @"using System;
+
+public class TestClass
+{
+    [Obsolete]
+    private int test1;
+    private bool test2;
+}";
+
+            await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
         protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
         {
             yield return new SA1516ElementsMustBeSeparatedByBlankLine();

StyleCop.Analyzers/StyleCop.Analyzers/LayoutRules/SA1516ElementsMustBeSeparatedByBlankLine.cs

@@ -192,24 +192,46 @@ private static void HandleMemberList(SyntaxNodeAnalysisContext context, SyntaxLi
                 if (!members[i - 1].ContainsDiagnostics && !members[i].ContainsDiagnostics)
                 {
                     // Report if
-                    // the previous declaration spans across multiple lines
+                    // the current declaration is not a field declaration
                     // or the previous declaration is of different type
-                    // or the current declaration has documentation
-                    // or the current declaration is not a field declaration,
-                    if (IsMultiline(members[i - 1])
+                    // or the previous declaration spans across multiple lines
+                    //
+                    // Note that the order of checking is important, as the call to IsMultiLine requires a FieldDeclarationSyntax,
+                    // something that can only be guaranteed if the first two checks fail.
+                    if (!members[i].IsKind(SyntaxKind.FieldDeclaration)
                         || !members[i - 1].IsKind(members[i].Kind())
-                        || !members[i].IsKind(SyntaxKind.FieldDeclaration))
+                        || IsMultiline((FieldDeclarationSyntax)members[i - 1]))
                     {
                         ReportIfThereIsNoBlankLine(context, members[i - 1], members[i]);
                     }
                 }
             }
         }
 
-        private static bool IsMultiline(SyntaxNode node)
+        private static bool IsMultiline(FieldDeclarationSyntax fieldDeclaration)
         {
-            var lineSpan = node.GetLineSpan();
+            var lineSpan = fieldDeclaration.GetLineSpan();
+            var attributeLists = fieldDeclaration.AttributeLists;
 
+            int startLine;
+
+            // Exclude attributes when determining if a field declaration spans multiple lines
+            if (attributeLists.Count > 0)
+            {
+                var lastAttributeSpan = fieldDeclaration.SyntaxTree.GetLineSpan(attributeLists.Last().FullSpan);
+                startLine = lastAttributeSpan.EndLinePosition.Line;
+            }
+            else
+            {
+                startLine = lineSpan.StartLinePosition.Line;
+            }
+
+            return startLine != lineSpan.EndLinePosition.Line;
+        }
+
+        private static bool IsMultiline(AccessorDeclarationSyntax accessorDeclaration)
+        {
+            var lineSpan = accessorDeclaration.GetLineSpan();
             return lineSpan.StartLinePosition.Line != lineSpan.EndLinePosition.Line;
         }