Resolve misc DTFIX0/1 (#85247)

* complete DTFIX0 after eval

* sunder-prefix Marker.concrete_subclasses

* re-home Jinja plugin decorators public API

* low-hanging/already fixed DTFIX cases

Co-authored-by: Matt Clay <matt@mystile.com>

---------

Co-authored-by: Matt Clay <matt@mystile.com>

lib/ansible/_internal/_templating/_jinja_bits.py

@@ -1050,7 +1050,7 @@ def _finalize_template_result(o: t.Any, mode: FinalizeMode) -> t.Any:
     if o_type in _FINALIZE_FAST_PATH_EXACT_ITERABLE_TYPES:  # silently convert known sequence types to list
         return _finalize_collection(o, mode, _finalize_list, list)
 
-    if o_type in Marker.concrete_subclasses:  # this early return assumes handle_marker follows our variable type rules
+    if o_type in Marker._concrete_subclasses:  # this early return assumes handle_marker follows our variable type rules
         return TemplateContext.current().templar.marker_behavior.handle_marker(o)
 
     if mode is not FinalizeMode.TOP_LEVEL:  # unsupported type (do not raise)

lib/ansible/_internal/_templating/_jinja_common.py

@@ -62,7 +62,7 @@ class Marker(StrictUndefined, Tripwire):
 
     __slots__ = ('_marker_template_source',)
 
-    concrete_subclasses: t.ClassVar[set[type[Marker]]] = set()
+    _concrete_subclasses: t.ClassVar[set[type[Marker]]] = set()
 
     def __init__(
         self,
@@ -136,7 +136,7 @@ class Marker(StrictUndefined, Tripwire):
     def __init_subclass__(cls, **kwargs) -> None:
         if not inspect.isabstract(cls):
             _untaggable_types.add(cls)
-            cls.concrete_subclasses.add(cls)
+            cls._concrete_subclasses.add(cls)
 
     @classmethod
     def _init_class(cls):
@@ -277,16 +277,12 @@ class VaultExceptionMarker(ExceptionMarker):
 
 def get_first_marker_arg(args: c.Sequence, kwargs: dict[str, t.Any]) -> Marker | None:
     """Utility method to inspect plugin args and return the first `Marker` encountered, otherwise `None`."""
-    # DTFIX0: this may or may not need to be public API, move back to utils or once usage is wrapped in a decorator?
+    # CAUTION: This function is exposed in public API as ansible.template.get_first_marker_arg.
-    for arg in iter_marker_args(args, kwargs):
+    return next(iter_marker_args(args, kwargs), None)
-        return arg
-
-    return None
 
 
 def iter_marker_args(args: c.Sequence, kwargs: dict[str, t.Any]) -> t.Generator[Marker]:
     """Utility method to iterate plugin args and yield any `Marker` encountered."""
-    # DTFIX0: this may or may not need to be public API, move back to utils or once usage is wrapped in a decorator?
     for arg in itertools.chain(args, kwargs.values()):
         if isinstance(arg, Marker):
             yield arg
@@ -301,7 +297,7 @@ class JinjaCallContext(NotifiableAccessContextBase):
     _mask = True
 
     def __init__(self, accept_lazy_markers: bool) -> None:
-        self._type_interest = frozenset() if accept_lazy_markers else frozenset(Marker.concrete_subclasses)
+        self._type_interest = frozenset() if accept_lazy_markers else frozenset(Marker._concrete_subclasses)
 
     def _notify(self, o: Marker) -> t.NoReturn:
         o.trip()

lib/ansible/_internal/_templating/_lazy_containers.py

@@ -43,7 +43,7 @@ _KNOWN_TYPES: t.Final[set[type]] = (
         TemplateModule,  # example: '{% import "importme.j2" as im %}{{ im | type_debug }}'
     }
     | set(PASS_THROUGH_SCALAR_VAR_TYPES)
-    | set(Marker.concrete_subclasses)
+    | set(Marker._concrete_subclasses)
 )
 """
 These types are known to the templating system.

lib/ansible/_internal/ansible_collections/ansible/_protomatter/plugins/filter/true_type.py

@@ -2,7 +2,7 @@ from __future__ import annotations
 
 import typing as t
 
-from ansible.plugins import accept_args_markers
+from ansible.template import accept_args_markers
 
 
 @accept_args_markers

lib/ansible/executor/task_result.py

@@ -230,8 +230,6 @@ class _RawTaskResult(_BaseTaskResult):
 class CallbackTaskResult(_BaseTaskResult):
     """Public contract of TaskResult """
 
-    # DTFIX1: find a better home for this since it's public API
-
     @property
     def _result(self) -> _c.MutableMapping[str, t.Any]:
         """Use the `result` property when supporting only ansible-core 2.19 or later."""

lib/ansible/parsing/ajson.py

@@ -4,10 +4,8 @@
 from __future__ import annotations as _annotations
 
 # from ansible.utils.display import Display as _Display
-
+#
-
+#
-# DTFIX1: The pylint deprecated checker does not detect `Display().deprecated` calls, of which we have many.
-
 # deprecated: description='deprecate ajson' core_version='2.23'
 # _Display().deprecated(
 #     msg='The `ansible.parsing.ajson` module is deprecated.',

lib/ansible/parsing/vault/__init__.py

@@ -1520,7 +1520,7 @@ class VaultHelper:
             tags = AnsibleTagHelper.tags(ciphertext)  # ciphertext has tags but value does not
         elif value_type is EncryptedString:
             ciphertext = value._ciphertext
-        elif value_type in _jinja_common.Marker.concrete_subclasses:  # avoid wasteful raise/except of Marker when calling get_tag below
+        elif value_type in _jinja_common.Marker._concrete_subclasses:  # avoid wasteful raise/except of Marker when calling get_tag below
             ciphertext = None
         elif vaulted_value := VaultedValue.get_tag(value):
             ciphertext = vaulted_value.ciphertext

lib/ansible/playbook/taggable.py

@@ -45,9 +45,6 @@ class Taggable:
             return ds
 
         if isinstance(ds, str):
-            # DTFIX0: this allows each individual tag to be templated, but prevents the use of commas in templates, is that what we want?
-            # DTFIX0: this can return empty tags (including a list of nothing but empty tags), is that correct?
-            # DTFIX0: the original code seemed to attempt to preserve `ds` if there were no commas, but it never ran, what should it actually do?
             return [AnsibleTagHelper.tag_copy(ds, item.strip()) for item in ds.split(',')]
 
         raise AnsibleError('tags must be specified as a list', obj=ds)

lib/ansible/plugins/__init__.py

@@ -189,28 +189,3 @@ class AnsibleJinja2Plugin(AnsiblePlugin, metaclass=abc.ABCMeta):
     @property
     def j2_function(self) -> t.Callable:
         return self._function
-
-
-_TCallable = t.TypeVar('_TCallable', bound=t.Callable)
-
-
-def accept_args_markers(plugin: _TCallable) -> _TCallable:
-    """
-    A decorator to mark a Jinja plugin as capable of handling `Marker` values for its top-level arguments.
-    Non-decorated plugin invocation is skipped when a top-level argument is a `Marker`, with the first such value substituted as the plugin result.
-    This ensures that only plugins which understand `Marker` instances for top-level arguments will encounter them.
-    """
-    plugin.accept_args_markers = True
-
-    return plugin
-
-
-def accept_lazy_markers(plugin: _TCallable) -> _TCallable:
-    """
-    A decorator to mark a Jinja plugin as capable of handling `Marker` values retrieved from lazy containers.
-    Non-decorated plugins will trigger a `MarkerError` exception when attempting to retrieve a `Marker` from a lazy container.
-    This ensures that only plugins which understand lazy retrieval of `Marker` instances will encounter them.
-    """
-    plugin.accept_lazy_markers = True
-
-    return plugin

lib/ansible/plugins/callback/__init__.py

@@ -280,8 +280,6 @@ class CallbackBase(AnsiblePlugin):
             # that want to further modify the result, or use custom serialization
             return abridged_result
 
-        # DTFIX0: Switch to stock json/yaml serializers here? We should always have a transformed plain-types result.
-
         if result_format == 'json':
             return json.dumps(abridged_result, cls=_fallback_to_str.Encoder, indent=indent, ensure_ascii=False, sort_keys=sort_keys)
 

lib/ansible/plugins/filter/core.py

@@ -34,7 +34,7 @@ from ansible.module_utils.common.text.converters import to_bytes, to_native, to_
 from ansible.module_utils.common.collections import is_sequence
 from ansible.module_utils.common.yaml import yaml_load, yaml_load_all
 from ansible.parsing.yaml.dumper import AnsibleDumper
-from ansible.plugins import accept_args_markers, accept_lazy_markers
+from ansible.template import accept_args_markers, accept_lazy_markers
 from ansible._internal._templating._jinja_common import MarkerError, UndefinedMarker, validate_arg_type
 from ansible.utils.display import Display
 from ansible.utils.encrypt import do_encrypt, PASSLIB_AVAILABLE
@@ -824,5 +824,3 @@ class FilterModule(object):
             'reject': wrapped_reject,
             'rejectattr': wrapped_rejectattr,
         }
-
-# DTFIX1: document protomatter plugins, or hide them from ansible-doc/galaxy (not related to this code, but needed some place to put this comment)

lib/ansible/plugins/filter/encryption.py

@@ -4,10 +4,10 @@ from __future__ import annotations
 
 from ansible.errors import AnsibleError
 from ansible.module_utils.common.text.converters import to_native, to_bytes
-from ansible.plugins import accept_args_markers
+from ansible._internal._templating._jinja_common import VaultExceptionMarker
-from ansible._internal._templating._jinja_common import get_first_marker_arg, VaultExceptionMarker
 from ansible._internal._datatag._tags import VaultedValue
 from ansible.parsing.vault import is_encrypted, VaultSecret, VaultLib, VaultHelper
+from ansible import template as _template
 from ansible.utils.display import Display
 
 display = Display()
@@ -43,12 +43,12 @@ def do_vault(data, secret, salt=None, vault_id='filter_default', wrap_object=Fal
     return vault
 
 
-@accept_args_markers
+@_template.accept_args_markers
 def do_unvault(vault, secret, vault_id='filter_default', vaultid=None):
     if isinstance(vault, VaultExceptionMarker):
         vault = vault._disarm()
 
-    if (first_marker := get_first_marker_arg((vault, secret, vault_id, vaultid), {})) is not None:
+    if (first_marker := _template.get_first_marker_arg((vault, secret, vault_id, vaultid), {})) is not None:
         return first_marker
 
     if not isinstance(secret, (str, bytes)):

lib/ansible/plugins/lookup/first_found.py

@@ -149,6 +149,7 @@ from ansible.errors import AnsibleError
 from ansible.plugins.lookup import LookupBase
 from ansible._internal._templating import _jinja_common
 from ansible._internal._templating import _jinja_plugins
+from ansible import template as _template
 from ansible.utils.path import unfrackpath
 from ansible.utils.display import Display
 from ansible.module_utils.datatag import native_type_name
@@ -222,7 +223,7 @@ class LookupModule(LookupBase):
         return total_search
 
     def run(self, terms: list, variables=None, **kwargs):
-        if (first_marker := _jinja_common.get_first_marker_arg((), kwargs)) is not None:
+        if (first_marker := _template.get_first_marker_arg((), kwargs)) is not None:
             first_marker.trip()
 
         if _jinja_plugins._LookupContext.current().invoked_as_with:

lib/ansible/plugins/test/core.py

@@ -31,7 +31,7 @@ from ansible import errors
 from ansible.module_utils.common.text.converters import to_native, to_text, to_bytes
 from ansible._internal._templating._jinja_common import Marker, UndefinedMarker
 from ansible.module_utils.parsing.convert_bool import boolean
-from ansible.plugins import accept_args_markers
+from ansible.template import accept_args_markers
 from ansible.parsing.vault import is_encrypted_file, VaultHelper, VaultLib
 from ansible.utils.display import Display
 from ansible.utils.version import SemanticVersion

lib/ansible/template/__init__.py

@@ -427,3 +427,31 @@ def is_trusted_as_template(value: object) -> bool:
     This function should not be needed for production code, but may be useful in unit tests.
     """
     return isinstance(value, _TRUSTABLE_TYPES) and _tags.TrustedAsTemplate.is_tagged_on(value)
+
+
+_TCallable = _t.TypeVar('_TCallable', bound=_t.Callable)
+
+
+def accept_args_markers(plugin: _TCallable) -> _TCallable:
+    """
+    A decorator to mark a Jinja plugin as capable of handling `Marker` values for its top-level arguments.
+    Non-decorated plugin invocation is skipped when a top-level argument is a `Marker`, with the first such value substituted as the plugin result.
+    This ensures that only plugins which understand `Marker` instances for top-level arguments will encounter them.
+    """
+    plugin.accept_args_markers = True
+
+    return plugin
+
+
+def accept_lazy_markers(plugin: _TCallable) -> _TCallable:
+    """
+    A decorator to mark a Jinja plugin as capable of handling `Marker` values retrieved from lazy containers.
+    Non-decorated plugins will trigger a `MarkerError` exception when attempting to retrieve a `Marker` from a lazy container.
+    This ensures that only plugins which understand lazy retrieval of `Marker` instances will encounter them.
+    """
+    plugin.accept_lazy_markers = True
+
+    return plugin
+
+
+get_first_marker_arg = _jinja_common.get_first_marker_arg

test/integration/targets/assert/tasks/main.yml

@@ -44,15 +44,27 @@
     - that: 1 == 1
     - that: a_var == "one"
     - that: '{{ a_var == "one" }}'
-# DTFIX1: loops are not lazily templated, so this is not possible today, but could be in the future
-#    - that: |
-#        "hi mom" == '{{ "hi mom" }}'
     - that:
         - 1 == 1
         - 2 == 2
         - a_var == "one"
         - '{{ a_var == "one" }}'
 
+- name: arg splat loopable failure case
+  assert: '{{ item }}'
+  args:
+    quiet: yes
+  ignore_errors: true
+  loop:
+    - that: |
+        "hi mom" == '{{ "hi mom" }}'  # not implemented; requires lazy loop templating
+  register: loop_fail
+
+- name: check results from previous assert failures
+  assert:
+    that:
+      - loop_fail is failed
+      - loop_fail.results[0].msg is contains "untrusted"
 
 - name: arg splat fail cases
   assert: '{{ item }}'