Skip to content

Commit b5162fa

Browse files
TimovzlTimovzl
committed
Enabled struct ValueObjects.
1 parent 149aaff commit b5162fa

8 files changed

Lines changed: 121 additions & 77 deletions

File tree

DomainModeling.Analyzer/Analyzers/WrapperValueObjectDefaultExpressionAnalyzer.cs renamed to DomainModeling.Analyzer/Analyzers/ValueObjectDefaultExpressionAnalyzer.cs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,15 @@
77
namespace Architect.DomainModeling.Analyzer.Analyzers;
88

99
/// <summary>
10-
/// Prevents the use of default expressions and literals on struct WrapperValueObject types, so that validation cannot be circumvented.
10+
/// Prevents the use of default expressions and literals on struct ValueObject and WrapperValueObject types, so that validation cannot be circumvented.
1111
/// </summary>
1212
[DiagnosticAnalyzer(LanguageNames.CSharp)]
13-
public sealed class WrapperValueObjectDefaultExpressionAnalyzer : DiagnosticAnalyzer
13+
public sealed class ValueObjectDefaultExpressionAnalyzer : DiagnosticAnalyzer
1414
{
1515
[System.Diagnostics.CodeAnalysis.SuppressMessage("CodeQuality", "IDE0079:Remove unnecessary suppression", Justification = "False positive.")]
1616
[System.Diagnostics.CodeAnalysis.SuppressMessage("MicrosoftCodeAnalysisReleaseTracking", "RS2008:Enable analyzer release tracking", Justification = "Not yet implemented.")]
1717
private static readonly DiagnosticDescriptor DiagnosticDescriptor = new DiagnosticDescriptor(
18-
id: "WrapperValueObjectDefaultExpression",
18+
id: "ValueObjectDefaultExpression",
1919
title: "Default expression instantiating unvalidated value object",
2020
messageFormat: "A 'default' expression would create an unvalidated instance of value object {0}. Use a parameterized constructor, or use IsDefault() to merely compare.",
2121
category: "Design",
@@ -48,8 +48,9 @@ private static void AnalyzeDefaultExpressionOrLiteral(SyntaxNodeAnalysisContext
4848
return;
4949

5050
// Only with WrapperValueObjectAttribute<TValue>
51-
if (!type.GetAttributes().Any(attr =>
52-
attr.AttributeClass is { Arity: 1, Name: "WrapperValueObjectAttribute", ContainingNamespace: { Name: "DomainModeling", ContainingNamespace: { Name: "Architect", ContainingNamespace.IsGlobalNamespace: true, } }, }))
51+
if (!type.GetAttributes().Any(attr => attr.AttributeClass is
52+
{ Arity: 0, Name: "ValueObjectAttribute", ContainingNamespace: { Name: "DomainModeling", ContainingNamespace: { Name: "Architect", ContainingNamespace.IsGlobalNamespace: true, } }, } or
53+
{ Arity: 1, Name: "WrapperValueObjectAttribute", ContainingNamespace: { Name: "DomainModeling", ContainingNamespace: { Name: "Architect", ContainingNamespace.IsGlobalNamespace: true, } }, }))
5354
return;
5455

5556
var diagnostic = Diagnostic.Create(

‎DomainModeling.Analyzer/Analyzers/ValueObjectMissingStringComparisonAnalyzer.cs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,16 @@ public sealed class ValueObjectMissingStringComparisonAnalyzer : DiagnosticAnaly
2929
public override void Initialize(AnalysisContext context)
3030
{
3131
context.EnableConcurrentExecution();
32-
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
33-
34-
context.RegisterSyntaxNodeAction(AnalyzeClassDeclaration,
32+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
33+
34+
context.RegisterSyntaxNodeAction(AnalyzeTypeDeclaration,
3535
SyntaxKind.ClassDeclaration,
36-
SyntaxKind.RecordDeclaration);
36+
SyntaxKind.StructDeclaration,
37+
SyntaxKind.RecordDeclaration,
38+
SyntaxKind.RecordStructDeclaration);
3739
}
3840

39-
private static void AnalyzeClassDeclaration(SyntaxNodeAnalysisContext context)
41+
private static void AnalyzeTypeDeclaration(SyntaxNodeAnalysisContext context)
4042
{
4143
var tds = (TypeDeclarationSyntax)context.Node;
4244

‎DomainModeling.Generator/ValueObjectGenerator.cs‎

Lines changed: 17 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,8 @@ private static bool FilterSyntaxNode(SyntaxNode node, CancellationToken cancella
6262

6363
var existingComponents = ValueObjectTypeComponents.None;
6464

65-
existingComponents |= ValueObjectTypeComponents.DefaultConstructor.If(type.InstanceConstructors.Any(ctor =>
66-
ctor.Parameters.Length == 0 /*&& ctor.DeclaringSyntaxReferences.Length > 0*/) ||
65+
existingComponents |= ValueObjectTypeComponents.DefaultConstructor.If(
66+
type.InstanceConstructors.Any(ctor => ctor.Parameters.Length == 0 && !ctor.ContainingType.IsValueType) ||
6767
!type.BasePermitsDefaultConstruction() || // Base offers no visible default ctor
6868
type.HasPrimaryConstructor()); // Type has a primary ctor
6969

@@ -181,14 +181,6 @@ private static void GenerateSource(SourceProductionContext context, (Generatable
181181
if (!generatable.IsPartial)
182182
return;
183183

184-
// Only if class
185-
if (!generatable.IsClass)
186-
{
187-
context.ReportDiagnostic("ValueObjectGeneratorValueType", "Source-generated struct value object",
188-
"The type was not source-generated because it is a struct, while a class was expected. To disable source generation, remove the 'partial' keyword.", DiagnosticSeverity.Warning, type);
189-
return;
190-
}
191-
192184
// Only if non-abstract
193185
if (generatable.IsAbstract)
194186
{
@@ -214,6 +206,7 @@ private static void GenerateSource(SourceProductionContext context, (Generatable
214206
}
215207

216208
var isRecord = generatable.IsRecord;
209+
var isClass = generatable.IsClass;
217210

218211
var typeName = type.Name; // Non-generic
219212
var containingNamespace = type.ContainingNamespace.ToString();
@@ -278,7 +271,7 @@ private static void GenerateSource(SourceProductionContext context, (Generatable
278271
279272
namespace {containingNamespace}
280273
{{
281-
[CompilerGenerated] {type.DeclaredAccessibility.ToCodeString()} sealed partial {(isRecord ? "record " : "")}class {typeName} :
274+
[CompilerGenerated] {type.DeclaredAccessibility.ToCodeString()} {(isClass ? "sealed" : "readonly")} partial {(isRecord ? "record " : "")}{(isClass ? "class" : "struct")} {typeName} :
282275
IValueObject,
283276
IEquatable<{typeName}>{(isComparable ? "" : "/*")},
284277
IComparable<{typeName}>{(isComparable ? "" : "*/")}
@@ -287,24 +280,27 @@ namespace {containingNamespace}
287280
288281
{(existingComponents.HasFlags(ValueObjectTypeComponents.DefaultConstructor) ? "/*" : "")}
289282
#pragma warning disable CS8618 // Deserialization constructor
283+
/// <summary>
284+
/// <strong>Obsolete:</strong> This constructor exists for deserialization purposes only.
285+
/// </summary>
290286
[System.Text.Json.Serialization.JsonConstructor]
291287
[Newtonsoft.Json.JsonConstructor]
292288
[Obsolete(""This constructor exists for deserialization purposes only."")]
293-
private {typeName}()
289+
{(isClass ? "private" : "public")} {typeName}()
294290
{{
295291
}}
296292
#pragma warning restore CS8618
297293
{(existingComponents.HasFlags(ValueObjectTypeComponents.DefaultConstructor) ? "*/" : "")}
298294
299295
{(existingComponents.HasFlags(ValueObjectTypeComponents.ToStringOverride) ? "/*" : "")}
300-
public sealed override string ToString()
296+
public {(isClass ? "sealed " : "")}override string ToString()
301297
{{
302298
{toStringBody}
303299
}}
304300
{(existingComponents.HasFlags(ValueObjectTypeComponents.ToStringOverride) ? "*/" : "")}
305301
306302
{(existingComponents.HasFlags(ValueObjectTypeComponents.GetHashCodeOverride) ? "/*" : "")}
307-
public sealed override int GetHashCode()
303+
public {(isClass ? "sealed " : "")}override int GetHashCode()
308304
{{
309305
#pragma warning disable RS1024 // Compare symbols correctly
310306
{getHashCodeBody}
@@ -313,34 +309,30 @@ public sealed override int GetHashCode()
313309
{(existingComponents.HasFlags(ValueObjectTypeComponents.GetHashCodeOverride) ? "*/" : "")}
314310
315311
{(existingComponents.HasFlags(ValueObjectTypeComponents.EqualsOverride) ? "/*" : "")}
316-
public sealed override bool Equals(object? other)
312+
public {(isClass ? "sealed " : "")}override bool Equals(object? other)
317313
{{
318314
return other is {typeName} otherValue && this.Equals(otherValue);
319315
}}
320316
{(existingComponents.HasFlags(ValueObjectTypeComponents.EqualsOverride) ? "*/" : "")}
321317
322318
{(existingComponents.HasFlags(ValueObjectTypeComponents.EqualsMethod) ? "/*" : "")}
323-
public bool Equals({typeName}? other)
319+
public bool Equals({typeName}{(isClass ? "?" : "")} other)
324320
{{
325-
if (other is null) return false;
326-
327-
{equalsBodyIfInstanceNonNull};
321+
{(!generatable.IsClass ? "" : "if (other is null) return false;\n\t\t\t")}{equalsBodyIfInstanceNonNull};
328322
}}
329323
{(existingComponents.HasFlags(ValueObjectTypeComponents.EqualsMethod) ? " */" : "")}
330324
331325
{(existingComponents.HasFlags(ValueObjectTypeComponents.CompareToMethod) ? "/*" : "")}
332326
{(isComparable ? "" : "/* Generated only if the ValueObject implements IComparable<T> against its own type and each data member implements IComparable<T> against its own type")}
333-
public int CompareTo({typeName}? other)
327+
public int CompareTo({typeName}{(isClass ? "?" : "")} other)
334328
{{
335-
if (other is null) return +1;
336-
337-
{compareToBodyIfInstanceNonNull}
329+
{(!generatable.IsClass ? "" : "if (other is null) return +1;\n\t\t\t")}{compareToBodyIfInstanceNonNull}
338330
}}
339331
{(isComparable ? "" : "*/")}
340332
{(existingComponents.HasFlags(ValueObjectTypeComponents.CompareToMethod) ? "*/" : "")}
341333
342-
{(existingComponents.HasFlags(ValueObjectTypeComponents.EqualsOperator) ? "//" : "")}public static bool operator ==({typeName}? left, {typeName}? right) => left is null ? right is null : left.Equals(right);
343-
{(existingComponents.HasFlags(ValueObjectTypeComponents.NotEqualsOperator) ? "//" : "")}public static bool operator !=({typeName}? left, {typeName}? right) => !(left == right);
334+
{(existingComponents.HasFlags(ValueObjectTypeComponents.EqualsOperator) ? "//" : "")}public static bool operator ==({typeName}{(generatable.IsClass ? "?" : "")} left, {typeName}{(generatable.IsClass ? "?" : "")} right) => {(generatable.IsClass ? "left is null ? right is null : left.Equals(right)" : "left.Equals(right)")};
335+
{(existingComponents.HasFlags(ValueObjectTypeComponents.NotEqualsOperator) ? "//" : "")}public static bool operator !=({typeName}{(generatable.IsClass ? "?" : "")} left, {typeName}{(generatable.IsClass ? "?" : "")} right) => !(left == right);
344336
345337
{(isComparable ? "" : "/*")}
346338
{(existingComponents.HasFlags(ValueObjectTypeComponents.GreaterThanOperator) ? "//" : "")}public static bool operator >({typeName} left, {typeName} right) => left.CompareTo(right) > 0;

‎DomainModeling.Tests/Analyzers/ValueObjectDefaultExpressionAnalyzerTests.cs‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
using System.Diagnostics.CodeAnalysis;
2+
3+
namespace Architect.DomainModeling.Tests.Analyzers;
4+
5+
[SuppressMessage("CodeQuality", "IDE0079:Remove unnecessary suppression", Justification = "False positive.")]
6+
[SuppressMessage("Design", "ValueObjectDefaultExpression:Default expression instantiating unvalidated value object", Justification = "Testing presence of warning.")]
7+
public class ValueObjectDefaultExpressionAnalyzerTests
8+
{
9+
// Unfortunately, we always get "unnecessary suppression" even when the warning is successfully suppressed
10+
// All we can do is manually outcomment the suppression temporarily to check that each statement in this file still warns
11+
12+
public static void UseDefaultExpressionOnWrapperValueObjectStruct_Always_ShouldWarn()
13+
{
14+
_ = default(Tests.WrapperValueObjectTestTypes.DecimalValue);
15+
}
16+
17+
public static void UseDefaultExpressionOnValueObjectStruct_Always_ShouldWarn()
18+
{
19+
_ = default(Tests.ValueObjectTestTypes.CustomCollectionValueObject);
20+
}
21+
22+
public static void UseDefaultLiteralOnWrapperValueObjectStruct_Always_ShouldWarn()
23+
{
24+
Tests.WrapperValueObjectTestTypes.DecimalValue value = default;
25+
_ = value;
26+
}
27+
28+
public static void UseDefaultLiteralOnValueObjectStruct_Always_ShouldWarn()
29+
{
30+
Tests.WrapperValueObjectTestTypes.DecimalValue value = default;
31+
_ = value;
32+
}
33+
}

‎DomainModeling.Tests/Analyzers/WrapperValueObjectDefaultExpressionAnalyzerTests.cs‎

Lines changed: 0 additions & 23 deletions
This file was deleted.

‎DomainModeling.Tests/EntityFramework/EntityFrameworkConfigurationGeneratorTests.cs‎

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using Microsoft.EntityFrameworkCore;
77
using Microsoft.EntityFrameworkCore.Metadata.Conventions;
88
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
9+
using Microsoft.Extensions.DependencyInjection;
910
using Microsoft.Extensions.Logging;
1011
using Xunit;
1112

@@ -46,8 +47,11 @@ public void ConfigureConventions_WithAllExtensionsCalled_ShouldBeAbleToWorkWithA
4647
new FormatAndParseTestingIntId(3),
4748
new LazyStringWrapper(new Lazy<string>("4")),
4849
new LazyIntWrapper(new Lazy<int>(5)),
49-
new NumericStringId("6"));
50-
var entity = new EntityForEF(values);
50+
new NumericStringId("6"));
51+
var otherValues = new StructValueObjectForEF(
52+
1,
53+
"2");
54+
var entity = new EntityForEF(values, otherValues);
5155
var domainEvent = new DomainEventForEF(id: 2, ignored: null!);
5256

5357
this.DbContext.Database.EnsureCreated();
@@ -79,9 +83,12 @@ public void ConfigureConventions_WithAllExtensionsCalled_ShouldBeAbleToWorkWithA
7983
Assert.Equal(3, reloadedEntity.Values.Three.Value?.Value.Value);
8084
Assert.Equal("4", reloadedEntity.Values.Four.Value.Value);
8185
Assert.Equal(5, reloadedEntity.Values.Five.Value.Value);
82-
Assert.Equal("6", reloadedEntity.Values.Six?.Value);
86+
Assert.Equal("6", reloadedEntity.Values.Six?.Value);
8387

84-
// This property should be mapped to int via ICoreValueWrapper<NumericStringId, int>
88+
Assert.Equal((byte)1, reloadedEntity.OtherValues.One);
89+
Assert.Equal("2", reloadedEntity.OtherValues.Two);
90+
91+
// This property should be mapped to int via ICoreValueWrapper<NumericStringId, int>
8592
var mappingForStringWithCustomIntCore = this.DbContext.Model.FindEntityType(typeof(EntityForEF))?.FindNavigation(nameof(EntityForEF.Values))?.TargetEntityType
8693
.FindProperty(nameof(EntityForEF.Values.Six));
8794
var columnTypeForStringWrapperWithCustomIntCore = mappingForStringWithCustomIntCore?.GetColumnType();
@@ -115,11 +122,11 @@ internal sealed class TestDbContext(
115122
[SuppressMessage("CodeQuality", "IDE0079:Remove unnecessary suppression", Justification = "Suppression is necessary.")]
116123
[SuppressMessage("Usage", "CA2263:Prefer generic overload when type is known", Justification = "We have no generic info for types received from callbacks.")]
117124
protected override void ConfigureConventions(ModelConfigurationBuilder configurationBuilder)
118-
{
125+
{
119126
configurationBuilder.Conventions.Remove(typeof(ConstructorBindingConvention));
120127
configurationBuilder.Conventions.Remove(typeof(RelationshipDiscoveryConvention));
121-
configurationBuilder.Conventions.Remove(typeof(PropertyDiscoveryConvention));
122-
128+
configurationBuilder.Conventions.Remove(typeof(PropertyDiscoveryConvention));
129+
123130
configurationBuilder.ConfigureDomainModelConventions(domainModel =>
124131
{
125132
domainModel.ConfigureIdentityConventions(new IdentityConfigurationOptions() { CaseSensitiveCollation = "BINARY", IgnoreCaseCollation = "NOCASE", });
@@ -138,7 +145,10 @@ protected override void ConfigureConventions(ModelConfigurationBuilder configura
138145
.HasConversion(typeof(LazyStringWrapperConverter));
139146
}
140147
});
141-
});
148+
});
149+
150+
// Pre-EF10 workaround for ComplexProperty() bug that requires ConstructorBindingConvention to be present (but it can fail if run before UninitializedInstantiationConvention): https://github.com/dotnet/efcore/issues/32437
151+
configurationBuilder.Conventions.Add(services => ActivatorUtilities.CreateInstance<ConstructorBindingConvention>(services));
142152
}
143153

144154
private class LazyStringWrapperConverter : ValueConverter<LazyStringWrapper, string>
@@ -167,7 +177,13 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
167177
values.Property(x => x.Three);
168178
values.Property(x => x.Four);
169179
values.Property(x => x.Five);
170-
values.Property(x => x.Six);
180+
values.Property(x => x.Six);
181+
});
182+
183+
builder.ComplexProperty(x => x.OtherValues, values =>
184+
{
185+
values.Property(x => x.One);
186+
values.Property(x => x.Two);
171187
});
172188

173189
builder.HasKey(x => x.Id);
@@ -219,15 +235,18 @@ internal sealed class EntityForEF : Entity<EntityForEFId>
219235
/// </summary>
220236
public bool HasFieldInitializerRun { get; } = true;
221237

222-
public ValueObjectForEF Values { get; }
238+
public ValueObjectForEF Values { get; }
239+
240+
public StructValueObjectForEF OtherValues { get; }
223241

224-
public EntityForEF(ValueObjectForEF values)
242+
public EntityForEF(ValueObjectForEF values, StructValueObjectForEF otherValues)
225243
: base(id: "A")
226244
{
227245
if (!EntityFrameworkConfigurationGeneratorTests.AllowParameterizedConstructors)
228246
throw new InvalidOperationException("Deserialization was not allowed to use the parameterized constructors.");
229247

230-
this.Values = values;
248+
this.Values = values;
249+
this.OtherValues = otherValues;
231250
}
232251

233252
#pragma warning disable IDE0079 // Remove unnecessary suppression -- Suppression below is falsely flagged as unnecessary
@@ -334,4 +353,19 @@ public ValueObjectForEF(
334353
this.Five = five;
335354
this.Six = six;
336355
}
356+
}
357+
358+
[ValueObject]
359+
internal readonly partial struct StructValueObjectForEF : IComparable<StructValueObjectForEF>
360+
{
361+
private StringComparison StringComparison => StringComparison.Ordinal;
362+
363+
public byte? One { get; private init; }
364+
public string Two { get; private init; }
365+
366+
public StructValueObjectForEF(byte? one, string two)
367+
{
368+
this.One = one;
369+
this.Two = two;
370+
}
337371
}

‎DomainModeling.Tests/ValueObjectTests.cs‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -565,8 +565,8 @@ public void GetHashCode_WithImmutableArray_ShouldReturnExpectedResult()
565565
[InlineData("A", "B", true)] // Custom collection's hash code always returns 1
566566
public void GetHashCode_WithCustomEquatableCollection_ShouldHonorItsOverride(string? one, string? two, bool expectedResult)
567567
{
568-
var left = new CustomCollectionValueObject() { Values = one is null ? null : new CustomCollectionValueObject.CustomCollection(one) };
569-
var right = new CustomCollectionValueObject() { Values = two is null ? null : new CustomCollectionValueObject.CustomCollection(two) };
568+
var left = new CustomCollectionValueObject(one is null ? null : new CustomCollectionValueObject.CustomCollection(one));
569+
var right = new CustomCollectionValueObject(two is null ? null : new CustomCollectionValueObject.CustomCollection(two));
570570

571571
var leftHashCode = left.GetHashCode();
572572
var rightHashCode = right.GetHashCode();
@@ -642,8 +642,8 @@ public void Equals_WithImmutableArray_ShouldReturnExpectedResult(string one, str
642642
[InlineData("A", "B", true)] // Custom collection's equality always returns true
643643
public void Equals_WithCustomEquatableCollection_ShouldHonorItsOverride(string? one, string? two, bool expectedResult)
644644
{
645-
var left = new CustomCollectionValueObject() { Values = one is null ? null : new CustomCollectionValueObject.CustomCollection(one) };
646-
var right = new CustomCollectionValueObject() { Values = two is null ? null : new CustomCollectionValueObject.CustomCollection(two) };
645+
var left = new CustomCollectionValueObject(one is null ? null : new CustomCollectionValueObject.CustomCollection(one));
646+
var right = new CustomCollectionValueObject(two is null ? null : new CustomCollectionValueObject.CustomCollection(two));
647647
Assert.Equal(expectedResult, left.Equals(right));
648648
}
649649

@@ -1128,9 +1128,14 @@ public ArrayValueObject(string?[]? stringValues, int?[] intValues)
11281128
}
11291129

11301130
[ValueObject]
1131-
public sealed partial record class CustomCollectionValueObject
1131+
public readonly partial record struct CustomCollectionValueObject
11321132
{
1133-
public CustomCollection? Values { get; set; }
1133+
public CustomCollection? Values { get; private init; }
1134+
1135+
public CustomCollectionValueObject(CustomCollection? values)
1136+
{
1137+
this.Values = values;
1138+
}
11341139

11351140
public class CustomCollection : IReadOnlyCollection<int>
11361141
{

0 commit comments

Comments
 (0)