diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/LayoutRules/SA1516UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/LayoutRules/SA1516UnitTests.cs index 39e9222af..697ae6776 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/LayoutRules/SA1516UnitTests.cs +++ b/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); } + /// + /// Verifies that private fields with attributes are handled properly. + /// This is a regression test for #1595 + /// + /// A representing the asynchronous unit test. + [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 GetCSharpDiagnosticAnalyzers() { yield return new SA1516ElementsMustBeSeparatedByBlankLine(); diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/LayoutRules/SA1516ElementsMustBeSeparatedByBlankLine.cs b/StyleCop.Analyzers/StyleCop.Analyzers/LayoutRules/SA1516ElementsMustBeSeparatedByBlankLine.cs index 7e09674a5..dc11a4894 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/LayoutRules/SA1516ElementsMustBeSeparatedByBlankLine.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/LayoutRules/SA1516ElementsMustBeSeparatedByBlankLine.cs @@ -192,13 +192,15 @@ 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]); } @@ -206,10 +208,30 @@ private static void HandleMemberList(SyntaxNodeAnalysisContext context, SyntaxLi } } - 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; }