Add pyright as a pre-commit check

Another pair of eyes, pyright is a type check plus a bit more, and will inspect the code from slightyl different point of view. As a result, it will report issues mypy cannot spot, or cannot spot *yet* because the set of implemented checks is simply different. The slow, gradual approach is the key here, like the one we know from mypy introduction, and slowly files and packages will get covered.
teemtee · Oct 6, 2023 · 1bc07d3 · 1bc07d3
1 parent 8e4f385
commit 1bc07d3
Show file tree

Hide file tree

Showing 4 changed files with 99 additions and 26 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -18,23 +18,59 @@ repos:
     rev: "v1.4.1"
     hooks:
       - id: mypy
-        # Do not install *types-click* - it's not recommended with Click 8 & newer,
-        # which is the version a developer encounters given the requirements are not
-        # frozen.
         additional_dependencies:
-          - click!=8.1.4  # TODO github.com/pallets/click/issues/2558
+          # Do not install *types-click* - it's not recommended with Click 8 & newer,
+          # which is the version a developer encounters given the requirements are not
+          # frozen.
+          - "click>=8.0.3,!=8.1.4"   # 8.1.3 / 8.1.6 TODO type annotations tmt.cli.Context -> click.core.Context click/issues/2558
+          - "fmf>=1.2.1"
+          - "jinja2>=2.11.3"         # 3.1.2 / 3.1.2
+          - "pint<=0.20"
+          - "requests>=2.25.1"       # 2.28.2 / 2.31.0
+          - "ruamel.yaml>=0.16.6"    # 0.17.32 / 0.17.32
+          - "urllib3>=1.26.5, <2.0"  # 1.26.16 / 2.0.4
+
+          - "typing-extensions>=4.4.0; python_version < '3.10'" # TypeAlias introduced in 3.10, Self in 3.11
+          - types-babel
+          - types-jinja2
+          - types-jsonschema
           - types-Markdown
           - types-requests
           - types-setuptools
-          - types-jsonschema
           - types-urllib3
-          - ruamel.yaml
-          - types-jinja2
-          - pint==0.20
-          - types-babel
+
         pass_filenames: false
         args: [--config-file=pyproject.toml]
 
+  - repo: https://github.com/RobertCraigie/pyright-python
+    rev: v1.1.329
+    hooks:
+      - id: pyright
+        # TODO: find out how to amend pyright, pre-commit and package requirements.
+        # Apparently, there is no easy way to avoid some level of duplication of
+        # information when the package is *not* installed, which is the case of tmt
+        # & pre-commit in our setup.
+        additional_dependencies:
+          # Do not install *types-click* - it's not recommended with Click 8 & newer,
+          # which is the version a developer encounters given the requirements are not
+          # frozen.
+          - "click>=8.0.3,!=8.1.4"   # 8.1.3 / 8.1.6 TODO type annotations tmt.cli.Context -> click.core.Context click/issues/2558
+          - "fmf>=1.2.1"
+          - "jinja2>=2.11.3"         # 3.1.2 / 3.1.2
+          - "pint<=0.20"
+          - "requests>=2.25.1"       # 2.28.2 / 2.31.0
+          - "ruamel.yaml>=0.16.6"    # 0.17.32 / 0.17.32
+          - "urllib3>=1.26.5, <2.0"  # 1.26.16 / 2.0.4
+
+          - "typing-extensions>=4.4.0; python_version < '3.10'" # TypeAlias introduced in 3.10, Self in 3.11
+          - types-babel
+          - types-jinja2
+          - types-jsonschema
+          - types-Markdown
+          - types-requests
+          - types-setuptools
+          - types-urllib3
+
   - repo: https://github.com/python-jsonschema/check-jsonschema
     rev: "0.23.3"
     hooks:
@@ -63,4 +99,4 @@ repos:
       additional_dependencies:
         # Make sure we use fmf compatible with tmt in the repo, not the 1.26.0 version.
         - "fmf>=1.3.0"
-        - pint==0.20
+        - "pint<=0.20"
diff --git a/pyproject.toml b/pyproject.toml
@@ -33,7 +33,7 @@ dependencies = [              # F39 / PyPI
     "click>=8.0.3,!=8.1.4",   # 8.1.3 / 8.1.6 TODO type annotations tmt.cli.Context -> click.core.Context click/issues/2558
     "fmf>=1.3.0",
     "jinja2>=2.11.3",         # 3.1.2 / 3.1.2
-    "pint>=0.16.1",           # 0.16.1 / 0.22
+    "pint<=0.20",             # 0.16.1 / 0.22
     "requests>=2.25.1",       # 2.28.2 / 2.31.0
     "ruamel.yaml>=0.16.6",    # 0.17.32 / 0.17.32
     "urllib3>=1.26.5, <2.0",  # 1.26.16 / 2.0.4
@@ -202,6 +202,49 @@ module = [
     ]
 ignore_missing_imports = true
 
+[tool.pyright]
+include = [
+    "tmt/**/*.py",
+]
+ignore = [
+    "docs/**",
+    "examples/**",
+    "tests/**",
+    "tmt/checks/*.py",
+    "tmt/export/*.py",
+    "tmt/frameworks/*.py",
+    "tmt/libraries/*.py",
+    "tmt/plugins/*.py",
+    "tmt/steps/**/*.py",
+    "tmt/base.py",
+    "tmt/cli.py",
+    "tmt/convert.py",
+    # "tmt/hardware.py",
+    # "tmt/identifier.py",
+    "tmt/__init__.py",
+    "tmt/lint.py",
+    "tmt/log.py",
+    "tmt/__main__.py",
+    "tmt/options.py",
+    # "tmt/queue.py",
+    # "tmt/result.py",
+    "tmt/templates.py",
+    "tmt/utils.py",
+]
+
+pythonVersion = "3.9"
+pythonPlatform = "Linux"
+
+typeCheckingMode = "strict"
+
+# Stub file not found for "fmf.utils"
+reportMissingTypeStubs = false
+reportPrivateUsage = false
+# Type of "listed" is partially unknown
+reportUnknownMemberType = false
+# Unnecessary "cast" call; type is already...
+reportUnnecessaryCast = false
+
 [tool.autopep8]
 max_line_length = 99
 in-place = true

diff --git a/tmt/hardware.py b/tmt/hardware.py
@@ -1222,9 +1222,6 @@ def report_support(
 
         for variant in self.constraint.variants():
             for constraint in variant:
-                if not isinstance(constraint, Constraint):
-                    continue
-
                 name, _, child_name = constraint.expand_name()
 
                 if name in names \

diff --git a/tmt/result.py b/tmt/result.py
@@ -1,11 +1,13 @@
 import dataclasses
 import enum
 import re
-from typing import TYPE_CHECKING, Dict, List, Optional, cast
+from typing import TYPE_CHECKING, Callable, Dict, List, Optional, cast
 
 import click
 import fmf
+import fmf.utils
 
+import tmt.identifier
 import tmt.utils
 from tmt.checks import CheckEvent
 from tmt.utils import Path, field
@@ -90,7 +92,7 @@ class BaseResult(tmt.utils.SerializableContainer):
         )
     note: Optional[str] = None
     log: List[Path] = field(
-        default_factory=list,
+        default_factory=cast(Callable[[], List[Path]], list),
         serialize=lambda logs: [str(log) for log in logs],
         unserialize=lambda value: [Path(log) for log in value])
 
@@ -142,7 +144,7 @@ class Result(BaseResult):
         )
 
     check: List[CheckResult] = field(
-        default_factory=list,
+        default_factory=cast(Callable[[], List[CheckResult]], list),
         serialize=lambda results: [result.to_serialized() for result in results],
         unserialize=lambda serialized: [
             CheckResult.from_serialized(check) for check in serialized]
@@ -178,11 +180,6 @@ def from_test(
         (see https://tmt.readthedocs.io/en/stable/spec/tests.html#result).
         """
 
-        from tmt.base import Test
-
-        if not isinstance(test, Test):
-            raise tmt.utils.SpecificationError(f"Invalid test '{test}'.")
-
         # Saving identifiable information for each test case so we can match them
         # to Polarion/Nitrate/other cases and report run results there
         # TODO: would an exception be better? Can test.id be None?
@@ -192,7 +189,7 @@ def from_test(
             }
 
         for key in EXTRA_RESULT_IDENTIFICATION_KEYS:
-            default_ids[key] = test.node.get(key)
+            default_ids[key] = cast(Optional[str], test.node.get(key))
 
         default_ids.update(ids)
         ids = default_ids
@@ -232,7 +229,7 @@ def interpret_result(self, interpret: ResultInterpret) -> 'Result':
             return self
 
         # Extend existing note or set a new one
-        if self.note and isinstance(self.note, str):
+        if self.note or self.note == '':
             self.note += f', original result: {self.result.value}'
 
         elif self.note is None:
@@ -272,7 +269,7 @@ def total(results: List['Result']) -> Dict[ResultOutcome, int]:
     def summary(results: List['Result']) -> str:
         """ Prepare a nice human summary of provided results """
         stats = Result.total(results)
-        comments = []
+        comments: List[str] = []
         if stats.get(ResultOutcome.PASS):
             passed = ' ' + click.style('passed', fg='green')
             comments.append(fmf.utils.listed(stats[ResultOutcome.PASS], 'test') + passed)
@@ -328,7 +325,7 @@ def failures(log: Optional[str], msg_type: str = 'FAIL') -> str:
         if re.search(':: \\[   FAIL   \\] ::', log):  # dumb check for a beakerlib log
             copy_line = False
             copy_phase_name = False
-            failure_log = []
+            failure_log: List[str] = []
             # we will be processing log lines in a reversed order
             iterator = iter(reversed(log.split("\n")))
             for line in iterator: