OneOf
OneOf copied to clipboard
Implements `IsX` and `AsX`
Closes #93
@mcintyre321 i think i'm done with the first part here
These examples are making me think that it's not going to be possible to come up with a default way to render the name elegantly every time.
It might be better to leave the default naming strategy as is (e.g. IsT0, IsT1, etc).
Then have the generator attribute take an optional string array where the names can be overridden and set explicitly.
On Fri, 10 Sep 2021, 13:47 Andre Soares, @.***> wrote:
@.**** commented on this pull request.
In OneOf.SourceGenerator/OneOfGenerator.cs https://github.com/mcintyre321/OneOf/pull/95#discussion_r706151816:
public bool Is{arg.Name} => this.Is{param.Name};
public {arg.ToDisplayString()} As{arg.Name} => this.As{param.Name};
public bool TryPick{arg.Name}(out {arg.ToDisplayString()} value, out {remainderArgsString} remainder) => this.TryPick{param.Name}(out value, out remainder);
Maybe we could try adding the number of type parameters before the type arguments, like:
- List
-> ListOf1String - Dictionary<string, int> -> DictionaryOf2StringInt32
for the problematic cases mentioned above:
- OneOf<OneOf<byte, short, int>, long> -> OneOf2OneOf3ByteShortIntLong
- OneOf<OneOf<byte, short>, int, long> - OneOf3OneOf2ByteShortIntLong
but it is very difficult to solve all conflicts, since an identifier can only have some characters but we are trying to represent characters that are not present. we must come out with some kind of escape character logic
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mcintyre321/OneOf/pull/95#discussion_r706151816, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACDJ6TVUTCBTK2NEPWNMBLUBH443ANCNFSM5DXR7UJA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
@mcintyre321 yeah, we kind of thought the same thing at the same time. I agree, but would suggest that we have at least a simple/incomplete/defective way shipped by default and if any conflict arise then we just dont render these properties.
And maybe raise a diagnostic warning.
But this default naming strategy could come in a next pull request. The array would be of much help already.
The biggest problem I see isn't conflicts, it's that ReallyLongTypeNamesAndGenericsWillBeVeryUgly
On Fri, 10 Sep 2021, 14:01 Andre Soares, @.***> wrote:
@mcintyre321 https://github.com/mcintyre321 yeah, we kind of thought the same thing at the same time. I agree, but would suggest that we had at least a simple incomplete way shipped by default and if any conflict arise then we just dont render these properties.
And maybe raise a diagnostic warning
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mcintyre321/OneOf/pull/95#issuecomment-916886145, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACDJ6TIZ44GJLLLYWRS2PLUBH6SDANCNFSM5DXR7UJA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
If we choose to stay with As/IsT1
we can add XML comments with appropriate descriptions explaining what 0/1/2 stands for eg:
but we will have similar problem because there is no way to reference closed generic type, this is the closest solution:
so in some edge cases it could be very long but it will work.
@romfir Is not possible to add this comments, since the classes OneOfBase<T1, T2, ... Tn>
are declared in the project OneOf
and for that reason are not generated by the source generator
@mcintyre321 I removed the default naming strategy and added a parameters to the attribute constructor in order to determine the naming. If the arguments are not passed, the generation of this named properties does not happen.
I have also added some diagnostic errors for special cases
- when the number names informed on the attribute does not match the number of cases
- when some name is null
I did extensions like this that are super handy ...
public static class OneOfIs
{
public static bool Is<T>(this IOneOf oneOf, [NotNullWhen(true)] out T? value)
{
if (oneOf.Value is T v)
{
value = v;
return true;
}
value = default;
return false;
}
public static bool Is<T>(this IOneOf oneOf)
{
if (oneOf.Value is T)
{
return true;
}
return false;
}
}
What is the future of this PR? This feature is the only blocker for us to migrate to OneOf library from a self-written union type.