From b00e21b45f40a5ed9b8d6b346c26c7f41419ddf2 Mon Sep 17 00:00:00 2001
From: Sorin Sbarnea <ssbarnea@redhat.com>
Date: Mon, 24 May 2021 09:56:34 +0100
Subject: [PATCH] Normalize action names using builtins (#1581)

Fixes: #1573
---
 .../rules/MissingFilePermissionsRule.py       | 23 ++++++++++---------
 src/ansiblelint/text.py                       |  8 +++++++
 src/ansiblelint/utils.py                      | 10 +++++++-
 3 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/src/ansiblelint/rules/MissingFilePermissionsRule.py b/src/ansiblelint/rules/MissingFilePermissionsRule.py
index 47ddeb47ca..512f0b0c75 100644
--- a/src/ansiblelint/rules/MissingFilePermissionsRule.py
+++ b/src/ansiblelint/rules/MissingFilePermissionsRule.py
@@ -214,6 +214,10 @@ def matchtask(
       file:
         path: foo
         state: touch
+    - name: permissions missing and might create file (fqcn)
+      ansible.builtin.file:
+        path: foo
+        state: touch
 '''
 
     FAIL_MISSING_PERMISSIONS_DIRECTORY = '''
@@ -223,6 +227,11 @@ def matchtask(
       file:
         path: foo
         state: directory
+    - name: lineinfile when create is true (fqcn)
+      ansible.builtin.lineinfile:
+        path: foo
+        create: true
+        line: some content here
 '''
 
     FAIL_LINEINFILE_CREATE = '''
@@ -242,14 +251,6 @@ def matchtask(
       replace:
         path: foo
         mode: preserve
-    - name: permissions are missing (fqcn)
-      ansible.builtin.file:
-        path: bar
-    - name: lineinfile when create is true (fqcn)
-      ansible.builtin.lineinfile:
-        path: foo
-        create: true
-        line: some content here
 '''
 
     FAIL_PERMISSION_COMMENT = '''
@@ -353,7 +354,7 @@ def test_fail_preserve_mode(rule_runner: RunFromText) -> None:
     def test_fail_missing_permissions_touch(rule_runner: RunFromText) -> None:
         """Missing permissions when possibly creating file."""
         results = rule_runner.run_playbook(FAIL_MISSING_PERMISSIONS_TOUCH)
-        assert len(results) == 1
+        assert len(results) == 2
 
     @pytest.mark.parametrize(
         'rule_runner', (MissingFilePermissionsRule,), indirect=['rule_runner']
@@ -361,7 +362,7 @@ def test_fail_missing_permissions_touch(rule_runner: RunFromText) -> None:
     def test_fail_missing_permissions_directory(rule_runner: RunFromText) -> None:
         """Missing permissions when possibly creating a directory."""
         results = rule_runner.run_playbook(FAIL_MISSING_PERMISSIONS_DIRECTORY)
-        assert len(results) == 1
+        assert len(results) == 2
 
     @pytest.mark.parametrize(
         'rule_runner', (MissingFilePermissionsRule,), indirect=['rule_runner']
@@ -377,7 +378,7 @@ def test_fail_lineinfile_create(rule_runner: RunFromText) -> None:
     def test_fail_replace_preserve(rule_runner: RunFromText) -> None:
         """Replace does not allow preserve mode."""
         results = rule_runner.run_playbook(FAIL_REPLACE_PRESERVE)
-        assert len(results) == 3
+        assert len(results) == 1
 
     @pytest.mark.parametrize(
         'rule_runner', (MissingFilePermissionsRule,), indirect=['rule_runner']
diff --git a/src/ansiblelint/text.py b/src/ansiblelint/text.py
index 7064a9913a..7bfce4711a 100644
--- a/src/ansiblelint/text.py
+++ b/src/ansiblelint/text.py
@@ -23,3 +23,11 @@ def toidentifier(text: str) -> str:
             "Unable to convert role name '%s' to valid variable name." % text
         )
     return result
+
+
+# https://www.python.org/dev/peps/pep-0616/
+def removeprefix(self: str, prefix: str) -> str:
+    """Remove prefix from string."""
+    if self.startswith(prefix):
+        return self[len(prefix) :]
+    return self[:]
diff --git a/src/ansiblelint/utils.py b/src/ansiblelint/utils.py
index d2079decbd..cae2f92808 100644
--- a/src/ansiblelint/utils.py
+++ b/src/ansiblelint/utils.py
@@ -74,6 +74,7 @@
 from ansiblelint.constants import FileType
 from ansiblelint.errors import MatchError
 from ansiblelint.file_utils import Lintable, discover_lintables
+from ansiblelint.text import removeprefix
 
 # ansible-lint doesn't need/want to know about encrypted secrets, so we pass a
 # string as the password to enable such yaml files to be opened and parsed
@@ -527,7 +528,7 @@ def _sanitize_task(task: Dict[str, Any]) -> Dict[str, Any]:
 
 
 def normalize_task_v2(task: Dict[str, Any]) -> Dict[str, Any]:
-    """Ensure tasks have an action key and strings are converted to python objects."""
+    """Ensure tasks have a normalized action key and strings are converted to python objects."""
     result = dict()
 
     sanitized_task = _sanitize_task(task)
@@ -556,6 +557,13 @@ def normalize_task_v2(task: Dict[str, Any]) -> Dict[str, Any]:
             continue
         result[k] = v
 
+    if not isinstance(action, str):
+        raise RuntimeError("Task actions can only be strings, got %s" % action)
+    # convert builtin fqn calls to short forms because most rules know only
+    # about short calls but in the future we may switch the normalization to do
+    # the opposite. Mainly we currently consider normalized the module listing
+    # used by `ansible-doc -t module -l 2>/dev/null`
+    action = removeprefix(action, "ansible.builtin.")
     result['action'] = dict(__ansible_module__=action)
 
     if '_raw_params' in arguments: