diff --git a/.github/workflows/tox.yml b/.github/workflows/tox.yml index 448dba4e9f..6fa6254f98 100644 --- a/.github/workflows/tox.yml +++ b/.github/workflows/tox.yml @@ -72,7 +72,7 @@ jobs: env: # Number of expected test passes, safety measure for accidental skip of # tests. Update value if you add/remove tests. - PYTEST_REQPASS: 879 + PYTEST_REQPASS: 882 steps: - uses: actions/checkout@v4 with: diff --git a/examples/playbooks/common-include-1.yml b/examples/playbooks/common-include-1.yml index 3a4691f3ba..9885d61e92 100644 --- a/examples/playbooks/common-include-1.yml +++ b/examples/playbooks/common-include-1.yml @@ -8,3 +8,5 @@ - name: Some include_tasks with file and jinja2 ansible.builtin.include_tasks: file: "{{ 'tasks/included-with-lint.yml' }}" + - name: Some include 3 + ansible.builtin.include_tasks: file=tasks/included-with-lint.yml diff --git a/examples/playbooks/common-include-wrong-syntax.yml b/examples/playbooks/common-include-wrong-syntax.yml new file mode 100644 index 0000000000..c59b41b318 --- /dev/null +++ b/examples/playbooks/common-include-wrong-syntax.yml @@ -0,0 +1,9 @@ +--- +- name: Fixture for test coverage + hosts: localhost + gather_facts: false + tasks: + - name: Some include with invalid syntax + ansible.builtin.include_tasks: "file=" + - name: Some include with invalid syntax + ansible.builtin.include_tasks: other=tasks/included-with-lint.yml diff --git a/examples/playbooks/common-include-wrong-syntax2.yml b/examples/playbooks/common-include-wrong-syntax2.yml new file mode 100644 index 0000000000..a4891c88fc --- /dev/null +++ b/examples/playbooks/common-include-wrong-syntax2.yml @@ -0,0 +1,8 @@ +--- +- name: Fixture for test coverage + hosts: localhost + gather_facts: false + tasks: + - name: Some include with invalid syntax + ansible.builtin.include_tasks: + file: null diff --git a/examples/playbooks/common-include-wrong-syntax3.yml b/examples/playbooks/common-include-wrong-syntax3.yml new file mode 100644 index 0000000000..21bba1eab5 --- /dev/null +++ b/examples/playbooks/common-include-wrong-syntax3.yml @@ -0,0 +1,7 @@ +--- +- name: Fixture + hosts: localhost + tasks: + - name: Fixture + ansible.builtin.include_role: + name: include_wrong_syntax diff --git a/examples/playbooks/include.yml b/examples/playbooks/include.yml index 559672892e..57fe58e517 100644 --- a/examples/playbooks/include.yml +++ b/examples/playbooks/include.yml @@ -11,6 +11,7 @@ tasks: - ansible.builtin.include_tasks: tasks/x.yml - ansible.builtin.include_tasks: tasks/x.yml y=z + - ansible.builtin.include_tasks: file=tasks/x.yml handlers: - ansible.builtin.include_tasks: handlers/y.yml diff --git a/examples/roles/include_wrong_syntax/tasks/main.yml b/examples/roles/include_wrong_syntax/tasks/main.yml new file mode 100644 index 0000000000..be269e5a4e --- /dev/null +++ b/examples/roles/include_wrong_syntax/tasks/main.yml @@ -0,0 +1,3 @@ +--- +- name: Invalid syntax for import (coverage) + ansible.builtin.import_tasks: wrong=imported_tasks.yml diff --git a/examples/roles/var_naming_pattern/tasks/include_task_with_vars.yml b/examples/roles/var_naming_pattern/tasks/include_task_with_vars.yml index 49822279c9..40b6729499 100644 --- a/examples/roles/var_naming_pattern/tasks/include_task_with_vars.yml +++ b/examples/roles/var_naming_pattern/tasks/include_task_with_vars.yml @@ -1,12 +1,15 @@ --- -- name: include_task_with_vars | Foo +- name: include_task_with_vars | Var1 + ansible.builtin.include_tasks: file=../tasks/included-task-with-vars.yml + +- name: include_task_with_vars | Var2 ansible.builtin.include_tasks: ../tasks/included-task-with-vars.yml vars: var_naming_pattern_1: bar _var_naming_pattern_2: ... # we allow _ before the prefix __var_naming_pattern_3: ... # we allow __ before the prefix -- name: include_task_with_vars | Foo +- name: include_task_with_vars | Var3 ansible.builtin.include_role: name: bobbins vars: diff --git a/playbook.yml b/playbook.yml index f55677edc3..ea510a46ad 100644 --- a/playbook.yml +++ b/playbook.yml @@ -1,6 +1,7 @@ --- - name: Example hosts: localhost + gather_facts: false tasks: - name: include extra tasks ansible.builtin.include_tasks: diff --git a/src/ansiblelint/rules/role_name.py b/src/ansiblelint/rules/role_name.py index ee59c9fba5..ebe0b1a179 100644 --- a/src/ansiblelint/rules/role_name.py +++ b/src/ansiblelint/rules/role_name.py @@ -164,7 +164,7 @@ def _infer_role_name(meta: Path, default: str) -> str: if meta_data: try: return str(meta_data["galaxy_info"]["role_name"]) - except KeyError: + except (KeyError, TypeError): pass return default diff --git a/src/ansiblelint/rules/syntax_check.py b/src/ansiblelint/rules/syntax_check.py index 2a74c11d2a..9b072f675e 100644 --- a/src/ansiblelint/rules/syntax_check.py +++ b/src/ansiblelint/rules/syntax_check.py @@ -27,6 +27,13 @@ class KnownError: re.MULTILINE | re.S | re.DOTALL, ), ), + KnownError( + tag="no-file", + regex=re.compile( + r"^ERROR! (?PNo file specified for [^\n]*)", + re.MULTILINE | re.S | re.DOTALL, + ), + ), KnownError( tag="empty-playbook", regex=re.compile( diff --git a/src/ansiblelint/runner.py b/src/ansiblelint/runner.py index 1f5ded4232..f48732940b 100644 --- a/src/ansiblelint/runner.py +++ b/src/ansiblelint/runner.py @@ -455,6 +455,9 @@ def _emit_matches(self, files: list[Lintable]) -> Generator[MatchError, None, No visited: set[Lintable] = set() while visited != self.lintables: for lintable in self.lintables - visited: + visited.add(lintable) + if not lintable.path.exists(): + continue try: children = self.find_children(lintable) for child in children: @@ -468,8 +471,10 @@ def _emit_matches(self, files: list[Lintable]) -> Generator[MatchError, None, No exc.rule = self.rules["load-failure"] yield exc except AttributeError: - yield MatchError(lintable=lintable, rule=self.rules["load-failure"]) - visited.add(lintable) + yield MatchError( + lintable=lintable, + rule=self.rules["load-failure"], + ) def find_children(self, lintable: Lintable) -> list[Lintable]: """Traverse children of a single file or folder.""" @@ -490,7 +495,6 @@ def find_children(self, lintable: Lintable) -> list[Lintable]: except AnsibleError as exc: msg = f"Loading {lintable.filename} caused an {type(exc).__name__} exception: {exc}, file was ignored." logging.exception(msg) - # raise SystemExit(exc) from exc return [] results = [] # playbook_ds can be an AnsibleUnicode string, which we consider invalid diff --git a/src/ansiblelint/utils.py b/src/ansiblelint/utils.py index 4cbf0fa0b7..3d0e535bfd 100644 --- a/src/ansiblelint/utils.py +++ b/src/ansiblelint/utils.py @@ -28,7 +28,7 @@ import logging import os import re -from collections.abc import ItemsView, Iterator, Mapping, Sequence +from collections.abc import ItemsView, Iterable, Iterator, Mapping, Sequence from dataclasses import _MISSING_TYPE, dataclass, field from functools import cache, lru_cache from pathlib import Path @@ -290,7 +290,7 @@ class HandleChildren: rules: RulesCollection = field(init=True, repr=False) app: App - def include_children( + def include_children( # pylint: disable=too-many-return-statements self, lintable: Lintable, k: str, @@ -325,14 +325,21 @@ def include_children( return [] # handle include: filename.yml tags=blah - (args, _) = tokenize(v) + (args, kwargs) = tokenize(v) - result = path_dwim(basedir, args[0]) + if args: + file = args[0] + elif "file" in kwargs: + file = kwargs["file"] + else: + return [] + + result = path_dwim(basedir, file) while basedir not in ["", "/"]: if os.path.exists(result): break basedir = os.path.dirname(basedir) - result = path_dwim(basedir, args[0]) + result = path_dwim(basedir, file) return [Lintable(result, kind=parent_type)] @@ -430,7 +437,7 @@ def roles_children( # pylint: disable=unused-argument # parent_type) basedir = str(lintable.path.parent) results: list[Lintable] = [] - if not v: + if not v or not isinstance(v, Iterable): # typing does not prevent junk from being passed in return results for role in v: @@ -467,10 +474,24 @@ def _get_task_handler_children_for_tasks_or_playbooks( if not task_handler or isinstance(task_handler, str): # pragma: no branch continue - file_name = task_handler[task_handler_key] - if isinstance(file_name, Mapping) and file_name.get("file", None): - file_name = file_name["file"] + file_name = "" + action_args = task_handler[task_handler_key] + if isinstance(action_args, str): + (args, kwargs) = tokenize(action_args) + if len(args) == 1: + file_name = args[0] + elif kwargs.get("file", None): + file_name = kwargs["file"] + else: + # ignore invalid data (syntax check will outside the scope) + continue + + if isinstance(action_args, Mapping) and action_args.get("file", None): + file_name = action_args["file"] + if not file_name: + # ignore invalid data (syntax check will outside the scope) + continue f = path_dwim(basedir, file_name) while basedir not in ["", "/"]: if os.path.exists(f): diff --git a/test/test_runner.py b/test/test_runner.py index c87ac60409..aa76b65a66 100644 --- a/test/test_runner.py +++ b/test/test_runner.py @@ -178,6 +178,52 @@ def test_files_not_scanned_twice(default_rules_collection: RulesCollection) -> N assert len(run2) == 0 +@pytest.mark.parametrize( + ("filename", "failures", "checked_files_no"), + ( + pytest.param( + "examples/playbooks/common-include-wrong-syntax.yml", + 1, + 1, + id="1", + ), + pytest.param( + "examples/playbooks/common-include-wrong-syntax2.yml", + 1, + 1, + id="2", + ), + pytest.param( + "examples/playbooks/common-include-wrong-syntax3.yml", + 0, + 2, + id="3", + ), + ), +) +def test_include_wrong_syntax( + filename: str, + failures: int, + checked_files_no: int, + default_rules_collection: RulesCollection, +) -> None: + """Ensure that lintables aren't double-checked.""" + checked_files: set[Lintable] = set() + + path = Path(filename).resolve() + runner = Runner( + path, + rules=default_rules_collection, + verbosity=0, + checked_files=checked_files, + ) + result = runner.run() + assert len(runner.checked_files) == checked_files_no + assert len(result) == failures, result + for item in result: + assert item.tag == "syntax-check[no-file]" + + def test_runner_not_found(default_rules_collection: RulesCollection) -> None: """Ensure that lintables aren't double-checked.""" checked_files: set[Lintable] = set() diff --git a/test/test_yaml_utils.py b/test/test_yaml_utils.py index 6f16c09a81..f4d9b46a30 100644 --- a/test/test_yaml_utils.py +++ b/test/test_yaml_utils.py @@ -768,12 +768,12 @@ def test_get_path_to_play( pytest.param( "examples/playbooks/include.yml", 14, - [0, "tasks", 1], + [0, "tasks", 2], id="playbook-multi_tasks_blocks-tasks_last_task_before_handlers", ), pytest.param( "examples/playbooks/include.yml", - 16, + 17, [0, "handlers", 0], id="playbook-multi_tasks_blocks-handlers_task", ),