Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add auto-fixing implementation for no-free-form rule #3725

Merged
merged 3 commits into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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:
WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:GITHUB_STEP_SUMMARY
# Number of expected test passes, safety measure for accidental skip of
# tests. Update value if you add/remove tests.
PYTEST_REQPASS: 822
PYTEST_REQPASS: 823
steps:
- name: Activate WSL1
if: "contains(matrix.shell, 'wsl')"
Expand Down
2 changes: 2 additions & 0 deletions examples/playbooks/rule-no-free-form-fail.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
- name: Create a placefolder file
ansible.builtin.command: chdir=/tmp touch foo # <-- don't use shorthand
changed_when: false

- name: Use raw to echo
ansible.builtin.raw: executable=/bin/bash echo foo # <-- don't use executable=
changed_when: false

- name: Testing anything else passed to raw except for string
ansible.builtin.raw:
args: "123"
Expand Down
15 changes: 15 additions & 0 deletions examples/playbooks/transform-no-free-form.transformed.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
- name: Example with discouraged free-form syntax
hosts: localhost
tasks:
- name: Create a placefolder file
ansible.builtin.command: # <-- don't use shorthand
chdir: /tmp
cmd: touch foo
changed_when: false

- name: Use raw to echo
ansible.builtin.raw: echo foo # <-- don't use executable=
args:
executable: /bin/bash
changed_when: false
11 changes: 11 additions & 0 deletions examples/playbooks/transform-no-free-form.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
- name: Example with discouraged free-form syntax
hosts: localhost
tasks:
- name: Create a placefolder file
ansible.builtin.command: chdir=/tmp touch foo # <-- don't use shorthand
changed_when: false

- name: Use raw to echo
ansible.builtin.raw: executable=/bin/bash echo foo # <-- don't use executable=
changed_when: false
77 changes: 74 additions & 3 deletions src/ansiblelint/rules/no_free_form.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,24 @@
"""Implementation of NoFreeFormRule."""
from __future__ import annotations

import functools
import re
import sys
from typing import TYPE_CHECKING
from typing import TYPE_CHECKING, Any

from ansiblelint.constants import INCLUSION_ACTION_NAMES, LINE_NUMBER_KEY
from ansiblelint.rules import AnsibleLintRule
from ansiblelint.rules import AnsibleLintRule, TransformMixin
from ansiblelint.rules.key_order import task_property_sorter

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

from ansiblelint.errors import MatchError
from ansiblelint.file_utils import Lintable
from ansiblelint.utils import Task


class NoFreeFormRule(AnsibleLintRule):
class NoFreeFormRule(AnsibleLintRule, TransformMixin):
"""Rule for detecting discouraged free-form syntax for action modules."""

id = "no-free-form"
Expand Down Expand Up @@ -89,6 +93,73 @@ def matchtask(
)
return results

def transform(
self,
match: MatchError,
lintable: Lintable,
data: CommentedMap | CommentedSeq | str,
) -> None:
if "no-free-form" in match.tag:
task = self.seek(match.yaml_path, data)

def filter_values(
val: str,
filter_key: str,
filter_dict: dict[str, Any],
) -> bool:
"""Return True if module option is not present in the string."""
if filter_key not in val:
return True

[k, v] = val.split("=")
filter_dict[k] = v
return False

if match.tag == "no-free-form":
module_opts: dict[str, Any] = {}
for _ in range(len(task)):
k, v = task.popitem(False)
# identify module as key and process its value
if len(k.split(".")) == 3 and isinstance(v, str):
# Filter the module options and command
module_opts["cmd"] = " ".join(
[
item
for item in v.split(" ")
if filter_values(item, "=", module_opts)
],
)

sorted_module_opts = {}
for key in sorted(
module_opts.keys(),
key=functools.cmp_to_key(task_property_sorter),
):
sorted_module_opts[key] = module_opts[key]

task[k] = sorted_module_opts
else:
task[k] = v

match.fixed = True
elif match.tag == "no-free-form[raw]":
exec_key_val: dict[str, Any] = {}
for _ in range(len(task)):
k, v = task.popitem(False)
if isinstance(v, str) and "executable" in v:
# Filter the executable and other parts from the string
task[k] = " ".join(
[
item
for item in v.split(" ")
if filter_values(item, "executable=", exec_key_val)
],
)
task["args"] = exec_key_val
else:
task[k] = v
match.fixed = True


if "pytest" in sys.modules:
import pytest
Expand Down
6 changes: 6 additions & 0 deletions test/test_transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ def fixture_runner_result(
True,
id="key_order_transform",
),
pytest.param(
"examples/playbooks/transform-no-free-form.yml",
2,
True,
id="no_free_form_transform",
),
pytest.param(
"examples/playbooks/transform-partial-become.yml",
4,
Expand Down
Loading