From bb7c51daa29c90f976ab164ab8677c2d8b75afb1 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 29 May 2024 09:59:34 -0700 Subject: [PATCH 1/2] Support 'use primary constructor' on no-parameter constructors with attributes --- ...UsePrimaryConstructorDiagnosticAnalyzer.cs | 9 ++++-- .../UsePrimaryConstructorTests.cs | 29 +++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UsePrimaryConstructor/CSharpUsePrimaryConstructorDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UsePrimaryConstructor/CSharpUsePrimaryConstructorDiagnosticAnalyzer.cs index 68983f7a69938..8d48275f66a5d 100644 --- a/src/Analyzers/CSharp/Analyzers/UsePrimaryConstructor/CSharpUsePrimaryConstructorDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UsePrimaryConstructor/CSharpUsePrimaryConstructorDiagnosticAnalyzer.cs @@ -189,7 +189,7 @@ private void OnSymbolEnd(SymbolAnalysisContext context) _primaryConstructorDeclaration.Identifier.GetLocation(), _styleOption.Notification, context.Options, - ImmutableArray.Create(_primaryConstructorDeclaration.GetLocation()), + [_primaryConstructorDeclaration.GetLocation()], properties)); _candidateMembersToRemove.Free(); @@ -288,7 +288,12 @@ void RegisterFieldOrPropertyAnalysisIfNecessary(Analyzer? analyzer) if (!TryFindPrimaryConstructorCandidate(namedType, out var primaryConstructor, out var primaryConstructorDeclaration)) return null; - if (primaryConstructor.Parameters.Length == 0) + // If we have no parameters and no attributes, then this is a trivial constructor. We don't want to + // spam the user to convert this to a primary constructor. What would likely be better would be to + // offer to remove the constructor entirely. We do offer when there are attributes to help with cases + // like simple DI constructors that have no parameters, but would still be nicer with the attributes + // moved to the type instead. + if (primaryConstructor.Parameters.Length == 0 && primaryConstructorDeclaration.AttributeLists.Count == 0) return null; // protected constructor in an abstract type is fine. It will stay protected even as a primary constructor. diff --git a/src/Analyzers/CSharp/Tests/UsePrimaryConstructor/UsePrimaryConstructorTests.cs b/src/Analyzers/CSharp/Tests/UsePrimaryConstructor/UsePrimaryConstructorTests.cs index 7e695ec9d2089..e1f02ac0b3c7d 100644 --- a/src/Analyzers/CSharp/Tests/UsePrimaryConstructor/UsePrimaryConstructorTests.cs +++ b/src/Analyzers/CSharp/Tests/UsePrimaryConstructor/UsePrimaryConstructorTests.cs @@ -4071,4 +4071,33 @@ public enum MyEnum ReferenceAssemblies = ReferenceAssemblies.Net.Net80, }.RunAsync(); } + + [Fact] + public async Task TestAttributeOnEmptyConstructor() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + [CLSCompliant(true)] + public [|C|]() + { + } + } + """, + FixedCode = """ + using System; + + [method: CLSCompliant(true)] + class C() + { + } + """, + CodeActionIndex = 0, + LanguageVersion = LanguageVersion.CSharp12, + }.RunAsync(); + } } From 4ba4a1b1b3737275258a14d62da2406689826c07 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 29 May 2024 09:59:53 -0700 Subject: [PATCH 2/2] Work item --- .../Tests/UsePrimaryConstructor/UsePrimaryConstructorTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analyzers/CSharp/Tests/UsePrimaryConstructor/UsePrimaryConstructorTests.cs b/src/Analyzers/CSharp/Tests/UsePrimaryConstructor/UsePrimaryConstructorTests.cs index e1f02ac0b3c7d..5fb52d431a110 100644 --- a/src/Analyzers/CSharp/Tests/UsePrimaryConstructor/UsePrimaryConstructorTests.cs +++ b/src/Analyzers/CSharp/Tests/UsePrimaryConstructor/UsePrimaryConstructorTests.cs @@ -4072,7 +4072,7 @@ public enum MyEnum }.RunAsync(); } - [Fact] + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/73695")] public async Task TestAttributeOnEmptyConstructor() { await new VerifyCS.Test