nirum-lang / nirum

Nirum: IDL compiler and RPC/distributed object framework for microservices
https://nirum.org/
GNU General Public License v3.0
121 stars 28 forks source link

Type names and union tags can't have the same name in Python target #68

Closed dahlia closed 6 years ago

dahlia commented 8 years ago

In Python target, because union tags are compiled to subtypes, if there's a name shared between a type and a union tag, one of them is overridden by other one. There can be also similar situation when there are tags of the same name but belonging to other unions.

Reported by @yjroot.

Kroisse commented 8 years ago

There is a several options:

Just restrict to use same name for type and union tag at the schema level.

record coin ( ... );
record cash ( ... );
union payment = coin ( coin coin ) | cash ( cash cash );

For Python target, compile union tags to be the inner class of its union type like:

class Payment:
    # ...
    class Coin(Payment):
        # ...
    class Cash(Payment):
        # ...
dahlia commented 8 years ago

@Kroisse I personally prefer the latter, but I think it can be mixed some pros of both. For example, if there aren't types of the same name, we can make a module-level alias for tags e.g. Coin = Payment.Coin. If there is already a type of the same name, we can do not define an alias for the tag with warning e.g. “A record type coin is already defined; alias for payment.coin will be not made.”

@admire93 Thoughts?

Kroisse commented 8 years ago

@dahlia It could be hard to expect. Because both of union tag and record are compiled to the Python class, they can be mixed in the user's code easliy, and lead to a bug which is hard to discover what is wrong.

The user also can make an alias, so I think it's better to let them do. We don't need to give a bad default.

dahlia commented 8 years ago

The user also can make an alias, so I think it's better to let them do. We don't need to give a bad default.

So you're suggesting ways like using annotation, right?

Kroisse commented 8 years ago

I'm prefer to provide an alias module, e.g.

from the_service import Payment
from the_service.payment import *

c = Coin( ... )  # from `the_service.payment.Coin`, alias of `Payment.Coin`
assert isinstance(c, Payment)

Still nasty, though.

dahlia commented 8 years ago

@Kroisse Unless I misunderstood it you are suggesting making a Python module named after the union name. For example, if there are payment union with coin/cash tags in the-service/entity.nrm the compile makes the following files, right?

kanghyojun commented 8 years ago

I prefer the latter also. however i don't have a strong opinion about aliasing.

Kroisse commented 8 years ago

And we need to discuss about the case that is the the origin of this issue. With this discussion, we can solve only the namespace conflict between union tags and other types. But some problem is still remained when I want to define several records and use them directly as a union:

from the_service import Payment, Coin

# too much "coin"
coin = Payment.Coin(coin=Coin(amount=100))
if isinstance(coin, Payment.Coin):
    print(coin.coin.amount)

Serialization is also problematic:

// serialization example of 'coin':
{
  "_tag": "coin",
  "coin": {
    "amount": 100
  }
}

updated: fixed "_type" to "_tag"

Kroisse commented 8 years ago

@dahlia: That's right, and I understand that can be lead a circular import.

dahlia commented 8 years ago

In the case of "_type" field in the JSON payload we can ignore it for the reasons:

  1. "_type" doesn't affect to programs. It's only for humans.
  2. Union tags are encoded in "_tag" (which affects to programs) e.g. {"_type": "payment", "_tag": "coin", "coin": {"_type": "coin", "amount": 100}}.
dahlia commented 8 years ago

Instead of making an alias module (which may introduce another name conflicts), how about making Python target to provide aliasing option through annotation? E.g.:

union payment
    = @python-alias ("Coin")
      coin (coin coin)
    | @python-alias ("Cash")
      cash (cash cash)
    ;
dahlia commented 6 years ago

Since Nirum 0.4 the compile will become to disallow to use the same name for a type name and a union tag name (or an enum member name). See also PR #254.