-
Notifications
You must be signed in to change notification settings - Fork 215
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
feature: "top-level-types required" mode for .bzl
files
#651
Comments
An example I can't quite anticipate OTTOMH: what |
Such enforcements probably belong to linters, rather than buck2 core. Internally we generally don't use types in bzl files outside of buck2 prelude. Could be a source-level hint recognized by buck2/starlark though, like this:
|
How do you feel about adding this as an option to |
Nit: it might also be nice if this enforced fields with types on providers and records, too? |
@JakobDegen I'm not sure about linters. If this is supposed to be an error, then it should always be an error, not lint. However, I have another possible idea to consider:
But I'm doubt about that, because:
Given that, an option for lint maybe not the worst option. |
For a lint, I think I could just make it a hard error among all the other things the CI system checks. It's mostly a matter of whether you get an error in |
@thoughtpolice feasibly, this is also the kind of thing that someone might prefer to not have block their local iteration, which would mean that warn-in-IDE and fail-in-CI would actually be the right balance |
I'm working on my own Prelude and occasionally I do this thing where I splat out some function, then run it with the wrong arguments later in the file. This is a time-honored tradition for those working in Python-derived languages.
Buck2 Starlark already has MyPy-style types, but what I'd like is to make sure types always have to be provided for all top-level
.bzl
code in my repository. That is, every singledef
in a.bzl
should have all arguments typed, and a return type. Note that I considertyping.Any
to be accetable, BTW — but only because it's much easier to spot and audit, and because having something for gradual migration is occasionally necessary. (This is coincidentally considered to be good practice or mandatory in most languages with inferred types, anyway.)Does this kind of feature make sense? I'm sure there's something I can't anticipate; I know the typing support has undergone a lot of work in the past year, so it would be great if this were possible.
I would also accept a
lint
for this, I suppose.The text was updated successfully, but these errors were encountered: