Merge pull request #3839 from MattFromRVA/SA1001_Change

Skip Warnings for Commas After Conditional Directives

Author: sharwell

StyleCop.Analyzers/StyleCop.Analyzers.Test/SpacingRules/SA1001UnitTests.cs

@@ -349,6 +349,124 @@ public void TestMethod()
             await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
         }
 
+        [Fact]
+        [WorkItem(3816, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3816")]
+        public async Task TestCommaFollowingPreprocessorDirectiveAsync()
+        {
+            var testCode = @"
+interface IFormattable {}
+interface ISpanFormattable {}
+
+partial struct Money : IFormattable
+#if true
+    , ISpanFormattable
+#endif
+{
+}
+";
+
+            var expected = DiagnosticResult.EmptyDiagnosticResults;
+
+            await VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
+        }
+
+        [Fact]
+        [WorkItem(3816, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3816")]
+        public async Task TestCommaFollowingElifDirectiveAsync()
+        {
+            var testCode = @"
+interface IFormattable {}
+interface ISpanFormattable {}
+
+partial struct Money : IFormattable
+#if false
+#elif true
+    , ISpanFormattable
+#endif
+{
+}
+";
+
+            var expected = DiagnosticResult.EmptyDiagnosticResults;
+
+            await VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
+        }
+
+        [Fact]
+        [WorkItem(3816, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3816")]
+        public async Task TestCommaFollowingElseDirectiveAsync()
+        {
+            var testCode = @"
+interface IFormattable {}
+interface ISpanFormattable {}
+
+partial struct Money : IFormattable
+#if false
+#else
+    , ISpanFormattable
+#endif
+{
+}
+";
+
+            var expected = DiagnosticResult.EmptyDiagnosticResults;
+
+            await VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
+        }
+
+        [Fact]
+        [WorkItem(3816, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3816")]
+        public async Task TestCommaFollowingEndIfDirectiveAsync()
+        {
+            var testCode = @"
+interface IFormattable {}
+interface ISpanFormattable {}
+
+partial struct Money : IFormattable
+#if false
+#endif
+    , ISpanFormattable
+{
+}
+";
+
+            var expected = DiagnosticResult.EmptyDiagnosticResults;
+
+            await VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
+        }
+
+        [Fact]
+        [WorkItem(3816, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3816")]
+        public async Task TestCommaNotFollowingDirectiveAsync()
+        {
+            var testCode = @"
+interface IFormattable {}
+interface ISpanFormattable {}
+
+partial struct Money : IFormattable
+    {|#0:,|} ISpanFormattable
+{
+}
+";
+
+            var fixedCode = @"
+interface IFormattable {}
+interface ISpanFormattable {}
+
+partial struct Money : IFormattable,
+    ISpanFormattable
+{
+}
+";
+
+            var expected = new[]
+            {
+                Diagnostic().WithLocation(0).WithArguments(" not", "preceded"),
+            };
+
+            await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
+        }
+
         private Task TestCommaInStatementOrDeclAsync(string originalStatement, DiagnosticResult expected, string fixedStatement)
         {
             return this.TestCommaInStatementOrDeclAsync(originalStatement, new[] { expected }, fixedStatement);

StyleCop.Analyzers/StyleCop.Analyzers/SpacingRules/SA1001CommasMustBeSpacedCorrectly.cs

@@ -7,6 +7,7 @@ namespace StyleCop.Analyzers.SpacingRules
 {
     using System;
     using System.Collections.Immutable;
+    using System.Linq;
     using Microsoft.CodeAnalysis;
     using Microsoft.CodeAnalysis.CSharp;
     using Microsoft.CodeAnalysis.Diagnostics;
@@ -76,6 +77,9 @@ private static void HandleCommaToken(SyntaxTreeAnalysisContext context, SyntaxTo
                 return;
             }
 
+            // Check if the comma follows a conditional preprocessor directive
+            bool followsDirective = token.HasLeadingTrivia && IsPrecededByDirectiveTrivia(token.LeadingTrivia);
+
             // check for a following space
             bool missingFollowingSpace = true;
 
@@ -102,7 +106,7 @@ private static void HandleCommaToken(SyntaxTreeAnalysisContext context, SyntaxTo
                 }
             }
 
-            if (token.IsFirstInLine() || token.IsPrecededByWhitespace(context.CancellationToken))
+            if (!followsDirective && (token.IsFirstInLine() || token.IsPrecededByWhitespace(context.CancellationToken)))
             {
                 // comma should{ not} be {preceded} by whitespace
                 context.ReportDiagnostic(Diagnostic.Create(Descriptor, token.GetLocation(), TokenSpacingProperties.RemovePrecedingPreserveLayout, " not", "preceded"));
@@ -120,5 +124,30 @@ private static void HandleCommaToken(SyntaxTreeAnalysisContext context, SyntaxTo
                 context.ReportDiagnostic(Diagnostic.Create(Descriptor, token.GetLocation(), TokenSpacingProperties.RemoveFollowing, " not", "followed"));
             }
         }
+
+        private static bool IsPrecededByDirectiveTrivia(SyntaxTriviaList triviaList)
+        {
+            int triviaIndex = triviaList.Count - 1;
+            while (triviaIndex >= 0)
+            {
+                switch (triviaList[triviaIndex].Kind())
+                {
+                case SyntaxKind.WhitespaceTrivia:
+                    triviaIndex--;
+                    break;
+
+                case SyntaxKind.IfDirectiveTrivia:
+                case SyntaxKind.ElifDirectiveTrivia:
+                case SyntaxKind.ElseDirectiveTrivia:
+                case SyntaxKind.EndIfDirectiveTrivia:
+                    return true;
+
+                default:
+                    return false;
+                }
+            }
+
+            return false;
+        }
     }
 }

documentation/SA1001.md

@@ -28,6 +28,12 @@ A comma should be followed by a single space, except in the following cases.
 * A comma may appear at the end of a line
 * A comma should not be followed by a space when used in an open generic type in a `typeof` expression
 * A comma is part of a string interpolation alignment component. For example:`$"{x,3}"`
+* A comma follows a conditional preprocessor directive (#if, #elif, #else, #endif). For example:
+  ```csharp
+  partial struct Money : IFormattable
+  #if !NETSTANDARD
+      , ISpanFormattable
+  #endif
 
 A comma should never be preceded by a space or appear as the first token on a line.