facebookincubator / antlir

ANoTher Linux Image buildeR
https://facebookincubator.github.io/antlir/docs/
MIT License
78 stars 14 forks source link

refactor shape.bzl code generator to better support type checkers #143

Open vmagro opened 3 years ago

vmagro commented 3 years ago

antlir/bzl/shape.bzl implements a type checker in starlark (the limited flavor of python that is used by buck). As part of this goal, it also includes a python code generator to enable type-checking serialized shape instances at runtime when they are loaded by python scripts.

Unfortunately, the code generator today does not play very nicely with Python type checkers (specifically we have been using Pyre).

If a shape type is nested inside of another shape type, we are not able to annotate the nested type with Pyre.

As an example, if you have:

type_a = shape.shape(...)

type_y = shape.shape(

   ...

   z = shape.list(type_a)

)

Then you export an instance of type_y into a Python file, you cannot annotate type_a, as it's a member of the generated class for type_y with a hidden type name.

Options here are to explore Pyre to find something that works, or change the shape codegenning in a way that makes readable class names for all nested types as well such that they could be imported.

Testing changes

The existing unit tests for shape.bzl are pretty good, and you can run them with buck test //antlir/bzl/tests/shapes/.... The generated code can be found in buck-out, at something like these paths buck-out/gen/antlir/bzl/tests/shapes/test-shape#binary,link-tree/antlir/bzl/tests/shapes/*.py. The pyre docs should have good instructions for getting set up, including instructions for getting it setup with buck integration

baioc commented 3 years ago

Adding more details, even if it's just for my own future reference:

Currently, inner shapes get inlined when the outer shape is generated. This is useful when we don't want to give a name to an inner shape. eg:

outer_t = shape.shape(
    nested = shape.shape(my_field=int),
)
# ... generate a class called `outer_t` for outer_t

The runtime typechecker doesn't complain when we use a dict to represent the inner type. Pyre is also fine with this, since Shape's constructor takes Any-typed arguments and that's inherited by the generated classes.

inner = {'my_field': 1}
outer: outer_t = outer_t(nested=inner)

The issue is that we may want to give a proper type to the nested shape ...

inner: inner_t = {'my_field': 2}  # => Error: name 'inner_t' is not defined
outer: outer_t = outer_t(nested=inner)

... or call its constructor

outer: outer_t = outer_t(
    nested = inner_t(my_field=2)  # => Error: name 'inner_t' is not defined
)

But those errors are to be expected, since we never generated anything called inner_t, just an anonymous type for the field nested.