From f1f6d83c1074476b005a3ea668a5773dc76bc76e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Hellander?= Date: Sat, 10 Feb 2024 09:19:55 +0100 Subject: [PATCH 1/2] Update SA1110 to also check the parameter list in primary constructors #3784 --- .../SA1110CSharp11UnitTests.cs | 11 ++++ .../SA1110CSharp9UnitTests.cs | 57 +++++++++++++++++++ .../Helpers/CommonMemberData.cs | 23 ++++++++ .../TypeDeclarationSyntaxExtensions.cs | 14 +++++ ...eningParenthesisMustBeOnDeclarationLine.cs | 16 ++++++ 5 files changed, 121 insertions(+) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp11/ReadabilityRules/SA1110CSharp11UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp11/ReadabilityRules/SA1110CSharp11UnitTests.cs index d2fba6df3..7c2153407 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp11/ReadabilityRules/SA1110CSharp11UnitTests.cs +++ b/StyleCop.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/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/ReadabilityRules/SA1110CSharp9UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/ReadabilityRules/SA1110CSharp9UnitTests.cs index bdaa844bf..68762bdd0 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/ReadabilityRules/SA1110CSharp9UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/ReadabilityRules/SA1110CSharp9UnitTests.cs @@ -3,9 +3,66 @@ 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); + } + + protected virtual DiagnosticResult[] GetExpectedResultTestPrimaryConstructor() + { + return new[] + { + 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 index 3920ac2f4..bf2d0d5ad 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/Helpers/CommonMemberData.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/Helpers/CommonMemberData.cs @@ -114,5 +114,28 @@ public static IEnumerable GenericTypeDeclarationKeywords .Concat(new[] { new[] { "delegate" } }); } } + + public static IEnumerable 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" }; + } + } + } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/TypeDeclarationSyntaxExtensions.cs b/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/TypeDeclarationSyntaxExtensions.cs index 64140adb3..c4ee988eb 100644 --- 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/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1110OpeningParenthesisMustBeOnDeclarationLine.cs b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1110OpeningParenthesisMustBeOnDeclarationLine.cs index 4b4477f75..733eb176f 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1110OpeningParenthesisMustBeOnDeclarationLine.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1110OpeningParenthesisMustBeOnDeclarationLine.cs @@ -53,6 +53,7 @@ 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 TypeDeclarationAction = HandleTypeDeclaration; private static readonly Action MethodDeclarationAction = HandleMethodDeclaration; private static readonly Action LocalFunctionStatementAction = HandleLocalFunctionStatement; private static readonly Action ConstructorDeclarationAction = HandleConstructorDeclaration; @@ -77,6 +78,7 @@ public override void Initialize(AnalysisContext context) context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); context.EnableConcurrentExecution(); + context.RegisterSyntaxNodeAction(TypeDeclarationAction, SyntaxKinds.TypeDeclaration); context.RegisterSyntaxNodeAction(MethodDeclarationAction, SyntaxKind.MethodDeclaration); context.RegisterSyntaxNodeAction(LocalFunctionStatementAction, SyntaxKindEx.LocalFunctionStatement); context.RegisterSyntaxNodeAction(ConstructorDeclarationAction, SyntaxKind.ConstructorDeclaration); @@ -329,6 +331,20 @@ 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 CheckIfLocationOfPreviousTokenAndOpenTokenAreTheSame(SyntaxNodeAnalysisContext context, SyntaxToken openToken, bool preserveLayout) { var previousToken = openToken.GetPreviousToken(); From 67e8fd517333d63b6010c9387edcf5d0f1958d64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Hellander?= Date: Fri, 15 Mar 2024 07:28:51 +0100 Subject: [PATCH 2/2] Update SA1110 to also check the primary constructor argument list in a base list #3784 --- .../SA1110CSharp9UnitTests.cs | 85 +++++++++++++++++++ .../Helpers/CommonMemberData.cs | 8 ++ .../Lightup/SyntaxKindEx.cs | 1 + ...eningParenthesisMustBeOnDeclarationLine.cs | 23 +++++ 4 files changed, 117 insertions(+) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/ReadabilityRules/SA1110CSharp9UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/ReadabilityRules/SA1110CSharp9UnitTests.cs index 68762bdd0..844b063f7 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/ReadabilityRules/SA1110CSharp9UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/ReadabilityRules/SA1110CSharp9UnitTests.cs @@ -56,10 +56,95 @@ public async Task TestPrimaryConstructorWithParametersAsync(string typeKeyword) 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 index bf2d0d5ad..6ab2508c5 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/Helpers/CommonMemberData.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/Helpers/CommonMemberData.cs @@ -137,5 +137,13 @@ public static IEnumerable TypeKeywordsWhichSupportPrimaryConstructors } } } + + public static IEnumerable 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 index b3c2d27c6..920ac7d12 100644 --- 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/ReadabilityRules/SA1110OpeningParenthesisMustBeOnDeclarationLine.cs b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1110OpeningParenthesisMustBeOnDeclarationLine.cs index 733eb176f..5634c7f22 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1110OpeningParenthesisMustBeOnDeclarationLine.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1110OpeningParenthesisMustBeOnDeclarationLine.cs @@ -54,6 +54,7 @@ internal class SA1110OpeningParenthesisMustBeOnDeclarationLine : DiagnosticAnaly new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.ReadabilityRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink); private static readonly Action TypeDeclarationAction = HandleTypeDeclaration; + private static readonly Action PrimaryConstructorBaseTypeAction = HandlePrimaryConstructorBaseType; private static readonly Action MethodDeclarationAction = HandleMethodDeclaration; private static readonly Action LocalFunctionStatementAction = HandleLocalFunctionStatement; private static readonly Action ConstructorDeclarationAction = HandleConstructorDeclaration; @@ -79,6 +80,7 @@ public override void Initialize(AnalysisContext context) 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); @@ -345,6 +347,27 @@ private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context) } } + private static void HandlePrimaryConstructorBaseType(SyntaxNodeAnalysisContext context) + { + var primaryConstructorBaseType = (PrimaryConstructorBaseTypeSyntaxWrapper)context.Node; + + var identifierName = ((BaseTypeSyntax)primaryConstructorBaseType).ChildNodes() + .OfType() + .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();