Enable pyright hook (#4426)

Author: ssbarnea

.pre-commit-config.yaml

@@ -7,6 +7,7 @@ ci:
   skip:
     # https://github.com/pre-commit-ci/issues/issues/55
     - pip-compile
+    - pyright
     - schemas
   submodules: true
 exclude: >
@@ -178,31 +179,31 @@ repos:
             test/local-content/.*|
             plugins/.*
           )$
-  # - repo: https://github.com/RobertCraigie/pyright-python
-  #   rev: v1.1.389
-  #   hooks:
-  #     - id: pyright
-  #       additional_dependencies:
-  #         - nodejs-wheel-binaries
-  #         - ansible-compat>=24.8.0
-  #         - black>=22.10.0
-  #         - cryptography>=39.0.1
-  #         - filelock>=3.12.2
-  #         - importlib_metadata
-  #         - jinja2
-  #         - license-expression >= 30.3.0
-  #         - pip>=22.3.1
-  #         - pytest-mock
-  #         - pytest>=7.2.2
-  #         - rich>=13.2.0
-  #         - ruamel-yaml-clib>=0.2.8
-  #         - ruamel-yaml>=0.18.6
-  #         - subprocess-tee
-  #         - types-PyYAML
-  #         - types-jsonschema>=4.20.0.0
-  #         - types-setuptools
-  #         - wcmatch
-  #         - yamllint
+  - repo: https://github.com/RobertCraigie/pyright-python
+    rev: v1.1.389
+    hooks:
+      - id: pyright
+        additional_dependencies:
+          - nodejs-wheel-binaries
+          - ansible-compat>=24.8.0
+          - black>=22.10.0
+          - cryptography>=39.0.1
+          - filelock>=3.12.2
+          - importlib_metadata
+          - jinja2
+          - license-expression >= 30.3.0
+          - pip>=22.3.1
+          - pytest-mock
+          - pytest>=7.2.2
+          - rich>=13.2.0
+          - ruamel-yaml-clib>=0.2.8
+          - ruamel-yaml>=0.18.6
+          - subprocess-tee
+          - types-PyYAML
+          - types-jsonschema>=4.20.0.0
+          - types-setuptools
+          - wcmatch
+          - yamllint
   - repo: https://github.com/pycqa/pylint
     rev: v3.3.1
     hooks:

.vscode/settings.json

@@ -17,9 +17,6 @@
         "**/*.txt",
         "**/*.md"
     ],
-    "python.analysis.exclude": [
-        "build"
-    ],
     "python.terminal.activateEnvironment": true,
     "python.testing.pytestEnabled": true,
     "python.testing.unittestEnabled": false,

pyproject.toml

@@ -153,7 +153,16 @@ output-format = "colorized"
 score = "n"
 
 [tool.pyright]
-exclude = ["venv", ".cache"]
+exclude = [
+  ".cache",
+  ".config",
+  ".tox",
+  "ansible_collections",
+  "build",
+  "dist",
+  "site",
+  "venv"
+]
 include = ["src"]
 mode = "standard"
 # https://github.com/microsoft/pyright/blob/main/docs/configuration.md#sample-pyprojecttoml-file
@@ -339,6 +348,7 @@ exclude = [
 ]
 ignore_names = [
   "_ANSIBLE_ARGS",
+  "__line__",
   "__rich_console__",
   "fixture_*",
   "pytest_addoption",

src/ansiblelint/utils.py

@@ -39,11 +39,13 @@
 import ruamel.yaml.parser
 import yaml
 from ansible.errors import AnsibleError, AnsibleParserError
+from ansible.module_utils._text import to_bytes
 from ansible.module_utils.parsing.convert_bool import boolean
 from ansible.parsing.dataloader import DataLoader
 from ansible.parsing.mod_args import ModuleArgsParser
 from ansible.parsing.plugin_docs import read_docstring
 from ansible.parsing.splitter import split_args
+from ansible.parsing.vault import PromptVaultSecret
 from ansible.parsing.yaml.constructor import AnsibleConstructor, AnsibleMapping
 from ansible.parsing.yaml.loader import AnsibleLoader
 from ansible.parsing.yaml.objects import AnsibleBaseYAMLObject, AnsibleSequence
@@ -56,7 +58,9 @@
 from ansible.template import Templar
 from ansible.utils.collection_loader import AnsibleCollectionConfig
 from yaml.composer import Composer
+from yaml.parser import ParserError
 from yaml.representer import RepresenterError
+from yaml.scanner import ScannerError
 
 from ansiblelint._internal.rules import (
     AnsibleParserErrorRule,
@@ -95,8 +99,11 @@
 def parse_yaml_from_file(filepath: str) -> AnsibleBaseYAMLObject:
     """Extract a decrypted YAML object from file."""
     dataloader = DataLoader()
-    if hasattr(dataloader, "set_vault_password"):
-        dataloader.set_vault_password(DEFAULT_VAULT_PASSWORD)
+    if hasattr(dataloader, "set_vault_secrets"):
+        dataloader.set_vault_secrets(
+            [("default", PromptVaultSecret(_bytes=to_bytes(DEFAULT_VAULT_PASSWORD)))]
+        )
+
     return dataloader.load_from_file(filepath)
 
 
@@ -254,7 +261,11 @@ def set_collections_basedir(basedir: Path) -> None:
     """Set the playbook directory as playbook_paths for the collection loader."""
     # Ansible expects only absolute paths inside `playbook_paths` and will
     # produce weird errors if we use a relative one.
-    AnsibleCollectionConfig.playbook_paths = str(basedir.resolve())
+    # https://github.com/psf/black/issues/4519
+    # fmt: off
+    AnsibleCollectionConfig.playbook_paths = (  # pyright: ignore[reportAttributeAccessIssue]
+        str(basedir.resolve()))
+    # fmt: on
 
 
 def template(
@@ -911,7 +922,7 @@ def task_in_list(
     """Get action tasks from block structures."""
 
     def each_entry(data: AnsibleBaseYAMLObject, position: str) -> Iterator[Task]:
-        if not data:
+        if not data or not isinstance(data, Iterable):
             return
         for entry_index, entry in enumerate(data):
             if not entry:
@@ -951,29 +962,35 @@ def each_entry(data: AnsibleBaseYAMLObject, position: str) -> Iterator[Task]:
         yield from each_entry(data, position)
 
 
-def add_action_type(actions: AnsibleBaseYAMLObject, action_type: str) -> list[Any]:
+def add_action_type(
+    actions: AnsibleBaseYAMLObject, action_type: str
+) -> AnsibleSequence:
     """Add action markers to task objects."""
-    results = []
-    for action in actions:
-        # ignore empty task
-        if not action:
-            continue
-        action["__ansible_action_type__"] = BLOCK_NAME_TO_ACTION_TYPE_MAP[action_type]
-        results.append(action)
+    results = AnsibleSequence()
+    if isinstance(actions, Iterable):
+        for action in actions:
+            # ignore empty task
+            if not action:
+                continue
+            action["__ansible_action_type__"] = BLOCK_NAME_TO_ACTION_TYPE_MAP[
+                action_type
+            ]
+            results.append(action)
     return results
 
 
 @cache
 def parse_yaml_linenumbers(
     lintable: Lintable,
-) -> AnsibleBaseYAMLObject:
+) -> AnsibleBaseYAMLObject | None:
     """Parse yaml as ansible.utils.parse_yaml but with linenumbers.
 
     The line numbers are stored in each node's LINE_NUMBER_KEY key.
     """
-    result = []
+    result = AnsibleSequence()
 
-    def compose_node(parent: yaml.nodes.Node, index: int) -> yaml.nodes.Node:
+    # signature of Composer.compose_node
+    def compose_node(parent: yaml.nodes.Node | None, index: int) -> yaml.nodes.Node:
         # the line number where the previous token has ended (plus empty lines)
         line = loader.line
         node = Composer.compose_node(loader, parent, index)
@@ -983,14 +1000,15 @@ def compose_node(parent: yaml.nodes.Node, index: int) -> yaml.nodes.Node:
         node.__line__ = line + 1  # type: ignore[attr-defined]
         return node
 
+    # signature of AnsibleConstructor.construct_mapping
     def construct_mapping(
-        node: AnsibleBaseYAMLObject,
-        *,
-        deep: bool = False,
+        node: yaml.MappingNode,
+        deep: bool = False,  # noqa: FBT002
     ) -> AnsibleMapping:
+        # pyright: ignore[reportArgumentType]
         mapping = AnsibleConstructor.construct_mapping(loader, node, deep=deep)
-        if hasattr(node, "__line__"):
-            mapping[LINE_NUMBER_KEY] = node.__line__
+        if hasattr(node, LINE_NUMBER_KEY):
+            mapping[LINE_NUMBER_KEY] = getattr(node, LINE_NUMBER_KEY)
         else:
             mapping[LINE_NUMBER_KEY] = mapping._line_number  # noqa: SLF001
         mapping[FILENAME_KEY] = lintable.path
@@ -1001,7 +1019,9 @@ def construct_mapping(
         if "vault_password" in inspect.getfullargspec(AnsibleLoader.__init__).args:
             kwargs["vault_password"] = DEFAULT_VAULT_PASSWORD
         loader = AnsibleLoader(lintable.content, **kwargs)
+        # redefine Composer.compose_node
         loader.compose_node = compose_node
+        # redefine AnsibleConstructor.construct_mapping
         loader.construct_mapping = construct_mapping
         # while Ansible only accepts single documents, we also need to load
         # multi-documents, as we attempt to load any YAML file, not only
@@ -1012,8 +1032,8 @@ def construct_mapping(
                 break
             result.append(data)
     except (
-        yaml.parser.ParserError,
-        yaml.scanner.ScannerError,
+        ParserError,
+        ScannerError,
         yaml.constructor.ConstructorError,
         ruamel.yaml.parser.ParserError,
     ) as exc:

src/ansiblelint/version.py

@@ -1,15 +1,20 @@
 """Ansible-lint version information."""
 
-try:
-    from ._version import version as __version__
-except ImportError:  # pragma: no cover
-    try:
-        import pkg_resources
+# this is the fallback SemVer version picked by setuptools_scm when tag
+# information is not available.
+__version__ = "0.1.dev1"
+__all__ = ("__version__",)
+from typing import TYPE_CHECKING
 
-        __version__ = pkg_resources.get_distribution("ansible-lint").version
-    except Exception:  # pylint: disable=broad-except # noqa: BLE001
-        # this is the fallback SemVer version picked by setuptools_scm when tag
-        # information is not available.
-        __version__ = "0.1.dev1"
+# as either pyright or mypy have problems with these import fallbacks, we
+# avoid running them when doing type checking
+if not TYPE_CHECKING:
+    try:
+        from ._version import version as __version__
+    except ImportError:  # pragma: no cover
+        try:
+            import pkg_resources
 
-__all__ = ("__version__",)
+            __version__ = pkg_resources.get_distribution("ansible-lint").version
+        except Exception:  # pylint: disable=broad-except  # noqa: BLE001, S110
+            pass

test/rules/fixtures/raw_task.py

@@ -15,7 +15,7 @@ class RawTaskRule(AnsibleLintRule):
     """Test rule that inspects the raw task."""
 
     id = "raw-task"
-    shortdesc = "Test rule that inspects the raw task"
+    _shortdesc = "Test rule that inspects the raw task"
     tags = ["fake", "dummy", "test3"]
     needs_raw_task = True
 

test/test_schemas.py

@@ -47,9 +47,11 @@ def test_request_timeouterror_handling(
 ) -> None:
     """Test that schema refresh can handle time out errors."""
     error_msg = "Simulating handshake operation time out."
-    mock_request.urlopen.side_effect = urllib.error.URLError(
-        TimeoutError(error_msg)
-    )  # pyright: reportAttributeAccessIssue=false
+    mock_request.urlopen.side_effect = (
+        urllib.error.URLError(  # pyright: ignore[reportAttributeAccessIssue]
+            TimeoutError(error_msg)
+        )
+    )
     with caplog.at_level(logging.DEBUG):
         assert refresh_schemas(min_age_seconds=0) == 0
     mock_request.urlopen.assert_called()

test/test_utils.py

@@ -28,6 +28,7 @@
 from typing import TYPE_CHECKING, Any
 
 import pytest
+from ansible.parsing.yaml.constructor import AnsibleMapping, AnsibleSequence
 from ansible.utils.sentinel import Sentinel
 from ansible_compat.runtime import Runtime
 
@@ -45,7 +46,6 @@
     from _pytest.capture import CaptureFixture
     from _pytest.logging import LogCaptureFixture
     from _pytest.monkeypatch import MonkeyPatch
-    from ansible.parsing.yaml.objects import AnsibleBaseYAMLObject
 
     from ansiblelint.rules import RulesCollection
 
@@ -221,7 +221,7 @@ def test_extract_from_list() -> None:
         "test_none": None,
         "test_string": "foo",
     }
-    blocks = [block]
+    blocks = AnsibleSequence([block])
 
     test_list = utils.extract_from_list(blocks, ["block"])
     test_none = utils.extract_from_list(blocks, ["test_none"])
@@ -234,10 +234,12 @@ def test_extract_from_list() -> None:
 
 def test_extract_from_list_recursive() -> None:
     """Check that tasks get extracted from blocks if present."""
-    block = {
-        "block": [{"block": [{"name": "hello", "command": "whoami"}]}],
-    }
-    blocks: AnsibleBaseYAMLObject = [block]
+    block = AnsibleMapping(
+        {
+            "block": [{"block": [{"name": "hello", "command": "whoami"}]}],
+        }
+    )
+    blocks = AnsibleSequence([block])
 
     test_list = utils.extract_from_list(blocks, ["block"])
     assert list(block["block"]) == test_list