Singleton improvements (#4)

- Better warning for implicit public parameterless constructor
- Emit one warning per public constructor

.github/workflows/test_on_push.yaml

@@ -27,5 +27,5 @@ jobs:
               with:
                   dotnet-version: ${{ matrix.dotnet-version }}
             - run: dotnet restore
-#            - run: dotnet build --configuration Debug --no-restore
+#            - run: dotnet build --configuration Debug --no-restore # TODO: breaks Metalama in 2024.2.8-preview, reintroduce after fixed
             - run: dotnet test --configuration Release --no-restore

Moyou.Aspects/Moyou.Aspects.Singleton/SingletonAttribute.cs

@@ -31,11 +31,23 @@ public class SingletonAttribute : TypeAspect
     /// INamedType should be the relevant type, string should be a comma separated list of violating constructor
     /// signatures.
     /// </remarks>
-    private static readonly DiagnosticDefinition<(INamedType, string)> WarningHasAccessibleConstructors =
-        new(Warnings.Singleton.HasAccessibleConstructorsId, Severity.Warning,
-            Warnings.Singleton.HasAccessibleConstructorsMessageFormat,
-            Warnings.Singleton.HasAccessibleConstructorsTitle,
-            Warnings.Singleton.HasAccessibleConstructorsCategory);
+    private static readonly DiagnosticDefinition<(INamedType, string)> WarningHasAccessibleConstructor =
+        new(Warnings.Singleton.HasAccessibleConstructorId, Severity.Warning,
+            Warnings.Singleton.HasAccessibleConstructorMessageFormat,
+            Warnings.Singleton.HasAccessibleConstructorTitle,
+            Warnings.Singleton.HasAccessibleConstructorCategory);
+
+    /// <summary>
+    /// MOYOU1102
+    /// </summary>
+    /// <remarks>
+    /// INamedType should be the relevant type
+    /// </remarks>
+    private static readonly DiagnosticDefinition<INamedType> WarningHasImplicitPublicConstructor =
+        new(Warnings.Singleton.HasImplicitPublicConstructorId, Severity.Warning,
+            Warnings.Singleton.HasImplicitPublicConstructorMessageFormat,
+            Warnings.Singleton.HasImplicitPublicConstructorTitle,
+            Warnings.Singleton.HasImplicitPublicConstructorCategory);
 
     public override void BuildEligibility(IEligibilityBuilder<INamedType> builder)
     {
@@ -64,28 +76,36 @@ public override void BuildAspect(IAspectBuilder<INamedType> builder)
 
 
         // warning if there are any non-private constructors
-        if (builder.Target.Constructors.Any(constructor => constructor.Accessibility != Accessibility.Private))
+        var constructors = builder.Target.Constructors.ToList();
+        var first = constructors.First();
+        if (constructors.Count == 1 && first is { IsImplicitlyDeclared: true, Parameters.Count: 0, Accessibility: Accessibility.Public })
         {
-            var constructorSignaturesString = CollectConstructorSignaturesAsString(builder);
-            builder.Diagnostics.Report(
-                WarningHasAccessibleConstructors.WithArguments((builder.Target, constructorSignaturesString)));
+            ReportImplicitConstructor(builder);
+        }
+        else
+        {
+            foreach (var constructor in constructors.Where(constructor => constructor.Accessibility != Accessibility.Private))
+            {
+                ReportPublicConstructor(builder, constructor);
+            }
         }
 
         if (Lazy) GenerateLazyImplementation(builder);
         else GenerateNonLazyImplementation(builder);
     }
 
-    private static string CollectConstructorSignaturesAsString(IAspectBuilder<INamedType> builder)
+    private static void ReportImplicitConstructor(IAspectBuilder<INamedType> builder)
+    {
+        //special warning for implicit constructor
+        builder.Diagnostics.Report(WarningHasImplicitPublicConstructor.WithArguments(builder.Target), builder.Target);
+    }
+
+    private static void ReportPublicConstructor(IAspectBuilder<INamedType> builder, IConstructor constructor)
     {
-        var constructorSignatures = builder.Target.Constructors
-            .Where(constructor => constructor.Accessibility != Accessibility.Private)
-            .Select(constructor => constructor.Parameters)
-            .Select(parameters => parameters.Select(parameter => parameter.Type.ToDisplayString()))
-            .Select(stringList => string.Join(",", stringList))
-            .Select(str => string.IsNullOrWhiteSpace(str) ? "void" : str)
-            .Select(str => $"({str})");
-        var constructorSignaturesString = string.Join(", ", constructorSignatures);
-        return constructorSignaturesString;
+        var typeSignature = string.Join(", ", constructor.Parameters.Select(param => $"{param.Type.ToDisplayString()} {param.Name}"));
+        var signatureString = $"({typeSignature})";
+        builder.Diagnostics.Report(WarningHasAccessibleConstructor.WithArguments((builder.Target, signatureString)),
+            constructor);
     }
 
     private void GenerateLazyImplementation(IAspectBuilder<INamedType> builder)

Moyou.Diagnostics/Warnings.cs

@@ -38,11 +38,19 @@ public static class Singleton
     {
         // MOYOU1101: Singleton class {0} should have no accessible constructors.
         // Found constructors with signatures: {1}
-        public static string HasAccessibleConstructorsId => "MOYOU1101";
-        public static string HasAccessibleConstructorsMessageFormat =>
-            "Singleton class {0} should have no accessible constructors. Found constructors with signatures: {1}";
-        public static string HasAccessibleConstructorsTitle => "Singleton class should have no accessible constructors";
-        public static string HasAccessibleConstructorsCategory => "Singleton";
-
+        public static string HasAccessibleConstructorId => "MOYOU1101";
+        public static string HasAccessibleConstructorMessageFormat =>
+            "Singleton class {0} should have no accessible constructors. Found constructor with signature: {1}.";
+        public static string HasAccessibleConstructorTitle => "Singleton class should have no accessible constructors";
+        public static string HasAccessibleConstructorCategory => "Singleton";
+        public static string HasImplicitPublicConstructorId => "MOYOU1102";
+
+        public static string HasImplicitPublicConstructorMessageFormat =>
+            "Singleton class {0} should have no implicit public constructor. Consider defining your own private " +
+            "constructor to override the implicit public constructor.";
+
+        public static string? HasImplicitPublicConstructorTitle =>
+            "Singleton class should have no implicit public constructor";
+        public static string? HasImplicitPublicConstructorCategory => "Singleton";
     }
 }

Moyou.Test/Singleton/SingletonAttributeTests/HasImplicitPublicConstructor.t.cs

@@ -1,4 +1,4 @@
-// Warning MOYOU1101 on `HasImplicitPublicConstructor`: `Singleton class HasImplicitPublicConstructor should have no accessible constructors. Found constructors with signatures: (void)`
+// Warning MOYOU1102 on `HasImplicitPublicConstructor`: `Singleton class HasImplicitPublicConstructor should have no implicit public constructor. Consider defining your own private constructor to override the implicit public constructor.`
 using Moyou.Aspects.Singleton;
 namespace Moyou.CompileTimeTest.Singleton.SingletonAttributeTests;
 [Singleton]

Moyou.Test/Singleton/SingletonAttributeTests/HasPublicConstructors.cs

@@ -15,5 +15,10 @@ public HasPublicConstructors(int a)
         A = a;
     }
 
+    public HasPublicConstructors(int a, float b)
+    {
+
+    }
+
     public int A { get; set; }
 }

Moyou.Test/Singleton/SingletonAttributeTests/HasPublicConstructors.t.cs

@@ -1,4 +1,5 @@
-// Warning MOYOU1101 on `HasPublicConstructors`: `Singleton class HasPublicConstructors should have no accessible constructors. Found constructors with signatures: (int)`
+// Warning MOYOU1101 on `HasPublicConstructors`: `Singleton class HasPublicConstructors should have no accessible constructors. Found constructor with signature: (int a).`
+// Warning MOYOU1101 on `HasPublicConstructors`: `Singleton class HasPublicConstructors should have no accessible constructors. Found constructor with signature: (int a, float b).`
 using Moyou.Aspects.Singleton;
 namespace Moyou.CompileTimeTest.Singleton.SingletonAttributeTests;
 [Singleton]
@@ -12,6 +13,9 @@ public HasPublicConstructors(int a)
   {
     A = a;
   }
+  public HasPublicConstructors(int a, float b)
+  {
+  }
   public int A { get; set; }
   private static global::System.Lazy<global::Moyou.CompileTimeTest.Singleton.SingletonAttributeTests.HasPublicConstructors> _instance;
   static HasPublicConstructors()