feat: Constructor selection (#30)

* feat: use constructor with least amount of parameters

* refactor: rename test

* test: GenericClassPrivateConstructor

* fix(ConstructorGenerator): add BindingFlags.NonPublic

* feat: report diagnostics for missing or ambiguous constructor

* chore: adjust analyzer release tracking

* fix(Analyzers): remove MissingConstructor diagnosis

* test: CanDetectAmbiguousConstructors

* chore: increase nuget versions to 1.8

* chore(ConstructorGenerator): add doc comment

* fix(ClassInfoFactory): comment

* refactor(ClassInfoFactory): improve variable name

Author: m31coding

README.md

@@ -37,7 +37,7 @@ PM> Install-Package M31.FluentApi
 A package reference will be added to your `csproj` file. Moreover, since this library provides code via source code generation, consumers of your project don't need the reference to `M31.FluentApi`. Therefore, it is recommended to use the `PrivateAssets` metadata tag:
 
 ```xml
-<PackageReference Include="M31.FluentApi" Version="1.7.0" PrivateAssets="all"/>
+<PackageReference Include="M31.FluentApi" Version="1.8.0" PrivateAssets="all"/>
 ```
 
 If you would like to examine the generated code, you may emit it by adding the following lines to your `csproj` file:

src/M31.FluentApi.Generator/AnalyzerReleases.Shipped.md

@@ -68,4 +68,19 @@ M31FA007 | M31.Usage | Error | Partial types are not supported
 
 Rule ID | Category | Severity | Notes
 --------|----------|----------|-------
-M31FA023 | M31.Usage | Error | Last builder step cannot be skipped
+M31FA023 | M31.Usage | Error | Last builder step cannot be skipped
+
+
+## Release 1.8.0
+
+### Removed Rules
+
+Rule ID | Category | Severity | Notes
+--------|----------|----------|-------
+M31FA011 | M31.Usage | Error | Default constructor is missing
+
+### New Rules
+
+Rule ID | Category | Severity | Notes
+--------|----------|----------|-------
+M31FA024 | M31.Usage | Error | Constructors are ambiguous

src/M31.FluentApi.Generator/CodeGeneration/CodeBoardActors/ConstructorGenerator.cs

@@ -18,20 +18,41 @@ public void Modify(CodeBoard codeBoard)
         }
 
         Method constructor = CreateConstructor(codeBoard.Info.BuilderClassName);
+        int nofParameters = codeBoard.Info.FluentApiTypeConstructorInfo.NumberOfParameters;
 
-        if (codeBoard.Info.FluentApiTypeHasPrivateConstructor)
+        if (codeBoard.Info.FluentApiTypeConstructorInfo.ConstructorIsNonPublic)
         {
-            // student = (Student<T1, T2>) Activator.CreateInstance(typeof(Student<T1, T2>), true)!;
-            constructor.AppendBodyLine(
-                $"{instanceName} = ({classNameWithTypeParameters}) " +
-                $"Activator.CreateInstance(typeof({classNameWithTypeParameters}), true)!;");
+            if (nofParameters == 0)
+            {
+                // student = (Student<T1, T2>) Activator.CreateInstance(typeof(Student<T1, T2>), true)!;
+                constructor.AppendBodyLine(
+                    $"{instanceName} = ({classNameWithTypeParameters}) " +
+                    $"Activator.CreateInstance(typeof({classNameWithTypeParameters}), true)!;");
 
-            codeBoard.CodeFile.AddUsing("System");
+                codeBoard.CodeFile.AddUsing("System");
+            }
+            else
+            {
+                // student = (Student<T1, T2>) Activator.CreateInstance(typeof(Student<T1, T2>), BindingFlags.Instance |
+                // BindingFlags.NonPublic, null, new object?[] { null, null }, null)!;
+                string parameters =
+                    $"new object?[] {{ {string.Join(", ", Enumerable.Repeat("null", nofParameters))} }}";
+
+                constructor.AppendBodyLine(
+                    $"{instanceName} = ({classNameWithTypeParameters}) " +
+                    $"Activator.CreateInstance(" +
+                    $"typeof({classNameWithTypeParameters}), BindingFlags.Instance | BindingFlags.NonPublic, null, {parameters}, null)!;");
+
+                codeBoard.CodeFile.AddUsing("System.Reflection");
+                codeBoard.CodeFile.AddUsing("System");
+            }
         }
         else
         {
-            // student = new Student<T1, T2>();
-            constructor.AppendBodyLine($"{instanceName} = new {classNameWithTypeParameters}();");
+            // student = new Student<T1, T2>(default!, default!);
+            string parameters = string.Join(", ",
+                Enumerable.Repeat("default!", nofParameters));
+            constructor.AppendBodyLine($"{instanceName} = new {classNameWithTypeParameters}({parameters});");
         }
 
         codeBoard.Constructor = constructor;

src/M31.FluentApi.Generator/CodeGeneration/CodeBoardElements/BuilderAndTargetInfo.cs

@@ -1,4 +1,5 @@
 using M31.FluentApi.Generator.Commons;
+using M31.FluentApi.Generator.SourceGenerators;
 using M31.FluentApi.Generator.SourceGenerators.Generics;
 
 namespace M31.FluentApi.Generator.CodeGeneration.CodeBoardElements;
@@ -11,7 +12,7 @@ internal BuilderAndTargetInfo(
         GenericInfo? genericInfo,
         bool fluentApiTypeIsStruct,
         bool fluentApiTypeIsInternal,
-        bool fluentApiTypeHasPrivateConstructor,
+        ConstructorInfo fluentApiTypeConstructorInfo,
         string builderClassName)
     {
         Namespace = @namespace;
@@ -21,7 +22,7 @@ internal BuilderAndTargetInfo(
         FluentApiTypeIsStruct = fluentApiTypeIsStruct;
         FluentApiTypeIsInternal = fluentApiTypeIsInternal;
         DefaultAccessModifier = fluentApiTypeIsInternal ? "internal" : "public";
-        FluentApiTypeHasPrivateConstructor = fluentApiTypeHasPrivateConstructor;
+        FluentApiTypeConstructorInfo = fluentApiTypeConstructorInfo;
         BuilderClassName = builderClassName;
         BuilderClassNameWithTypeParameters = WithTypeParameters(builderClassName, genericInfo);
         BuilderInstanceName = builderClassName.FirstCharToLower();
@@ -36,7 +37,7 @@ internal BuilderAndTargetInfo(
     internal bool FluentApiTypeIsStruct { get; }
     internal bool FluentApiTypeIsInternal { get; }
     internal string DefaultAccessModifier { get; }
-    internal bool FluentApiTypeHasPrivateConstructor { get; }
+    internal ConstructorInfo FluentApiTypeConstructorInfo { get; }
     internal string BuilderClassName { get; }
     internal string BuilderClassNameWithTypeParameters { get; }
     internal string BuilderInstanceName { get; }

src/M31.FluentApi.Generator/CodeGeneration/CodeGenerator.cs

@@ -19,7 +19,7 @@ internal static CodeGeneratorResult GenerateCode(FluentApiClassInfo classInfo, C
                 classInfo.GenericInfo,
                 classInfo.IsStruct,
                 classInfo.IsInternal,
-                classInfo.HasPrivateConstructor,
+                classInfo.ConstructorInfo,
                 classInfo.BuilderClassName);
 
         CodeBoard codeBoard = CodeBoard.Create(

src/M31.FluentApi.Generator/M31.FluentApi.Generator.csproj

@@ -11,7 +11,7 @@
     <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
     <SuppressDependenciesWhenPacking>true</SuppressDependenciesWhenPacking>
-    <PackageVersion>1.7.0</PackageVersion>
+    <PackageVersion>1.8.0</PackageVersion>
     <Authors>Kevin Schaal</Authors>
     <Description>The generator package for M31.FluentAPI. Don't install this package explicitly, install M31.FluentAPI instead.</Description>
     <PackageTags>fluentapi fluentbuilder fluentinterface fluentdesign fluent codegeneration</PackageTags>

src/M31.FluentApi.Generator/SourceAnalyzers/FluentApiAnalyzer.cs

@@ -66,11 +66,6 @@ private void AnalyzeNodeInternal(SyntaxNodeAnalysisContext context)
             return;
         }
 
-        if (!symbol.InstanceConstructors.Any(m => m.Parameters.Length == 0))
-        {
-            context.ReportDiagnostic(MissingDefaultConstructor.CreateDiagnostic(symbol));
-        }
-
         ClassInfoResult classInfoResult =
             ClassInfoFactory.CreateFluentApiClassInfo(
                 context.SemanticModel,

src/M31.FluentApi.Generator/SourceAnalyzers/FluentApiDiagnostics.cs

@@ -21,7 +21,6 @@ internal static class FluentApiDiagnostics
         InvalidFluentPredicateType.Descriptor,
         InvalidFluentNullableType.Descriptor,
         FluentNullableTypeWithoutNullableAnnotation.Descriptor,
-        MissingDefaultConstructor.Descriptor,
         CodeGenerationException.Descriptor,
         GenericException.Descriptor,
         OrthogonalAttributeMisusedWithCompound.Descriptor,
@@ -33,6 +32,7 @@ internal static class FluentApiDiagnostics
         ReservedMethodName.Descriptor,
         FluentLambdaMemberWithoutFluentApi.Descriptor,
         LastBuilderStepCannotBeSkipped.Descriptor,
+        AmbiguousConstructors.Descriptor,
     };
 
     internal static class MissingSetAccessor
@@ -195,23 +195,6 @@ internal static Diagnostic CreateDiagnostic(TypeSyntax actualType)
         }
     }
 
-    internal static class MissingDefaultConstructor
-    {
-        internal static readonly DiagnosticDescriptor Descriptor = new DiagnosticDescriptor(
-            id: "M31FA011",
-            title: "Default constructor is missing",
-            messageFormat: "The fluent API requires a default constructor. " +
-                           "Add a default constructor to type '{0}'.",
-            category: "M31.Usage",
-            defaultSeverity: DiagnosticSeverity.Error,
-            isEnabledByDefault: true);
-
-        internal static Diagnostic CreateDiagnostic(INamedTypeSymbol symbol)
-        {
-            return Diagnostic.Create(Descriptor, symbol.Locations[0], symbol.Name);
-        }
-    }
-
     /// <summary>
     /// Diagnostic used for <see cref="GenerationException"/>s.
     /// </summary>
@@ -409,4 +392,21 @@ internal static Diagnostic CreateDiagnostic(AttributeDataExtended attributeData)
             return Diagnostic.Create(Descriptor, location);
         }
     }
+
+    internal static class AmbiguousConstructors
+    {
+        internal static readonly DiagnosticDescriptor Descriptor = new DiagnosticDescriptor(
+            id: "M31FA024",
+            title: "Constructors are ambiguous",
+            messageFormat: "The fluent API creates instances by invoking the constructor with the fewest parameters " +
+                           "with default values. Found more than one constructor with {0} parameter(s).",
+            category: "M31.Usage",
+            defaultSeverity: DiagnosticSeverity.Error,
+            isEnabledByDefault: true);
+
+        internal static Diagnostic CreateDiagnostic(IMethodSymbol constructorSymbol, int numberOfParameters)
+        {
+            return Diagnostic.Create(Descriptor, constructorSymbol.Locations[0], numberOfParameters);
+        }
+    }
 }

src/M31.FluentApi.Generator/SourceGenerators/ClassInfoFactory.cs

@@ -1,3 +1,4 @@
+using M31.FluentApi.Generator.Commons;
 using M31.FluentApi.Generator.SourceGenerators.AttributeElements;
 using M31.FluentApi.Generator.SourceGenerators.AttributeInfo;
 using M31.FluentApi.Generator.SourceGenerators.Generics;
@@ -85,7 +86,7 @@ private ClassInfoResult CreateFluentApiClassInfoInternal(
         string className = type.Name;
         string? @namespace = type.ContainingNamespace.IsGlobalNamespace ? null : type.ContainingNamespace.ToString();
         bool isInternal = type.DeclaredAccessibility == Accessibility.Internal;
-        bool hasPrivateConstructor = HasPrivateConstructor(type);
+        ConstructorInfo? constructorInfo = TryGetConstructorInfo(type);
         FluentApiAttributeInfo fluentApiAttributeInfo =
             FluentApiAttributeInfo.Create(attributeDataExtended.AttributeData, className);
 
@@ -114,25 +115,54 @@ private ClassInfoResult CreateFluentApiClassInfoInternal(
             genericInfo,
             isStruct,
             isInternal,
-            hasPrivateConstructor,
+            constructorInfo!,
             fluentApiAttributeInfo.BuilderClassName,
             newLineString,
             infos,
             usingStatements,
             new FluentApiClassAdditionalInfo(groups));
     }
 
-    private bool HasPrivateConstructor(INamedTypeSymbol type)
+    private ConstructorInfo? TryGetConstructorInfo(INamedTypeSymbol type)
     {
-        IMethodSymbol[] defaultInstanceConstructors =
-            type.InstanceConstructors.Where(c => c.Parameters.Length == 0).ToArray();
+        /* Look for the default constructor. If it is not present, take the constructor
+           with the fewest parameters that is explicitly declared. */
+
+#pragma warning disable RS1024
+        IGrouping<int, IMethodSymbol>[] constructorsGroupedByNumberOfParameters =
+            type.InstanceConstructors
+                .Where(c => c.Parameters.Length == 0 || !c.IsImplicitlyDeclared)
+                .GroupBy(c => c.Parameters.Length)
+                .OrderBy(g => g.Key)
+                .ToArray();
+#pragma warning restore RS1024
+
+        IGrouping<int, IMethodSymbol>? constructorsWithFewestParameters =
+            constructorsGroupedByNumberOfParameters.FirstOrDefault();
+
+        if (constructorsWithFewestParameters == null)
+        {
+            throw new GenerationException(
+                $"The type {type.Name} has neither a default constructor nor explicitly declared constructors.");
+        }
 
-        if (defaultInstanceConstructors.Length == 0)
+        IMethodSymbol[] constructors = constructorsWithFewestParameters.ToArray();
+
+        if (constructors.Length != 1)
         {
-            return false;
+            int nofParameters = constructorsWithFewestParameters.Key;
+
+            foreach (IMethodSymbol constructor in constructors)
+            {
+                report.ReportDiagnostic(AmbiguousConstructors.CreateDiagnostic(constructor, nofParameters));
+            }
+
+            return null;
         }
 
-        return !defaultInstanceConstructors.Any(c => c.DeclaredAccessibility == Accessibility.Public);
+        return new ConstructorInfo(
+            constructors[0].Parameters.Length,
+            constructors[0].DeclaredAccessibility != Accessibility.Public);
     }
 
     private FluentApiInfo? TryCreateFluentApiInfo(ISymbol symbol)

src/M31.FluentApi.Generator/SourceGenerators/ConstructorInfo.cs

@@ -0,0 +1,13 @@
+namespace M31.FluentApi.Generator.SourceGenerators;
+
+internal record ConstructorInfo
+{
+    public ConstructorInfo(int numberOfParameters, bool constructorIsNonPublic)
+    {
+        NumberOfParameters = numberOfParameters;
+        ConstructorIsNonPublic = constructorIsNonPublic;
+    }
+
+    internal int NumberOfParameters { get; }
+    internal bool ConstructorIsNonPublic { get; }
+}

src/M31.FluentApi.Generator/SourceGenerators/FluentApiClassInfo.cs

@@ -16,7 +16,7 @@ internal FluentApiClassInfo(
         GenericInfo? genericInfo,
         bool isStruct,
         bool isInternal,
-        bool hasPrivateConstructor,
+        ConstructorInfo constructorInfo,
         string builderClassName,
         string newLineString,
         IReadOnlyCollection<FluentApiInfo> fluentApiInfos,
@@ -28,7 +28,7 @@ internal FluentApiClassInfo(
         GenericInfo = genericInfo;
         IsStruct = isStruct;
         IsInternal = isInternal;
-        HasPrivateConstructor = hasPrivateConstructor;
+        ConstructorInfo = constructorInfo;
         BuilderClassName = builderClassName;
         NewLineString = newLineString;
         FluentApiInfos = fluentApiInfos;
@@ -41,7 +41,7 @@ internal FluentApiClassInfo(
     internal GenericInfo? GenericInfo { get; }
     internal bool IsStruct { get; }
     internal bool IsInternal { get; }
-    internal bool HasPrivateConstructor { get; }
+    internal ConstructorInfo ConstructorInfo { get; }
     internal string BuilderClassName { get; }
     internal string NewLineString { get; }
     internal IReadOnlyCollection<FluentApiInfo> FluentApiInfos { get; }
@@ -57,7 +57,7 @@ public bool Equals(FluentApiClassInfo? other)
                Equals(GenericInfo, other.GenericInfo) &&
                IsStruct == other.IsStruct &&
                IsInternal == other.IsInternal &&
-               HasPrivateConstructor == other.HasPrivateConstructor &&
+               ConstructorInfo.Equals(other.ConstructorInfo) &&
                BuilderClassName == other.BuilderClassName &&
                NewLineString == other.NewLineString &&
                FluentApiInfos.SequenceEqual(other.FluentApiInfos) &&
@@ -76,7 +76,7 @@ public override int GetHashCode()
     {
         return new HashCode()
             .Add(Name, Namespace, GenericInfo)
-            .Add(IsStruct, IsInternal, HasPrivateConstructor)
+            .Add(IsStruct, IsInternal, ConstructorInfo)
             .Add(BuilderClassName)
             .Add(NewLineString)
             .AddSequence(FluentApiInfos)

src/M31.FluentApi.Tests/AnalyzerAndCodeFixes/AnalyzerAndCodeFixTests.cs

@@ -12,6 +12,22 @@ namespace M31.FluentApi.Tests.AnalyzerAndCodeFixes;
 
 public class AnalyzerAndCodeFixTests
 {
+    [Fact]
+    public async Task CanDetectAmbiguousConstructors()
+    {
+        SourceWithFix source = ReadSource("AmbiguousConstructorsClass", "Student");
+
+        var expectedDiagnostic1 = Verifier.Diagnostic(AmbiguousConstructors.Descriptor.Id)
+            .WithLocation(10, 12)
+            .WithArguments(1);
+
+        var expectedDiagnostic2 = Verifier.Diagnostic(AmbiguousConstructors.Descriptor.Id)
+            .WithLocation(15, 12)
+            .WithArguments(1);
+
+        await Verifier.VerifyCodeFixAsync(source, expectedDiagnostic1, expectedDiagnostic2);
+    }
+
     [Fact]
     public async Task CanDetectConflictingControlAttributes1()
     {
@@ -188,18 +204,6 @@ public async Task CanDetectMissingBuilderStep()
         await Verifier.VerifyCodeFixAsync(source, expectedDiagnostic);
     }
 
-    [Fact]
-    public async Task CanDetectMissingDefaultConstructor()
-    {
-        SourceWithFix source = ReadSource("MissingDefaultConstructorClass", "Student");
-
-        var expectedDiagnostic = Verifier.Diagnostic(MissingDefaultConstructor.Descriptor.Id)
-            .WithLocation(8, 14)
-            .WithArguments("Student");
-
-        await Verifier.VerifyCodeFixAsync(source, expectedDiagnostic);
-    }
-
     [Fact]
     public async Task CanDetectNullableTypeNoNullableAnnotation()
     {