Skip to content

Add a stricter config pass for pyright #5612

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jun 10, 2021
Merged

Add a stricter config pass for pyright #5612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0be9307
Add a stricter config pass for pyright
jakebailey Jun 10, 2021
029e637
Flip exclude/include to opt stubs into stricter by default
jakebailey Jun 10, 2021
06451c4
Document stricter config in testing readme
jakebailey Jun 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,20 @@ jobs:
python-platform: ["Linux", "Windows", "Darwin"]
python-version: [3.6, 3.7, 3.8, 3.9, '3.10']
fail-fast: false
env:
PYRIGHT_VERSION: 1.1.148 # Must match pyright_test.py.
steps:
- uses: actions/checkout@v2
- uses: jakebailey/pyright-action@v1
with:
version: 1.1.144 # Must match pyright_test.py.
version: ${{ env.PYRIGHT_VERSION }}
python-platform: ${{ matrix.python-platform }}
python-version: ${{ matrix.python-version }}
no-comments: ${{ matrix.python-version != '3.9' || matrix.python-platform != 'Linux' }} # Having each job create the same comment is too noisy.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should make only one of the strict and the lenient run generate comments?

Copy link
Contributor Author

@jakebailey jakebailey Jun 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the first one fails, then it will leave comments and the next step will not run. If the first one passes, then it won't leave any comments and the second one will continue as normal. There shouldn't end up being any duplication.

project: ./pyrightconfig.stricter.json
- uses: jakebailey/pyright-action@v1
with:
version: ${{ env.PYRIGHT_VERSION }}
python-platform: ${{ matrix.python-platform }}
python-version: ${{ matrix.python-version }}
no-comments: ${{ matrix.python-version != '3.9' || matrix.python-platform != 'Linux' }} # Having each job create the same comment is too noisy.
Expand Down
2 changes: 1 addition & 1 deletion pyrightconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
"reportInvalidTypeVarUse": "error",
"reportPropertyTypeMismatch": "error",
"reportSelfClsParameterName": "error",
// Overloapping overloads cannot be enabled at this time because
// Overlapping overloads cannot be enabled at this time because
// of the "factions.Fraction.__pow__" method and "tasks.gather" function.
// Mypy's overlapping overload logic misses these issues (see mypy
// issue #10143 and #10157).
Expand Down
93 changes: 93 additions & 0 deletions pyrightconfig.stricter.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
{
"$schema": "https://raw.githubusercontent.com/microsoft/pyright/main/packages/vscode-pyright/schemas/pyrightconfig.schema.json",
"typeshedPath": ".",
"include": [
"stdlib",
"stubs"
],
"exclude": [
// Python 2 only modules.
"**/@python2",
"stubs/enum34",
"stubs/fb303",
"stubs/futures",
"stubs/ipaddress",
"stubs/kazoo",
"stubs/openssl-python",
"stubs/pathlib2",
"stubs/pymssql",
"stubs/Routes",
"stubs/scribe",
"stubs/tornado",
// Modules that are incomplete in some way.
"stdlib/sqlite3/dbapi2.pyi",
"stdlib/tkinter",
"stdlib/xml/dom",
"stdlib/xml/sax",
"stubs/backports",
"stubs/backports_abc",
"stubs/boto",
"stubs/cryptography",
"stubs/docutils",
"stubs/Flask",
"stubs/Jinja2",
"stubs/Markdown",
"stubs/Pillow",
"stubs/paramiko",
"stubs/protobuf",
"stubs/PyMySQL",
"stubs/python-dateutil",
"stubs/pyvmomi",
"stubs/PyYAML",
"stubs/redis",
"stubs/requests",
"stubs/simplejson",
"stubs/waitress",
"stubs/Werkzeug"
],
"typeCheckingMode": "basic",
"strictListInference": true,
"strictDictionaryInference": true,
"strictParameterNoneValue": true,
"reportFunctionMemberAccess": "error",
"reportMissingModuleSource": "none",
"reportMissingTypeStubs": "error",
"reportUnusedImport": "error",
"reportUnusedClass": "error",
"reportUnusedFunction": "error",
"reportUnusedVariable": "error",
"reportDuplicateImport": "error",
"reportOptionalSubscript": "error",
"reportOptionalMemberAccess": "error",
"reportOptionalCall": "error",
"reportOptionalIterable": "error",
"reportOptionalContextManager": "error",
"reportOptionalOperand": "error",
"reportUntypedFunctionDecorator": "error",
"reportUntypedClassDecorator": "error",
"reportUntypedBaseClass": "error",
"reportUntypedNamedTuple": "error",
"reportPrivateUsage": "error",
"reportConstantRedefinition": "error",
"reportIncompatibleMethodOverride": "error",
"reportIncompatibleVariableOverride": "error",
"reportInvalidStringEscapeSequence": "error",
"reportUnknownParameterType": "error",
"reportUnknownArgumentType": "error",
"reportUnknownLambdaType": "error",
"reportUnknownVariableType": "error",
"reportUnknownMemberType": "error",
"reportMissingTypeArgument": "error",
"reportUndefinedVariable": "error",
"reportUnboundVariable": "error",
"reportInvalidStubStatement": "error",
"reportUnsupportedDunderAll": "error",
"reportInvalidTypeVarUse": "error",
"reportPropertyTypeMismatch": "error",
"reportSelfClsParameterName": "error",
// Overlapping overloads cannot be enabled at this time because
// of the "factions.Fraction.__pow__" method and "tasks.gather" function.
// Mypy's overlapping overload logic misses these issues (see mypy
// issue #10143 and #10157).
"reportOverlappingOverload": "none"
}
9 changes: 7 additions & 2 deletions tests/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,15 @@ This test requires [Node.js](https://nodejs.org) to be installed. It is
currently not part of the CI,
but it uses the same pyright version and configuration as the CI.
```
(.venv3)$ python3 tests/pyright_test.py # Check all files
(.venv3)$ python3 tests/pyright_test.py stdlib/sys.pyi # Check one file
(.venv3)$ python3 tests/pyright_test.py # Check all files
(.venv3)$ python3 tests/pyright_test.py stdlib/sys.pyi # Check one file
(.venv3)$ python3 tests/pyright_test.py -p pyrightconfig.stricter.json # Check with the stricter config.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should document somewhere (not necessarily here) what the "stricter config" is and why it exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where would you suggest? I can leave a comment at the top of the stricter config, or put it somewhere else.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of checking a single file?

tests/README.md seems like a good location to document this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be useful; I didn't add this instruction though (it was already there).

I'll stick something in that readme.

```

`pyrightconfig.stricter.json` is a stricter configuration that enables additional
checks that would typically fail on incomplete stubs (such as `Unknown` checks),
and is run on a subset of stubs (including the standard library).

## check\_consistent.py

Run using:
Expand Down
2 changes: 1 addition & 1 deletion tests/pyright_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import sys
from pathlib import Path

_PYRIGHT_VERSION = "1.1.144" # Must match tests.yml.
_PYRIGHT_VERSION = "1.1.148" # Must match tests.yml.
_WELL_KNOWN_FILE = Path("tests", "pyright_test.py")


Expand Down