From 7050a8aeaaf6e7a3b5579fac86a9cfcf93219a38 Mon Sep 17 00:00:00 2001 From: Bernhard Kaindl Date: Fri, 15 Nov 2024 12:00:00 +0100 Subject: [PATCH 1/6] tox.ini: Complete the transition from pyre to pyright Signed-off-by: Bernhard Kaindl --- tox.ini | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/tox.ini b/tox.ini index 5db78e4e..ba49e39b 100644 --- a/tox.ini +++ b/tox.ini @@ -4,8 +4,8 @@ # 1. python 2.7 and 3.10 coverage test for changed, but not covered lines and mypy check # 2. python 3.6 test and pylint warnings from changed lines # 3. pytype (needs Python 3.8 for best results) -# 4. pyre and pyright checks, pytest test report as markdown for GitHub Actions summary -envlist = py38-covcombine-check, py311-lint-test, py310-pytype, py311-pyre-mdreport +# 4. pyright checks, pytest test report as markdown for GitHub Actions summary +envlist = py38-covcombine-check, py311-lint-test, py310-pytype, py311-pyright-mdreport isolated_build = true skip_missing_interpreters = true requires = @@ -42,15 +42,15 @@ description = Run in a {basepython} virtualenv: lint: {[lint]description} mdreport: Make a test report (which is shown in the GitHub Actions Summary Page) test: {[test]description} - # https://pypi.org/project/pyre-check/ pyre intro: https://youtu.be/0FSXS5kw2m4 - pyre: Run pyre for static analyis, only passes using: tox -e py311-pyre + # See https://microsoft.github.io/pyright/#/configuration for more in pyright + pyright: Run pyright for static analyis check: Run mypy for static analyis pytype: Run pytype for static analyis, intro: https://youtu.be/abvW0mOrDiY # checkers(mypy) need the pytest dependices as well: extras = {check,pytype}: {[check]extras} - {cov,covcp,covcombine,fox,check,lint,test,pytype,pyre,mdreport}: {[test]extras} - {cov,covcp,covcombine,fox}: {[cov]extras} + {cov,covcp,covcombine,fox,check,lint,test,pytype,pyright,mdreport}: {[test]extras} + {cov,covcp,covcombine,fox}: {[cov]extras} deps = mdreport: pytest-md-report {py27-test,py27-cov}: pyftpdlib @@ -58,9 +58,7 @@ deps = {cov,covcp,covcombine,fox}: coverage[toml] {cov,covcp,covcombine,fox}: diff-cover {lint,fox}: {[lint]deps} - pyre: pyre-check - pyre: pyre-extensions - pyre: pyright + pyright: pyright pytype: {[pytype]deps} allowlist_externals = {cov,covcp,covcombine,fox,check,lint,test,mdreport}: echo @@ -81,7 +79,6 @@ passenv = covcp: HOME check: MYPY_FORCE_COLOR check: MYPY_FORCE_TERMINAL_WIDTH - pyre: PYRE_TYPESHED {fox,check,pytype}: TERM fox: DISPLAY fox: XAUTHORITY @@ -104,7 +101,7 @@ setenv = {[cov]setenv} commands = lint: {[lint]commands} - pyre: {[pyre]commands} + pyright: {[pyright]commands} check: {[check]commands} pytype: {[pytype]commands} {cov,covcp,covcombine,check,fox,test,mdreport}: {[test]commands} @@ -200,7 +197,7 @@ commands = ignore = W191,W293,W504,E101,E126,E127,E201,E202,E203,E221,E222,E226,E227,E241,E251,E261,E262,E265,E301,E302,E303,E305,E722,W391,E401,E402,E741 max-line-length = 129 -[pyre] +[pyright] commands = -pyright From d335446fab20ff2f166621c1c1dbfe221660a4a2 Mon Sep 17 00:00:00 2001 From: Bernhard Kaindl Date: Fri, 15 Nov 2024 12:00:00 +0100 Subject: [PATCH 2/6] pre-commit: Remove the remaining call to pyre in pre-commit Signed-off-by: Bernhard Kaindl --- .pre-commit-config.yaml | 7 ----- pyproject.toml | 5 ---- pyre_runner.py | 62 ----------------------------------------- 3 files changed, 74 deletions(-) delete mode 100755 pyre_runner.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4dea24a9..36b678db 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -101,13 +101,6 @@ repos: - toml - repo: local hooks: - - id: run-pyre - name: run-pyre (expect this to take 30 seconds) - entry: python pyre_runner.py - types: [python] - language: python - log_file: ".git/pre-commit-pyre.log" - additional_dependencies: [pyre-check,mock] - id: pytype name: pytype (may take up to two minutes) entry: sh -c "pytype >/dev/tty" diff --git a/pyproject.toml b/pyproject.toml index 8edd0ba3..ac1cabf4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -71,11 +71,6 @@ mypy = [ "types-six", "types-toml", ] -# pyre introduced two false-postives recently, pin it to prevent further surprises: -pyre = [ - "pyre-check == 0.9.21", - "pyre-extensions == 0.0.30", -] pytype = [ "pandas", "pytype", diff --git a/pyre_runner.py b/pyre_runner.py deleted file mode 100755 index a67d2427..00000000 --- a/pyre_runner.py +++ /dev/null @@ -1,62 +0,0 @@ -#!/usr/bin/env python -""" -Run a one-time pyre static analysis check without needing a .pyre_configuration -Gets the paths dynamically so it can be used in tox and GitHub CI -""" -import os -import sys -import time - -import mock - -me = os.path.basename(__file__) + ":" - -pyre_typesched = os.environ.get("PYRE_TYPESHED", None) -if pyre_typesched and os.path.exists(pyre_typesched + "/stdlib/os/path.pyi"): - print("Using {env:PYRE_TYPESHED}:", pyre_typesched) -else: - pyre_typesched = sys.path[-1] + "/mypy/typeshed" - if os.path.exists(pyre_typesched + "/stdlib/os/path.pyi"): - print("Using python_lib:", pyre_typesched) - else: - pyre_typesched = "/tmp/typeshed" - if os.path.exists(pyre_typesched + "/stdlib/os/path.pyi"): - print("Using:", pyre_typesched) - else: - clone = "git clone --depth 1 https://github.com/python/typeshed " - print(me, "Falling back to:", clone + pyre_typesched) - ret = os.system(clone + pyre_typesched) - if ret or not os.path.exists(pyre_typesched + "/stdlib/os/path.pyi"): - print(me, "Could not find or clone typeshed, giving up.") - sys.exit(0) - -command = ( - "pyre", - "--source-directory", - "xcp", - "--source-directory", - "tests", - "--search-path", - "stubs", - "--search-path", - ".", - "--search-path", - os.path.dirname(mock.__file__), - "--typeshed", - pyre_typesched, - "check", -) -cmd = " ".join(command) -print(me, "Running:", cmd) -start_time = time.time() -ret = os.system(cmd) -duration = time.time() - start_time -r = os.waitstatus_to_exitcode(ret) # type: ignore[module-addr] # newer versions have it -if r == 0: - print(me, f"OK pyre took: {duration:.1f}s") -else: - print(me, "Ran:", cmd) - print(me, "exit code:", r) - if os.environ.get("ACT", None): - time.sleep(10) -sys.exit(r) From 230782364099c939babacef68e657bd96cbc21b8 Mon Sep 17 00:00:00 2001 From: Bernhard Kaindl Date: Fri, 15 Nov 2024 12:00:00 +0100 Subject: [PATCH 3/6] docs: Complete the transition from pyre to pyright Signed-off-by: Bernhard Kaindl --- CONTRIBUTING.md | 5 ++--- README.md | 7 +++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 910602de..8ba18bc9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -50,13 +50,12 @@ This project uses `tox` to run the tests for different python versions. Intro: > A comprehensive beginner's introduction to `tox`":_ > https://www.seanh.cc/2018/09/01/tox-tutorial/ -`tox` runs `pytest`, `pylint` and static analysis using `mypy`, `pyre`, `pytype`, and `pyright`. +`tox` runs `pytest`, `pylint` and static analysis using `mypy`, `pytype`, and `pyright`. Links: - https://mypy.readthedocs.io/en/stable/ - https://microsoft.github.io/pyright/ - https://google.github.io/pytype/ -- https://pyre-check.org/ With `tox`, developers can run the full test suite for Python 2.7 and 3.x. The same test suite is used in GitHub CI: @@ -78,7 +77,7 @@ Using pip-tools, you can extract the requirements and extras from `pyptoject.tom ```bash PYTHON=python3.10 -EXTRAS=.,test,mypy,pyre,pytype,tox +EXTRAS=.,test,mypy,pytype,tox PFLAGS="--no-warn-conflicts" $PYTHON -m pip install pip-tools==7.3.0 $PYTHON -m piptools compile --extra=$EXTRAS -o - pyproject.toml | diff --git a/README.md b/README.md index 2f24022a..fa00abca 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ The Continuous Integration Tests feature: - Checking of the combined coverage against the diff to master: Fails if changes are not covered! - Pylint report in the GitHub Action Summary page, with Warning and Error annotatios, even in the code review. - Check that changes don't generate pylint warnings (if warning classes which are enabled in .pylintrc) -- Static analysis using `mypy`, `pyre` and `pytype` +- Static analysis using `mypy`, `pylint`, `pyright` and `pytype` This enforces that any change (besides whitespace): @@ -72,7 +72,6 @@ For the installation of the general development dependencies, visit [INSTALL.md] - Test with `python2.7 -m pytest` - Run `mypy` (without any arguments - The configuration is in `pyproject.toml`) - Run `./pytype_runner.py` -- Run `./pyre_runner.py` - Run `tox -e py36-lint` and fix any `Pylint` warnings - Run `tox -e py310-covcombine-check` and fix any missing diff-coverage. - Run `tox` for the full CI test suite @@ -86,7 +85,7 @@ The list of `virtualenvs` configured in tox can be shown using this command: `to $ tox -av default environments: py36-lint -> Run in a py36 virtualenv: Run pylint and fail on warnings remaining on lines in the diff to master -py311-pyre -> Run in a py311 virtualenv: Run pyre for static analyis, only passes using: tox -e py311-pyre +py311-pyright -> Run in a py311 virtualenv: Run pyright for static analyis py38-pytype -> Run in a py38 virtualenv: Run pytype for static analyis, intro: https://youtu.be/abvW0mOrDiY py310-covcombine-check -> Run in a py310 virtualenv: Generate combined coverage reports with py27-test coverage merged Run mypy for static analyis @@ -100,7 +99,7 @@ test -> Run in a python virtualenv: Run pytest in this environ If you have only one version of Python3, that works too. Use: `tox -e py-test` -## Static analysis using mypy, pyre, pyright and pytype +## Static analysis using mypy, pylint, pyright and pytype The preconditions for using static analysis with `mypy` (which passes now but has only a few type comments) and `pyright` are present now and `mypy` is enabled in `tox` From be1336de3ac432b41f38a4e390b636273ef86e86 Mon Sep 17 00:00:00 2001 From: Bernhard Kaindl Date: Fri, 15 Nov 2024 12:00:00 +0100 Subject: [PATCH 4/6] `tox.ini/pyright`: Fail `tox` if `pyright` fails (enforce check) Signed-off-by: Bernhard Kaindl --- tox.ini | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index ba49e39b..0de3fed6 100644 --- a/tox.ini +++ b/tox.ini @@ -199,7 +199,9 @@ max-line-length = 129 [pyright] commands = - -pyright + pyright --version + # To make pyright checks optional, change the next line to '-pyright': + pyright [pytype] deps = pytype From b5752464fb92ed86fcf59441886cec139355cac1 Mon Sep 17 00:00:00 2001 From: Bernhard Kaindl Date: Fri, 15 Nov 2024 12:00:00 +0100 Subject: [PATCH 5/6] pyre: Finally, remove obsolete pyre error suppressions Signed-off-by: Bernhard Kaindl --- tests/conftest.py | 4 ++-- tests/test_cpiofile.py | 1 - tests/test_httpaccessor.py | 1 - xcp/accessor.py | 1 - xcp/bootloader.py | 1 - xcp/cpiofile.py | 2 -- 6 files changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 1a2a4974..da395853 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,10 +5,10 @@ This module is run automatically by pytest to define and enable fixtures. """ -# pyre-ignore-all-errors[21] import warnings -import pytest # pyre does not find the module when run by tox -e py311-pyre +import pytest + @pytest.fixture(autouse=True) def set_warnings(): diff --git a/tests/test_cpiofile.py b/tests/test_cpiofile.py index cd36a2f2..0371d2df 100644 --- a/tests/test_cpiofile.py +++ b/tests/test_cpiofile.py @@ -1,5 +1,4 @@ # suppress false positive on pytest missing pytest.raises(): -# pyre-ignore-all-errors[16] """ Test various modes of creating and extracting CpioFile using different compression types, opening the archive as stream and as file, using pyfakefs as filesystem without diff --git a/tests/test_httpaccessor.py b/tests/test_httpaccessor.py index d0248f7f..c6bd38fa 100644 --- a/tests/test_httpaccessor.py +++ b/tests/test_httpaccessor.py @@ -43,7 +43,6 @@ def http_get_request_data(self, url, read_file, error_handler): def assert_http_get_request_data(self, url, read_file, error_handler): # type:(str, str, ErrorHandler) -> HTTPAccessor - # pyre-ignore[23]: silence false positive with self.http_get_request_data(url, read_file, error_handler) as (httpaccessor, ref): http_accessor_filehandle = httpaccessor.openAddress(read_file) if sys.version_info >= (3, 0): diff --git a/xcp/accessor.py b/xcp/accessor.py index 0ceec1c1..afdb643d 100644 --- a/xcp/accessor.py +++ b/xcp/accessor.py @@ -24,7 +24,6 @@ """accessor - provide common interface to access methods""" import errno -# pyre-ignore-all-errors[6,16] import ftplib import io import os diff --git a/xcp/bootloader.py b/xcp/bootloader.py index 695c3413..a7fc1fa2 100644 --- a/xcp/bootloader.py +++ b/xcp/bootloader.py @@ -33,7 +33,6 @@ import xcp.cmd -# pyre-ignore-all-errors[21] try: # xenserver-release.rpm puts a branding.py into our xcp installation directory: from xcp import branding # type:ignore[attr-defined] # pytype: disable=import-error except ImportError: # For CI, use stubs/branding.py (./stubs is added to pythonpath) diff --git a/xcp/cpiofile.py b/xcp/cpiofile.py index 995da94c..ff8f5c7c 100644 --- a/xcp/cpiofile.py +++ b/xcp/cpiofile.py @@ -36,8 +36,6 @@ Derived from Lars Gustäbel's tarfile.py """ from __future__ import print_function -# pyre is not as good as other static analysis tools in inferring the correct types: -# pyre-ignore-all-errors[6,16] __version__ = "0.1" __author__ = "Simon Rowe" From 5203159866cbad231739c41108fe68fe3ca41c4c Mon Sep 17 00:00:00 2001 From: Bernhard Kaindl Date: Fri, 15 Aug 2025 12:00:00 +0200 Subject: [PATCH 6/6] Fix remaining pyright warnings found by the unit tests Signed-off-by: Bernhard Kaindl --- pyproject.toml | 4 ++-- tests/test_bootloader.py | 1 + tests/test_xmlunwrap.py | 1 + xcp/bootloader.py | 2 +- 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index ac1cabf4..ae27d940 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -195,8 +195,8 @@ include = ["xcp", "tests"] pythonPlatform = "Linux" pythonVersion = "3.11" reportFunctionMemberAccess = true -reportGeneralTypeIssues = "warning" -reportOptionalMemberAccess = "warning" +reportGeneralTypeIssues = "error" +reportOptionalMemberAccess = "error" reportPrivateUsage = true reportPropertyTypeMismatch = true reportUnnecessaryTypeIgnoreComment = false diff --git a/tests/test_bootloader.py b/tests/test_bootloader.py index 7a0a7a0a..4a34cec6 100644 --- a/tests/test_bootloader.py +++ b/tests/test_bootloader.py @@ -18,6 +18,7 @@ def _test_cfg(self, cfg): stdout = subprocess.PIPE, universal_newlines=True) + assert proc.stdout is not None # for pyright, to ensure it is valid self.assertEqual(proc.stdout.read(), '''5a6,13 > if [ -s $prefix/grubenv ]; then > load_env diff --git a/tests/test_xmlunwrap.py b/tests/test_xmlunwrap.py index 19d9241b..2c47e6af 100644 --- a/tests/test_xmlunwrap.py +++ b/tests/test_xmlunwrap.py @@ -15,6 +15,7 @@ def setUp(self): self.top_el = xmldoc.documentElement def test(self): + assert self.top_el is not None # This test requires that top_el is not None self.assertEqual(self.top_el.tagName, "installation") self.assertEqual([getText(el) diff --git a/xcp/bootloader.py b/xcp/bootloader.py index a7fc1fa2..6bb17459 100644 --- a/xcp/bootloader.py +++ b/xcp/bootloader.py @@ -60,7 +60,7 @@ def __init__(self, hypervisor, hypervisor_args, kernel, kernel_args, self.initrd = initrd self.title = title self.root = root - self.entry_format = None + self.entry_format = None # type: Grub2Format | None def getHypervisorArgs(self): return re.findall(r'\S[^ "]*(?:"[^"]*")?\S*', self.hypervisor_args)