Skip to content

Commit

Permalink
Add auto-fixing implementation for no-free-form rule (#3725)
Browse files Browse the repository at this point in the history
  • Loading branch information
audgirka authored Sep 12, 2023
1 parent 8cd9281 commit 2dd44ba
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 4 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:
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

0 comments on commit 2dd44ba

Please sign in to comment.