Repository navigation
Open
Labels
Description
Along with data classes and enum classes, dart also lacks a support for defining custom union types. I think built_value could contain another generator for union types that followed the same principles as enum and data classes. The union class should be:
- immutable
- comparable
- serializable
- implement toString
I think it would look something like this:
/// how the user defines a union
@union
const myUnion = const <Type>[int, String];generated in a .g.dart file:
/// the generated union class. It should be immutable
/// but probably not a built_value since it only takes
/// 1 parameter to build a new value.
@immutable
class MyUnion {
/// every union type has `value` and `type` accessors
/// `value` must be dynamic, and be casted to `type`
final dynamic value;
final Type type;
/// a typed constructor is generated for each possible type
MyUnion.int(int value)
: value = value,
type = int;
MyUnion.string(String value)
: value = value,
type = String;
/// a convience function for switching on type
/// similiar switching on types using Kotlin's when statement.
T when<T>(
T intCallback(int value),
T stringCallback(String value),
) {
switch (type) {
case int:
return intCallback(value as int);
case String:
return stringCallback(value as String);
default:
throw Exception('wtf');
}
}
String toString() => (newBuiltValueToStringHelper('MyUnion')
..add('value', value)
..add('type', type))
.toString();
int hashCode() => $jf($jc($jc(0, value.hashCode), type.hashCode));
@override
bool operator ==(dynamic other) {
if (identical(other, this)) return true;
if (other is! MyUnion) return false;
return type == other.type && value == other.value;
}
}Reacted by Adel Nizamutdinov, Fabrizio Cacicia, Timo von Holtz, Gregory Sech, Marc Prengemann, Victor, Mutlu Simsek, Giacomo Randazzo, Peter Leibiger, sāmīr and 1 more
Activity
davidmarne commentedon Apr 14, 2018
the method for defining a union might be up for debate as you couldn't use types with generics in the List method:
Maybe an alternative could be:
But that is debatable. Not sure the use case is strong enough to merit the more awkward syntax.
davidmorgan commentedon Apr 16, 2018
Interesting, thanks. Will give it some thought.
Does
valuehave to bedynamic? I think I'd prefer it to beObject. You could haveasInt,asStringgetters?What if instead of union types, we allow built_value classes to have union fields?
These would map to the same field internally. Not sure what the rest of the API would look like.
davidmarne commentedon Apr 17, 2018
Yea i think it could be Object, not dynamic, and getters that return they typed value would be a great addition.
Union fields is an interesting idea, but I'm not quite sure that will work. In your example, I assume at the very least you would want to generate:
in doing so there would be no way to access isFoo and isBar from an instance of Foo. There is no consistent interface to tell Foo to implement because the set of fields that get generated is dynamic.
Also, I personally like the idea of a given union being self-contained in a single object rather than being included in a built_value that may include other fields or even other unions. However, that's just my opinion, and I understand going the union fields route if we could figure out how to make the codegen work.
MikeMannox commentedon Apr 24, 2018
How about https://pub.dartlang.org/packages/sealed_unions ?
amnykon commentedon Nov 22, 2019
I miss swift enums with associated values... I feel that a union would fill the gap, though they can only hold 1 value. Multiple values can be achieved by using a class (hopefully immutable) containing the multiple values.
I propose to use the following format:
my_union.dart:
my_union.g.dart:
I did not include serialization in my solution. This should be added.
Initializers should be used to determine the generated code as initializers are not inherited and therefore must be specified anyway.
I did consider having a class for each type, but chose a different approach because each type should not include Union members.
Support for a variable amount of associated types could be added (including 0) at a later time, removing the need of the Foo class.
MikeMannox commentedon Nov 26, 2019
But you can do that with
Sealed Unions?realcr commentedon Jan 16, 2020
I am also very much interested in this feature. I want to be able to get an interop with my Rust code through json serialization. Here is an example for a Rust enum (Which is like the proposed union here):
Note that the Rust example above also requires the implementation of tuples, which we currently don't have, but I can definitely live without it for now (The last variant, Node, contains a tuple of
(NodeId, CompactToUser)).What are the obstacles to getting a union in
built_value?I looked at what we currently have with enum in
built_value, and it seems pretty close to what I would like to have in a union. The only thing missing is the associated types. Maybe we can even extend enum without breaking existing code, to contain associated types?Some inspiration from related projects:
I played with the code of the projects above for the whole day, trying to make them work with
built_value, but I didn't get very far. For example, withsum-types, the design of the automatic code-gen goes in a very different direction from the code-gen inbuilt_value, which makes interoperability somewhat difficult.davidmorgan commentedon Jan 17, 2020
Enums in built_value aren't a particularly good fit as they don't carry data; I think a built_value class is closer.
But I don't think either can be directly adapted, it would be an entirely new feature with new support.
I think the only obstacle is that this would take quite a big time investment :)
realcr commentedon Jan 21, 2020
@davidmorgan : I am willing to give it a try, but I need some help.
I looked at the code and began some small attempt to implement the suggestion of @amnykon.
My very general plan is as follows:
union_source_class.dartandunion_source_variant.dartfiles.Of course most of the work goes in the first bullet.
I believe that there are some major things I don't understand:
I noticed that the
built_value_generatorpackage uses its own code generation capabilities to generate.g.dartfiles for itself. How is this actually working? My guess would be that the code generation has to be done in a certain order, with some bootstrapping hack in the end. I couldn't spot it anywhere in the code.I am not sure about the correct workflow to make incremental progress. I began by copying:
value_source_class.dart->union_source_class.dartvalue_source_field.dart->union_source_variant.dartAnd then began with renaming some classes, just to make sure
pub run build_runner buildworks without errors. This worked at first, but after a while it broke with this error:What is the usual thing you would do to debug this kind of error? I have no idea where the error comes from, and at this point my only option is to revert to the previous state with my source control, starting all over.
Is there a way for me to test only the part I am working on? Can I add some debug/log prints to aid with debugging the code-gen I am working on?
*.g.dartautogenerated files checked into the repository?EDIT: I expanded on the build issue above in #807
davidmorgan commentedon Jan 21, 2020
I suggest comitting locally a lot, so you can easily revert when you run into trouble. Yeah, it's a bit tricky.
A few tips: 1) run
tool/use_local_depsso that the packages depend on your local versions; you'll need this to run the end to end tests. And, 2) use the end to end tests :) ... they're the best way to specify what the generation is supposed to be achieving.You're very welcome to look at this, but I'd like to caution that this is likely to be a hard PR to write/accept; I don't have a lot of spare time for built_value, so I can give some pointers to help you but not much more than that; and even if it works, I'm a little hesitant about accepting the change because I'll need to maintain it. e.g. it'll soon need updating to work with None Null By Default.
That warning aside, I'm very interested to see how it goes :)
Thanks.
realcr commentedon Jan 21, 2020
I just managed to remove the dependency of
built_value_generatoron its own codegen. I did this by changing all theBuilt<...>statements frombuilt_value_generator. I suspect that you might not like the result, but at least at this point I can change things in the code base without being affected by issue #807.If you are interested to see what it looks like, you can see this branch:
https://github.com/freedomlayer/built_value.dart/tree/real/feat/union-class
Even if you choose not to merge those changes eventually, I will still be happy to use it with my code, because I am very interested in having sum-types. In addition, it could be an interesting experiment about how to implement sum-types that you might be able to utilize later if you ever want to add this feature.
realcr commentedon Jan 22, 2020
I spent some time reading the proposals in this issue, and thinking about reasonable design that will go in harmony with the rest of
built_value. I include here my current idea, I haven't yet written the code generator, but the code below actually does compile and pass tests.@davidmorgan , @amnykon, @davidmarne , @werediver: Comments and ideas are highly appreciated, it is very likely that I might be missing something.
EDIT: Updated Builder setters in autogenerated code to discourage the use of
..when building unions. Using avoidmethod instead.User code
Example test code
Automatically generated code
davidmorgan commentedon Jan 22, 2020
Thanks! Looks promising. Just a few notes:
operator==can be simpler: just check that _type is equal to other._type and _value is equal to other._valuebuilt_valueor union types ... likebuilt_valuedoes. This would complicate things quite a bit, but is interesting to think about :)1 remaining item
realcr commentedon Jan 22, 2020
Hi, thanks for the feedback!
@davidmorgan : Regarding the
operator==: I wish so too, but it seems like I can not accessother._type, becauseotheris of typeSimpleUnion, and not of type_$SimpleUnion.About the nested builders: Could you point me to an example of what it means? I have seen this expression in the documentation and repository more than once, but couldn't find it. Maybe it will be possible to add this.
@amnykon :
I understand that your main concern is that we should have chosen the opposite design and invert the inheritance. I actually began from your proposed code yesterday, but at some point I realized I do not know how to express something like empty variant or tuple variant using the syntax proposed in your example.
I made some progress, and this is the current state of user code:
Note that above,
emptymeans an empty variant, andtupleis a tuple of two items: integer and a string. I remember that you also cared about having something like this. I was wondering if you know of a reasonable way to represent tuple variants and empty variants in the proposed "constructor design".EDIT: Thinking about it, maybe it is not so difficult to do, and using the "constructor design" should add the wanted extra ergonomics for
is{TypeName}etc. @davidmorgan : Are you cool with a design of inverted inheritance? I am not sure I have enough view at this point to understand the advantages of extendingBuilt<...>over using the inverted inheritance idea proposed by @amnykon.As an appendix, I include the full user code, example code and generated code.
We have everything compiling and the tests pass.
user code
test code
Autogenerated code
amnykon commentedon Jan 22, 2020
I like the
final List<Object> _values;it will work in either case, though we would need to think about the get functions I proposed. Maybe useGet{TypeName}{Member}if the type is a tuple?The gets are not as important for a tuple, probably could leave them out for now and add them in latter.
davidmorgan commentedon Jan 23, 2020
Nested builders would mean that when you use the union builder, you can also use the nested field builder. (Actually, have to).
davidmorgan commentedon Jan 23, 2020
Re:
operator==, you could just check/cast for_$SimpleUnioninstead ofSimpleUnion. It probably makes sense to forbid other implementations of the type.realcr commentedon Jan 23, 2020
Hey, so I gave the
class SimpleUnion extends _$SimpleUniondesign a try,I think that the result is cleaner than what we had earlier.
The ergonomics
is{Type}methods were easy to add with this design.The
operator==implementation was updated to avoid usingmatch. Thatsaid, I believe that what I did there is still not very clean (You can check
it out below).
I wasn't sure if having
BuiltList<Object> _valuescould therefore be a betterdesign choice than
List<Object> _values, but eventually decided against itbecause using
BuiltList<Object>requires an extra dependency.Not fully sure what are the implications of having the
class SimpleUnion extends _$SimpleUniondesign over nested builders.I hope to start exploring code generation implementation soon.
User code
Example test code
Autogenerated code
davidmorgan commentedon Jan 23, 2020
The problem with using
class SimpleUnion extends _$SimpleUnionis that it means your class exposes API that is purely in the generated code. Generated code can be harder to deal with--it might not be generated yet, it might change when the generator changes--and so this makes the public API of the class less accessible.It's true that
built_valuecurrently does this by default for builders. This is a compromise; the builder API is much less important than the value type API, so it's less of a problem that it's hidden.I don't see a great way to do this for union types, since they need more API surface in the union class that we can't predict. One option is to have the analyzer plugin add the boilerplate for you. In the long run I think that's a good way to do it, because it makes all the API visible. But it has to work really well everywhere, and we're not currently at that point.
There has been quite a lot of discussion about improving how codegen works so we can better separate the public API and the generated code; in particular about using the
externalkeyword to let you declare explicitly that you're writing out an API that is going to be generated later.realcr commentedon Jan 23, 2020
I understand now for the first time the rationale behind the
extends Built<...>design decision: it makes sure autogenerated code is never exposed as part of the API.To summarize, I currently see two strategies of adding a Union:
class SimpleUnion extends _$SimpleUnionis{Type})abstract class SimpleUnion implements Built<SimpleUnion, SimpleUnionBuilder>matchfunction), somewhat of a stranger way to declare the variants (Usingmatchdeclaration, instead of using constructors)Regarding the extra ergonomics: As a Rust programmer all you have to deal with a Rust enum (equivalent to Union in this case) is a
matchstatement. There is nothing else. It is sometimes a bit not ergonomic, but I lived with it a few years already and stayed alive, so I think it is a reasonable option too.@davidmorgan : I think that your last comment was too polite for me to be able to deduce what to do next (: Which strategy would you like to have implemented?
One more thought: Possibly if you do not intend to merge such feature to
built_valueat all at this point, maybe there is a possibility to add the union feature as a separate dart package, and in some way let the two packages co-exist with the code-gen? I am not sure how to do something like that, but you might have an idea of what is possible.EDIT: Extra thought: In the autogenerated implementation of
SimpleUnionBuilderwith theabstract class SimpleUnion implements Built<SimpleUnion, SimpleUnionBuilder>strategy, isn't the user exposed to generated code at the API level?davidmorgan commentedon Jan 23, 2020
I didn't make a clear recommendation because I'm not hugely satisfied with either option :) but on the whole I think option two is the way to go, it's consistent with Built and EnumClass, and it's more future proof. This only works if the result is good enough that people want to use it, though.
Yes, it's possible to have it be a separate package. Originally the serializers codegen was in a separate package to the value type codegen. It basically just works automatically :) ... the output from multiple generators is collected in the same
.g.dartfile. So if you copied/renamed thebuilt_valuepackages they'd work.Re: your edit, yes,
SimpleUnionBuilderis generated API. That's what I meant in my comment by 'It's true that built_value currently does this by default for builders. This is a compromise...'; you do have the option to explicitly write out the builder API, but people didn't want to do that. For comparison, note that AutoValue for Java requires to explicitly write out the full builder API.realcr commentedon Jan 23, 2020
Sounds reasonable.
I am currently also exploring the option of making an external dart package that will provide a Union. I think it might allow us to initially check what it is like, without putting on the maintainers of
built_valuethe burden of maintaining the newly exposed API forever.My first attempt was to try to use the
sum_typesdart package, which implements a concept similar to theclass SimpleUnion extends _$SimpleUniondesign we discussed here.I hit an issue with the serializers, I thought that you might have an idea of how to fix it.
I added this line in the original dart file:
Above,
MyStructis abuilt_valueclass (extendingBuilt<...>), whileNatis a structure extended by thesum_typesdart package. However, in the code generated bybuilt_valuethe Nat serializer I implemented by hand is missing:And therefore I get this exception during runtime when trying to serialize
MyStruct:When I added the missing serializer manually by adding:
..add(Nat.serializer), serialization worked as expected.Full code:
lib/ser.dart
bin/ser.dart
davidmorgan commentedon Jan 28, 2020
built_valuewill only auto-add serializers that belong tobuilt_valueclasses; so it's expected that it won't add theNatserializer automatically.There's an issue open that it should be a compile time error if you add
NatinSerializersForsince it didn't know what to do with it.realcr commentedon Jan 29, 2020
Hey, I tried to play a bit with the idea of having an external library that will implement a built_value compatible union. I think that the current list of constraints leaves us in a catch-22 situation:
Built<SimpleUnion, SimpleUnionBuilder>, which implies we have to use theabstract class SimpleUnion implements Built<SimpleUnion, SimpleUnionBuilder>design.abstract class SimpleUnion implements Built<SimpleUnion, SimpleUnionBuilder>inside the code, the automatic code generator ofbuilt_valuewill attempt to generate code for the union class as if it was a usualbuilt_valueclass. Therefore the code will not compile.I am not sure how to continue from here. Some ideas I had:
built_valueautomatic code generation for a certain class? Maybe decorate it somehow or put it somewhere else?Built<...>?)The way I see it the hygienic way for code generation is to have a decorator above every class, this way the code generation module doesn't have to guess which classes should be dealt with. This also allows different code generation libraries to work well together. However,
built_valuecode generator already tries to inspect every class, and changing this behaviour will break all existing code usingbuilt_value, so this is probably not an option.If none of these options are possible, I think there is no hope left but to fully fork the
built_valuecode generator if we want to have abuilt_valuestyle union.davidmorgan commentedon Jan 29, 2020
For #1: what is it you're getting from the
Builtinterface? Would it work to have your own (renamed) copy of it?Thanks.
realcr commentedon Jan 29, 2020
Hey, I hope that I understood the question correctly. The only reason I am using the
Builtinterface is to be able to have automatic serialization. If I have my own copy of it (LikeMyBuilt<...>) my class's serializer will not get into the serializers list, and therefore I will not be able serialize it (As an example, see the issue I was having when trying to use thesum_typesdart package, here: #395 (comment))davidmorgan commentedon Jan 30, 2020
Ah, I see.
I think it's probably neatest to ask people to explicitly modify the
@SerializersFordeclaration to add in the union serializers. That way we don't run into undefined behaviour about howBuiltbehaves when reused by other packages :)What do you think? Thanks.
realcr commentedon Jan 31, 2020
Hey, I managed to make a working dart package for unions that integrates well enough with
built_value.dart, this is the repository: https://github.com/freedomlayer/built_union.dartI haven't uploaded a pub package yet. This is my first dart contribution, I am not yet really sure how to do it, but I will find out.
Check out the tests to see what the usage syntax looks like.
I made sure the license is compatible with this repository, so that if in the future you might want to merge some of the code from
built_union.dartit wouldn't be a problem.Hacks
There are two hacks worth mentioning in making
built_union.dartwork in harmony withbuilt_value.dart, mostly on the serialization side:There was a special case for serialization: A tuple with an empty variant, that did not serialize well with the existing
StandardJsonPlugin, so I created aCustomJsonPluginthat extends it and made a little fix. You can see it here. You might consider adding this change upstream to StandardJsonPlugin code.As noted before
@SerializersFor(...)doesn't work properly with anything that does not inherit fromBuilt<...>, so I had to createserializersmanually. It is good enough for my use case, but maybe in the future we can find a solution for this.Things not implemented
I did not yet implement generics. I think it is possible, but it wasn't important enough for me at this point. If someone wants to implement it, this is something I will be happy to merge.
In addition, the only ergonomics methods that were added are the
is{Type}style methods. Other ideas are welcome.EDIT: now on pub.dev: