Fully qualified collection name automatically stripped away from matchtask

[StopMotionCuber]

Summary

We are currently using a rule that doesn't exist upstream to check whether we use the fully qualified collection name for builtins, as we recently migrated to fully qualified collection name and have integrated a check to CI to be consistent here.
Code of it is seen below:

from typing import Any, Dict, Union, Optional

from ansiblelint.rules import AnsibleLintRule

from ansiblelint.file_utils import Lintable

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 FQCNBuiltinsLint(AnsibleLintRule):
    id = "fqcn"
    shortdesc = "Use fqcn for builtins"
    description = "Use fqcn for builtins"
    tags = ["productivity"]

    def matchtask(
        self, task: Dict[str, Any], file: "Optional[Lintable]" = None
    ) -> Union[bool, str]:
        if task["action"]["__ansible_module__"] in builtins:
            return (f"{task['__file__']}:{task['action']['__line__'] - 1}: "
                    f"{task['action']['__ansible_module__']}")

        return False

Our problem is now that #1581 got merged, task["action"]["__ansible_module__"] will falsely flag basically all of our roles, with https://github.com/ansible-community/ansible-lint/pull/1581/files#diff-d453523679145f511ca4e8fb97be96e7dd664ae88373e2e5e9a90a65fd751cabR562 as the exact line of this problem.

If you would be fine with expanding the normalization to also include the unnormalized way (e.g. as seen in desired behavior) I can happily create a PR for that.

Issue Type

• Bug Report

Ansible and Ansible Lint details

ansible --version
ansible [core 2.11.1] 
  config file = /home/ruben/Projects/ansible/ansible.cfg
  configured module search path = ['/home/ruben/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/ruben/Projects/ansible/venv/lib/python3.9/site-packages/ansible
  ansible collection location = /home/ruben/Projects/ansible/collections
  executable location = /home/ruben/Projects/ansible/venv/bin/ansible
  python version = 3.9.5 (default, May 11 2021, 15:40:31) [GCC 10.2.0]
  jinja version = 3.0.1
  libyaml = True

ansible-lint --version
ansible-lint 5.0.11 using ansible 2.11.1

• ansible installation method: pip
• ansible-lint installation method: pip

STEPS TO REPRODUCE

I hope this is not really needed as I pointed to the related MR.

Desired Behaviour

Have a task["action"]["__ansible_module_original__"] to check against the exact name that we have in the role

Actual Behaviour

task["action"]["__ansible_module__"] only contains the base name and it is not possible to derive from the task dictionary whether a fully qualified name was used or not

[ssbarnea]

That is related to a change made one day before you filed it, https://github.com/ansible-community/ansible-lint/pull/1581/files -- mainly we consider the normalized name to be the short form not the long one. That is because we have a huge number of places where the short/old name is used.

Still, if you want to make a PR to switch normalized form to be the long one and replace all occurrences in the code to use long ones you care welcome. Keep in mind NOT to do replacement for tests. For tests you will likely want to add extra checks to also include for fully qualified names, in addition to existing ones.

While Ansible core team already replaced module calls with long form in the docs, there are longer debates regarding if we do really need this kind of verbosity for builtins. That is the reason why we did not make a rule that forces people to use only full names. Still, if you want to add one and make it an opt-in, I would not refuse you, especially as we may go towards this direction in the future anyway.

[StopMotionCuber]

Thanks for your input, I understand that having the short form as the normalized one helps to have current code about specific builtins working without having to refactor a huge amount of code.

Actually, normalizing to the long name wouldn't help here as we wouldn't find anything with our rule (as all the short names already got transformed to their long version).
My proposal would be to extend the task["action"] dictionary with the form that is not normalized, and I'll create a PR for that later on, possibly also including an optional rule for FQCNs there. I'm not sure about the naming of variables, but I think this is better discussed within that PR.

[StopMotionCuber]

Fixed with #1614