Add no-shorthand rule as experimental

Fixes: #2117

.github/workflows/tox.yml

@@ -166,7 +166,7 @@ jobs:
       WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:TOX_PARALLEL_NO_SPINNER
       # Number of expected test passes, safety measure for accidental skip of
       # tests. Update value if you add/remove tests.
-      PYTEST_REQPASS: 702
+      PYTEST_REQPASS: 704
 
     steps:
       - name: Activate WSL1

examples/playbooks/contains_secrets.transformed.yml

@@ -14,4 +14,5 @@
       3066
   tasks:
     - name: Just a debug task
-      ansible.builtin.debug: msg="hello world"
+      ansible.builtin.debug:
+        msg: hello world

examples/playbooks/contains_secrets.yml

@@ -14,4 +14,5 @@
       3066
   tasks:
     - name: Just a debug task
-      ansible.builtin.debug: msg="hello world"
+      ansible.builtin.debug:
+        msg: hello world

examples/playbooks/playbook-imported.yml

@@ -4,7 +4,8 @@
   connection: local
   gather_facts: false
   tasks:
-    - ansible.builtin.command: echo "no name" # should generate unnamed-task
+    - ansible.builtin.command:
+        cmd: echo "no name" # should generate unnamed-task
     - name: Another task
       ansible.builtin.debug:
         msg: debug message

examples/playbooks/playbook-parent.yml

@@ -3,7 +3,7 @@
   ansible.builtin.import_playbook: playbook-imported.yml
 
 - name: Importing playbook from collection
-  import_playbook: community.molecule.validate
+  import_playbook: community.molecule.validate # noqa: fqcn
 
 - name: Importing playbook using jinja2
-  import_playbook: "{{ 'community.molecule.validate' }}"
+  import_playbook: "{{ 'community.molecule.validate' }}" # noqa: fqcn

examples/playbooks/rule-no-shorthand-fail.yml

@@ -0,0 +1,7 @@
+---
+- name: Example with discouraged shorthand syntax
+  hosts: localhost
+  tasks:
+    - name: Create a placefolder file
+      ansible.builtin.command: chdir=/tmp touch foo # <-- don't use shorthand
+      changed_when: false

examples/playbooks/rule-no-shorthand-pass.yml

@@ -0,0 +1,9 @@
+---
+- name: Example with discouraged shorthand syntax
+  hosts: localhost
+  tasks:
+    - name: Create a placefolder file
+      ansible.builtin.command:
+        cmd: touch foo
+        chdir: /tmp
+      changed_when: false

examples/playbooks/test_skip_inside_yaml.yml

@@ -28,13 +28,13 @@
         dest: "{{dest_proj_path}}/foo.conf" # noqa jinja[spacing]
 
     - name: Test deprecated-command-syntax # <-- 3 deprecated-command-syntax
-      ansible.builtin.command: creates=B chmod 644 A
+      ansible.builtin.command: creates=B chmod 644 A # noqa: no-shorthand
     - name: Test deprecated-command-syntax # <-- 4 deprecated-command-syntax
-      ansible.builtin.command: warn=yes creates=B chmod 644 A
+      ansible.builtin.command: warn=yes creates=B chmod 644 A # noqa: no-shorthand
     - name: Test deprecated-command-syntax (skipped via no warn)
-      ansible.builtin.command: warn=no creates=B chmod 644 A
+      ansible.builtin.command: warn=no creates=B chmod 644 A # noqa: no-shorthand
     - name: Test deprecated-command-syntax (skipped via skip_ansible_lint)
-      ansible.builtin.command: creates=B chmod 644 A
+      ansible.builtin.command: creates=B chmod 644 A # noqa: no-shorthand
       tags:
         - skip_ansible_lint
 

examples/playbooks/unicode.transformed.yml

@@ -4,4 +4,5 @@
   tasks:
     # cspell:disable-next-line
     - name: Some тест
-      ansible.builtin.command: uname
+      ansible.builtin.command:
+        cmd: uname

examples/playbooks/unicode.yml

@@ -4,4 +4,5 @@
   tasks:
     # cspell:disable-next-line
     - name: Some тест
-      ansible.builtin.command: uname
+      ansible.builtin.command:
+        cmd: uname

examples/roles/test-role/tasks/main.yml

@@ -1,3 +1,4 @@
 ---
 - name: Shell instead of command
-  shell: echo hello world
+  ansible.builtin.shell:
+    cmd: echo hello world

examples/roles/test-role/tasks/world.yml

@@ -1,2 +1,3 @@
 ---
-- command: echo this is a task without a name
+- command: # noqa: fqcn
+    cmd: echo this is a task without a name # noqa: no-shorthand

src/ansiblelint/rules/command_instead_of_shell.py

@@ -34,15 +34,18 @@
   hosts: localhost
   tasks:
   - name: Shell no pipe
-    ansible.builtin.shell: echo hello
+    ansible.builtin.shell:
+      cmd: echo hello
     changed_when: false
 
   - name: Shell with jinja filter
-    ansible.builtin.shell: echo {{ "hello" | upper }}
+    ansible.builtin.shell:
+      cmd: echo {{ "hello" | upper }}
     changed_when: false
 
   - name: Sshell with jinja filter (fqcn)
-    ansible.builtin.shell: echo {{ "hello" | upper }}
+    ansible.builtin.shell:
+      cmd: echo {{ "hello" | upper }}
     changed_when: false
 """
 
@@ -51,43 +54,53 @@
   hosts: localhost
   tasks:
   - name: Shell with pipe
-    ansible.builtin.shell: echo hello | true  # noqa: risky-shell-pipe
+    ansible.builtin.shell:
+      cmd: echo hello | true  # noqa: risky-shell-pipe
     changed_when: false
 
   - name: Shell with redirect
-    ansible.builtin.shell: echo hello >  /tmp/hello
+    ansible.builtin.shell:
+      cmd: echo hello >  /tmp/hello
     changed_when: false
 
   - name: Chain two shell commands
-    ansible.builtin.shell: echo hello && echo goodbye
+    ansible.builtin.shell:
+      cmd: echo hello && echo goodbye
     changed_when: false
 
   - name: Run commands in succession
-    ansible.builtin.shell: echo hello ; echo goodbye
+    ansible.builtin.shell:
+      cmd: echo hello ; echo goodbye
     changed_when: false
 
   - name: Use variables
-    ansible.builtin.shell: echo $HOME $USER
+    ansible.builtin.shell:
+      cmd: echo $HOME $USER
     changed_when: false
 
   - name: Use * for globbing
-    ansible.builtin.shell: ls foo*
+    ansible.builtin.shell:
+      cmd: ls foo*
     changed_when: false
 
   - name: Use ? for globbing
-    ansible.builtin.shell: ls foo?
+    ansible.builtin.shell:
+      cmd: ls foo?
     changed_when: false
 
   - name: Use [] for globbing
-    ansible.builtin.shell: ls foo[1,2,3]
+    ansible.builtin.shell:
+      cmd: ls foo[1,2,3]
     changed_when: false
 
   - name: Use shell generator
-    ansible.builtin.shell: ls foo{.txt,.xml}
+    ansible.builtin.shell:
+      cmd: ls foo{.txt,.xml}
     changed_when: false
 
   - name: Use backticks
-    ansible.builtin.shell: ls `ls foo*`
+    ansible.builtin.shell:
+      cmd: ls `ls foo*`
     changed_when: false
 
   - name: Use shell with cmd

src/ansiblelint/rules/no_shorthand.md

@@ -0,0 +1,39 @@
+# no-shorthand
+
+This rule identifies any use of [short-hand (free-form)](https://docs.ansible.com/ansible/2.7/user_guide/playbooks_intro.html#action-shorthand)
+module calling syntax and asks for switching to the full syntax.
+
+**Shorthand** syntax, also known as **free-form**, is known to produce
+subtle bugs. Short-hand syntax also prevents editors from providing feedback,
+autocomplete and validation for the edited line.
+
+```{note}
+As long you just pass a YAML string that contains a `=` character inside as the
+parameter to the action module name, we consider this as being shorthand
+syntax. Be sure you pass a dictionary to the module, so the short-hand parsing
+is never triggered.
+```
+
+## Problematic code
+
+```yaml
+---
+- name: Example with discouraged shorthand syntax
+  hosts: localhost
+  tasks:
+    - name: Create a placefolder file
+      ansible.builtin.command: chdir=/tmp touch foo # <-- don't use shorthand
+```
+
+## Correct code
+
+```yaml
+---
+- name: Example that avoids shorthand syntax
+  hosts: localhost
+  tasks:
+    - name: Create a placefolder file
+      ansible.builtin.command:
+        cmd: touch foo # <-- ansible will not touch it
+        chdir: /tmp
+```

src/ansiblelint/rules/no_shorthand.py

@@ -0,0 +1,69 @@
+"""Implementation of NoShorthandRule."""
+from __future__ import annotations
+
+import sys
+from typing import TYPE_CHECKING, Any
+
+from ansiblelint.constants import INCLUSION_ACTION_NAMES, LINE_NUMBER_KEY
+from ansiblelint.errors import MatchError
+from ansiblelint.rules import AnsibleLintRule
+
+if TYPE_CHECKING:
+    from ansiblelint.file_utils import Lintable
+
+
+class NoShorthandRule(AnsibleLintRule):
+    """Rule for detecting discouraged shorthand syntax for action modules."""
+
+    id = "no-shorthand"
+    description = "Avoid shorthand inside files as it can produce subtile bugs."
+    severity = "MEDIUM"
+    tags = ["syntax", "risk", "experimental"]
+    version_added = "v6.8.0"
+    needs_raw_task = True
+
+    def matchtask(
+        self, task: dict[str, Any], file: Lintable | None = None
+    ) -> list[MatchError]:
+        results: list[MatchError] = []
+        action = task["action"]["__ansible_module_original__"]
+
+        if action in INCLUSION_ACTION_NAMES:
+            return results
+
+        action_value = task["__raw_task__"].get(action, None)
+        if isinstance(action_value, str) and "=" in action_value:
+            results.append(
+                self.create_matcherror(
+                    message="Avoid using shorthand (free-form) when calling module actions.",
+                    linenumber=task[LINE_NUMBER_KEY],
+                    filename=file,
+                    tag=f"{self.id}[{action}]",
+                )
+            )
+        return results
+
+
+if "pytest" in sys.modules:  # noqa: C901
+
+    import pytest
+
+    from ansiblelint.rules import RulesCollection  # pylint: disable=ungrouped-imports
+    from ansiblelint.runner import Runner  # pylint: disable=ungrouped-imports
+
+    @pytest.mark.parametrize(
+        ("file", "expected"),
+        (
+            pytest.param("examples/playbooks/rule-no-shorthand-pass.yml", 0, id="pass"),
+            pytest.param("examples/playbooks/rule-no-shorthand-fail.yml", 1, id="fail"),
+        ),
+    )
+    def test_rule_no_shorthand(
+        default_rules_collection: RulesCollection, file: str, expected: int
+    ) -> None:
+        """Validate that rule works as intended."""
+        results = Runner(file, rules=default_rules_collection).run()
+
+        for result in results:
+            assert result.rule.id == NoShorthandRule.id, result
+        assert len(results) == expected

test/test_examples.py

@@ -22,7 +22,7 @@ def _change_into_examples_dir(request: FixtureRequest) -> Generator[None, None,
 def test_example(default_rules_collection: RulesCollection) -> None:
     """example.yml is expected to have exact number of errors inside."""
     result = Runner("playbooks/example.yml", rules=default_rules_collection).run()
-    assert len(result) == 17
+    assert len(result) == 21
 
 
 @pytest.mark.parametrize(

test/test_import_include_role.py

@@ -12,13 +12,14 @@
 ROLE_TASKS_MAIN = """\
 ---
 - name: Shell instead of command
-  shell: echo hello world
+  shell: echo hello world # noqa fqcn no-shorthand
   changed_when: false
 """
 
 ROLE_TASKS_WORLD = """\
 ---
-- debug: msg="this is a task without a name"
+- ansible.builtin.debug:
+    msg: "this is a task without a name"
 """
 
 PLAY_IMPORT_ROLE = """\
@@ -27,7 +28,7 @@
   hosts: all
 
   tasks:
-    - name: Some import
+    - name: Some import # noqa: fqcn
       import_role:
         name: test-role
 """
@@ -49,7 +50,7 @@
   hosts: all
   tasks:
     - name: Some import
-      import_role: name=test-role
+      import_role: name=test-role  # noqa: no-shorthand fqcn
 """
 
 PLAY_INCLUDE_ROLE = """\
@@ -80,7 +81,7 @@
   hosts: all
   tasks:
     - name: Some import
-      include_role: name=test-role tasks_from=world
+      include_role: name=test-role tasks_from=world  # noqa: no-shorthand
 """
 
 

test/test_rules_collection.py

@@ -166,5 +166,5 @@ def test_rules_id_format() -> None:
             rule.help != "" or rule.description or rule.__doc__
         ), f"Rule {rule.id} must have at least one of:  .help, .description, .__doc__"
     assert "yaml" in keys, "yaml rule is missing"
-    assert len(rules) == 45  # update this number when adding new rules!
+    assert len(rules) == 46  # update this number when adding new rules!
     assert len(keys) == len(rules), "Duplicate rule ids?"

test/test_skip_import_playbook.py

@@ -11,7 +11,7 @@
 - name: Fixture
   hosts: all
   tasks:
-    - name: Success
+    - name: Success # noqa: no-shorthand
       ansible.builtin.fail: msg="fail"
       when: false
 """
@@ -22,7 +22,7 @@
   hosts: all
 
   tasks:
-    - name: Should be shell  # noqa command-instead-of-shell no-changed-when
+    - name: Should be shell  # noqa command-instead-of-shell no-changed-when no-shorthand
       ansible.builtin.shell: echo lol
 
 - name: Should not be imported

test/test_skip_inside_yaml.py

@@ -55,7 +55,7 @@ def test_role_tasks_with_block(default_text_runner: RunFromText) -> None:
 
 @pytest.mark.parametrize(
     ("lintable", "expected"),
-    (pytest.param("examples/playbooks/test_skip_inside_yaml.yml", 6, id="yaml"),),
+    (pytest.param("examples/playbooks/test_skip_inside_yaml.yml", 7, id="yaml"),),
 )
 def test_inline_skips(
     default_rules_collection: RulesCollection, lintable: str, expected: int