Merge pull request #721 from SteveDunn/cast-operator-bug

Feature: more detailed compilation errors when both explicit and implicit casting operators are specific
SteveDunn · Dec 3, 2024 · f6d4c9b · f6d4c9b
2 parents 8356bd6 + a08d800
commit f6d4c9b
Show file tree

Hide file tree

Showing 15 changed files with 1,001 additions and 25 deletions.
diff --git a/src/Vogen/BuildWorkItems.cs b/src/Vogen/BuildWorkItems.cs
@@ -75,6 +75,12 @@ internal static class BuildWorkItems
             globalConfig,
             funcForDefaultUnderlyingType: () => vogenKnownSymbols.Int32);
 
+        if (DuplicateCastOperatorsSpecified(config))
+        {
+            context.ReportDiagnostic(DiagnosticsCatalogue.BothImplicitAndExplicitCastsSpecified(target.VoSymbolInformation));
+            return null;
+        }
+
         ReportErrorIfNestedType(context, voSymbolInformation, target.NestingInfo);
 
         if (config.UnderlyingType is null)
@@ -157,6 +163,18 @@ RecordDeclarationSyntax rds when rds.IsKind(SyntaxKind.RecordStructDeclaration)
         };
     }
 
+    private static bool DuplicateCastOperatorsSpecified(VogenConfiguration config)
+    {
+        var sag = config.StaticAbstractsGeneration;
+        var explicitFromPrimitive = config.FromPrimitiveCasting == CastOperator.Explicit || sag.HasFlag(StaticAbstractsGeneration.ExplicitCastFromPrimitive);
+        var implicitFromPrimitive = config.FromPrimitiveCasting == CastOperator.Implicit || sag.HasFlag(StaticAbstractsGeneration.ImplicitCastFromPrimitive);
+
+        var explicitToPrimitive = config.ToPrimitiveCasting == CastOperator.Explicit || sag.HasFlag(StaticAbstractsGeneration.ExplicitCastToPrimitive);
+        var implicitToPrimitive = config.ToPrimitiveCasting == CastOperator.Implicit || sag.HasFlag(StaticAbstractsGeneration.ImplicitCastToPrimitive);
+
+        return (explicitFromPrimitive && implicitFromPrimitive) || (explicitToPrimitive && implicitToPrimitive);
+    }
+
     private static bool ShouldShowNullAnnotations(Compilation compilation, VoTarget target)
     {
         SemanticModel sm = compilation.GetSemanticModel(target.VoSyntaxInformation.SyntaxTree);

diff --git a/src/Vogen/Diagnostics/DiagnosticsCatalogue.cs b/src/Vogen/Diagnostics/DiagnosticsCatalogue.cs
@@ -114,6 +114,11 @@ internal static class DiagnosticsCatalogue
         "Invalid custom exception",
         "{0} must have at least 1 public constructor with 1 parameter of type System.String");
 
+    private static readonly DiagnosticDescriptor _bothImplicitAndExplicitCastsSpecified = CreateDescriptor(
+        RuleIdentifiers.BothImplicitAndExplicitCastsSpecified,
+        "Both implicit and explicit casts specified",
+        "'{0}' should have either an explicit or implicit cast for casting to or from the wrapper or primitive, but not both. Check that the global config isn't specifying a conflicting casting operator. Check 'toPrimitiveCasting', 'fromPrimitiveCasting', and 'staticAbstractsGeneration'. 'staticAbstractGeneration' defaults to explicit casting, so if you change the default, you need to change it here too. See issue 720 (https://github.com/SteveDunn/Vogen/issues/720) for more information.");
+
     private static readonly DiagnosticDescriptor _voReferencedInAConversionMarkerMustExplicitlySpecifyPrimitive = CreateDescriptor(
         RuleIdentifiers.VoReferencedInAConversionMarkerMustExplicitlySpecifyPrimitive,
         "Value objects that are referenced in a conversion marker attribute must explicitly specify the primitive type",
@@ -194,6 +199,9 @@ public static Diagnostic VoReferencedInAConversionMarkerMustExplicitlySpecifyPri
             voSymbol.Name,
             markerClassSymbol.Name);
 
+    public static Diagnostic BothImplicitAndExplicitCastsSpecified(INamedTypeSymbol voSymbol) =>
+        Create(_bothImplicitAndExplicitCastsSpecified, voSymbol.Locations, voSymbol.Name); 
+
     public static Diagnostic TypesReferencedInAConversionMarkerMustBeaValueObjects(INamedTypeSymbol markerClassSymbol, INamedTypeSymbol voSymbol) => 
         Create(_typesReferencedInAConversionMarkerMustBeaValueObjects, voSymbol.Locations, markerClassSymbol.Name, voSymbol.Name);
 

diff --git a/src/Vogen/Diagnostics/RuleIdentifiers.cs b/src/Vogen/Diagnostics/RuleIdentifiers.cs
@@ -42,4 +42,5 @@ public static class RuleIdentifiers
     public const string UseReadonlyStructInsteadOfStruct = "VOG033";
     public const string DoNotCompareWithPrimitivesInEfCore = "VOG034";
     public const string VoReferencedInAConversionMarkerMustExplicitlySpecifyPrimitive = "VOG035";
+    public const string BothImplicitAndExplicitCastsSpecified = "VOG036";
 }
diff --git a/src/Vogen/GenerateCodeForCastingOperators.cs b/src/Vogen/GenerateCodeForCastingOperators.cs
@@ -1,54 +1,59 @@
 using System.Text;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Vogen.Generators;
 
 namespace Vogen;
 
 public static class GenerateCodeForCastingOperators
 {
-    public static string GenerateImplementations(VoWorkItem item, TypeDeclarationSyntax tds)
+    public static string GenerateImplementations(GenerationParameters p, TypeDeclarationSyntax tds)
     {
-        var className = tds.Identifier;
-        var itemUnderlyingType = item.UnderlyingTypeFullName;
+        var item = p.WorkItem;
+        var wrapper = tds.Identifier;
+        var primitive = item.UnderlyingTypeFullName;
 
         string primitiveBang = item.Nullable.BangForUnderlying;
 
         StringBuilder sb = new();
 
-        if (item.Config.FromPrimitiveCasting == CastOperator.Explicit)
+        var config = item.Config;
+        var sag = config.StaticAbstractsGeneration;
+
+        if (config.FromPrimitiveCasting == CastOperator.Explicit || sag.HasFlag(StaticAbstractsGeneration.ExplicitCastFromPrimitive))
         {
-            sb.AppendLine($"public static explicit operator {className}({itemUnderlyingType} value) => From(value);");
+            sb.AppendLine($"public static explicit operator {wrapper}({primitive} value) => From(value);");
         }
 
         // Generate the call to the Value property so that it throws if uninitialized.
-        if (item.Config.ToPrimitiveCasting == CastOperator.Explicit)
+        if (config.ToPrimitiveCasting == CastOperator.Explicit || sag.HasFlag(StaticAbstractsGeneration.ExplicitCastToPrimitive))
         {
-            sb.AppendLine($"public static explicit operator {itemUnderlyingType}({className} value) => value.Value;");
+            sb.AppendLine($"public static explicit operator {primitive}({wrapper} value) => value.Value;");
         }
 
         // Generate the call to the _value field so that it doesn't throw if uninitialized.
-        if (item.Config.ToPrimitiveCasting == CastOperator.Implicit)
+        if (config.ToPrimitiveCasting == CastOperator.Implicit || sag.HasFlag(StaticAbstractsGeneration.ImplicitCastToPrimitive))
         {
-            sb.AppendLine($"public static implicit operator {itemUnderlyingType}({className} vo) => vo._value{primitiveBang};");
+            sb.AppendLine($"public static implicit operator {primitive}({wrapper} vo) => vo._value{primitiveBang};");
         }
 
-        if (item.Config.FromPrimitiveCasting == CastOperator.Implicit)
+        if (config.FromPrimitiveCasting == CastOperator.Implicit || sag.HasFlag(StaticAbstractsGeneration.ImplicitCastFromPrimitive))
         {
             if (item.NormalizeInputMethod is not null)
             {
                 sb.AppendLine($$"""
-                                public static implicit operator {{className}}({{itemUnderlyingType}} value) 
+                                public static implicit operator {{wrapper}}({{primitive}} value) 
                                 {
-                                    return new {{className}}({{className}}.NormalizeInput(value));
+                                    return new {{wrapper}}({{wrapper}}.NormalizeInput(value));
                                 }
                                 """);
 
             }
             else
             {
                 sb.AppendLine($$"""
-                                public static implicit operator {{className}}({{itemUnderlyingType}} value) 
+                                public static implicit operator {{wrapper}}({{primitive}} value) 
                                 {
-                                  return new {{className}}(value);
+                                  return new {{wrapper}}(value);
                                 }
                                 """);
             }

diff --git a/src/Vogen/Generators/ClassGenerator.cs b/src/Vogen/Generators/ClassGenerator.cs
@@ -123,7 +123,7 @@ string GenerateCode() => $@"
         public static global::System.Boolean operator !=({className}{wrapperQ} left, {className}{wrapperQ} right) => !Equals(left, right);
         {GenerateCodeForEqualsMethodsAndOperators.GenerateEqualsOperatorsForPrimitivesIfNeeded(itemUnderlyingType, className, item)}
 
-        {GenerateCodeForCastingOperators.GenerateImplementations(item,tds)}{Util.GenerateGuidFactoryMethodIfNeeded(item)}
+        {GenerateCodeForCastingOperators.GenerateImplementations(parameters,tds)}{Util.GenerateGuidFactoryMethodIfNeeded(item)}
         {GenerateCodeForComparables.GenerateIComparableImplementationIfNeeded(item, tds)}
 
         {GenerateCodeForTryParse.GenerateAnyHoistedTryParseMethods(item)}{GenerateCodeForParse.GenerateAnyHoistedParseMethods(item)}

diff --git a/src/Vogen/Generators/RecordClassGenerator.cs b/src/Vogen/Generators/RecordClassGenerator.cs
@@ -127,7 +127,7 @@ string GenerateCode() => $@"
         {GenerateCodeForEqualsMethodsAndOperators.GenerateEqualsMethodsForAClass(item, tds)}
 {GenerateCodeForEqualsMethodsAndOperators.GenerateEqualsOperatorsForPrimitivesIfNeeded(itemUnderlyingType, wrapperName, item)}
 
-{GenerateCodeForCastingOperators.GenerateImplementations(item,tds)}{Util.GenerateGuidFactoryMethodIfNeeded(item)}
+{GenerateCodeForCastingOperators.GenerateImplementations(parameters,tds)}{Util.GenerateGuidFactoryMethodIfNeeded(item)}
         {GenerateCodeForComparables.GenerateIComparableImplementationIfNeeded(item, tds)}
 
         {GenerateCodeForTryParse.GenerateAnyHoistedTryParseMethods(item)}{GenerateCodeForParse.GenerateAnyHoistedParseMethods(item)}

diff --git a/src/Vogen/Generators/RecordStructGenerator.cs b/src/Vogen/Generators/RecordStructGenerator.cs
@@ -113,7 +113,7 @@ public readonly {itemUnderlyingType} Value
 {Util.GenerateIsInitializedMethod(true, item)}
 
 {GenerateCodeForStringComparers.GenerateIfNeeded(item, tds)}        
-{GenerateCodeForCastingOperators.GenerateImplementations(item,tds)}{Util.GenerateGuidFactoryMethodIfNeeded(item)}
+{GenerateCodeForCastingOperators.GenerateImplementations(parameters,tds)}{Util.GenerateGuidFactoryMethodIfNeeded(item)}
         // only called internally when something has been deserialized into
         // its primitive type.
         private static {wrapperName} __Deserialize({itemUnderlyingType} value)

diff --git a/src/Vogen/Generators/StructGenerator.cs b/src/Vogen/Generators/StructGenerator.cs
@@ -105,7 +105,7 @@ public readonly {itemUnderlyingType} Value
 
 {GenerateCodeForStringComparers.GenerateIfNeeded(item, tds)}        
 
-{GenerateCodeForCastingOperators.GenerateImplementations(item,tds)}{Util.GenerateGuidFactoryMethodIfNeeded(item)}
+{GenerateCodeForCastingOperators.GenerateImplementations(parameters, tds)}{Util.GenerateGuidFactoryMethodIfNeeded(item)}
         // only called internally when something has been deserialized into
         // its primitive type.
         private static {structName} __Deserialize({itemUnderlyingType} value)

diff --git a/tests/AnalyzerTests/GlobalConfig/SadTests.cs b/tests/AnalyzerTests/GlobalConfig/SadTests.cs
@@ -9,6 +9,40 @@ namespace AnalyzerTests.GlobalConfig;
 
 public class SadTests
 {
+    [Fact]
+    public async Task Conflicting_casts()
+    {
+        var source = """
+            using Vogen;
+            
+            [assembly: VogenDefaults(
+            	toPrimitiveCasting: CastOperator.Implicit,
+            	staticAbstractsGeneration: StaticAbstractsGeneration.MostCommon)]
+            
+            namespace MyApp;
+            
+            [ValueObject<int>]
+            public readonly partial record struct ToDoItemId;
+            """;
+
+        await new TestRunner<ValueObjectGenerator>()
+            .WithSource(source)
+            .ValidateWith(Validate)
+            .RunOnAllFrameworks();
+        return;
+
+        static void Validate(ImmutableArray<Diagnostic> diagnostics)
+        {
+            diagnostics.Should().HaveCount(1);
+
+            Diagnostic diagnostic = diagnostics.Single();
+
+            diagnostic.Id.Should().Be("VOG036");
+            diagnostic.ToString().Should().Be(
+                "(10,39): error VOG036: 'ToDoItemId' should have either an explicit or implicit cast for casting to or from the wrapper or primitive, but not both. Check that the global config isn't specifying a conflicting casting operator. Check 'toPrimitiveCasting', 'fromPrimitiveCasting', and 'staticAbstractsGeneration'. 'staticAbstractGeneration' defaults to explicit casting, so if you change the default, you need to change it here too. See issue 720 (https://github.com/SteveDunn/Vogen/issues/720) for more information.");
+        }
+    }
+
     [Fact]
     public async Task Missing_any_constructors()
     {

diff --git a/tests/SnapshotTests/BugFixes/Bug720_Inconsistent_casting_mixed_with_IVogen_generation.cs b/tests/SnapshotTests/BugFixes/Bug720_Inconsistent_casting_mixed_with_IVogen_generation.cs
@@ -0,0 +1,37 @@
+using System.Threading.Tasks;
+using Shared;
+using Vogen;
+
+namespace SnapshotTests.BugFixes;
+
+// See https://github.com/SteveDunn/Vogen/issues/720
+public class Bug720_Inconsistent_casting_mixed_with_IVogen_generation
+{
+    [Fact]
+    public async Task Works_when_the_static_abstracts_and_implementation_have_same_casting()
+    {
+        // we say that to primitive is implicit and we say that the static abstract interface matches.
+        var source = """
+                     using Vogen;
+                     using static Vogen.StaticAbstractsGeneration;
+                     
+                     [assembly: VogenDefaults(
+                         toPrimitiveCasting: CastOperator.Implicit,
+                         staticAbstractsGeneration: ValueObjectsDeriveFromTheInterface |
+                                                    EqualsOperators |
+                                                    ExplicitCastFromPrimitive |
+                                                    ImplicitCastToPrimitive |
+                                                    FactoryMethods)]
+
+                     namespace MyApp;
+                     
+                     [ValueObject<int>]
+                     public readonly partial record struct ToDoItemId;
+                     """;
+
+        await new SnapshotRunner<ValueObjectGenerator>()
+                .WithSource(source)
+                .IgnoreInitialCompilationErrors()
+                .RunOn(TargetFramework.AspNetCore8_0);
+    }
+}