Merge pull request #626 from SteveDunn/fix-efcore

Fix EFCore code generation and allow the users marker method be either internal or public

docs/site/Writerside/topics/how-to/efcore-tips.md

@@ -9,7 +9,8 @@ Thank you to [@jeffward01](https://github.com/jeffward01) for this item.
 The goal of this is to identify types that are generated by Vogen.
 
 ### Use Case
-I use Vogen alongside EfCore, I like to programmatically add ValueConverters by convention, I need to identity which properties on my entities are Vogen Generated value objects.
+I use Vogen alongside EfCore, I like to programmatically add `ValueConverter`s by convention,
+I need to identity which properties on my entities are generated value objects.
 
 ### Solution
 Vogen decorates the source it generates with the `GeneratedCodeAttribute`. This provides metadata about the tool which generated the code, this is what we'll use as an identifier.
@@ -32,12 +33,15 @@ internal static class AttributeHelper
 {
     public static bool IsVogenValueObject(this Type targetType)
     {
-        Maybe<GeneratedCodeAttribute> generatedCodeAttribute = targetType.GetClassAttribute<GeneratedCodeAttribute>();
-        return generatedCodeAttribute.HasValue && generatedCodeAttribute.Value.Tool == "Vogen";
+        Maybe<GeneratedCodeAttribute> generatedCodeAttribute = 
+            targetType.GetClassAttribute<GeneratedCodeAttribute>();
+
+        return generatedCodeAttribute.HasValue && 
+            generatedCodeAttribute.Value.Tool == "Vogen";
     }
 
-    private static Maybe<TAttribute> GetClassAttribute<TAttribute>(this Type targetType)
-        where TAttribute : Attribute
+    private static Maybe<TAttribute> GetClassAttribute<TAttribute>(
+        this Type targetType) where TAttribute : Attribute
     {
         return targetType.GetAttribute<TAttribute>();
     }
@@ -47,16 +51,17 @@ internal static class AttributeHelper
 Usage Example (From EfCore)
 
 ```c#
- foreach (IMutableEntityType entityType in builder.Model.GetEntityTypes())
+ foreach (IMutableEntityType et in builder.Model.GetEntityTypes())
         {
-            PropertyInfo[] properties = entityType.ClrType.GetProperties();
+            PropertyInfo[] properties = et.ClrType.GetProperties();
 
             foreach (PropertyInfo propertyInfo in properties)
             {
                 if (propertyInfo.PropertyType.IsVogenValueObject())
                 {
                      // Huzzah!
-                     // Do something with the property that is a value object generated by Vogen....
+                     // Do something with the property that is a value 
+                     // object generated by Vogen....
                 }
             }
         }
@@ -82,11 +87,14 @@ public class VogenStronglyTypedIdTests
     {
         Type vogenType = typeof(VogenStronglyTypedId);
 
-        Maybe<GeneratedCodeAttribute> generatedCodeAttribute = vogenType.GetClassAttribute<GeneratedCodeAttribute>();
+        Maybe<GeneratedCodeAttribute> generatedCodeAttribute = 
+            vogenType.GetClassAttribute<GeneratedCodeAttribute>();
         Assert.True(generatedCodeAttribute.HasValue);
-        GeneratedCodeAttribute? valueOfAttribute = generatedCodeAttribute.Value;
-        valueOfAttribute.Tool.Should()
-            .Be("Vogen");
+
+        GeneratedCodeAttribute? valueOfAttribute = 
+            generatedCodeAttribute.Value;
+
+        valueOfAttribute.Tool.Should().Be("Vogen");
     }
 
     [Fact]
@@ -117,26 +125,37 @@ Add all VOs that have EfCoreValueConverter to the `ModelConfigurationBuilder`:
 ```C#
 internal static class VogenExtensions
 {
-    public static void ApplyVogenEfConvertersFromAssembly(this ModelConfigurationBuilder configurationBuilder, Assembly assembly)
+    public static void ApplyVogenEfConvertersFromAssembly(
+        this ModelConfigurationBuilder configurationBuilder, 
+        Assembly assembly)
     {
         var types = assembly.GetTypes();
 
 		foreach (var type in types)
 		{
-			if (IsVogenValueObject(type) && TryGetEfValueConverter(type, out var efCoreConverterType))
+			if (IsVogenValueObject(type) && TryGetEfValueConverter(
+			    type, 
+			    out var efCoreConverterType))
 			{
-				configurationBuilder.Properties(type).HaveConversion(efCoreConverterType);
+				configurationBuilder
+				    .Properties(type)
+				    .HaveConversion(efCoreConverterType);
 			}
 		}
 	}
 
-	private static bool TryGetEfValueConverter(Type type, [NotNullWhen(true)]out Type? efCoreConverterType)
+	private static bool TryGetEfValueConverter(
+	    Type type, 
+	    [NotNullWhen(true)]out Type? efCoreConverterType)
 	{
 		var inner = type.GetNestedTypes();
 
 		foreach (var innerType in inner)
 		{
-			if (!typeof(ValueConverter).IsAssignableFrom(innerType) || !"EfCoreValueConverter".Equals(innerType.Name, StringComparison.Ordinal))
+			if (!typeof(ValueConverter).IsAssignableFrom(innerType) || 
+			    !"EfCoreValueConverter".Equals(
+			        innerType.Name, 
+			        StringComparison.Ordinal))
 			{
 				continue;
 			}
@@ -151,17 +170,23 @@ internal static class VogenExtensions
 
 	private static bool IsVogenValueObject(MemberInfo targetType)
 	{
-		var generatedCodeAttribute =targetType.GetCustomAttribute<GeneratedCodeAttribute>();
-		return "Vogen".Equals(generatedCodeAttribute?.Tool, StringComparison.Ordinal);
+		var generatedCodeAttribute = 
+		    targetType.GetCustomAttribute<GeneratedCodeAttribute>();
+
+		return "Vogen".Equals(
+		    generatedCodeAttribute?.Tool, 
+		    StringComparison.Ordinal);
 	}
 }
 ```
 
 Usage: In your `dbcontext`:
 ```C#
-protected override void ConfigureConventions(ModelConfigurationBuilder configurationBuilder)
+protected override void ConfigureConventions(
+    ModelConfigurationBuilder configurationBuilder)
 {
-    configurationBuilder.ApplyVogenEfConvertersFromAssembly(typeof(YOURDBCONTEXT).Assembly);
+    configurationBuilder.ApplyVogenEfConvertersFromAssembly(
+        typeof(YOURDBCONTEXT).Assembly);
 }
 ```
 

docs/site/Writerside/topics/reference/EfCoreIntegrationHowTo.md

@@ -1,6 +1,6 @@
 # Integration with Entity Framework Core
 
-It is possible to use value objects in EFCore.
+It is possible to use value objects (VOs) in EFCore.
 Using VO structs is straightforward, and no converter is required.
 Using VO classes requires generating a converter.
 
@@ -14,19 +14,22 @@ public partial class Name
 }
 ```
 
-Another way, if you're using .NET 8 or greater, is to use `EfCoreConverter` attributes:
+Another way, if you're using .NET 8 or greater, is to use `EfCoreConverter` attributes on
+a marker class:
 
 ```c#
 [EfCoreConverter<Domain.CustomerId>]
 [EfCoreConverter<Domain.CustomerName>]
-public partial class VogenEfCoreConverters;
+internal partial class VogenEfCoreConverters;
 ```
 
-This allows you to create the generator in a separate project,
-which is useful if you're using something like Onion architecture,
+The source generator augments this type with EFCore converters and comparers.
+This allows you to create these types in a separate project,
+which is useful if you're using something like Onion Architecture,
 where you don't want your domain objects to reference infrastructure code.
 
-Now the converters are generated, in your database context, you then specify the conversion:
+Now that the converters are generated by one of the approaches above, 
+in your database context, you can now specify the conversion:
 
 ```c#
     protected override void OnModelCreating(ModelBuilder builder)
@@ -39,6 +42,22 @@ Now the converters are generated, in your database context, you then specify the
     }
 ```
 
+`HasConversion` is an extension method that Vogen generates.
+
+Another approach, if you're using .NET 8 or greater, is to override `ConfigureConventions`
+
+```c#
+    protected override void ConfigureConventions(
+        ModelConfigurationBuilder configurationBuilder)
+    {
+        base.ConfigureConventions(configurationBuilder);
+
+        configurationBuilder.RegisterAllInVogenEfCoreConverters();
+    }
+```
+
+`RegisterAllInVogenEfCoreConverters` is a generated extension method.
+Note that the extension method's name after `RegisterAllIn` relates to the name of marker class.
 
 An [EFCore example is included in the source](https://github.com/SteveDunn/Vogen/tree/main/samples/Vogen.Examples/SerializationAndConversion/EFCore).
 
@@ -115,8 +134,6 @@ public static class __IdEfCoreExtensions
 
 For the `Id` field, because it's being used as a primary key, it needs to be a class because EFCore compares it to null to determine if it should be auto-generated. When it is null, EFCore will use the specified `SomeIdValueGenerator`, which looks like:
 
-
-
 ```c#
 internal class SomeIdValueGenerator : ValueGenerator<SomeId>
 {

samples/Onion/Infra/EfCoreScenario.cs

@@ -7,27 +7,27 @@
 namespace UsingTypesGeneratedInTheSameProject;
 
 /*
- * In this scenario, we want to use the System.Text.Json converters
- * for the value objects that were generated by Vogen in this project.
+ * In this scenario, we want to use the EFCore value converters and comparers
+ * for the value objects that were generated by Vogen in the Domain project.
  *
- * We create a `Supplier` which contains value objects, we serialize it to a string,
- * and we deserialize back into an Order.
+ * We create an `EfCoreConverters` class, and decorate it with attributes, where each attribute says which value object should have
+ * a converter generated for it.
  *
- * We use the `SupplierGenerationContext` below and tell it about `Supplier`. It then goes through its properties
- * and builds a mapping of converters.
- *
- * **NOTE** - because the value objects WERE BUILT IN THIS PROJECT, they are not 'fully formed', so we need to
- * tell System.Text.Json to use the 'type factory' that Vogen generates (Infra.VogenTypesFactory) to get its hints about
- * mapping types to converters.
+ * Then, we have two ways of registering all of these converters;
+ * 1. override `ConfigureConventions` and call `RegisterAllInEfCoreConverters`
+ * 2. call `HasVogenConversion` in `OnModelCreating`.
  */
 
 
+// By having this partial marker class, Vogen will generate converters for each value object mentioned in the attributes.
+// The naming of this class is later used to get the converters, or for when registering them via the generated extension
+// methods like `RegisterAllInEfCoreConverters` or `HasVogenConversion`
 [EfCoreConverter<Id>]
 [EfCoreConverter<Name>]
 [EfCoreConverter<Age>]
 [EfCoreConverter<Department>]
 [EfCoreConverter<HireDate>]
-public partial class EfCoreConverters;
+internal sealed partial class EfCoreConverters;
 
 public static class EfCoreScenario
 {
@@ -82,23 +82,34 @@ internal class DbContext : Microsoft.EntityFrameworkCore.DbContext
     //     return Math.Max(maxLocalId, maxSavedId) + 1;
     // }
 
+    protected override void ConfigureConventions(ModelConfigurationBuilder configurationBuilder)
+    {
+        base.ConfigureConventions(configurationBuilder);
+
+        // There are two ways of registering these, you can call the generated extension method here,
+        // or register the converters individually, like below in `OnModelCreating`.
+        configurationBuilder.RegisterAllInEfCoreConverters();
+    }
+
     protected override void OnModelCreating(ModelBuilder builder)
     {
         builder.Entity<EmployeeEntity>(b =>
         {
             b.HasKey(x => x.Id);
             b.Property(e => e.Id).HasValueGenerator<SomeIdValueGenerator>();
-            b.Property(e => e.Id).HasVogenConversion();
-            b.Property(e => e.Name).HasVogenConversion();
-            b.Property(e => e.Department).HasVogenConversion();
-            b.Property(e => e.HireDate).HasVogenConversion();
+
+            // There are two ways of registering these, you can do them inline here,
+            // or with the `RegisterAllIn[xxx]` like above in `ConfigureConventions`
+
+            // b.Property(e => e.Id).HasVogenConversion();
+            // b.Property(e => e.Name).HasVogenConversion();
+            // b.Property(e => e.Department).HasVogenConversion();
+            // b.Property(e => e.HireDate).HasVogenConversion();
         });
     }
 
-    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
-    {
+    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) => 
         optionsBuilder.UseInMemoryDatabase("SomeDB");
-    }
 }
 
 internal class SomeIdValueGenerator : ValueGenerator<Id>

samples/Onion/README.md

@@ -1,4 +1,4 @@
-This folder contains a small 'Onion'-like project consisting of two projects, Doman and Infra.
+This folder contains a small 'Onion'-like project consisting of two projects, Domain and Infra.
 
 It demonstrates using value objects that were created in the Domain layer, in the Infra(structure) layer.
 

samples/Vogen.Examples/SerializationAndConversion/EFCore/EfCoreExamples.cs

@@ -14,7 +14,7 @@ public class EfCoreExamples : IScenario
         public string GetDescription() => """
                                           Uses value objects in EF Core. 
                                           * It creates DB contexts and adds values to it, and saves
-                                          * It then create another context and lists the items
+                                          * It then creates another context and lists the items
 
                                           It demonstrates:
                                           * how to use value objects in a model

src/Vogen/BuildEfCoreConverterSpecsFromAttributes.cs

@@ -11,7 +11,7 @@ namespace Vogen;
 internal static class BuildEfCoreConverterSpecsFromAttributes
 {
 
-    public static EfCoreConverterSpecResult? TryBuild(AttributeData att, INamedTypeSymbol? symbol)
+    public static EfCoreConverterSpecResult? TryBuild(AttributeData att, in INamedTypeSymbol? markerClassSymbol)
     {
         ImmutableArray<TypedConstant> args = att.ConstructorArguments;
 
@@ -28,16 +28,23 @@ internal static class BuildEfCoreConverterSpecsFromAttributes
             return null;
         }
 
-        ImmutableArray<ITypeSymbol> t = att.AttributeClass.TypeArguments;
+        ImmutableArray<ITypeSymbol> argumentSymbols = att.AttributeClass.TypeArguments;
 
-        if (t.Length != 1) return null;
+        if (argumentSymbols.Length != 1)
+        {
+            return null;
+        }
 
-        var voSymbol = t[0] as INamedTypeSymbol;
-        if (voSymbol is null) return null;
+        var voSymbol = argumentSymbols[0] as INamedTypeSymbol;
+
+        if (voSymbol is null)
+        {
+            return null;
+        }
 
         if (!VoFilter.IsTarget(voSymbol))
         {
-            return EfCoreConverterSpecResult.Error(DiagnosticsCatalogue.EfCoreTargetMustBeAVo(symbol!, voSymbol));
+            return EfCoreConverterSpecResult.Error(DiagnosticsCatalogue.EfCoreTargetMustBeAVo(markerClassSymbol!, voSymbol));
         }
 
         List<AttributeData> attrs = VoFilter.TryGetValueObjectAttributes(voSymbol).ToList();
@@ -59,7 +66,7 @@ internal static class BuildEfCoreConverterSpecsFromAttributes
             return EfCoreConverterSpecResult.Error(DiagnosticsCatalogue.VoMustExplicitlySpecifyPrimitiveToBeAnEfCoreTarget(voSymbol));
         }
 
-        return EfCoreConverterSpecResult.Ok(voSymbol, underlyingType, symbol!);
+        return EfCoreConverterSpecResult.Ok(voSymbol, underlyingType, markerClassSymbol!);
     }
 
     private static INamedTypeSymbol? ResolveUnderlyingType(INamedTypeSymbol method)

src/Vogen/CompilationExtensions.cs

@@ -2,11 +2,15 @@
 using Microsoft.CodeAnalysis;
 using System.Collections.Generic;
 using System.Diagnostics;
+using Microsoft.CodeAnalysis.CSharp;
 
 namespace Vogen;
 
 internal static class CompilationExtensions
 {
+    public static bool IsAtLeastCSharpVersion(this Compilation compilation, LanguageVersion langVersion) => 
+        (compilation as CSharpCompilation)?.LanguageVersion >= langVersion;
+
     public static IEnumerable<INamedTypeSymbol> GetTypesByMetadataName(this Compilation compilation, string typeMetadataName)
     {
         var symbol = compilation.Assembly.GetTypeByMetadataName(typeMetadataName);

src/Vogen/Diagnostics/DiagnosticsCatalogue.cs

@@ -194,8 +194,8 @@ public static Diagnostic CustomExceptionMustHaveValidConstructor(INamedTypeSymbo
     public static Diagnostic VoMustExplicitlySpecifyPrimitiveToBeAnEfCoreTarget(INamedTypeSymbol symbol) => 
         Create(_efCoreTargetMustExplicitlySpecifyItsPrimitive, symbol.Locations, symbol.Name);
 
-    public static Diagnostic EfCoreTargetMustBeAVo(INamedTypeSymbol sourceSymbol, INamedTypeSymbol targetSymbol) => 
-        Create(_efCoreTargetMustBeAVo, sourceSymbol.Locations, sourceSymbol.Name, targetSymbol.Name);
+    public static Diagnostic EfCoreTargetMustBeAVo(INamedTypeSymbol markerClassSymbol, INamedTypeSymbol voSymbol) => 
+        Create(_efCoreTargetMustBeAVo, voSymbol.Locations, markerClassSymbol.Name, voSymbol.Name);
 
     private static DiagnosticDescriptor CreateDescriptor(string code, string title, string messageFormat, DiagnosticSeverity severity = DiagnosticSeverity.Error)
     {