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

Support type annotations #900

Closed
ndmitchell opened this issue Sep 8, 2020 · 8 comments · Fixed by #946
Closed

Support type annotations #900

ndmitchell opened this issue Sep 8, 2020 · 8 comments · Fixed by #946

Comments

@ndmitchell
Copy link

The code:

def foo(x: 1):
    pass

Currently gives:

$ buildifier foo.bzl
foo.bzl:1:11: syntax error near :

That code is accepted by python3. I think it would be useful to support in buildifer in particular because types are an extension that some Starlark implementations support, much the same way that nested-def is a Starlark extension that is supported by buildifier.

If the above is accepted, I'd be happy to do a PR to enable it.

@laurentlb
Copy link
Contributor

Earlier discussion: https://groups.google.com/d/msg/bazel-dev/Pk9VrPCqby0/zoOL1IkxEAAJ

I think there should be a standard way for annotating types in Starlark. This is probably something to discuss in http://github.com/bazelbuild/starlark.

types are an extension that some Starlark implementations support

Is there anything public?

@ndmitchell
Copy link
Author

A standard way for annotating types in Starlark would be nice, but is slightly more than I'm aiming for here. Starlark can't gain support for the above form of type annotation, because its not compatible with Python 2.

However, if the "extended Starlark" tools (e.g. those with support nested def etc and other non-Starlark features) could merely ignore type annotations, then we could use existing Python type checkers like Pyre or MyPy to check Starlark files.

Is there anything public?

Not yet, but I hope so in the near/medium term.

@laurentlb
Copy link
Contributor

Python 2 compatibility is no longer a concern (bazelbuild/starlark#27).
Do you think existing Python type checkers can be reused easily? That'd need to handle load statements to be useful.

@ndmitchell
Copy link
Author

Probably not easily, but it doesn't seem infeasible to add knowledge of load statements - although I haven't actually tried. I did try adding runtime type checking to Starlark, and that was pretty successful.

@ndmitchell
Copy link
Author

I raised an issue at bazelbuild/starlark#106.

@laurentlb
Copy link
Contributor

@vladmos What do you think of allowing type annotations in the parser, maybe behind a flag?
Type annotations are supported in https://github.com/facebookexperimental/starlark-rust. Changing the spec may take a lot of time because the semantics are complicated, but the syntax change should be simple and allow users experiment with the feature.

@vladmos
Copy link
Member

vladmos commented Jan 29, 2021

I think it can be implelented even without a flag, Buildifier already allows things that are not allowed by the interpreter (such as functions in BUILD files).

How does the language spec allowing type annotations look like? Compared with the official spec, is it enough and correct to just change the following:

  • add another definition for Parameter: Parameter = ... | Identifier ':' Test,
  • modify the DefStmt: DefStmt = 'def' identifier '(' [Parameters [',']] ')' {'->' Test} ':' Suite .

@ndmitchell
Copy link
Author

https://github.com/facebookexperimental/starlark-rust/blob/master/starlark/src/syntax/grammar.lalrpop#L68 is the grammar we used. Note that because lambda x: x you can't have types on lambdas, so lambda parameters end up being a bit different. And because of defaults/args/kwargs etc, you end up having ident: Type, ident: Type = default, *args: Type` etc.

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

Successfully merging a pull request may close this issue.

3 participants