From 9ae3006630086d56343c3a5bb4de2610470d3d51 Mon Sep 17 00:00:00 2001 From: stevedunnhq Date: Fri, 3 Dec 2021 07:16:17 +0000 Subject: [PATCH] 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]