Merge pull request #3800 from bjornhellander/feature/sa1110-primary-c…

…tor-3784 Update SA1110 to also check the parameter list in primary constructors
DotNetAnalyzers · Mar 19, 2024 · 834d939 · 834d939
2 parents cedfcf9 + 67e8fd5
commit 834d939
Show file tree

Hide file tree

Showing 6 changed files with 238 additions and 0 deletions.
diff --git a/...op.Analyzers/StyleCop.Analyzers.Test.CSharp11/ReadabilityRules/SA1110CSharp11UnitTests.cs b/...op.Analyzers/StyleCop.Analyzers.Test.CSharp11/ReadabilityRules/SA1110CSharp11UnitTests.cs
@@ -3,9 +3,20 @@
 
 namespace StyleCop.Analyzers.Test.CSharp11.ReadabilityRules
 {
+    using Microsoft.CodeAnalysis.Testing;
     using StyleCop.Analyzers.Test.CSharp10.ReadabilityRules;
+    using static StyleCop.Analyzers.Test.Verifiers.StyleCopCodeFixVerifier<
+        StyleCop.Analyzers.ReadabilityRules.SA1110OpeningParenthesisMustBeOnDeclarationLine,
+        StyleCop.Analyzers.SpacingRules.TokenSpacingCodeFixProvider>;
 
     public partial class SA1110CSharp11UnitTests : SA1110CSharp10UnitTests
     {
+        protected override DiagnosticResult[] GetExpectedResultTestPrimaryConstructor()
+        {
+            return new[]
+            {
+                Diagnostic().WithLocation(0),
+            };
+        }
     }
 }
diff --git a/...eCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/ReadabilityRules/SA1110CSharp9UnitTests.cs b/...eCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/ReadabilityRules/SA1110CSharp9UnitTests.cs
@@ -3,9 +3,151 @@
 
 namespace StyleCop.Analyzers.Test.CSharp9.ReadabilityRules
 {
+    using System.Threading;
+    using System.Threading.Tasks;
+    using Microsoft.CodeAnalysis.Testing;
     using StyleCop.Analyzers.Test.CSharp8.ReadabilityRules;
+    using StyleCop.Analyzers.Test.Helpers;
+    using Xunit;
+    using static StyleCop.Analyzers.Test.Verifiers.StyleCopCodeFixVerifier<
+        StyleCop.Analyzers.ReadabilityRules.SA1110OpeningParenthesisMustBeOnDeclarationLine,
+        StyleCop.Analyzers.SpacingRules.TokenSpacingCodeFixProvider>;
 
     public partial class SA1110CSharp9UnitTests : SA1110CSharp8UnitTests
     {
+        [Theory]
+        [MemberData(nameof(CommonMemberData.TypeKeywordsWhichSupportPrimaryConstructors), MemberType = typeof(CommonMemberData))]
+        [WorkItem(3784, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3784")]
+        public async Task TestPrimaryConstructorWithoutParametersAsync(string typeKeyword)
+        {
+            var testCode = $@"
+{typeKeyword} Foo
+    {{|#0:(|}})
+{{
+}}";
+
+            var fixedCode = $@"
+{typeKeyword} Foo()
+{{
+}}";
+
+            var expected = this.GetExpectedResultTestPrimaryConstructor();
+            await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
+        }
+
+        [Theory]
+        [MemberData(nameof(CommonMemberData.TypeKeywordsWhichSupportPrimaryConstructors), MemberType = typeof(CommonMemberData))]
+        [WorkItem(3784, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3784")]
+        public async Task TestPrimaryConstructorWithParametersAsync(string typeKeyword)
+        {
+            var testCode = $@"
+{typeKeyword} Foo
+    {{|#0:(|}}int x)
+{{
+}}";
+
+            var fixedCode = $@"
+{typeKeyword} Foo(
+    int x)
+{{
+}}";
+
+            var expected = this.GetExpectedResultTestPrimaryConstructor();
+            await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
+        }
+
+        [Theory]
+        [MemberData(nameof(CommonMemberData.ReferenceTypeKeywordsWhichSupportPrimaryConstructors), MemberType = typeof(CommonMemberData))]
+        [WorkItem(3784, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3784")]
+        public async Task TestPrimaryConstructorBaseListWithParametersOnSameLineAsync(string typeKeyword)
+        {
+            var testCode = $@"
+{typeKeyword} Foo(int x)
+{{
+}}
+
+{typeKeyword} Bar(int x) : Foo(x)
+{{
+}}";
+
+            await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
+        [Theory]
+        [MemberData(nameof(CommonMemberData.ReferenceTypeKeywordsWhichSupportPrimaryConstructors), MemberType = typeof(CommonMemberData))]
+        [WorkItem(3784, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3784")]
+        public async Task TestPrimaryConstructorBaseListWithParametersAsync(string typeKeyword)
+        {
+            var testCode = $@"
+{typeKeyword} Foo(int x)
+{{
+}}
+
+{typeKeyword} Bar(int x) : Foo
+    {{|#0:(|}}x)
+{{
+}}";
+
+            var fixedCode = $@"
+{typeKeyword} Foo(int x)
+{{
+}}
+
+{typeKeyword} Bar(int x) : Foo(
+    x)
+{{
+}}";
+
+            var expected = this.GetExpectedResultTestPrimaryConstructorBaseList();
+            await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
+        }
+
+        [Theory]
+        [MemberData(nameof(CommonMemberData.ReferenceTypeKeywordsWhichSupportPrimaryConstructors), MemberType = typeof(CommonMemberData))]
+        [WorkItem(3784, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3784")]
+        public async Task TestPrimaryConstructorBaseListWithoutParametersAsync(string typeKeyword)
+        {
+            var testCode = $@"
+{typeKeyword} Foo()
+{{
+}}
+
+{typeKeyword} Bar(int x) : Foo
+    {{|#0:(|}})
+{{
+}}";
+
+            var fixedCode = $@"
+{typeKeyword} Foo()
+{{
+}}
+
+{typeKeyword} Bar(int x) : Foo()
+{{
+}}";
+
+            var expected = this.GetExpectedResultTestPrimaryConstructorBaseList();
+            await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
+        }
+
+        protected virtual DiagnosticResult[] GetExpectedResultTestPrimaryConstructor()
+        {
+            return new[]
+            {
+                // Diagnostic issued twice because of https://github.com/dotnet/roslyn/issues/53136
+                Diagnostic().WithLocation(0),
+                Diagnostic().WithLocation(0),
+            };
+        }
+
+        protected virtual DiagnosticResult[] GetExpectedResultTestPrimaryConstructorBaseList()
+        {
+            return new[]
+            {
+                // Diagnostic issued twice because of https://github.com/dotnet/roslyn/issues/70488
+                Diagnostic().WithLocation(0),
+                Diagnostic().WithLocation(0),
+            };
+        }
     }
 }
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/Helpers/CommonMemberData.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/Helpers/CommonMemberData.cs
@@ -114,5 +114,36 @@ public static IEnumerable<object[]> GenericTypeDeclarationKeywords
                     .Concat(new[] { new[] { "delegate" } });
             }
         }
+
+        public static IEnumerable<object[]> TypeKeywordsWhichSupportPrimaryConstructors
+        {
+            get
+            {
+                if (LightupHelpers.SupportsCSharp9)
+                {
+                    yield return new[] { "record" };
+                }
+
+                if (LightupHelpers.SupportsCSharp10)
+                {
+                    yield return new[] { "record class" };
+                    yield return new[] { "record struct" };
+                }
+
+                if (LightupHelpers.SupportsCSharp12)
+                {
+                    yield return new[] { "class" };
+                    yield return new[] { "struct" };
+                }
+            }
+        }
+
+        public static IEnumerable<object[]> ReferenceTypeKeywordsWhichSupportPrimaryConstructors
+        {
+            get
+            {
+                return TypeKeywordsWhichSupportPrimaryConstructors.Where(x => !((string)x[0]).Contains("struct"));
+            }
+        }
     }
 }
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/SyntaxKindEx.cs b/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/SyntaxKindEx.cs
@@ -67,6 +67,7 @@ internal static class SyntaxKindEx
         public const SyntaxKind WithExpression = (SyntaxKind)9061;
         public const SyntaxKind WithInitializerExpression = (SyntaxKind)9062;
         public const SyntaxKind RecordDeclaration = (SyntaxKind)9063;
+        public const SyntaxKind PrimaryConstructorBaseType = (SyntaxKind)9065;
         public const SyntaxKind FunctionPointerUnmanagedCallingConventionList = (SyntaxKind)9066;
         public const SyntaxKind RecordStructDeclaration = (SyntaxKind)9068;
         public const SyntaxKind CollectionExpression = (SyntaxKind)9076;

diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/TypeDeclarationSyntaxExtensions.cs b/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/TypeDeclarationSyntaxExtensions.cs
@@ -19,6 +19,20 @@ static TypeDeclarationSyntaxExtensions()
 
     public static ParameterListSyntax? ParameterList(this TypeDeclarationSyntax syntax)
     {
+        if (!LightupHelpers.SupportsCSharp12)
+        {
+            // Prior to C# 12, the ParameterList property in RecordDeclarationSyntax did not override a base method.
+            switch (syntax.Kind())
+            {
+            case SyntaxKindEx.RecordDeclaration:
+            case SyntaxKindEx.RecordStructDeclaration:
+                return ((RecordDeclarationSyntaxWrapper)syntax).ParameterList;
+
+            default:
+                return null;
+            }
+        }
+
         return ParameterListAccessor(syntax);
     }
 

diff --git a/...rs/StyleCop.Analyzers/ReadabilityRules/SA1110OpeningParenthesisMustBeOnDeclarationLine.cs b/...rs/StyleCop.Analyzers/ReadabilityRules/SA1110OpeningParenthesisMustBeOnDeclarationLine.cs
@@ -53,6 +53,8 @@ internal class SA1110OpeningParenthesisMustBeOnDeclarationLine : DiagnosticAnaly
         private static readonly DiagnosticDescriptor Descriptor =
             new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.ReadabilityRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink);
 
+        private static readonly Action<SyntaxNodeAnalysisContext> TypeDeclarationAction = HandleTypeDeclaration;
+        private static readonly Action<SyntaxNodeAnalysisContext> PrimaryConstructorBaseTypeAction = HandlePrimaryConstructorBaseType;
         private static readonly Action<SyntaxNodeAnalysisContext> MethodDeclarationAction = HandleMethodDeclaration;
         private static readonly Action<SyntaxNodeAnalysisContext> LocalFunctionStatementAction = HandleLocalFunctionStatement;
         private static readonly Action<SyntaxNodeAnalysisContext> ConstructorDeclarationAction = HandleConstructorDeclaration;
@@ -77,6 +79,8 @@ public override void Initialize(AnalysisContext context)
             context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
             context.EnableConcurrentExecution();
 
+            context.RegisterSyntaxNodeAction(TypeDeclarationAction, SyntaxKinds.TypeDeclaration);
+            context.RegisterSyntaxNodeAction(PrimaryConstructorBaseTypeAction, SyntaxKindEx.PrimaryConstructorBaseType);
             context.RegisterSyntaxNodeAction(MethodDeclarationAction, SyntaxKind.MethodDeclaration);
             context.RegisterSyntaxNodeAction(LocalFunctionStatementAction, SyntaxKindEx.LocalFunctionStatement);
             context.RegisterSyntaxNodeAction(ConstructorDeclarationAction, SyntaxKind.ConstructorDeclaration);
@@ -329,6 +333,41 @@ private static void HandleLocalFunctionStatement(SyntaxNodeAnalysisContext conte
             }
         }
 
+        private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context)
+        {
+            var typeDeclaration = (TypeDeclarationSyntax)context.Node;
+            var parameterList = typeDeclaration.ParameterList();
+
+            if (parameterList != null
+                && !parameterList.OpenParenToken.IsMissing
+                && !typeDeclaration.Identifier.IsMissing)
+            {
+                bool preserveLayout = parameterList.Parameters.Any();
+                CheckIfLocationOfPreviousTokenAndOpenTokenAreTheSame(context, parameterList.OpenParenToken, preserveLayout);
+            }
+        }
+
+        private static void HandlePrimaryConstructorBaseType(SyntaxNodeAnalysisContext context)
+        {
+            var primaryConstructorBaseType = (PrimaryConstructorBaseTypeSyntaxWrapper)context.Node;
+
+            var identifierName = ((BaseTypeSyntax)primaryConstructorBaseType).ChildNodes()
+                .OfType<IdentifierNameSyntax>()
+                .FirstOrDefault();
+            if (identifierName == null || identifierName.Identifier.IsMissing)
+            {
+                return;
+            }
+
+            var argumentListSyntax = primaryConstructorBaseType.ArgumentList;
+
+            if (argumentListSyntax != null && !argumentListSyntax.OpenParenToken.IsMissing)
+            {
+                bool preserveLayout = argumentListSyntax.Arguments.Any();
+                CheckIfLocationOfPreviousTokenAndOpenTokenAreTheSame(context, argumentListSyntax.OpenParenToken, preserveLayout);
+            }
+        }
+
         private static void CheckIfLocationOfPreviousTokenAndOpenTokenAreTheSame(SyntaxNodeAnalysisContext context, SyntaxToken openToken, bool preserveLayout)
         {
             var previousToken = openToken.GetPreviousToken();