From d9ce687748650b3dd2c5bebc1ee68466745e8458 Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Tue, 19 Jan 2021 15:34:45 +0000 Subject: [PATCH] More type --- .pre-commit-config.yaml | 11 ++++++++-- conftest.py | 4 +++- docs/rules_table_generator_ext.py | 2 +- mypy.ini | 14 ++++++++++++- src/ansiblelint/__main__.py | 8 ++++--- src/ansiblelint/app.py | 3 ++- src/ansiblelint/color.py | 4 ++-- .../rules/AnsibleSyntaxCheckRule.py | 4 ++-- src/ansiblelint/rules/NoSameOwnerRule.py | 5 ++++- src/ansiblelint/rules/__init__.py | 8 ------- src/ansiblelint/skip_utils.py | 16 +++++++------- src/ansiblelint/utils.py | 21 +++++++------------ 12 files changed, 57 insertions(+), 43 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 55ccb085444..c8496c02ada 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -76,7 +76,7 @@ repos: - flake8-docstrings>=1.5.0 - flake8-pytest-style>=1.2.2 - repo: https://github.com/pre-commit/mirrors-mypy - rev: v0.800 + rev: v0.812 hooks: - id: mypy # empty args needed in order to match mypy cli behavior @@ -85,8 +85,15 @@ repos: - Sphinx>=3.1.2 - enrich - flaky - - yamllint + - pytest + - types-PyYAML - wcmatch + - yamllint + exclude: > + (?x)^( + test/.*| + plugins/.* + )$ - repo: https://github.com/pre-commit/mirrors-pylint rev: v2.6.0 hooks: diff --git a/conftest.py b/conftest.py index 5dcfb0f125c..b740875b84a 100644 --- a/conftest.py +++ b/conftest.py @@ -3,11 +3,13 @@ import re import sys +from typing import List + os.environ["NO_COLOR"] = "1" pytest_plugins = ["ansiblelint.testing.fixtures"] -def pytest_cmdline_preparse(args): +def pytest_cmdline_preparse(args: List[str]) -> None: """Pytest hook.""" # disable xdist when called with -k args (filtering) # https://stackoverflow.com/questions/66407583/how-to-disable-pytest-xdist-only-when-pytest-is-called-with-filters diff --git a/docs/rules_table_generator_ext.py b/docs/rules_table_generator_ext.py index c6629f01aa4..9ecdd4222dd 100644 --- a/docs/rules_table_generator_ext.py +++ b/docs/rules_table_generator_ext.py @@ -36,7 +36,7 @@ def _nodes_from_rst( ), node=node, ) - return node.children + return node.children # type: ignore class AnsibleLintDefaultRulesDirective(SphinxDirective): diff --git a/mypy.ini b/mypy.ini index 1baa6e01b2c..a070457673f 100644 --- a/mypy.ini +++ b/mypy.ini @@ -3,11 +3,23 @@ python_version = 3.6 color_output = True error_summary = True disallow_untyped_calls = True -; warn_redundant_casts=True +disallow_untyped_defs = True +; disallow_any_generics = True +; disallow_any_unimported = True +; warn_redundant_casts = True +; warn_return_any = True +; warn_unused_configs = True [mypy-ansiblelint.*] ignore_missing_imports = True +[mypy-ansiblelint.testing.*] +disallow_untyped_defs = False + +[mypy-test.*] +disallow_untyped_defs = False + + # 3rd party ignores [mypy-ansible] ignore_missing_imports = True diff --git a/src/ansiblelint/__main__.py b/src/ansiblelint/__main__.py index f153358ab52..64a8da85d6a 100755 --- a/src/ansiblelint/__main__.py +++ b/src/ansiblelint/__main__.py @@ -20,6 +20,7 @@ # THE SOFTWARE. """Command line implementation.""" +from argparse import Namespace import errno import logging import os @@ -27,7 +28,8 @@ import subprocess import sys from contextlib import contextmanager -from typing import TYPE_CHECKING, List, Optional +from typing import TYPE_CHECKING, List, Iterator, Optional + from enrich.console import should_do_markup @@ -108,7 +110,7 @@ def initialize_options(arguments: Optional[List[str]] = None) -> None: options.configured = True -def report_outcome(result: "LintResult", options, mark_as_success=False) -> int: +def report_outcome(result: "LintResult", options: Namespace, mark_as_success: bool = False) -> int: """Display information about how to skip found rules. Returns exit code, 2 if errors were found, 0 when only warnings were found. @@ -249,7 +251,7 @@ def main(argv: List[str] = None) -> int: @contextmanager -def _previous_revision(): +def _previous_revision() -> Iterator[None]: """Create or update a temporary workdir containing the previous revision.""" worktree_dir = ".cache/old-rev" revision = subprocess.run( diff --git a/src/ansiblelint/app.py b/src/ansiblelint/app.py index 2d135642966..32b6062d0d3 100644 --- a/src/ansiblelint/app.py +++ b/src/ansiblelint/app.py @@ -5,6 +5,7 @@ from ansiblelint import formatters from ansiblelint.color import console +from ansiblelint.errors import MatchError if TYPE_CHECKING: from argparse import Namespace @@ -26,7 +27,7 @@ def __init__(self, options: "Namespace"): formatter_factory = choose_formatter_factory(options) self.formatter = formatter_factory(options.cwd, options.display_relative_path) - def render_matches(self, matches: List) -> None: + def render_matches(self, matches: List[MatchError]) -> None: """Display given matches.""" if isinstance(self.formatter, formatters.CodeclimateJSONFormatter): # If formatter CodeclimateJSONFormatter is chosen, diff --git a/src/ansiblelint/color.py b/src/ansiblelint/color.py index 69687af54b3..febde67526b 100644 --- a/src/ansiblelint/color.py +++ b/src/ansiblelint/color.py @@ -25,7 +25,7 @@ console_stderr = Console(**console_options_stderr) -def reconfigure(new_options: Dict[str, Any]): +def reconfigure(new_options: Dict[str, Any]) -> None: """Reconfigure console options.""" global console_options # pylint: disable=global-statement global console_stderr # pylint: disable=global-statement @@ -39,6 +39,6 @@ def reconfigure(new_options: Dict[str, Any]): console_stderr.__dict__ = tmp_console.__dict__ -def render_yaml(text: str): +def render_yaml(text: str) -> Syntax: """Colorize YAMl for nice display.""" return Syntax(text, 'yaml', theme="ansi_dark") diff --git a/src/ansiblelint/rules/AnsibleSyntaxCheckRule.py b/src/ansiblelint/rules/AnsibleSyntaxCheckRule.py index ddf5026bc29..b8414003d08 100644 --- a/src/ansiblelint/rules/AnsibleSyntaxCheckRule.py +++ b/src/ansiblelint/rules/AnsibleSyntaxCheckRule.py @@ -3,7 +3,7 @@ import re import subprocess import sys -from typing import List +from typing import List, Any from ansiblelint._internal.rules import BaseRule, RuntimeErrorRule from ansiblelint.config import options @@ -141,7 +141,7 @@ def test_empty_playbook() -> None: assert result[0].message == "Empty playbook, nothing to do" assert len(result) == 1 - def test_extra_vars_passed_to_command(config_options) -> None: + def test_extra_vars_passed_to_command(config_options: Any) -> None: """Validate `extra-vars` are passed to syntax check command.""" config_options.extra_vars = { 'foo': 'bar', diff --git a/src/ansiblelint/rules/NoSameOwnerRule.py b/src/ansiblelint/rules/NoSameOwnerRule.py index 98e345aceb1..e76ee8e00a4 100644 --- a/src/ansiblelint/rules/NoSameOwnerRule.py +++ b/src/ansiblelint/rules/NoSameOwnerRule.py @@ -109,6 +109,7 @@ def handle_unarchive(task: Any) -> bool: import pytest + from ansiblelint.rules import RulesCollection from ansiblelint.runner import Runner # pylint: disable=ungrouped-imports @pytest.mark.parametrize( @@ -122,7 +123,9 @@ def handle_unarchive(task: Any) -> bool: ), ), ) - def test_no_same_owner_rule(default_rules_collection, test_file, failures) -> None: + def test_no_same_owner_rule( + default_rules_collection: RulesCollection, test_file: str, failures: int + ) -> None: """Test rule matches.""" results = Runner(test_file, rules=default_rules_collection).run() assert len(results) == failures diff --git a/src/ansiblelint/rules/__init__.py b/src/ansiblelint/rules/__init__.py index a7d166ad537..6b444ab7129 100644 --- a/src/ansiblelint/rules/__init__.py +++ b/src/ansiblelint/rules/__init__.py @@ -130,14 +130,6 @@ def matchtasks(self, file: Lintable) -> List[MatchError]: matches.append(m) return matches - @staticmethod - def _matchplay_linenumber(play, optional_linenumber): - try: - (linenumber,) = optional_linenumber - except ValueError: - linenumber = play[ansiblelint.utils.LINE_NUMBER_KEY] - return linenumber - def matchyaml(self, file: Lintable) -> List[MatchError]: matches: List[MatchError] = [] if not self.matchplay or file.base_kind != 'text/yaml': diff --git a/src/ansiblelint/skip_utils.py b/src/ansiblelint/skip_utils.py index 8010b71f1ef..823f696edfe 100644 --- a/src/ansiblelint/skip_utils.py +++ b/src/ansiblelint/skip_utils.py @@ -52,7 +52,7 @@ def get_rule_skips_from_line(line: str) -> List[str]: def append_skipped_rules( pyyaml_data: "AnsibleBaseYAMLObject", lintable: Lintable -) -> Sequence[Any]: +) -> "AnsibleBaseYAMLObject": """Append 'skipped_rules' to individual tasks or single metadata block. For a file, uses 2nd parser (ruamel.yaml) to pull comments out of @@ -65,12 +65,14 @@ def append_skipped_rules( :returns: original pyyaml_data altered with a 'skipped_rules' list added to individual tasks, or added to the single metadata block. """ - try: - yaml_skip = _append_skipped_rules(pyyaml_data, lintable) - except RuntimeError: - # Notify user of skip error, do not stop, do not change exit code - _logger.error('Error trying to append skipped rules', exc_info=True) - return pyyaml_data + if isinstance(pyyaml_data, Sequence): + try: + yaml_skip = _append_skipped_rules(pyyaml_data, lintable) + except RuntimeError: + # Notify user of skip error, do not stop, do not change exit code + _logger.error('Error trying to append skipped rules', exc_info=True) + return pyyaml_data + return yaml_skip diff --git a/src/ansiblelint/utils.py b/src/ansiblelint/utils.py index 6f0963e9f44..e4a80873470 100644 --- a/src/ansiblelint/utils.py +++ b/src/ansiblelint/utils.py @@ -186,7 +186,7 @@ def _playbook_items(pb_data: dict) -> ItemsView: return [item for play in pb_data if play for item in play.items()] -def _set_collections_basedir(basedir: str): +def _set_collections_basedir(basedir: str) -> None: # Sets the playbook directory as playbook_paths for the collection loader try: # Ansible 2.10+ @@ -480,16 +480,7 @@ def _look_for_role_files(basedir: str, role: str, main='main') -> List[Lintable] return results -def rolename(filepath): - idx = filepath.find('roles/') - if idx < 0: - return '' - role = filepath[idx + 6 :] - role = role[: role.find('/')] - return role - - -def _kv_to_dict(v): +def _kv_to_dict(v: str) -> Dict[str, Any]: (command, args, kwargs) = tokenize(v) return dict(__ansible_module__=command, __ansible_arguments__=args, **kwargs) @@ -560,7 +551,7 @@ def normalize_task_v1(task): # noqa: C901 for (k, v) in task.items(): if k in VALID_KEYS or k.startswith('with_'): if k in ('local_action', 'action'): - if not isinstance(v, dict): + if isinstance(v, str): v = _kv_to_dict(v) v['__ansible_arguments__'] = v.get('__ansible_arguments__', list()) result['action'] = v @@ -712,11 +703,13 @@ def parse_yaml_linenumbers(lintable: Lintable) -> AnsibleBaseYAMLObject: The line numbers are stored in each node's LINE_NUMBER_KEY key. """ - def compose_node(parent, index): + def compose_node(parent, index) -> 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) + node = Composer.compose_node(loader, parent, index) # type: ignore node.__line__ = line + 1 + if not isinstance(node, yaml.nodes.Node): + raise RuntimeError("Unexpected yaml data.") return node def construct_mapping(node, deep=False):