minor detection improvement on j2plugin loader (#73714)

* minor  improvement on j2plugin loader, comments and var names clarified

2 files changed, 32 insertions, 21 deletions

diff --git a/changelogs/fragments/j2ploader_minor.yml b/changelogs/fragments/j2ploader_minor.yml
new file mode 100644
index 0000000000..b8b9874267
--- /dev/null
+++ b/changelogs/fragments/j2ploader_minor.yml
@@ -0,0 +1,2 @@
+bugfixes:
+  - j2 plugin loader clarified comments, made note with better fqcn detection.
diff --git a/lib/ansible/plugins/loader.py b/lib/ansible/plugins/loader.py
index eaf6df894f..dad5d38a58 100644
--- a/lib/ansible/plugins/loader.py
+++ b/lib/ansible/plugins/loader.py
@@ -966,50 +966,59 @@ class Jinja2Loader(PluginLoader):
 
     The filter and test plugins are Jinja2 plugins encapsulated inside of our plugin format.
     The way the calling code is setup, we need to do a few things differently in the all() method
+
+    We can't use the base class version because of file == plugin assumptions and dedupe logic
     """
     def find_plugin(self, name, collection_list=None):
-        # Nothing using Jinja2Loader use this method.  We can't use the base class version because
-        # we deduplicate differently than the base class
-        if '.' in name:
+
+        if '.' in name:  # NOTE: this is wrong way, use: AnsibleCollectionRef.is_valid_fqcr(name) or collection_list
             return super(Jinja2Loader, self).find_plugin(name, collection_list=collection_list)
 
-        raise AnsibleError('No code should call find_plugin for Jinja2Loaders (Not implemented)')
+        # Nothing is currently using this method
+        raise AnsibleError('No code should call "find_plugin" for Jinja2Loaders (Not implemented)')
 
     def get(self, name, *args, **kwargs):
-        # Nothing using Jinja2Loader use this method.  We can't use the base class version because
-        # we deduplicate differently than the base class
-        if '.' in name:
+
+        if '.' in name:  # NOTE: this is wrong way to detect collection, see note above for example
             return super(Jinja2Loader, self).get(name, *args, **kwargs)
 
-        raise AnsibleError('No code should call find_plugin for Jinja2Loaders (Not implemented)')
+        # Nothing is currently using this method
+        raise AnsibleError('No code should call "get" for Jinja2Loaders (Not implemented)')
 
     def all(self, *args, **kwargs):
         """
         Differences with :meth:`PluginLoader.all`:
 
-        * We do not deduplicate ansible plugin names.  This is because we don't care about our
-          plugin names, here.  We care about the names of the actual jinja2 plugins which are inside
-          of our plugins.
-        * We reverse the order of the list of plugins compared to other PluginLoaders.  This is
+        * Unlike other plugin types, file != plugin, a file can contain multiple plugins (of same type).
+          This is why we do not deduplicate ansible file names at this point, we mostly care about
+          the names of the actual jinja2 plugins which are inside of our files.
+        * We reverse the order of the list of files compared to other PluginLoaders.  This is
           because of how calling code chooses to sync the plugins from the list.  It adds all the
-          Jinja2 plugins from one of our Ansible plugins into a dict.  Then it adds the Jinja2
-          plugins from the next Ansible plugin, overwriting any Jinja2 plugins that had the same
+          Jinja2 plugins from one of our Ansible files into a dict.  Then it adds the Jinja2
+          plugins from the next Ansible file, overwriting any Jinja2 plugins that had the same
           name.  This is an encapsulation violation (the PluginLoader should not know about what
           calling code does with the data) but we're pushing the common code here.  We'll fix
           this in the future by moving more of the common code into this PluginLoader.
         * We return a list.  We could iterate the list instead but that's extra work for no gain because
           the API receiving this doesn't care.  It just needs an iterable
+        * This method will NOT fetch collection plugins, only those that would be expected under 'ansible.legacy'.
         """
-        # We don't deduplicate ansible plugin names.  Instead, calling code deduplicates jinja2
-        # plugin names.
+        # We don't deduplicate ansible file names.
+        # Instead, calling code deduplicates jinja2 plugin names when loading each file.
         kwargs['_dedupe'] = False
 
-        # We have to instantiate a list of all plugins so that we can reverse it.  We reverse it so
-        # that calling code will deduplicate this correctly.
-        plugins = list(super(Jinja2Loader, self).all(*args, **kwargs))
-        plugins.reverse()
+        # TODO: move this to initalization and extract/dedupe plugin names in loader and offset this from
+        # caller. It would have to cache/refresh on add_directory to reevaluate plugin list and dedupe.
+        # Another option is to always prepend 'ansible.legac'y and force the collection path to
+        # load/find plugins, just need to check compatiblity of that approach.
+        # This would also enable get/find_plugin for these type of plugins.
+
+        # We have to instantiate a list of all files so that we can reverse the list.
+        # We reverse it so that calling code will deduplicate this correctly.
+        files = list(super(Jinja2Loader, self).all(*args, **kwargs))
+        files .reverse()
 
-        return plugins
+        return files
 
 
 def _load_plugin_filter():