cve-bin-tool
cve-bin-tool copied to clipboard
Add type checker to Github Actions
- related to #1539
Now that we're starting to get type hints in more files, I'd like to have CI give us feedback on type hints.
This can be allowed to fail on most files right now. I don't love the way github actions handles that (last I checked, it made "allow fail" items look like they've passed even if they haven't so people may not look at them) but even if I'm not sure new contributors will know to look, it will be useful for more experienced contributors and folk reviewing pull requests to have the report generated for us.
As we get files into a good state, we could add them to the list of files where type checker fails are no longer allowed, and eventually the whole code base should be required to pass the type checker tests.
mypy was the top of my list at one point but python type checkers generally follow the same PEPs so if something else works better we can go there. I use Visual Studio Code while reviewing markdown files so a recommendation for tooling that works with that particular IDE might be nice for our documentation too.
Some talks that might be useful for someone interested in this issue:
- mypy: oldest, bit slower to update, well-supported by tools/CI
- pyright : good support for vscode, faster moving, already has 3.11 support, provided by Microsoft
- there's also one from google (pytype?)
I would suggest pyright, it has been working fine for me, switched to it from mypy. mypy sometimes gives errors that are not very helpful.
Top two contenders would be mypy and pyright, (pytype and pyre are too lenient for type checking sometimes). Both are extremely good for usage as well and very similar, key differences being,
Mypy works best if the codebase is already typed out or to get the codebase aggressively typed out. Does not do any inference, completely typing based. Sometimes throws unhelpful errors. The existing standard for python type checks.
Pyright on the other hand allows for friendly gradual typing of the codebase and also tries to infer the types itself via the code flow. Is also way faster than mypy, throws helpful errors, the fact that its treated as a first class citizen in vscode is also a plus.
I guess its a matter of when we plan to integrate type checking enforcement in ci, parallelly or after the existing codebase is typed.
I'm intending to integrate something in CI sooner rather than later. As in, if someone makes a workable pull request that doesn't break CI, I'd be willing to integrate it this week. I'd like any file that passes checks to have enforcement turned on, the rest can be added to the enforced list as they become ready. So yes, parallel integration with CI, enforcement on a per-file basis until the codebase is fully updated.
Sounds like there's a slight preference for pyright, so I'd recommend anyone working on this issue start there.
Note: I'd be willing to consider this as a potential short (175hr) gsoc project, but I'm a bit concerned that we might be able to finish it before applications closed, which is why it hasn't been added to the gsoc list directly.
I'll look into getting this implemented.
Should we have this only in CI, or also in the pre-commit-hook?
I suppose we could enforce type checks locally once we have most of the code base typed out, as enforcing them right now would lead to a lot of fails while making commits. Or maybe exclude files that are not typed yet?
I think type checking the files that already have type hints, in pre-commit would be very useful. These can be the files that may not fail type checking in CI.
I would suggest to take a look at the PR and the generated logs for pyright, to identify what all should we exclude from checking (imports, stubs, files/directories etc) and in general the pyright config file.
For now I suppose we should use pyrightconfig.json file for the config temporarily this is because I was also looking into #1595 to get it implemented so once we have moved to pyproject.toml we could get rid of many of these singular config files which could easily be moved to the .toml file.
Update: I switched from using pyright to mypy due to npm errors that were incredibly poorly timed, but thanks to the folk working on hacktoberfest we're getting close to being able to enable mypy across the repo. I don't know that I'm going to push for it on 3.2 (depends on how many more bugs I file and get fixes for this week) but I'm going to flag this with the future milestone so it keeps showing up on my dashboard.
Yay! This has been an ongoing effort for about almost a year at this point, or even more than that if I remember correctly.
Closed via #2488