Add opt-in rule to check whether FQCN is used for builtins (#1614)

* Add __ansible_module_original__ to determine the unmodified name for an ansible module that was used

* Add specific rule for a rule_runner to enable_list, such that this rule is never ignored

* Add opt-in rule that checks for builtins to use the fully qualified collection name

* Clarification on FQCN for builtins rule

* Adapt shortdesc to include specification of builtin actions

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: Felix Fontein <felix@fontein.de>

src/ansiblelint/rules/EnvVarsInCommandRule.py

@@ -51,6 +51,7 @@ class EnvVarsInCommandRule(AnsibleLintRule):
         'strip_empty_ends',
         'cmd',
         '__ansible_module__',
+        '__ansible_module_original__',
         '__ansible_arguments__',
         LINE_NUMBER_KEY,
         FILENAME_KEY,

src/ansiblelint/rules/FQCNBuiltinsRule.py

@@ -0,0 +1,131 @@
+"""Rule definition for usage of fully qualified collection names for builtins."""
+import sys
+from typing import Any, Dict, Optional, Union
+
+from ansiblelint.file_utils import Lintable
+from ansiblelint.rules import AnsibleLintRule
+from ansiblelint.testing import RunFromText
+
+builtins = [
+    "add_host",
+    "apt",
+    "apt_key",
+    "apt_repository",
+    "assemble",
+    "assert",
+    "async_status",
+    "blockinfile",
+    "command",
+    "copy",
+    "cron",
+    "debconf",
+    "debug",
+    "dnf",
+    "dpkg_selections",
+    "expect",
+    "fail",
+    "fetch",
+    "file",
+    "find",
+    "gather_facts",
+    "get_url",
+    "getent",
+    "git",
+    "group",
+    "group_by",
+    "hostname",
+    "import_playbook",
+    "import_role",
+    "import_tasks",
+    "include",
+    "include_role",
+    "include_tasks",
+    "include_vars",
+    "iptables",
+    "known_hosts",
+    "lineinfile",
+    "meta",
+    "package",
+    "package_facts",
+    "pause",
+    "ping",
+    "pip",
+    "raw",
+    "reboot",
+    "replace",
+    "rpm_key",
+    "script",
+    "service",
+    "service_facts",
+    "set_fact",
+    "set_stats",
+    "setup",
+    "shell",
+    "slurp",
+    "stat",
+    "subversion",
+    "systemd",
+    "sysvinit",
+    "tempfile",
+    "template",
+    "unarchive",
+    "uri",
+    "user",
+    "wait_for",
+    "wait_for_connection",
+    "yum",
+    "yum_repository",
+]
+
+
+class FQCNBuiltinsRule(AnsibleLintRule):
+    id = "fqcn-builtins"
+    shortdesc = "Use FQCN for builtin actions"
+    description = (
+        'Check whether the long version starting with ``ansible.builtin`` '
+        'is used in the playbook'
+    )
+    tags = ["opt-in", "formatting", "experimental"]
+
+    def matchtask(
+        self, task: Dict[str, Any], file: Optional[Lintable] = None
+    ) -> Union[bool, str]:
+        return task["action"]["__ansible_module_original__"] in builtins
+
+
+# testing code to be loaded only with pytest or when executed the rule file
+if "pytest" in sys.modules:
+
+    import pytest
+
+    SUCCESS_PLAY = '''
+- hosts: localhost
+  tasks:
+  - name: shell (fqcn)
+    ansible.builtin.shell: echo This rule should not get matched by the fqcn-builtins rule
+    '''
+
+    FAIL_PLAY = '''
+- hosts: localhost
+  tasks:
+  - name: shell (fqcn)
+    shell: echo This rule should get matched by the fqcn-builtins rule
+    '''
+
+    @pytest.mark.parametrize(
+        'rule_runner', (FQCNBuiltinsRule,), indirect=['rule_runner']
+    )
+    def test_fqcn_builtin_fail(rule_runner: RunFromText) -> None:
+        """Test rule matches."""
+        results = rule_runner.run_playbook(FAIL_PLAY)
+        assert len(results) == 1
+        for result in results:
+            assert result.message == FQCNBuiltinsRule.shortdesc
+
+    @pytest.mark.parametrize(
+        'rule_runner', (FQCNBuiltinsRule,), indirect=['rule_runner']
+    )
+    def test_fqcn_builtin_pass(rule_runner: RunFromText) -> None:
+        """Test rule does not match."""
+        results = rule_runner.run_playbook(SUCCESS_PLAY)
+        assert len(results) == 0, results

src/ansiblelint/testing/fixtures.py

@@ -53,10 +53,11 @@ def default_text_runner(default_rules_collection: RulesCollection) -> RunFromTex
 
 
 @pytest.fixture
-def rule_runner(request: SubRequest) -> RunFromText:
+def rule_runner(request: SubRequest, config_options: Namespace) -> RunFromText:
     """Return runner for a specific rule class."""
     rule_class = request.param
-    collection = RulesCollection()
+    config_options.enable_list.append(rule_class().id)
+    collection = RulesCollection(options=config_options)
     collection.register(rule_class())
     return RunFromText(collection)
 

src/ansiblelint/utils.py

@@ -559,12 +559,15 @@ def normalize_task_v2(task: Dict[str, Any]) -> Dict[str, Any]:
 
     if not isinstance(action, str):
         raise RuntimeError("Task actions can only be strings, got %s" % action)
+    action_unnormalized = 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)
+    result['action'] = dict(
+        __ansible_module__=action, __ansible_module_original__=action_unnormalized
+    )
 
     if '_raw_params' in arguments:
         result['action']['__ansible_arguments__'] = arguments['_raw_params'].split(' ')
@@ -604,6 +607,7 @@ def task_to_str(task: Dict[str, Any]) -> str:
             if k
             not in [
                 "__ansible_module__",
+                "__ansible_module_original__",
                 "__ansible_arguments__",
                 "__line__",
                 "__file__",