Merge pull request #1653 from vweijsters/fix-1623

sharwell · sharwell · commit a6f0ea7b34f7 · 2015-10-23T17:11:49.000-05:00
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1114UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1114UnitTests.cs
@@ -1067,6 +1067,35 @@ private void SomeOtherMethod(int someParameter)
             await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
         }
 
+        /// <summary>
+        /// Verifies that directive trivia will not result in diagnostics.
+        /// This is a regression test for #1623
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Fact]
+        public async Task TestWithDirectiveTriviaAsync()
+        {
+            var testCode = @"
+public interface ITestInterface1 { }
+
+public interface ITestInterface2 { }
+
+public class TestClass
+{
+    public void TestMethod(
+#if TESTSYMBOL
+        ITestInterface1 instance)
+#else
+        ITestInterface2 instance)
+#endif
+    {
+    }
+}
+";
+
+            await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
         protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
         {
             yield return new SA1114ParameterListMustFollowDeclaration();
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1115UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1115UnitTests.cs
@@ -1099,6 +1099,36 @@ public class Foo
             await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
         }
 
+        /// <summary>
+        /// Verifies that directive trivia will not result in diagnostics.
+        /// This is a regression test for #1623
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Fact]
+        public async Task TestWithDirectiveTriviaAsync()
+        {
+            var testCode = @"
+public interface ITestInterface1 { }
+
+public interface ITestInterface2 { }
+
+public class TestClass
+{
+    public void TestMethod(
+        int parameter1,
+#if TESTSYMBOL
+        ITestInterface1 instance)
+#else
+        ITestInterface2 instance)
+#endif
+    {
+    }
+}
+";
+
+            await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
         protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
         {
             yield return new SA1115ParameterMustFollowComma();
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1114ParameterListMustFollowDeclaration.cs b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1114ParameterListMustFollowDeclaration.cs
@@ -4,6 +4,7 @@
 namespace StyleCop.Analyzers.ReadabilityRules
 {
     using System.Collections.Immutable;
+    using System.Linq;
     using Microsoft.CodeAnalysis;
     using Microsoft.CodeAnalysis.CSharp;
     using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -64,9 +65,8 @@ public override void Initialize(AnalysisContext context)
 
         private static void HandleCompilationStart(CompilationStartAnalysisContext context)
         {
-            context.RegisterSyntaxNodeActionHonorExclusions(HandleMethodDeclaration, SyntaxKind.MethodDeclaration);
+            context.RegisterSyntaxNodeActionHonorExclusions(HandleBaseMethodDeclaration, SyntaxKind.MethodDeclaration, SyntaxKind.ConstructorDeclaration, SyntaxKind.OperatorDeclaration);
             context.RegisterSyntaxNodeActionHonorExclusions(HandleMethodInvocation, SyntaxKind.InvocationExpression);
-            context.RegisterSyntaxNodeActionHonorExclusions(HandleConstructorDeclaration, SyntaxKind.ConstructorDeclaration);
             context.RegisterSyntaxNodeActionHonorExclusions(HandleObjectCreation, SyntaxKind.ObjectCreationExpression);
             context.RegisterSyntaxNodeActionHonorExclusions(HandleIndexerDeclaration, SyntaxKind.IndexerDeclaration);
             context.RegisterSyntaxNodeActionHonorExclusions(HandleArrayCreation, SyntaxKind.ArrayCreationExpression);
@@ -77,13 +77,6 @@ private static void HandleCompilationStart(CompilationStartAnalysisContext conte
             context.RegisterSyntaxNodeActionHonorExclusions(HandleAnonymousMethod, SyntaxKind.AnonymousMethodExpression);
             context.RegisterSyntaxNodeActionHonorExclusions(HandleLambdaExpression, SyntaxKind.ParenthesizedLambdaExpression);
             context.RegisterSyntaxNodeActionHonorExclusions(HandleConversionOperatorDeclaration, SyntaxKind.ConversionOperatorDeclaration);
-            context.RegisterSyntaxNodeActionHonorExclusions(HandleOperatorDeclaration, SyntaxKind.OperatorDeclaration);
-        }
-
-        private static void HandleOperatorDeclaration(SyntaxNodeAnalysisContext context)
-        {
-            var operatorDeclaration = (OperatorDeclarationSyntax)context.Node;
-            AnalyzeParametersList(context, operatorDeclaration.ParameterList);
         }
 
         private static void HandleConversionOperatorDeclaration(SyntaxNodeAnalysisContext context)
@@ -155,21 +148,15 @@ private static void HandleObjectCreation(SyntaxNodeAnalysisContext context)
             }
         }
 
-        private static void HandleConstructorDeclaration(SyntaxNodeAnalysisContext context)
-        {
-            var constructorDeclaration = (ConstructorDeclarationSyntax)context.Node;
-            AnalyzeParametersList(context, constructorDeclaration.ParameterList);
-        }
-
         private static void HandleMethodInvocation(SyntaxNodeAnalysisContext context)
         {
             var invocationExpression = (InvocationExpressionSyntax)context.Node;
             AnalyzeArgumentList(context, invocationExpression.ArgumentList);
         }
 
-        private static void HandleMethodDeclaration(SyntaxNodeAnalysisContext context)
+        private static void HandleBaseMethodDeclaration(SyntaxNodeAnalysisContext context)
         {
-            var methodDeclaration = (MethodDeclarationSyntax)context.Node;
+            var methodDeclaration = (BaseMethodDeclarationSyntax)context.Node;
 
             AnalyzeParametersList(context, methodDeclaration.ParameterList);
         }
@@ -391,24 +378,40 @@ private static void AnalyzeParametersList(SyntaxNodeAnalysisContext context, Par
             }
 
             var firstParameter = parameterListSyntax.Parameters[0];
+            int firstParameterLine;
 
-            var firstParameterLineSpan = firstParameter.GetLineSpan();
-            if (!firstParameterLineSpan.IsValid)
+            if (firstParameter.HasLeadingTrivia && firstParameter.GetLeadingTrivia().All(trivia => IsValidTrivia(trivia)))
             {
-                return;
+                firstParameterLine = firstParameter.SyntaxTree.GetLineSpan(firstParameter.FullSpan).StartLinePosition.Line;
             }
-
-            var openParenLineSpan = parameterListSyntax.OpenParenToken.GetLineSpan();
-            if (!openParenLineSpan.IsValid)
+            else
             {
-                return;
+                firstParameterLine = firstParameter.GetLineSpan().StartLinePosition.Line;
             }
 
-            if (openParenLineSpan.EndLinePosition.Line != firstParameterLineSpan.StartLinePosition.Line &&
-                openParenLineSpan.EndLinePosition.Line != (firstParameterLineSpan.StartLinePosition.Line - 1))
+            var parenLine = parameterListSyntax.OpenParenToken.GetLineSpan().EndLinePosition.Line;
+
+            if ((firstParameterLine - parenLine) > 1)
             {
                 context.ReportDiagnostic(Diagnostic.Create(Descriptor, firstParameter.GetLocation()));
             }
         }
+
+        private static bool IsValidTrivia(SyntaxTrivia trivia)
+        {
+            switch (trivia.Kind())
+            {
+            case SyntaxKind.IfDirectiveTrivia:
+            case SyntaxKind.ElseDirectiveTrivia:
+            case SyntaxKind.ElifDirectiveTrivia:
+            case SyntaxKind.EndIfDirectiveTrivia:
+            case SyntaxKind.DisabledTextTrivia:
+            case SyntaxKind.WhitespaceTrivia:
+                return true;
+
+            default:
+                return false;
+            }
+        }
     }
 }
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1115ParameterMustFollowComma.cs b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1115ParameterMustFollowComma.cs
@@ -4,6 +4,7 @@
 namespace StyleCop.Analyzers.ReadabilityRules
 {
     using System.Collections.Immutable;
+    using System.Linq;
     using Microsoft.CodeAnalysis;
     using Microsoft.CodeAnalysis.CSharp;
     using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -269,18 +270,44 @@ private static void AnalyzeParameterList(SyntaxNodeAnalysisContext context, Base
                 return;
             }
 
-            var previousLine = parameterListSyntax.Parameters[0].GetLineSpan().EndLinePosition.Line;
+            var previousParameterLine = parameterListSyntax.Parameters[0].GetLineSpan().EndLinePosition.Line;
             for (int i = 1; i < parameterListSyntax.Parameters.Count; i++)
             {
                 var currentParameter = parameterListSyntax.Parameters[i];
-                var lineSpan = currentParameter.GetLineSpan();
-                var currentLine = lineSpan.StartLinePosition.Line;
-                if (currentLine - previousLine > 1)
+                int currentParameterLine;
+
+                if (currentParameter.HasLeadingTrivia && currentParameter.GetLeadingTrivia().All(trivia => IsValidTrivia(trivia)))
+                {
+                    currentParameterLine = currentParameter.SyntaxTree.GetLineSpan(currentParameter.FullSpan).StartLinePosition.Line;
+                }
+                else
+                {
+                    currentParameterLine = currentParameter.GetLineSpan().StartLinePosition.Line;
+                }
+
+                if (currentParameterLine - previousParameterLine > 1)
                 {
                     context.ReportDiagnostic(Diagnostic.Create(Descriptor, currentParameter.GetLocation()));
                 }
 
-                previousLine = lineSpan.EndLinePosition.Line;
+                previousParameterLine = currentParameterLine;
+            }
+        }
+
+        private static bool IsValidTrivia(SyntaxTrivia trivia)
+        {
+            switch (trivia.Kind())
+            {
+            case SyntaxKind.IfDirectiveTrivia:
+            case SyntaxKind.ElseDirectiveTrivia:
+            case SyntaxKind.ElifDirectiveTrivia:
+            case SyntaxKind.EndIfDirectiveTrivia:
+            case SyntaxKind.DisabledTextTrivia:
+            case SyntaxKind.WhitespaceTrivia:
+                return true;
+
+            default:
+                return false;
             }
         }
     }

Original file line number	Diff line number	Diff line change
`@@ -4,6 +4,7 @@`
`4`	`4`	`namespace StyleCop.Analyzers.ReadabilityRules`
`5`	`5`	`{`
`6`	`6`	`using System.Collections.Immutable;`
	`7`	`+ using System.Linq;`
`7`	`8`	`using Microsoft.CodeAnalysis;`
`8`	`9`	`using Microsoft.CodeAnalysis.CSharp;`
`9`	`10`	`using Microsoft.CodeAnalysis.CSharp.Syntax;`
`@@ -64,9 +65,8 @@ public override void Initialize(AnalysisContext context)`
`64`	`65`
`65`	`66`	`private static void HandleCompilationStart(CompilationStartAnalysisContext context)`
`66`	`67`	`{`
`67`		`- context.RegisterSyntaxNodeActionHonorExclusions(HandleMethodDeclaration, SyntaxKind.MethodDeclaration);`
	`68`	`+ context.RegisterSyntaxNodeActionHonorExclusions(HandleBaseMethodDeclaration, SyntaxKind.MethodDeclaration, SyntaxKind.ConstructorDeclaration, SyntaxKind.OperatorDeclaration);`
`68`	`69`	`context.RegisterSyntaxNodeActionHonorExclusions(HandleMethodInvocation, SyntaxKind.InvocationExpression);`
`69`		`- context.RegisterSyntaxNodeActionHonorExclusions(HandleConstructorDeclaration, SyntaxKind.ConstructorDeclaration);`
`70`	`70`	`context.RegisterSyntaxNodeActionHonorExclusions(HandleObjectCreation, SyntaxKind.ObjectCreationExpression);`
`71`	`71`	`context.RegisterSyntaxNodeActionHonorExclusions(HandleIndexerDeclaration, SyntaxKind.IndexerDeclaration);`
`72`	`72`	`context.RegisterSyntaxNodeActionHonorExclusions(HandleArrayCreation, SyntaxKind.ArrayCreationExpression);`
`@@ -77,13 +77,6 @@ private static void HandleCompilationStart(CompilationStartAnalysisContext conte`
`77`	`77`	`context.RegisterSyntaxNodeActionHonorExclusions(HandleAnonymousMethod, SyntaxKind.AnonymousMethodExpression);`
`78`	`78`	`context.RegisterSyntaxNodeActionHonorExclusions(HandleLambdaExpression, SyntaxKind.ParenthesizedLambdaExpression);`
`79`	`79`	`context.RegisterSyntaxNodeActionHonorExclusions(HandleConversionOperatorDeclaration, SyntaxKind.ConversionOperatorDeclaration);`
`80`		`- context.RegisterSyntaxNodeActionHonorExclusions(HandleOperatorDeclaration, SyntaxKind.OperatorDeclaration);`
`81`		`- }`
`82`		`-`
`83`		`- private static void HandleOperatorDeclaration(SyntaxNodeAnalysisContext context)`
`84`		`- {`
`85`		`- var operatorDeclaration = (OperatorDeclarationSyntax)context.Node;`
`86`		`- AnalyzeParametersList(context, operatorDeclaration.ParameterList);`
`87`	`80`	`}`
`88`	`81`
`89`	`82`	`private static void HandleConversionOperatorDeclaration(SyntaxNodeAnalysisContext context)`
`@@ -155,21 +148,15 @@ private static void HandleObjectCreation(SyntaxNodeAnalysisContext context)`
`155`	`148`	`}`
`156`	`149`	`}`
`157`	`150`
`158`		`- private static void HandleConstructorDeclaration(SyntaxNodeAnalysisContext context)`
`159`		`- {`
`160`		`- var constructorDeclaration = (ConstructorDeclarationSyntax)context.Node;`
`161`		`- AnalyzeParametersList(context, constructorDeclaration.ParameterList);`
`162`		`- }`
`163`		`-`
`164`	`151`	`private static void HandleMethodInvocation(SyntaxNodeAnalysisContext context)`
`165`	`152`	`{`
`166`	`153`	`var invocationExpression = (InvocationExpressionSyntax)context.Node;`
`167`	`154`	`AnalyzeArgumentList(context, invocationExpression.ArgumentList);`
`168`	`155`	`}`
`169`	`156`
`170`		`- private static void HandleMethodDeclaration(SyntaxNodeAnalysisContext context)`
	`157`	`+ private static void HandleBaseMethodDeclaration(SyntaxNodeAnalysisContext context)`
`171`	`158`	`{`
`172`		`- var methodDeclaration = (MethodDeclarationSyntax)context.Node;`
	`159`	`+ var methodDeclaration = (BaseMethodDeclarationSyntax)context.Node;`
`173`	`160`
`174`	`161`	`AnalyzeParametersList(context, methodDeclaration.ParameterList);`
`175`	`162`	`}`
`@@ -391,24 +378,40 @@ private static void AnalyzeParametersList(SyntaxNodeAnalysisContext context, Par`
`391`	`378`	`}`
`392`	`379`
`393`	`380`	`var firstParameter = parameterListSyntax.Parameters[0];`
	`381`	`+ int firstParameterLine;`
`394`	`382`
`395`		`- var firstParameterLineSpan = firstParameter.GetLineSpan();`
`396`		`- if (!firstParameterLineSpan.IsValid)`
	`383`	`+ if (firstParameter.HasLeadingTrivia && firstParameter.GetLeadingTrivia().All(trivia => IsValidTrivia(trivia)))`
`397`	`384`	`{`
`398`		`- return;`
	`385`	`+ firstParameterLine = firstParameter.SyntaxTree.GetLineSpan(firstParameter.FullSpan).StartLinePosition.Line;`
`399`	`386`	`}`
`400`		`-`
`401`		`- var openParenLineSpan = parameterListSyntax.OpenParenToken.GetLineSpan();`
`402`		`- if (!openParenLineSpan.IsValid)`
	`387`	`+ else`
`403`	`388`	`{`
`404`		`- return;`
	`389`	`+ firstParameterLine = firstParameter.GetLineSpan().StartLinePosition.Line;`
`405`	`390`	`}`
`406`	`391`
`407`		`- if (openParenLineSpan.EndLinePosition.Line != firstParameterLineSpan.StartLinePosition.Line &&`
`408`		`- openParenLineSpan.EndLinePosition.Line != (firstParameterLineSpan.StartLinePosition.Line - 1))`
	`392`	`+ var parenLine = parameterListSyntax.OpenParenToken.GetLineSpan().EndLinePosition.Line;`
	`393`	`+`
	`394`	`+ if ((firstParameterLine - parenLine) > 1)`
`409`	`395`	`{`
`410`	`396`	`context.ReportDiagnostic(Diagnostic.Create(Descriptor, firstParameter.GetLocation()));`
`411`	`397`	`}`
`412`	`398`	`}`
	`399`	`+`
	`400`	`+ private static bool IsValidTrivia(SyntaxTrivia trivia)`
	`401`	`+ {`
	`402`	`+ switch (trivia.Kind())`
	`403`	`+ {`
	`404`	`+ case SyntaxKind.IfDirectiveTrivia:`
	`405`	`+ case SyntaxKind.ElseDirectiveTrivia:`
	`406`	`+ case SyntaxKind.ElifDirectiveTrivia:`
	`407`	`+ case SyntaxKind.EndIfDirectiveTrivia:`
	`408`	`+ case SyntaxKind.DisabledTextTrivia:`
	`409`	`+ case SyntaxKind.WhitespaceTrivia:`
	`410`	`+ return true;`
	`411`	`+`
	`412`	`+ default:`
	`413`	`+ return false;`
	`414`	`+ }`
	`415`	`+ }`
`413`	`416`	`}`
`414`	`417`	`}`