Skip to content

Commit

Permalink
Make fix more resilient to syntax-check errors
Browse files Browse the repository at this point in the history
Fixes: #4114
  • Loading branch information
ssbarnea committed Apr 30, 2024
1 parent 9a8db9f commit a61ae88
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 6 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/tox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ jobs:
env:
# Number of expected test passes, safety measure for accidental skip of
# tests. Update value if you add/remove tests.
PYTEST_REQPASS: 855
PYTEST_REQPASS: 856
steps:
- uses: actions/checkout@v4
with:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
- name: Reproducer for bug 4114
hosts: localhost
roles:
- this_role_is_missing
tasks:
- name: Task referring to a missing module
this_module_does_not_exist:
foo: bar

- name: Use raw to echo
ansible.builtin.debug: # <-- this should be converted to fqcn
msg: some message!
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
- name: Reproducer for bug 4114
hosts: localhost
roles:
- this_role_is_missing
tasks:
- name: Task referring to a missing module
this_module_does_not_exist:
foo: bar

- name: Use raw to echo
debug: # <-- this should be converted to fqcn
msg: some message!
7 changes: 6 additions & 1 deletion src/ansiblelint/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,12 @@ class Options: # pylint: disable=too-many-instance-attributes
ignore_file: Path | None = None
max_tasks: int = 100
max_block_depth: int = 20
nodeps: bool = bool(int(os.environ.get("ANSIBLE_LINT_NODEPS", "0")))

@property
def nodeps(self) -> bool:
"""Returns value of nodeps feature."""
# We do not want this to be cached as it would affect our testings.
return bool(int(os.environ.get("ANSIBLE_LINT_NODEPS", "0")))

def __post_init__(self) -> None:
"""Extra initialization logic."""
Expand Down
6 changes: 5 additions & 1 deletion src/ansiblelint/rules/fqcn.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@
import sys
from typing import TYPE_CHECKING, Any

from ruamel.yaml.comments import CommentedSeq

from ansiblelint.constants import LINE_NUMBER_KEY
from ansiblelint.rules import AnsibleLintRule, TransformMixin
from ansiblelint.utils import load_plugin

if TYPE_CHECKING:
from ruamel.yaml.comments import CommentedMap, CommentedSeq
from ruamel.yaml.comments import CommentedMap

from ansiblelint.errors import MatchError
from ansiblelint.file_utils import Lintable
Expand Down Expand Up @@ -242,6 +244,8 @@ def transform(
current_action = match.message.split("`")[3]
new_action = match.message.split("`")[1]
for _ in range(len(target_task)):
if isinstance(target_task, CommentedSeq):
continue

Check warning on line 248 in src/ansiblelint/rules/fqcn.py

View check run for this annotation

Codecov / codecov/patch

src/ansiblelint/rules/fqcn.py#L248

Added line #L248 was not covered by tests
k, v = target_task.popitem(False)
target_task[new_action if k == current_action else k] = v
match.fixed = True
Expand Down
8 changes: 8 additions & 0 deletions src/ansiblelint/rules/syntax_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ class KnownError:
re.MULTILINE | re.S | re.DOTALL,
),
),
# "ERROR! the role 'this_role_is_missing' was not found in ROLE_INCLUDE_PATHS\n\nThe error appears to be in 'FILE_PATH': line 5, column 7, but may\nbe elsewhere in the file depending on the exact syntax problem.\n\nThe offending line appears to be:\n\n roles:\n - this_role_is_missing\n ^ here\n"
KnownError(
tag="specific",
regex=re.compile(
r"^ERROR! (?P<title>the role '.*' was not found in[^\n]*)'(?P<filename>[\w\/\.\-]+)': line (?P<line>\d+), column (?P<column>\d+)",
re.MULTILINE | re.S | re.DOTALL,
),
),
)


Expand Down
5 changes: 4 additions & 1 deletion src/ansiblelint/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,10 @@ def _get_ansible_syntax_check_matches(
filename = lintable
column = int(groups.get("column", 1))

if pattern.tag == "unknown-module" and app.options.nodeps:
if (
pattern.tag in ("unknown-module", "specific")
and app.options.nodeps
):
ignore_rc = True
else:
results.append(
Expand Down
15 changes: 13 additions & 2 deletions test/test_transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,11 @@ def fixture_runner_result(
config_options: Options,
default_rules_collection: RulesCollection,
playbook_str: str,
monkeypatch: pytest.MonkeyPatch,
) -> LintResult:
"""Fixture that runs the Runner to populate a LintResult for a given file."""
# needed for testing transformer when roles/modules are missing:
monkeypatch.setenv("ANSIBLE_LINT_NODEPS", "1")
config_options.lintables = [playbook_str]
result = get_matches(rules=default_rules_collection, options=config_options)
return result
Expand Down Expand Up @@ -191,6 +194,13 @@ def fixture_runner_result(
True,
id="name_case_roles",
),
pytest.param(
"examples/playbooks/4114/transform-with-missing-role-and-modules.yml",
1,
True,
True,
id="4114",
),
),
)
@mock.patch.dict(os.environ, {"ANSIBLE_LINT_WRITE_TMP": "1"}, clear=True)
Expand All @@ -210,12 +220,13 @@ def test_transformer( # pylint: disable=too-many-arguments
assert Lintable(playbook_str).is_owned_by_ansible() == is_owned_by_ansible
playbook = Path(playbook_str)
config_options.write_list = ["all"]
transformer = Transformer(result=runner_result, options=config_options)
transformer.run()

matches = runner_result.matches
assert len(matches) == matches_count

transformer = Transformer(result=runner_result, options=config_options)
transformer.run()

orig_content = playbook.read_text(encoding="utf-8")
if transformed:
expected_content = playbook.with_suffix(
Expand Down

0 comments on commit a61ae88

Please sign in to comment.