From b7782cc95afc542db1a85ef106c0ab75af4ec500 Mon Sep 17 00:00:00 2001 From: Alex Zissis Date: Sat, 5 Apr 2025 22:18:29 +1100 Subject: [PATCH] If a record has multiple constructors, default to the primary constructor --- .../Internal/Inspection.cs | 31 ++++++++++++++++--- .../Internal/Roslyn/TypeSymbolExtensions.cs | 30 ++++++++++++++++-- test/Dapper.AOT.Test/Verifiers/DAP036.cs | 10 ++++++ 3 files changed, 64 insertions(+), 7 deletions(-) diff --git a/src/Dapper.AOT.Analyzers/Internal/Inspection.cs b/src/Dapper.AOT.Analyzers/Internal/Inspection.cs index ed114976..006c4a0c 100644 --- a/src/Dapper.AOT.Analyzers/Internal/Inspection.cs +++ b/src/Dapper.AOT.Analyzers/Internal/Inspection.cs @@ -830,6 +830,8 @@ internal static ConstructorResult ChooseConstructor(ITypeSymbol? typeSymbol, out return ConstructorResult.SuccessSingleExplicit; } + List implicitCtors = new(); + // look for remaining constructors foreach (var ctor in ctors) { @@ -883,13 +885,34 @@ internal static ConstructorResult ChooseConstructor(ITypeSymbol? typeSymbol, out continue; } - if (constructor is not null) + implicitCtors.Add(ctor); + } + + if (implicitCtors.Count == 0) + { + return ConstructorResult.NoneFound; + } + + if (implicitCtors.Count == 1) + { + constructor = implicitCtors[0]; + return ConstructorResult.SuccessSingleImplicit; + } + + // if it's a record without an [ExplicitConstructor] use the primary constructor, as you can't assign the attribute to the primary constructor + if (named.IsRecord) + { + bool hasPrimaryConstructor = named.TryGetPrimaryConstructor(out var primaryConstructor); + + if (hasPrimaryConstructor && primaryConstructor != null) { - return ConstructorResult.FailMultipleImplicit; + constructor = primaryConstructor; + return ConstructorResult.SuccessSingleImplicit; } - constructor = ctor; } - return constructor is null ? ConstructorResult.NoneFound : ConstructorResult.SuccessSingleImplicit; + + constructor = implicitCtors[0]; + return ConstructorResult.FailMultipleImplicit; } /// diff --git a/src/Dapper.AOT.Analyzers/Internal/Roslyn/TypeSymbolExtensions.cs b/src/Dapper.AOT.Analyzers/Internal/Roslyn/TypeSymbolExtensions.cs index 7d9e70a3..07161d50 100644 --- a/src/Dapper.AOT.Analyzers/Internal/Roslyn/TypeSymbolExtensions.cs +++ b/src/Dapper.AOT.Analyzers/Internal/Roslyn/TypeSymbolExtensions.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; +using Microsoft.CodeAnalysis.CSharp.Syntax; namespace Dapper.Internal.Roslyn; @@ -13,7 +14,7 @@ internal static class TypeSymbolExtensions Func? filter = null) { if (typeSymbol is null) yield break; - + foreach (var methodSymbol in typeSymbol.GetMembers() .OfType() .Where(m => m.MethodKind is MethodKind.Ordinary or MethodKind.DeclareMethod)) @@ -202,7 +203,7 @@ public static bool ImplementsIReadOnlyCollection(this ITypeSymbol? typeSymbol, o /// True, if passed implements . False otherwise /// /// if found, an interface type symbol - public static bool ImplementsIReadOnlyList(this ITypeSymbol? typeSymbol, out ITypeSymbol? searchedTypeSymbol) + public static bool ImplementsIReadOnlyList(this ITypeSymbol? typeSymbol, out ITypeSymbol? searchedTypeSymbol) => typeSymbol.ImplementsInterface(SpecialType.System_Collections_Generic_IReadOnlyList_T, out searchedTypeSymbol); private static bool ImplementsInterface( @@ -254,4 +255,27 @@ private static bool ImplementsInterface( found = null; return false; } -} \ No newline at end of file + + internal static bool TryGetPrimaryConstructor( + this INamedTypeSymbol typeSymbol, + out IMethodSymbol? primaryConstructor) + { + // from Roslyn + // https://github.com/dotnet/roslyn/blob/b6d2dc0d1be19ea0169f710bb370479734f746b9/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Extensions/ITypeSymbolExtensions.cs#L40 + if (typeSymbol.TypeKind is TypeKind.Class or TypeKind.Struct) + { + // A bit hacky to determine the parameters of primary constructor associated with a given record. + // Simplifying is tracked by: https://github.com/dotnet/roslyn/issues/53092. Note: When the issue is + // handled, we can remove the logic here and handle things in GetParameters extension. BUT if GetParameters + // extension method gets updated to handle records, we need to test EVERY usage of the extension method and + // make sure the change is applicable to all these usages. + + primaryConstructor = typeSymbol.InstanceConstructors.FirstOrDefault( + c => c.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax() is TypeDeclarationSyntax); + return primaryConstructor is not null; + } + + primaryConstructor = null; + return false; + } +} diff --git a/test/Dapper.AOT.Test/Verifiers/DAP036.cs b/test/Dapper.AOT.Test/Verifiers/DAP036.cs index 155b52e7..cbc0cd98 100644 --- a/test/Dapper.AOT.Test/Verifiers/DAP036.cs +++ b/test/Dapper.AOT.Test/Verifiers/DAP036.cs @@ -23,7 +23,9 @@ public void Foo(DbConnection conn) _ = conn.Query("storedproc"); _ = conn.Query("storedproc"); + _ = conn.Query("storedproc"); _ = conn.Query("storedproc"); + _ = conn.Query("storedproc"); _ = conn.Query("storedproc"); } @@ -51,7 +53,15 @@ public MultipleImplicit(string b) {} public MultipleImplicit(MultipleImplicit c) {} } record class RecordClass(int a); + public record class RecordClassWithMultipleConstructors(int a, bool b) + { + public RecordClassWithMultipleConstructors(int a) : this(a, false) {} + } record struct RecordStruct(int a); + public record struct RecordStructWithMultipleConstructors(int a, bool b) + { + public RecordStructWithMultipleConstructors(int a) : this(a, false) {} + } readonly record struct ReadOnlyRecordStruct(int a); namespace System.Runtime.CompilerServices