Vogen icon indicating copy to clipboard operation
Vogen copied to clipboard

Allow unvalidated instances to be specified

Open SteveDunn opened this issue 3 years ago • 1 comments

Describe the feature

The 'Instance' idea is rather limited when it comes to types that are non-const, e.g. decimal, and other non-primitive types, such as DateTime.

For instance, in the examples, we have:

[ValueObject(typeof(int))]
[Instance(name: "Invalid", value: -1)]
[Instance(name: "Unspecified", value: -2)]
public partial class MyIntWithTwoInstanceOfInvalidAndUnspecified
{
    private static Validation Validate(int value)
    {
        if (value > 0)
            return Validation.Ok;
        
        return Validation.Invalid("must be greater than zero");
    }
}

This is fine for int, but for decimal, the experience isn't great:

[ValueObject(typeof(decimal))]
[Instance(name: "Invalid", value: -1.23m)]

... results in error CS0182: An attribute argument must be a constant expression, typeof expression or array creation expression of an attribute parameter type

That's because decimal is not a constant in C#.

We can get around this particular problem by specifying the decimal as a string:

[ValueObject(typeof(decimal))]
[Instance(name: "Invalid", value: "-1.23")]

This parses the string as adds the m suffix. We do the same for:

  • double (d)
  • float (f)

Things get a bit messy for DateTime and DateTimeOffset. We parse them (as ISO8601 and 'roundtrip kind) into an instance of DateTime (and DateTimeOffset). Any errors are reported as compilation errors. We then generate the code to parse them again.

This all seem a bit cumbersome and means Vogen has to know about all possible types of values, which it obviously cannot do. It also means its limiting for the user, e.g. for DateTime they can't use any of the many overloads and are tied to the ISO8601 format.

A better way it to allow instances to be part of the Value Object itself, e.g.

[ValueObject(typeof(DateTimeOffset))]
public partial struct BirthDate
{
    public static BirthDate Unspecified = new BirthDate(DateTimeOffset.MinValue);
}

There are a couple of things that we need to do to allow this

  1. Vogen needs to allow fields constructed with new in Value Objects themselves (of the same type) - but still disallow default
  2. It needs to consider these fields when deserializing, the same way it does with Instances now.
  3. And maybe mark the Instance attribute as depracated? (or at least document it as such)

SteveDunn avatar Sep 14 '22 05:09 SteveDunn

This should be easy enough to implement although the testing mechanism needs updating. The following test fails due to it not being compilable:

        [Fact]
        public async Task Skip_check_when_creating_from_inside_VO_itself()
        {
            var source = $@"using Vogen;
namespace Whatever;

[ValueObject(typeof(int))]
public partial class MyVo 
{{
    public static readonly MyVo Unspecified = new MyVo(-1); 
}}
";
            await Run(
                source,
                Enumerable.Empty<DiagnosticResult>());
        }

It fails with a message saying that there isn't a constructor that takes one argument. This is because those tests are not first running the generator, only the analyzer.

For these tests, we need something that a) runs the generator, and then b) runs the analyzer.

I'm parking this one for now but intend to take a closer look at the excellent ProjectBuilder from Gérald Barré's Meziantou.Analyzer

SteveDunn avatar Dec 24 '22 07:12 SteveDunn