google
Add pyright support.
Adds support for type checking python code using pyright.
Also adds a dependency on mypy-protobuf in order to generate .pyi type stubs for generated protobuf code, without which the type checker gets very confused.
pyright was chosen because it seems like the leading python type checker these days, and because it has broad python3 version support.
This PR also integrates pyright with butler.py lint, such that changed files can optionally be type-checked if the --type-check command-line argument is set. This allows for local experimentation before we decide on the next steps.
Why use this instead of pytype? Is it because pyright doesn't fail on 3.11?
Right, as I recall one of the problems you ran into with pytype is that it did not support 3.11, whereas this does (edit: this changed, see below). It also seems to have some traction generally as a good python type checker, faster and less idiosyncratic than mypy.
To be a bit more precise:
- https://github.com/google/pytype has 4.4k stars, supports 3.8-3.11
- https://github.com/facebook/pyre-check has 6.5k stars, requires >= 3.8
- https://github.com/microsoft/pyright has 11.1k stars, so it seems to be leading the pack, and supports all versions of python 3
Rebased this PR, and added it to python butler.py lint in the way we chatted about last week: only changed files are typechecked. This is still likely to raise a ton of errors on new PRs that touch ~anything, but at least it scopes it down to what's being changed in the PR.
This approach also won't prevent breakages from making it into the repo (like the linter), since the checks do not propagate through dependency relationships: we do not check files that depend on changed files.
Latest commit adds a --type-check argument to butler.py lint that controls whether to run the type checker or not, it defaults to false. I can switch it back to true if you have appetite for aggressively fixing type-checking errors on every PR in the near future. Another approach would be to default it to true, but set it to false on CI. That way PRs aren't blocked from landing but local linting surfaces errors. WDYT?
Another option is to set pyright to only output warnings. This annoyingly requires explicitly listing all diagnostic types and setting them to "warning" in pyrightconfig.json, but it's not terribly hard.
Latest commit adds a
--type-checkargument tobutler.py lintthat controls whether to run the type checker or not, it defaults to false. I can switch it back to true if you have appetite for aggressively fixing type-checking errors on every PR in the near future. Another approach would be to default it to true, but set it to false on CI. That way PRs aren't blocked from landing but local linting surfaces errors. WDYT?
I think let's merge this as is, I'll play with it, and determine if it's ok to run this on every PR?