dev: Add pyright as a pre-commit hook [WOR-1318] #29280
Conversation
14bdfa8 to
3b36cb1
Compare
| "src" | ||
| ], | ||
|
|
||
| "exclude": [ |
There was a problem hiding this comment.
I wonder if .venv needs to be excluded as well.
There was a problem hiding this comment.
Its excluded by default
pyrightconfig.json
Outdated
|
|
||
| "ignore": [], | ||
|
|
||
| "strictListInference": false, |
There was a problem hiding this comment.
We might have future rules to add to this list.
I personally would prefer if we only used pyright but I understand how they each might catch different set of problems.
There was a problem hiding this comment.
Apparently there is a "typeCheckingMode" mode flag that can be set and suppresses all errors instead of setting all the rules statuses
It doesn't suppress warnings for some rules tho but just for two rules so I guess its better adding these two rules specifically so they don't show in warnings
pyrightconfig.json
Outdated
| "useLibraryCodeForTypes": false, | ||
|
|
||
| "pythonVersion": "3.6", | ||
| "pythonPlatform": "Linux" |
There was a problem hiding this comment.
What are the implications of these two last settings?
What happens when we move to Python 3.8? Does this work well on Mac OS?
There was a problem hiding this comment.
What happens when we move to Python 3.8?
If a version is provided, pyright will generate errors if the source code makes use of language features that are not supported in that version.
I guess we can remove this, since we are using pyright only for un bound local variable inspection.
Does this work well on Mac OS?
Should be one of "Windows", "Darwin" or "Linux". If specified, pyright will tailor its use of type stub files, which conditionalize type definitions based on the platform. If no platform is specified, pyright will use the current platform.
I guess we could remove it here as well
3b36cb1 to
b8bd898
Compare
Adds Pyright as a pre-commit hook check for potential unbound variables
b8bd898 to
d6b3693
Compare
|
Nice, thanks for putting this check in place! |
| @@ -103,6 +103,7 @@ | |||
| "prism-sentry": "^1.0.2", | |||
| "process": "^0.11.10", | |||
| "prop-types": "^15.6.0", | |||
| "pyright": "^1.1.178", | |||
There was a problem hiding this comment.
not that it matters much but should pyright be in devDependencies?
Adds Pyright as a pre-commit hook check for potential unbound variables, and all type checks are disabled given that we use mypy for that
Future Preventive measures against https://getsentry.atlassian.net/browse/INC-31
In my particular case, PyCharm linter usually alerts me when there is a potential unbound variable. However, in this case it didn't because the code block was wrapped in a context manager, and so adding Pyright to alert in the future against unbound local variables