Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
vmagro opened this issue Jul 9, 2021 · 1 comment
Open

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

vmagro opened this issue Jul 9, 2021 · 1 comment

Comments

@vmagro
Copy link
Contributor

vmagro commented Jul 9, 2021

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
Copy link
Contributor

baioc commented Jul 21, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants