From 67ac39495cbe55f192f2c7386486cb741f5cfc07 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Thu, 2 Dec 2021 09:02:33 +0000 Subject: [PATCH 1/3] Filled out readme a bit more --- README.md | 72 ++++++++++++++------ src/Vogen.Examples/RepresentingNullObject.cs | 31 ++++++++- 2 files changed, 82 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 9ce5d8b8ac..763fcda611 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ ![Build](https://github.com/stevedunn/vogen/actions/workflows/build.yaml/badge.svg) [![GitHub release](https://img.shields.io/github/release/stevedunn/vogen.svg)](https://GitHub.com/stevedunn/vogen/releases/) [![GitHub license](https://img.shields.io/github/license/stevedunn/vogen.svg)](https://github.com/SteveDunn/Vogen/blob/main/LICENSE) [![GitHub issues](https://img.shields.io/github/issues/Naereen/StrapDown.js.svg)](https://GitHub.com/stevedunn/vogen/issues/) [![GitHub issues-closed](https://img.shields.io/github/issues-closed/Naereen/StrapDown.js.svg)](https://GitHub.com/stevedunn/vogen/issues?q=is%3Aissue+is%3Aclosed) -[![Vogen stable version](https://badgen.net/nuget/v/vogen)](https://nuget.org/packages/newtonsoft.json) +[![Vogen stable version](https://badgen.net/nuget/v/vogen)](https://nuget.org/packages/vogen)

@@ -16,7 +16,7 @@ Primitive Obsession AKA **StringlyTyped** means being obsessed with primitives. ## What is the repository? -This is a semi-opinionated library which generates [Value Objects](https://wiki.c2.com/?ValueObject) that wrap simple primitives such as `int`, `string`, `double` etc. The main goal of this project is to have almost the same speed and memory performance as using primitives. +This is a semi-opinionated library which generates [Value Objects](https://wiki.c2.com/?ValueObject) that wrap simple primitives such as `int`, `string`, `double` etc. The main goal of this project is to achieve **almost the same speed and memory performance as using primitives directly**. Some examples: @@ -46,7 +46,7 @@ var customerId = CustomerId.From(42); ```csharp [ValueObject(typeof(int))] -public partial class CustomerId +public partial struct CustomerId { } ``` @@ -63,19 +63,12 @@ public partial class CustomerId } ``` -This generates the constructor and equality code. If your type better suits a value-type, which the example above does (being an `int`), then just change `class` to `struct`: -```csharp -[ValueObject(typeof(int))] -public partial struct CustomerId -{ -} -``` - -This allows us to have more _strongly typed_ domain objects instead of primitives, which makes the code easier to read and enforces better method signatures, so instead of: +Value Object allow more _strongly typed_ domain objects instead of primitives, which makes the code easier to read and enforces tighter method signatures, so instead of: ``` cs public void DoSomething(int customerId, int supplierId, int amount) ``` + we can have: ``` cs @@ -102,21 +95,60 @@ A customer ID likely cannot be *fully* represented by an `int`. An `int` can be So, we need some validation to ensure the **constraints** of a customer ID are met. Because it's in `int`, we can't be sure if it's been checked beforehand, so we need to check it every time we use it. Because it's a primitive, someone might've changed the value, so even if we're 100% sure we've checked it before, it still might need checking again. -So far, we've used as an example, a customer ID of value `42`. In C#, it may come as no surprise that "`42 == 42`" (*I haven't checked that in JavaScript!*). But, in our **domain**, should `42` always equal `42`? Probably not if you're comparing a Supplier ID of `42` to a Customer ID of `42`! But primitives won't help you here (remember, `42 == 42`!) Given this signature: +So far, we've used as an example, a customer ID of value `42`. In C#, it may come as no surprise that "`42 == 42`" (*I haven't checked that in JavaScript!*). But, in our **domain**, should `42` always equal `42`? Probably not if you're comparing a Supplier ID of `42` to a Customer ID of `42`! But primitives won't help you here (remember, `42 == 42`!). -``` cs -public void DoSomething(int customerId, int supplierId, int amount) +```csharp +(42 == 42) // true +(SuppliedId.From(42) == SupplierId.From(42)) // true +(SuppliedId.From(42) == VendorId.From(42)) // compilation error ``` -.. the compiler won't tell you you've messed it up by accidentally swapping customer and supplier IDs. +But sometimes, we need to denote that a Value Object isn't valid or hasn't been set. We don't want anyone _outside_ of the object doing this as it could be used accidentally. It's common to have `Unspecified` instances, e.g. -But by using ValueObjects, that signature becomes much more strongly typed: +```csharp +public class Person +{ + public Age Age { get; } = Age.Unspecified; +} +``` -``` cs -public void DoSomething(CustomerId customerId, SupplierId supplierId, Amount amount) +We can do that with an `Instance` attribute: + +```csharp + [ValueObject(typeof(int))] + [Instance("Unspecified", -1)] + public readonly partial struct Age + { + public static Validation Validate(int value) => + value > 0 ? Validation.Ok : Validation.Invalid("Must be greater than zero."); + } +``` + +This generates `public static Age Unspecified = new Age(-1);`. The constructor is `private`, so only this type can (deliberately) create _invalid_ instances. + +Now, when we use `Age`, our validation becomes clearer: + +```csharp +public void Process(Person person) { + if(person.Age == Age.Unspecified) { + // age not specified. + } +} +``` + +We can also specify other instance properties: + +```csharp +[ValueObject(typeof(int))] +[Instance("Freezing", 0)] +[Instance("Boiling", 100)] +public readonly partial struct Centigrade +{ + public static Validation Validate(float value) => + value >= -273 ? Validation.Ok : Validation.Invalid("Cannot be colder than absolute zero"); +} ``` -Now, the caller can't mess up the ordering of parameters, and the objects themselves are guaranteed to be valid and immutable. # FAQ diff --git a/src/Vogen.Examples/RepresentingNullObject.cs b/src/Vogen.Examples/RepresentingNullObject.cs index 8ba733e5c8..fe6df3d704 100644 --- a/src/Vogen.Examples/RepresentingNullObject.cs +++ b/src/Vogen.Examples/RepresentingNullObject.cs @@ -5,8 +5,37 @@ using System; using Vogen; -namespace Vogen.Examples +namespace Vogen.Examples.Instances { + [ValueObject(typeof(int))] + [Instance("Freezing", 0)] + [Instance("Boiling", 100)] + public readonly partial struct Centigrade + { + public static Validation Validate(float value) => + value >= -273 ? Validation.Ok : Validation.Invalid("Cannot be colder than absolute zero"); + } + + // bug - https://github.com/SteveDunn/Vogen/issues/10 + // [ValueObject(typeof(float))] + // [Instance("Freezing", 0.0f)] + // [Instance("Boiling", 100.0f)] + // [Instance("AbsoluteZero", -273.15f)] + // public readonly partial struct Centigrade + // { + // public static Validation Validate(float value) => + // value >= AbsoluteZero.Value ? Validation.Ok : Validation.Invalid("Cannot be colder than absolute zero"); + // } + + [ValueObject(typeof(int))] + [Instance("Unspecified", -1)] + [Instance("Invalid", -2)] + public readonly partial struct Age + { + public static Validation Validate(int value) => + value > 0 ? Validation.Ok : Validation.Invalid("Must be greater than zero."); + } + [ValueObject(typeof(int))] [Instance("Unspecified", 0)] [Instance("Invalid", -1)] From 1619790549d8a04d90ab210df03887905f033f37 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Thu, 2 Dec 2021 09:04:16 +0000 Subject: [PATCH 2/3] Updated build workflow to build from `develop` --- .github/workflows/build.yaml | Bin 1070 -> 1106 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 1fa5d3b7379f83cc541e607608d6b8fc8524c90d..04ec61468b0a11dba7eabce0954278045af0c4dd 100644 GIT binary patch delta 49 ncmZ3-afxF>k)RHP0z(QzDnl6%=P=|m6ih4(#uVImIGY&&G6D=@ delta 13 Ucmcb_v5sRx(ZrUZjW4p80V;h400000 From 9ae3006630086d56343c3a5bb4de2610470d3d51 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Fri, 3 Dec 2021 07:16:17 +0000 Subject: [PATCH 3/3] Fixes #9 - Disallow default(Foo) by emitting a build error --- Vogen.sln.DotSettings | 1 + src/Vogen.Examples/NoDefaulting.cs | 30 +++ src/Vogen/Diagnostics/DiagnosticCode.cs | 3 +- src/Vogen/Diagnostics/DiagnosticCollection.cs | 23 +- src/Vogen/ValueObjectReceiver.cs | 255 ++++++++++++------ tests/Vogen.Tests/StructTests.cs | 21 -- 6 files changed, 225 insertions(+), 108 deletions(-) create mode 100644 src/Vogen.Examples/NoDefaulting.cs diff --git a/Vogen.sln.DotSettings b/Vogen.sln.DotSettings index 033bfe3095..7fcfc3ffdd 100644 --- a/Vogen.sln.DotSettings +++ b/Vogen.sln.DotSettings @@ -1,4 +1,5 @@  + ERROR WARNING True True diff --git a/src/Vogen.Examples/NoDefaulting.cs b/src/Vogen.Examples/NoDefaulting.cs new file mode 100644 index 0000000000..e1c59ad906 --- /dev/null +++ b/src/Vogen.Examples/NoDefaulting.cs @@ -0,0 +1,30 @@ +using System; +using Vogen; +#pragma warning disable CS0219 + +namespace Vogen.Examples.NoDefaulting +{ + /* + You shouldn't be allowed to `default` a Value Object as it bypasses + any validation you might have added. + */ + + public class Naughty + { + public Naughty() + { + // uncomment for - error VOG009: Type 'CustomerId' cannot be constructed with default as it is prohibited. + // CustomerId c = default; + // var c2 = default(CustomerId); + + // VendorId v = default; + // var v2 = default(VendorId); + } + } + + [ValueObject(typeof(int))] + public partial struct CustomerId { } + + [ValueObject(typeof(int))] + public partial class VendorId { } +} \ No newline at end of file diff --git a/src/Vogen/Diagnostics/DiagnosticCode.cs b/src/Vogen/Diagnostics/DiagnosticCode.cs index 1e05f593a1..631a1e1a88 100644 --- a/src/Vogen/Diagnostics/DiagnosticCode.cs +++ b/src/Vogen/Diagnostics/DiagnosticCode.cs @@ -14,5 +14,6 @@ public enum DiagnosticCode ValidationMustBeStatic = 5, InstanceMethodCannotHaveNullArgumentName = 6, InstanceMethodCannotHaveNullArgumentValue = 7, - CannotHaveUserConstructors = 8 + CannotHaveUserConstructors = 8, + UsingDefaultProhibited = 9 } \ No newline at end of file diff --git a/src/Vogen/Diagnostics/DiagnosticCollection.cs b/src/Vogen/Diagnostics/DiagnosticCollection.cs index 8a4d7572a2..572b7afe54 100644 --- a/src/Vogen/Diagnostics/DiagnosticCollection.cs +++ b/src/Vogen/Diagnostics/DiagnosticCollection.cs @@ -23,6 +23,11 @@ internal class DiagnosticCollection : IEnumerable "Types cannot be nested", "Type '{0}' must specify an underlying type"); + private static readonly DiagnosticDescriptor _usingDefaultProhibited = CreateDescriptor( + DiagnosticCode.UsingDefaultProhibited, + "Using default of Value Objects is prohibited", + "Type '{0}' cannot be constructed with default as it is prohibited."); + private static readonly DiagnosticDescriptor _cannotHaveUserConstructors = CreateDescriptor( DiagnosticCode.CannotHaveUserConstructors, "Cannot have user defined constructors", @@ -70,6 +75,9 @@ public void AddValidationMustBeStatic(MethodDeclarationSyntax member) => public void AddMustSpecifyUnderlyingType(INamedTypeSymbol underlyingType) => AddDiagnostic(_mustSpecifyUnderlyingType, underlyingType.Locations, underlyingType.Name); + public void AddUsingDefaultProhibited(Location locationOfDefaultStatement, string voClassName) => + AddDiagnostic(_usingDefaultProhibited, voClassName, locationOfDefaultStatement); + public void AddCannotHaveUserConstructors(IMethodSymbol constructor) => AddDiagnostic(_cannotHaveUserConstructors, constructor.Locations); @@ -92,11 +100,24 @@ private static DiagnosticDescriptor CreateDescriptor(DiagnosticCode code, string return new DiagnosticDescriptor(code.Format(), title, messageFormat, "RestEaseGeneration", severity, isEnabledByDefault: true, customTags: tags); } + private void AddDiagnostic(DiagnosticDescriptor descriptor, string name, Location location) + { + var diagnostic = Diagnostic.Create(descriptor, location, name); + + AddDiagnostic(diagnostic); + } + private void AddDiagnostic(DiagnosticDescriptor descriptor, IEnumerable locations, params object?[] args) { var locationsList = (locations as IReadOnlyList) ?? locations.ToList(); + + var diagnostic = Diagnostic.Create( + descriptor, + locationsList.Count == 0 ? Location.None : locationsList[0], + locationsList.Skip(1), + args); - AddDiagnostic(Diagnostic.Create(descriptor, locationsList.Count == 0 ? Location.None : locationsList[0], locationsList.Skip(1), args)); + AddDiagnostic(diagnostic); } private void AddDiagnostic(DiagnosticDescriptor descriptor, Location? location, params object?[] args) => diff --git a/src/Vogen/ValueObjectReceiver.cs b/src/Vogen/ValueObjectReceiver.cs index 9925d260c8..e5c63f93e3 100644 --- a/src/Vogen/ValueObjectReceiver.cs +++ b/src/Vogen/ValueObjectReceiver.cs @@ -22,123 +22,208 @@ public void OnVisitSyntaxNode(GeneratorSyntaxContext context) { try { - if (context.Node is not TypeDeclarationSyntax typeDeclarationSyntax) + if (context.Node is TypeDeclarationSyntax typeDeclarationSyntax) { - return; - } + HandleType(context, typeDeclarationSyntax); - var voClass = (INamedTypeSymbol) context.SemanticModel.GetDeclaredSymbol(context.Node)!; - - var attributes = voClass.GetAttributes(); - - if (attributes.Length == 0) - { return; } - AttributeData? voAttribute = - attributes.SingleOrDefault(a => - { - var fullName = a.AttributeClass?.FullName(); - return fullName is "Vogen.ValueObjectAttribute"; - }); - - if (voAttribute is null) + if (context.Node is LiteralExpressionSyntax literalExpressionSyntax) { - return; - } - - if (voAttribute.ConstructorArguments.Length == 0) - { - DiagnosticMessages.AddMustSpecifyUnderlyingType(voClass); + HandleDefaultLiteralExpression(context, literalExpressionSyntax); return; } - foreach (var eachConstructor in voClass.Constructors) - { - // no need to check for default constructor as it's already defined - // and the user will see: error CS0111: Type 'Foo' already defines a member called 'Foo' with the same parameter type - if (eachConstructor.Parameters.Length > 0) - { - DiagnosticMessages.AddCannotHaveUserConstructors(eachConstructor); - } - - } - - var underlyingType = (INamedTypeSymbol?) voAttribute.ConstructorArguments[0].Value; - - if (underlyingType is null) + if (context.Node is DefaultExpressionSyntax defaultExpressionSyntax) { - DiagnosticMessages.AddMustSpecifyUnderlyingType(voClass); + HandleDefaultExpression(context, defaultExpressionSyntax); return; } + } + catch (Exception ex) when (LogException(ex)) + { + } + } + + private void HandleDefaultLiteralExpression(GeneratorSyntaxContext context, LiteralExpressionSyntax literalExpressionSyntax) + { + if (literalExpressionSyntax.Kind() != SyntaxKind.DefaultLiteralExpression) + { + return; + } + + var ancestor = literalExpressionSyntax.Ancestors(false) + .FirstOrDefault(a => a.IsKind(SyntaxKind.VariableDeclaration)); + + if (ancestor is not VariableDeclarationSyntax variableDeclarationSyntax) + { + return; + } + + if (!IsOneOfOurValueObjects(context, variableDeclarationSyntax.Type, out string name)) + { + return; + } + + DiagnosticMessages.AddUsingDefaultProhibited(literalExpressionSyntax.GetLocation(), name); + } + + private void HandleDefaultExpression(GeneratorSyntaxContext context, DefaultExpressionSyntax defaultExpressionSyntax) + { + TypeSyntax? typeSyntax = defaultExpressionSyntax?.Type; + + if (typeSyntax == null) + { + return; + } + + if (!IsOneOfOurValueObjects(context, typeSyntax, out string name)) + { + return; + } + + DiagnosticMessages.AddUsingDefaultProhibited(typeSyntax.GetLocation(), name); + } + + private bool IsOneOfOurValueObjects(GeneratorSyntaxContext context, TypeSyntax typeSyntax, out string name) + { + name = string.Empty; + + SymbolInfo typeSymbolInfo = context.SemanticModel.GetSymbolInfo(typeSyntax); + + var voClass = typeSymbolInfo.Symbol; + + if (voClass == null) + { + return false; + } + + var attributes = voClass.GetAttributes(); + + if (attributes.Length == 0) + { + return false; + } - var containingType = context.SemanticModel.GetDeclaredSymbol(context.Node)!.ContainingType; - if (containingType != null) + AttributeData? voAttribute = + attributes.SingleOrDefault(a => a.AttributeClass?.FullName() is "Vogen.ValueObjectAttribute"); + + if (voAttribute is null) + { + return false; + } + + name = voClass.Name; + + return true; + } + + private void HandleType(GeneratorSyntaxContext context, TypeDeclarationSyntax typeDeclarationSyntax) + { + var voClass = (INamedTypeSymbol) context.SemanticModel.GetDeclaredSymbol(context.Node)!; + + var attributes = voClass.GetAttributes(); + + if (attributes.Length == 0) + { + return; + } + + AttributeData? voAttribute = attributes.SingleOrDefault( + a => a.AttributeClass?.FullName() is "Vogen.ValueObjectAttribute"); + + if (voAttribute is null) + { + return; + } + + if (voAttribute.ConstructorArguments.Length == 0) + { + DiagnosticMessages.AddMustSpecifyUnderlyingType(voClass); + return; + } + + foreach (var eachConstructor in voClass.Constructors) + { + // no need to check for default constructor as it's already defined + // and the user will see: error CS0111: Type 'Foo' already defines a member called 'Foo' with the same parameter type + if (eachConstructor.Parameters.Length > 0) { - DiagnosticMessages.AddTypeCannotBeNested(voClass, containingType); + DiagnosticMessages.AddCannotHaveUserConstructors(eachConstructor); } + } + + var underlyingType = (INamedTypeSymbol?) voAttribute.ConstructorArguments[0].Value; + + if (underlyingType is null) + { + DiagnosticMessages.AddMustSpecifyUnderlyingType(voClass); + return; + } + + var containingType = context.SemanticModel.GetDeclaredSymbol(context.Node)!.ContainingType; + if (containingType != null) + { + DiagnosticMessages.AddTypeCannotBeNested(voClass, containingType); + } - var instanceProperties = TryBuildInstanceProperties(attributes, voClass); + var instanceProperties = TryBuildInstanceProperties(attributes, voClass); - MethodDeclarationSyntax? validateMethod = null; + MethodDeclarationSyntax? validateMethod = null; - // add any validator methods it finds - foreach (var memberDeclarationSyntax in typeDeclarationSyntax.Members) + // add any validator methods it finds + foreach (var memberDeclarationSyntax in typeDeclarationSyntax.Members) + { + if (memberDeclarationSyntax is MethodDeclarationSyntax mds) { - if (memberDeclarationSyntax is MethodDeclarationSyntax mds) + if (!(mds.DescendantTokens().Any(t => t.IsKind(SyntaxKind.StaticKeyword)))) { - if (!(mds.DescendantTokens().Any(t => t.IsKind(SyntaxKind.StaticKeyword)))) - { - DiagnosticMessages.AddValidationMustBeStatic(mds); - } + DiagnosticMessages.AddValidationMustBeStatic(mds); + } - object? value = mds.Identifier.Value; + object? value = mds.Identifier.Value; - Log.Add($" Found method named {value}"); + Log.Add($" Found method named {value}"); - if (value?.ToString() == "Validate") + if (value?.ToString() == "Validate") + { + TypeSyntax returnTypeSyntax = mds.ReturnType; + if (returnTypeSyntax.ToString() != "Validation") { - TypeSyntax returnTypeSyntax = mds.ReturnType; - if (returnTypeSyntax.ToString() != "Validation") - { - DiagnosticMessages.AddValidationMustReturnValidationType(mds); - Log.Add($" Validate return type is {returnTypeSyntax}"); - - } + DiagnosticMessages.AddValidationMustReturnValidationType(mds); + Log.Add($" Validate return type is {returnTypeSyntax}"); - Log.Add($" Added and will call {value}"); - - validateMethod = mds; } - } - } - if (SymbolEqualityComparer.Default.Equals(voClass, underlyingType)) - { - DiagnosticMessages.AddUnderlyingTypeMustNotBeSameAsValueObjectType(voClass); - } + Log.Add($" Added and will call {value}"); - if (underlyingType.ImplementsInterfaceOrBaseClass(typeof(ICollection))) - { - DiagnosticMessages.AddUnderlyingTypeCannotBeCollection(voClass, underlyingType); + validateMethod = mds; + } } - - bool isValueType = underlyingType.IsValueType; + } - WorkItems.Add(new ValueObjectWorkItem - { - InstanceProperties = instanceProperties.ToList(), - TypeToAugment = typeDeclarationSyntax, - IsValueType = isValueType, - UnderlyingType = underlyingType, - ValidateMethod = validateMethod, - FullNamespace = voClass.FullNamespace() - }); + if (SymbolEqualityComparer.Default.Equals(voClass, underlyingType)) + { + DiagnosticMessages.AddUnderlyingTypeMustNotBeSameAsValueObjectType(voClass); } - catch (Exception ex) when (LogException(ex)) + + if (underlyingType.ImplementsInterfaceOrBaseClass(typeof(ICollection))) { + DiagnosticMessages.AddUnderlyingTypeCannotBeCollection(voClass, underlyingType); } + + bool isValueType = underlyingType.IsValueType; + + WorkItems.Add(new ValueObjectWorkItem + { + InstanceProperties = instanceProperties.ToList(), + TypeToAugment = typeDeclarationSyntax, + IsValueType = isValueType, + UnderlyingType = underlyingType, + ValidateMethod = validateMethod, + FullNamespace = voClass.FullNamespace() + }); } private bool LogException(Exception ex) @@ -177,7 +262,7 @@ private IEnumerable TryBuildInstanceProperties( { Log.Add($"name symbol for InstanceAttribute is null"); DiagnosticMessages.AddInstanceMethodCannotHaveNullArgumentName(voClass); - // continue; + // continue; } var value = constructorArguments[1].Value; diff --git a/tests/Vogen.Tests/StructTests.cs b/tests/Vogen.Tests/StructTests.cs index dbf40937d6..24bddebba0 100644 --- a/tests/Vogen.Tests/StructTests.cs +++ b/tests/Vogen.Tests/StructTests.cs @@ -19,27 +19,6 @@ private static Validation Validate(string value) => string.IsNullOrEmpty(value) ? Validation.Invalid("cannot be null or empty") : Validation.Ok; } -public class StructTests_creating_with_default -{ - [Fact] - public void Throws_when_accessing_Value_after_creating_with_default() - { - var sut = default(CustomerId); - Func action = () => sut.Value; - - action.Should().Throw().WithMessage("Validation skipped by default initialisation. Please use the 'From' method for construction."); - } - - [Fact] - public void Throws_when_accessing_Value_after_creating_with_default_2() - { - var sut = default(VendorName); - Func action = () => sut.Value; - - action.Should().Throw().WithMessage("Validation skipped by default initialisation. Please use the 'From' method for construction."); - } -} - public class StructTests_creating_with_parameterless_constructor { [Fact]