asarazan / martok

https://www.npmjs.com/package/martok
10 stars 0 forks source link

Tagged unions can generate confusing "duplicate" classes. #47

Closed asarazan closed 2 years ago

asarazan commented 2 years ago

The tagged union generator creates a great sealed class cluster, but if the types it's referencing are declared/named elsewhere in the codebase, it does not delete those types from the output, causing some very confusing and similarly named classes, which are ripe for user error. You can see the failing tests in the https://github.com/asarazan/martok/tree/fix/vestigial_code branch.

Some possible approaches:

Delete the node from input as soon as we know it'll be in a tagged union

This is tricky because A) It's order-dependent and our compiler is single-pass, and B) it doesn't guarantee that other sections of the code aren't doing weird things with that reference in their own right. We could try walking all references and swapping them for the tagged version, but that might not be provably correct in all cases.

Delete the kotlin class from the output

It might actually be simpler to let the compiler do its magic and then erase/postprocess the undesired classes out of existence before returning. It shares some drawbacks with above, and has an added complication of "not all paths return klasses, some return strings (we kludged out on things like typedef).

Just do the multiple-pass upgrade already

Please no 😭

Just let folks know that if they don't want extraneous output, they should declare the unioned types anonymously

I fully admit that this kinda sucks, and becomes an overly prescriptive constraint on how backend organizes their code. I don't like it much either.

asarazan commented 2 years ago

Pending Resolution in https://github.com/asarazan/martok/pull/48