Merge pull request #1981 from vweijsters/fix-1921

SA1115 will now allow comments between parameters

Author: sharwell

StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1115UnitTests.cs

@@ -1154,6 +1154,96 @@ public void TestMethod(
             await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
         }
 
+        /// <summary>
+        /// Verifies that comment lines will not reported a diagnostic.
+        /// This is a regression test for #1921.
+        /// </summary>
+        /// <param name="commentText">The comment test to use for the test.</param>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Theory]
+        [InlineData("/* comment */")]
+        [InlineData("// comment")]
+        [InlineData("//// comment")]
+        public async Task TestWithCommentLinesAsync(string commentText)
+        {
+            var testCode = $@"
+public class TestClass
+{{
+    public void MyTest(int v1, int v2)
+    {{
+    }}
+
+    public void MyTest2(
+        int v1,
+        {commentText}
+        int v2)
+    {{
+    }}
+
+    public void TestMethod()
+    {{
+        this.MyTest(
+            1,
+            {commentText}
+            2);
+    }}
+}}
+";
+
+            await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
+        /// <summary>
+        /// Verifies that multiple comment lines will not reported a diagnostic.
+        /// This is a regression test for #1921.
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Fact]
+        public async Task TestWithMultipleCommentLinesAsync()
+        {
+            var testCode = @"
+public class TestClass
+{
+    public void MyTest1(
+        int v1,
+        //// bool b1,
+        //// bool b2,
+        int v2)
+    {
+    }
+
+    public void MyTest2(
+        int v1,
+        /*
+        bool b1,
+        bool b2,
+        */
+        int v2)
+    {
+    }
+
+    public void TestMethod()
+    {
+        this.MyTest1(
+            1,
+            //// true,
+            //// false,
+            2);
+
+        this.MyTest2(
+            1,
+            /*
+            true,
+            false,
+            */
+            2);
+    }
+}
+";
+
+            await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
         protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
         {
             yield return new SA1115ParameterMustFollowComma();

StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1115ParameterMustFollowComma.cs

@@ -277,7 +277,7 @@ private static void AnalyzeArgumentList(SyntaxNodeAnalysisContext context, BaseA
                 int currentArgumentStartLine;
                 int currentArgumentEndLine;
 
-                if (currentArgument.HasLeadingTrivia && currentArgument.GetLeadingTrivia().All(trivia => IsValidTrivia(trivia)))
+                if (currentArgument.HasLeadingTrivia && IsValidTrivia(currentArgument.GetLeadingTrivia()))
                 {
                     var lineSpan = currentArgument.SyntaxTree.GetLineSpan(currentArgument.FullSpan);
                     currentArgumentStartLine = lineSpan.StartLinePosition.Line;
@@ -313,7 +313,7 @@ private static void AnalyzeParameterList(SyntaxNodeAnalysisContext context, Base
                 var currentParameter = parameterListSyntax.Parameters[i];
                 int currentParameterLine;
 
-                if (currentParameter.HasLeadingTrivia && currentParameter.GetLeadingTrivia().All(trivia => IsValidTrivia(trivia)))
+                if (currentParameter.HasLeadingTrivia && IsValidTrivia(currentParameter.GetLeadingTrivia()))
                 {
                     currentParameterLine = currentParameter.SyntaxTree.GetLineSpan(currentParameter.FullSpan).StartLinePosition.Line;
                 }
@@ -331,22 +331,46 @@ private static void AnalyzeParameterList(SyntaxNodeAnalysisContext context, Base
             }
         }
 
-        private static bool IsValidTrivia(SyntaxTrivia trivia)
+        private static bool IsValidTrivia(SyntaxTriviaList triviaList)
         {
-            if (trivia.IsDirective)
-            {
-                return true;
-            }
+            var inBlankLine = true;
 
-            switch (trivia.Kind())
+            for (var i = 0; i < triviaList.Count; i++)
             {
-            case SyntaxKind.DisabledTextTrivia:
-            case SyntaxKind.WhitespaceTrivia:
-                return true;
+                var trivia = triviaList[i];
+
+                if (trivia.IsDirective)
+                {
+                    inBlankLine = false;
+                    continue;
+                }
+
+                switch (trivia.Kind())
+                {
+                case SyntaxKind.WhitespaceTrivia:
+                    break;
 
-            default:
-                return false;
+                case SyntaxKind.EndOfLineTrivia:
+                    if (inBlankLine)
+                    {
+                        return false;
+                    }
+
+                    inBlankLine = true;
+                    break;
+
+                case SyntaxKind.DisabledTextTrivia:
+                case SyntaxKind.SingleLineCommentTrivia:
+                case SyntaxKind.MultiLineCommentTrivia:
+                    inBlankLine = false;
+                    break;
+
+                default:
+                    return false;
+                }
             }
+
+            return true;
         }
     }
 }