Fix implicit conversion clash with required union properties (#127)

* Adds additional implicit conversions support check.
* Move implicit conversion support logic into union declaration.
* Clean up record declaration parameter vs property models.
* Update version and readme.

Readme.md

@@ -85,9 +85,10 @@ Console.WriteLine(output); // "12345".
 
 Dunet generates implicit conversions between union variants and the union type if your union meets all of the following conditions:
 
+- The union has no required properties.
 - All variants contain a single property.
 - Each variant's property is unique within the union.
-- No property is an interface type.
+- No variant's property is an interface type.
 
 For example, consider a `Result` union type that represents success as a `double` and failure as an `Exception`:
 

src/Dunet.csproj

@@ -14,10 +14,11 @@
         <PackageReadmeFile>Readme.md</PackageReadmeFile>
         <RepositoryUrl>https://github.com/domn1995/dunet</RepositoryUrl>
         <PackageTags>source; generator; discriminated; union; functional; tagged;</PackageTags>
-        <AssemblyVersion>1.7.0</AssemblyVersion>
-        <FileVersion>1.7.0</FileVersion>
+        <AssemblyVersion>1.7.1</AssemblyVersion>
+        <FileVersion>1.7.1</FileVersion>
         <PackageLicenseExpression>MIT</PackageLicenseExpression>
-        <PackageReleaseNotes>1.7.0 - Match on specific union variant.
+        <PackageReleaseNotes>1.7.1 - Disables implicit conversions when union has a required property.
+1.7.0 - Match on specific union variant.
 1.6.0 - Support generator cancellation.
 1.5.0 - Support async Action match functions.
 1.4.2 - Disables implicit conversions when union variant is an interface type.
@@ -43,7 +44,7 @@
         <RepositoryType>git</RepositoryType>
         <PackageIcon>favicon.png</PackageIcon>
         <SignAssembly>False</SignAssembly>
-        <Version>1.7.0</Version>
+        <Version>1.7.1</Version>
         <NeutralLanguage>en</NeutralLanguage>
         <DevelopmentDependency>true</DevelopmentDependency>
         <NoWarn>$(NoWarn);NU5128</NoWarn>

src/GenerateUnionRecord/RecordDeclarationSyntaxParser.cs

@@ -35,26 +35,52 @@ this RecordDeclarationSyntax record
         );
 
     /// <summary>
-    /// Gets the properties within this record declaration.
+    /// Gets the parameters in this record's primary constructor.
     /// </summary>
     /// <param name="record">This record declaration.</param>
     /// <param name="semanticModel">The semantic model associated with this record declaration.</param>
-    /// <returns>The sequence of properties, if any. Otherwise, <see langword="null"/>.</returns>
-    public static IEnumerable<Property>? GetProperties(
+    /// <returns>The sequence of parameters, if any. Otherwise, <see langword="null"/>.</returns>
+    public static IEnumerable<Parameter>? GetParameters(
         this RecordDeclarationSyntax record,
         SemanticModel semanticModel
     ) =>
         record.ParameterList?.Parameters.Select(
             parameter =>
-                new Property(
-                    Type: new PropertyType(
+                new Parameter(
+                    Type: new ParameterType(
                         Identifier: parameter.Type?.ToString() ?? "",
                         IsInterface: parameter.Type.IsInterfaceType(semanticModel)
                     ),
                     Identifier: parameter.Identifier.ToString()
                 )
         );
 
+    /// <summary>
+    /// Gets the properties declared in this record.
+    /// </summary>
+    /// <param name="record">This record declaration.</param>
+    /// <param name="semanticModel">The semantic model associated with this record declaration.</param>
+    /// <returns>The sequence of properties.</returns>
+    public static IEnumerable<Property> GetProperties(
+        this RecordDeclarationSyntax record,
+        SemanticModel semanticModel
+    ) =>
+        record.Members
+            .OfType<PropertyDeclarationSyntax>()
+            .Select(
+                propertyDeclaration =>
+                    new Property(
+                        Type: new PropertyType(
+                            Identifier: propertyDeclaration.Type.ToString(),
+                            IsInterface: propertyDeclaration.Type.IsInterfaceType(semanticModel)
+                        ),
+                        Identifier: propertyDeclaration.Identifier.ToString(),
+                        IsRequired: propertyDeclaration.Modifiers.Any(
+                            modifier => modifier.Value is "required"
+                        )
+                    )
+            );
+
     /// <summary>
     /// Gets the record declarations within this record declaration.
     /// </summary>
@@ -75,7 +101,7 @@ SemanticModel semanticModel
                     {
                         Identifier = nestedRecord.Identifier.ToString(),
                         TypeParameters = nestedRecord.GetTypeParameters()?.ToList() ?? new(),
-                        Properties = nestedRecord.GetProperties(semanticModel)?.ToList() ?? new()
+                        Parameters = nestedRecord.GetParameters(semanticModel)?.ToList() ?? new()
                     }
             );
 

src/GenerateUnionRecord/UnionDeclaration.cs

@@ -10,27 +10,60 @@ internal sealed record UnionDeclaration(
     List<TypeParameter> TypeParameters,
     List<TypeParameterConstraint> TypeParameterConstraints,
     List<VariantDeclaration> Variants,
-    Stack<ParentType> ParentTypes
+    Stack<ParentType> ParentTypes,
+    List<Property> Properties
 )
 {
     // Extension methods cannot be generated for a union declared in a top level program (no namespace).
     // It also doesn't make sense to generate Match extensions if there are no variants to match aginst.
     public bool SupportsAsyncMatchExtensionMethods() => Namespace is not null && Variants.Count > 0;
+
+    public bool SupportsImplicitConversions()
+    {
+        var allVariantsHaveSingleProperty = () =>
+            Variants.All(static variant => variant.Parameters.Count is 1);
+
+        var allVariantsHaveNoInterfaceParameters = () =>
+            Variants
+                .SelectMany(static variant => variant.Parameters)
+                .All(static property => !property.Type.IsInterface);
+
+        var allVariantsHaveUniquePropertyTypes = () =>
+        {
+            var allPropertyTypes = Variants
+                .SelectMany(static variant => variant.Parameters)
+                .Select(static property => property.Type);
+            var allPropertyTypesCount = allPropertyTypes.Count();
+            var uniquePropertyTypesCount = allPropertyTypes.Distinct().Count();
+            return allPropertyTypesCount == uniquePropertyTypesCount;
+        };
+
+        var hasNoRequiredProperties = () => !Properties.Any(property => property.IsRequired);
+
+        return allVariantsHaveSingleProperty()
+            && allVariantsHaveNoInterfaceParameters()
+            && allVariantsHaveUniquePropertyTypes()
+            && hasNoRequiredProperties();
+    }
 }
 
 internal sealed record VariantDeclaration
 {
     public required string Identifier { get; init; }
     public required List<TypeParameter> TypeParameters { get; init; }
-    public required List<Property> Properties { get; init; }
+    public required List<Parameter> Parameters { get; init; }
 }
 
 internal sealed record TypeParameter(string Identifier)
 {
     public override string ToString() => Identifier;
 }
 
-internal sealed record Property(PropertyType Type, string Identifier);
+internal sealed record Parameter(ParameterType Type, string Identifier);
+
+internal sealed record Property(PropertyType Type, string Identifier, bool IsRequired);
+
+internal sealed record ParameterType(string Identifier, bool IsInterface);
 
 internal sealed record PropertyType(string Identifier, bool IsInterface);
 

src/GenerateUnionRecord/UnionGenerator.cs

@@ -111,6 +111,7 @@ CancellationToken cancellation
             var typeParameterConstraints = declaration.GetTypeParameterConstraints();
             var variants = declaration.GetNestedRecordDeclarations(semanticModel);
             var parentTypes = declaration.GetParentTypes(semanticModel);
+            var properties = declaration.GetProperties(semanticModel);
 
             yield return new UnionDeclaration(
                 Imports: imports.ToList(),
@@ -120,7 +121,8 @@ CancellationToken cancellation
                 TypeParameters: typeParameters?.ToList() ?? new(),
                 TypeParameterConstraints: typeParameterConstraints?.ToList() ?? new(),
                 Variants: variants.ToList(),
-                ParentTypes: parentTypes
+                ParentTypes: parentTypes,
+                Properties: properties.ToList()
             );
         }
     }

src/GenerateUnionRecord/UnionSourceBuilder.cs

@@ -38,14 +38,14 @@ public static string Build(UnionDeclaration union)
         builder.AppendAbstractMatchMethods(union);
         builder.AppendAbstractSpecificMatchMethods(union);
 
-        if (SupportsImplicitConversions(union))
+        if (union.SupportsImplicitConversions())
         {
             foreach (var variant in union.Variants)
             {
                 builder.Append($"    public static implicit operator {union.Name}");
                 builder.AppendTypeParams(union.TypeParameters);
                 builder.AppendLine(
-                    $"({variant.Properties[0].Type.Identifier} value) => new {variant.Identifier}(value);"
+                    $"({variant.Parameters[0].Type.Identifier} value) => new {variant.Identifier}(value);"
                 );
             }
             builder.AppendLine();
@@ -76,31 +76,6 @@ public static string Build(UnionDeclaration union)
         return builder.ToString();
     }
 
-    private static bool SupportsImplicitConversions(UnionDeclaration union)
-    {
-        var allVariantsHaveSingleProperty = () =>
-            union.Variants.All(static variant => variant.Properties.Count is 1);
-
-        var allVariantsHaveNoInterfaceParameters = () =>
-            union.Variants
-                .SelectMany(static variant => variant.Properties)
-                .All(static property => !property.Type.IsInterface);
-
-        var allVariantsHaveUniquePropertyTypes = () =>
-        {
-            var allPropertyTypes = union.Variants
-                .SelectMany(static variant => variant.Properties)
-                .Select(static property => property.Type);
-            var allPropertyTypesCount = allPropertyTypes.Count();
-            var uniquePropertyTypesCount = allPropertyTypes.Distinct().Count();
-            return allPropertyTypesCount == uniquePropertyTypesCount;
-        };
-
-        return allVariantsHaveSingleProperty()
-            && allVariantsHaveNoInterfaceParameters()
-            && allVariantsHaveUniquePropertyTypes();
-    }
-
     private static StringBuilder AppendAbstractMatchMethods(
         this StringBuilder builder,
         UnionDeclaration union

test/GenerateUnionRecord/GenerationTests.cs

@@ -176,4 +176,35 @@ partial record Failure(Exception Error);
         result.CompilationErrors.Should().BeEmpty();
         result.GenerationDiagnostics.Should().BeEmpty();
     }
+
+    [Fact]
+    public void UnionTypeMayHaveRequiredProperties()
+    {
+        // Arrange.
+        var programCs = """
+using Dunet;
+using System;
+
+Result result1 = new Result.Success(Guid.NewGuid()) { Name = "Success" };
+Result result2 = new Result.Failure(new Exception("Boom!")) { Name = "Failure" };
+
+var result1Name = result1.Name;
+var result2Name = result2.Name;
+
+[Union]
+partial record Result
+{
+    public required string Name { get; init; }
+    partial record Success(Guid Id);
+    partial record Failure(Exception Error);
+}
+""";
+        // Act.
+        var result = Compiler.Compile(programCs);
+
+        // Assert.
+        using var scope = new AssertionScope();
+        result.CompilationErrors.Should().BeEmpty();
+        result.GenerationDiagnostics.Should().BeEmpty();
+    }
 }