Review c# 8 features

StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/OrderingRules/SA1206CodeFixProvider.cs

@@ -14,6 +14,7 @@ namespace StyleCop.Analyzers.OrderingRules
     using Microsoft.CodeAnalysis.CodeFixes;
     using Microsoft.CodeAnalysis.CSharp.Syntax;
     using StyleCop.Analyzers.Helpers;
+    using StyleCop.Analyzers.Lightup;
     using static StyleCop.Analyzers.OrderingRules.ModifierOrderHelper;
 
     /// <summary>
@@ -53,29 +54,42 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
         {
             var syntaxRoot = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
 
-            var memberDeclaration = syntaxRoot.FindNode(diagnostic.Location.SourceSpan).FirstAncestorOrSelf<MemberDeclarationSyntax>();
-            if (memberDeclaration == null)
+            var declaration = FindDeclaration(syntaxRoot, diagnostic);
+            if (declaration == null)
             {
                 return document;
             }
 
-            var modifierTokenToFix = memberDeclaration.FindToken(diagnostic.Location.SourceSpan.Start);
+            var modifierTokenToFix = declaration.FindToken(diagnostic.Location.SourceSpan.Start);
             if (GetModifierType(modifierTokenToFix) == ModifierType.None)
             {
                 return document;
             }
 
-            var newModifierList = PartiallySortModifiers(memberDeclaration.GetModifiers(), modifierTokenToFix);
-            syntaxRoot = UpdateSyntaxRoot(memberDeclaration, newModifierList, syntaxRoot);
+            var newModifierList = PartiallySortModifiers(DeclarationModifiersHelper.GetModifiers(declaration), modifierTokenToFix);
+            syntaxRoot = UpdateSyntaxRoot(declaration, newModifierList, syntaxRoot);
 
             return document.WithSyntaxRoot(syntaxRoot);
         }
 
-        private static SyntaxNode UpdateSyntaxRoot(MemberDeclarationSyntax memberDeclaration, SyntaxTokenList newModifiers, SyntaxNode syntaxRoot)
+        /// <summary>
+        /// Finds the declaration a diagnostic was reported on. A local function is a statement rather than a member
+        /// declaration, so it cannot be found by looking for a <see cref="MemberDeclarationSyntax"/> alone.
+        /// </summary>
+        /// <param name="syntaxRoot">The root of the syntax tree.</param>
+        /// <param name="diagnostic">The diagnostic to find the declaration for.</param>
+        /// <returns>The declaration, or <see langword="null"/> if none was found.</returns>
+        private static SyntaxNode FindDeclaration(SyntaxNode syntaxRoot, Diagnostic diagnostic)
         {
-            var newDeclaration = memberDeclaration.WithModifiers(newModifiers);
+            return syntaxRoot.FindNode(diagnostic.Location.SourceSpan)
+                .AncestorsAndSelf()
+                .FirstOrDefault(node => node is MemberDeclarationSyntax || LocalFunctionStatementSyntaxWrapper.IsInstance(node));
+        }
 
-            return syntaxRoot.ReplaceNode(memberDeclaration, newDeclaration);
+        private static SyntaxNode UpdateSyntaxRoot(SyntaxNode declaration, SyntaxTokenList newModifiers, SyntaxNode syntaxRoot)
+        {
+            var newDeclaration = DeclarationModifiersHelper.WithModifiers(declaration, newModifiers);
+            return syntaxRoot.ReplaceNode(declaration, newDeclaration);
         }
 
         /// <summary>
@@ -173,31 +187,31 @@ private class FixAll : DocumentBasedFixAllProvider
 
                 // because all modifiers can be fixed in one run, we
                 // only need to store each declaration once
-                var trackedDiagnosticMembers = new HashSet<MemberDeclarationSyntax>();
+                var trackedDiagnosticMembers = new HashSet<SyntaxNode>();
                 foreach (var diagnostic in diagnostics)
                 {
-                    var memberDeclaration = syntaxRoot.FindNode(diagnostic.Location.SourceSpan).FirstAncestorOrSelf<MemberDeclarationSyntax>();
-                    if (memberDeclaration == null)
+                    var declaration = FindDeclaration(syntaxRoot, diagnostic);
+                    if (declaration == null)
                     {
                         continue;
                     }
 
-                    var modifierToken = memberDeclaration.FindToken(diagnostic.Location.SourceSpan.Start);
+                    var modifierToken = declaration.FindToken(diagnostic.Location.SourceSpan.Start);
                     if (GetModifierType(modifierToken) == ModifierType.None)
                     {
                         continue;
                     }
 
-                    trackedDiagnosticMembers.Add(memberDeclaration);
+                    trackedDiagnosticMembers.Add(declaration);
                 }
 
                 syntaxRoot = syntaxRoot.TrackNodes(trackedDiagnosticMembers);
 
                 foreach (var member in trackedDiagnosticMembers)
                 {
-                    var memberDeclaration = syntaxRoot.GetCurrentNode(member);
-                    var newModifierList = FullySortModifiers(memberDeclaration.GetModifiers());
-                    syntaxRoot = UpdateSyntaxRoot(memberDeclaration, newModifierList, syntaxRoot);
+                    var declaration = syntaxRoot.GetCurrentNode(member);
+                    var newModifierList = FullySortModifiers(DeclarationModifiersHelper.GetModifiers(declaration));
+                    syntaxRoot = UpdateSyntaxRoot(declaration, newModifierList, syntaxRoot);
                 }
 
                 return syntaxRoot;

StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp6/ReadabilityRules/SA1122UnitTests.cs

@@ -6,6 +6,7 @@ namespace StyleCop.Analyzers.Test.CSharp6.ReadabilityRules
     using System.Threading;
     using System.Threading.Tasks;
     using Microsoft.CodeAnalysis.Testing;
+    using StyleCop.Analyzers.Lightup;
     using StyleCop.Analyzers.ReadabilityRules;
     using Xunit;
     using static StyleCop.Analyzers.Test.CSharp6.Verifiers.StyleCopCodeFixVerifier<
@@ -16,8 +17,99 @@ namespace StyleCop.Analyzers.Test.CSharp6.ReadabilityRules
     /// This class contains unit tests for <see cref="SA1122UseStringEmptyForEmptyStrings"/> and
     /// <see cref="SA1122CodeFixProvider"/>.
     /// </summary>
+    // TODO: Check if this can be simplified, using the theory tests
     public class SA1122UnitTests
     {
+        public static TheoryData<string> EmptyStringLiterals
+        {
+            get
+            {
+                var data = new TheoryData<string>()
+                {
+                    "\"\"",
+                    "@\"\"",
+                    "$\"\"",
+                    "$@\"\"",
+                };
+
+                if (LightupHelpers.SupportsCSharp8)
+                {
+                    data.Add("@$\"\"");
+                }
+
+                if (LightupHelpers.SupportsCSharp11)
+                {
+                    // Only the multi-line form of a raw string literal can be empty, written as a single blank line
+                    // between the delimiters.
+                    data.Add("\"\"\"\r\n\r\n            \"\"\"");
+                }
+
+                return data;
+            }
+        }
+
+        public static TheoryData<string> NotReportedStringLiterals
+        {
+            get
+            {
+                var data = new TheoryData<string>()
+                {
+                    "\"text\"",
+                    "@\"text\"",
+                    "$\"text\"",
+                    "$@\"text\"",
+                    "$\"{value}\"",
+                };
+
+                if (LightupHelpers.SupportsCSharp8)
+                {
+                    data.Add("@$\"text\"");
+                }
+
+                if (LightupHelpers.SupportsCSharp11)
+                {
+                    data.Add("\"\"\"text\"\"\"");
+
+                    // Two blank lines is the boundary of the empty case above: the newline ending the last content
+                    // line belongs to the closing delimiter, so this value is a single line break, not empty.
+                    data.Add("\"\"\"\r\n\r\n\r\n            \"\"\"");
+
+                    // A UTF-8 string literal is a ReadOnlySpan<byte> rather than a string, so string.Empty can never replace it.
+                    data.Add("\"\"u8");
+                    data.Add("\"text\"u8");
+                }
+
+                return data;
+            }
+        }
+
+        public static TheoryData<string> EmptyStringLiteralsAllowedAsConstant
+        {
+            get
+            {
+                var data = new TheoryData<string>()
+                {
+                    "\"\"",
+                    "@\"\"",
+                };
+
+                // An interpolated string is only a constant expression from C# 10 onwards.
+                if (LightupHelpers.SupportsCSharp10)
+                {
+                    data.Add("$\"\"");
+                    data.Add("$@\"\"");
+                    data.Add("@$\"\"");
+                }
+
+                if (LightupHelpers.SupportsCSharp11)
+                {
+                    data.Add("\"\"\"\r\n\r\n            \"\"\"");
+                }
+
+                return data;
+            }
+        }
+
         [Theory]
         [InlineData(true)]
         [InlineData(false)]
@@ -70,6 +162,60 @@ public void Bar()
             await VerifyCSharpFixAsync(oldSource, expected, newSource, CancellationToken.None).ConfigureAwait(true);
         }
 
+        [Theory]
+        [MemberData(nameof(EmptyStringLiterals))]
+        public async Task TestEmptyStringLiteralIsReportedAsync(string literal)
+        {
+            var testCode = $@"public class Foo
+{{
+    public void Bar(string value)
+    {{
+        var test = [|{literal}|];
+    }}
+}}";
+            var fixedCode = @"public class Foo
+{
+    public void Bar(string value)
+    {
+        var test = string.Empty;
+    }
+}";
+
+            await VerifyCSharpFixAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, fixedCode, CancellationToken.None).ConfigureAwait(true);
+        }
+
+        [Theory]
+        [MemberData(nameof(NotReportedStringLiterals))]
+        public async Task TestStringLiteralIsNotReportedAsync(string literal)
+        {
+            var testCode = $@"public class Foo
+{{
+    public void Bar(string value)
+    {{
+        var test = {literal};
+    }}
+}}";
+
+            await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(true);
+        }
+
+        [Theory]
+        [MemberData(nameof(EmptyStringLiteralsAllowedAsConstant))]
+        public async Task TestEmptyStringLiteralAsConstantIsNotReportedAsync(string literal)
+        {
+            var testCode = $@"public class Foo
+{{
+    private const string TestField = {literal};
+
+    public void Bar()
+    {{
+        const string test = {literal};
+    }}
+}}";
+
+            await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(true);
+        }
+
         [Theory]
         [InlineData(true)]
         [InlineData(false)]

StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp8/DocumentationRules/SA1600CSharp8UnitTests.cs

@@ -3,11 +3,55 @@
 
 namespace StyleCop.Analyzers.Test.CSharp8.DocumentationRules
 {
+    using System.Threading;
+    using System.Threading.Tasks;
     using Microsoft.CodeAnalysis.CSharp;
+    using Microsoft.CodeAnalysis.Testing;
+    using Xunit;
+    using static StyleCop.Analyzers.Test.CSharp6.Verifiers.StyleCopCodeFixVerifier<
+        StyleCop.Analyzers.DocumentationRules.SA1600ElementsMustBeDocumented,
+        StyleCop.Analyzers.DocumentationRules.SA1600CodeFixProvider>;
 
     public partial class SA1600CSharp8UnitTests
     {
         // Using 'Default' here makes sure that later test projects also run these tests with their own language version, without having to override this property
         protected override LanguageVersion LanguageVersion => LanguageVersion.Default;
+
+        /// <summary>
+        /// Verifies that the members an interface may hold from C# 8 onwards need documentation just like any other
+        /// interface member.
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        // TODO: Investigate this behavior (the private members)!
+        [Fact]
+        public async Task TestInterfaceMembersWithoutDocumentationAsync()
+        {
+            var testCode = @"/// <summary>
+/// A summary.
+/// </summary>
+public interface ITest
+{
+    void [|TestMethod1|]()
+    {
+    }
+
+    private void [|TestMethod2|]()
+    {
+    }
+
+    static void [|TestMethod3|]()
+    {
+    }
+
+    private static void [|TestMethod4|]()
+    {
+    }
+}
+";
+
+            // Only the diagnostic is verified, as in every other SA1600 test: undocumented members also produce
+            // CS1591 warnings, which the code fix verification would require to be declared here as well.
+            await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(true);
+        }
     }
 }

StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp8/LayoutRules/SA1500CSharp8UnitTests.cs

@@ -0,0 +1,69 @@
+// Copyright (c) Contributors to the New StyleCop Analyzers project.
+// Licensed under the MIT License. See LICENSE in the project root for license information.
+
+namespace StyleCop.Analyzers.Test.CSharp8.LayoutRules
+{
+    using System.Threading;
+    using System.Threading.Tasks;
+    using Microsoft.CodeAnalysis.Testing;
+    using Xunit;
+    using static StyleCop.Analyzers.Test.CSharp6.Verifiers.StyleCopCodeFixVerifier<
+        StyleCop.Analyzers.LayoutRules.SA1500BracesForMultiLineStatementsMustNotShareLine,
+        StyleCop.Analyzers.LayoutRules.SA1500CodeFixProvider>;
+
+    public partial class SA1500CSharp8UnitTests
+    {
+        /// <summary>
+        /// Verifies that a single-line switch expression is not inspected.
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Fact]
+        public async Task TestSwitchExpressionSingleLineAsync()
+        {
+            var testCode = @"public class TestClass
+{
+    public int TestMethod(int value)
+    {
+        return value switch { 0 => 0, _ => 1 };
+    }
+}
+";
+
+            await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(true);
+        }
+
+        /// <summary>
+        /// Verifies that diagnostics will be reported for the braces of a multi-line switch expression when they share a line with other code.
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Fact]
+        public async Task TestSwitchExpressionInvalidAsync()
+        {
+            var testCode = @"public class TestClass
+{
+    public int TestMethod(int value)
+    {
+        return value switch [|{|]
+            0 => 0,
+            _ => 1 [|}|];
+    }
+}
+";
+
+            var fixedCode = @"public class TestClass
+{
+    public int TestMethod(int value)
+    {
+        return value switch
+        {
+            0 => 0,
+            _ => 1
+        };
+    }
+}
+";
+
+            await VerifyCSharpFixAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, fixedCode, CancellationToken.None).ConfigureAwait(true);
+        }
+    }
+}