validate-modules and return fragments: fix bug in markup check, fix bug in missing doc fragment check, add tests (#85638)

* Prevent crashing on invalid structure.

* Also process return doc fragments.

* Fix handling of missing doc fragments.

changelogs/fragments/85638-ansible-test-validate-modules-doc-fragments.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - "validate-modules sanity test - fix handling of missing doc fragments (https://github.com/ansible/ansible/pull/85638)."

lib/ansible/utils/plugin_docs.py

@@ -126,6 +126,10 @@ def remove_current_collection_from_versions_and_dates(fragment, collection_name,
     _process_versions_and_dates(fragment, is_module, return_docs, remove)
 
 
+class AnsibleFragmentError(AnsibleError):
+    pass
+
+
 def add_fragments(doc, filename, fragment_loader, is_module=False, section='DOCUMENTATION'):
 
     if section not in _FRAGMENTABLE:
@@ -185,7 +189,7 @@ def add_fragments(doc, filename, fragment_loader, is_module=False, section='DOCU
                         doc[doc_key].extend(entries)
 
             if 'options' not in fragment and 'attributes' not in fragment:
-                raise Exception("missing options or attributes in fragment (%s), possibly misformatted?: %s" % (fragment_name, filename))
+                raise AnsibleFragmentError("missing options or attributes in fragment (%s), possibly misformatted?: %s" % (fragment_name, filename))
 
             # ensure options themselves are directly merged
             for doc_key in ['options', 'attributes']:
@@ -194,7 +198,7 @@ def add_fragments(doc, filename, fragment_loader, is_module=False, section='DOCU
                         try:
                             merge_fragment(doc[doc_key], fragment.pop(doc_key))
                         except Exception as e:
-                            raise AnsibleError("%s %s (%s) of unknown type: %s" % (to_native(e), doc_key, fragment_name, filename))
+                            raise AnsibleFragmentError("%s %s (%s) of unknown type: %s" % (to_native(e), doc_key, fragment_name, filename))
                     else:
                         doc[doc_key] = fragment.pop(doc_key)
 
@@ -202,10 +206,10 @@ def add_fragments(doc, filename, fragment_loader, is_module=False, section='DOCU
         try:
             merge_fragment(doc, fragment)
         except Exception as e:
-            raise AnsibleError("%s (%s) of unknown type: %s" % (to_native(e), fragment_name, filename))
+            raise AnsibleFragmentError("%s (%s) of unknown type: %s" % (to_native(e), fragment_name, filename))
 
     if unknown_fragments:
-        raise AnsibleError('unknown doc_fragment(s) in file {0}: {1}'.format(filename, to_native(', '.join(unknown_fragments))))
+        raise AnsibleFragmentError('unknown doc_fragment(s) in file {0}: {1}'.format(filename, to_native(', '.join(unknown_fragments))))
 
 
 def get_docstring(filename, fragment_loader, verbose=False, ignore_errors=False, collection_name=None, is_module=None, plugin_type=None):

test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/doc_fragments/return_doc_fragment.py

@@ -0,0 +1,19 @@
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+from __future__ import annotations
+
+
+class ModuleDocFragment:
+    DOCUMENTATION = r"""
+options: {}
+"""
+
+    RETURN = r"""
+bar:
+  description:
+    - Some foo bar.
+    - P(a.b.asfd#dfsa) this is an error.
+  returned: success
+  type: int
+  sample: 42
+"""

test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/modules/doc_fragments_not_exist.py

@@ -0,0 +1,28 @@
+#!/usr/bin/python
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+from __future__ import annotations
+
+DOCUMENTATION = """
+module: doc_fragments_not_exist
+short_description: Non-existing doc fragment
+description: A module with a non-existing doc fragment
+author:
+  - Ansible Core Team
+extends_documentation_fragment:
+  - does.not.exist
+"""
+
+EXAMPLES = """#"""
+
+RETURN = """"""
+
+from ansible.module_utils.basic import AnsibleModule
+
+
+def main():
+    AnsibleModule().exit_json()
+
+
+if __name__ == '__main__':
+    main()

test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/modules/return_fragments.py

@@ -0,0 +1,29 @@
+#!/usr/bin/python
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+from __future__ import annotations
+
+DOCUMENTATION = """
+module: return_fragments
+short_description: Uses return fragments
+description: A module with a return doc fragment.
+author:
+  - Ansible Core Team
+"""
+
+EXAMPLES = """#"""
+
+RETURN = """
+extends_documentation_fragment:
+  - ns.col.return_doc_fragment
+"""
+
+from ansible.module_utils.basic import AnsibleModule
+
+
+def main():
+    AnsibleModule().exit_json(bar=42)
+
+
+if __name__ == '__main__':
+    main()

test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/modules/return_fragments_not_exist.py

@@ -0,0 +1,29 @@
+#!/usr/bin/python
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+from __future__ import annotations
+
+DOCUMENTATION = """
+module: return_fragments_not_exist
+short_description: Non-existing return doc fragment
+description: A module with a non-existing return doc fragment.
+author:
+  - Ansible Core Team
+"""
+
+EXAMPLES = """#"""
+
+RETURN = """
+extends_documentation_fragment:
+  - does.not.exist
+"""
+
+from ansible.module_utils.basic import AnsibleModule
+
+
+def main():
+    AnsibleModule().exit_json(bar=42)
+
+
+if __name__ == '__main__':
+    main()

test/integration/targets/ansible-test-sanity-validate-modules/expected.txt

@@ -3,6 +3,7 @@ plugins/modules/check_mode_attribute_1.py:0:0: attributes-check-mode: The module
 plugins/modules/check_mode_attribute_2.py:0:0: attributes-check-mode: The module does not declare support for check mode, but the check_mode attribute's support value is 'partial' and not 'none'
 plugins/modules/check_mode_attribute_3.py:0:0: attributes-check-mode: The module does declare support for check mode, but the check_mode attribute's support value is 'none'
 plugins/modules/check_mode_attribute_4.py:0:0: attributes-check-mode-details: The module declares it does not fully support check mode, but has no details on what exactly that means
+plugins/modules/doc_fragments_not_exist.py:0:0: doc-fragment-error: Error while adding fragments: unknown doc_fragment(s) in file plugins/modules/doc_fragments_not_exist.py: does.not.exist
 plugins/modules/import_order.py:7:0: import-before-documentation: Import found before documentation variables. All imports must appear below DOCUMENTATION/EXAMPLES/RETURN.
 plugins/modules/invalid_argument_spec_extra_key.py:0:0: invalid-ansiblemodule-schema: AnsibleModule.argument_spec.foo.extra_key: extra keys not allowed @ data['argument_spec']['foo']['extra_key']. Got 'bar'
 plugins/modules/invalid_argument_spec_incorrect_context.py:0:0: invalid-ansiblemodule-schema: AnsibleModule.argument_spec.foo.context: expected dict for dictionary value @ data['argument_spec']['foo']['context']. Got 'bar'
@@ -14,6 +15,8 @@ plugins/modules/invalid_yaml_syntax.py:11:15: invalid-examples: EXAMPLES is not
 plugins/modules/invalid_yaml_syntax.py:15:15: return-syntax-error: RETURN is not valid YAML
 plugins/modules/option_name_casing.py:0:0: option-equal-up-to-casing: Multiple options/aliases are equal up to casing: option 'Bar', alias 'baR' of option 'bam', alias 'bar' of option 'foo'
 plugins/modules/option_name_casing.py:0:0: option-equal-up-to-casing: Multiple options/aliases are equal up to casing: option 'Foo', option 'foo'
+plugins/modules/return_fragments.py:0:0: invalid-documentation-markup: RETURN.bar.description.1: Directive "P(a.b.asfd#dfsa)" must contain a valid plugin type; found "dfsa" @ data['bar']['description'][1]. Got 'P(a.b.asfd#dfsa) this is an error.'
+plugins/modules/return_fragments_not_exist.py:0:0: return-fragment-error: Error while adding fragments: unknown doc_fragment(s) in file plugins/modules/return_fragments_not_exist.py: does.not.exist
 plugins/modules/semantic_markup.py:0:0: invalid-documentation-markup: DOCUMENTATION.options.a11.suboptions.b1.description.0: While parsing "V(C\(" at index 1: Unnecessarily escaped "(" @ data['options']['a11']['suboptions']['b1']['description'][0]. Got 'V(C\\(foo\\)).'
 plugins/modules/semantic_markup.py:0:0: invalid-documentation-markup: DOCUMENTATION.options.a11.suboptions.b1.description.2: While parsing "P(foo.bar#baz)" at index 1: Plugin name "foo.bar" is not a FQCN @ data['options']['a11']['suboptions']['b1']['description'][2]. Got 'P(foo.bar#baz).'
 plugins/modules/semantic_markup.py:0:0: invalid-documentation-markup: DOCUMENTATION.options.a11.suboptions.b1.description.3: While parsing "P(foo.bar.baz)" at index 1: Parameter "foo.bar.baz" is not of the form FQCN#type @ data['options']['a11']['suboptions']['b1']['description'][3]. Got 'P(foo.bar.baz).'

test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/main.py

@@ -72,7 +72,7 @@ from ansible.module_utils.compat.version import StrictVersion, LooseVersion
 from ansible.module_utils.basic import to_bytes
 from ansible.plugins.loader import fragment_loader
 from ansible.plugins.list import IGNORE as REJECTLIST
-from ansible.utils.plugin_docs import add_collection_to_versions_and_dates, add_fragments, get_docstring
+from ansible.utils.plugin_docs import AnsibleFragmentError, add_collection_to_versions_and_dates, add_fragments, get_docstring
 from ansible.utils.version import SemanticVersion
 
 from .module_args import AnsibleModuleImportError, AnsibleModuleNotInitialized, get_py_argument_spec, get_ps_argument_spec
@@ -1003,21 +1003,15 @@ class ModuleValidator(Validator):
             add_collection_to_versions_and_dates(doc, self.collection_name,
                                                  is_module=self.plugin_type == 'module')
 
-            missing_fragment = False
             with CaptureStd():
                 try:
                     get_docstring(os.path.abspath(self.path), fragment_loader=fragment_loader,
                                   verbose=True,
                                   collection_name=self.collection_name,
                                   plugin_type=self.plugin_type)
-                except AssertionError:
-                    fragment = doc['extends_documentation_fragment']
-                    self.reporter.error(
-                        path=self.object_path,
-                        code='missing-doc-fragment',
-                        msg='DOCUMENTATION fragment missing: %s' % fragment
-                    )
-                    missing_fragment = True
+                except AnsibleFragmentError:
+                    # Will be re-triggered below when explicitly calling add_fragments()
+                    pass
                 except Exception as e:
                     self.reporter.trace(
                         path=self.object_path,
@@ -1029,9 +1023,16 @@ class ModuleValidator(Validator):
                         msg='Unknown DOCUMENTATION error, see TRACE: %s' % e
                     )
 
-            if not missing_fragment:
+            try:
                 add_fragments(doc, os.path.abspath(self.object_path), fragment_loader=fragment_loader,
-                              is_module=self.plugin_type == 'module')
+                              is_module=self.plugin_type == 'module', section='DOCUMENTATION')
+            except AnsibleFragmentError as exc:
+                error = str(exc).replace(os.path.abspath(self.object_path), self.object_path)
+                self.reporter.error(
+                    path=self.object_path,
+                    code='doc-fragment-error',
+                    msg=f'Error while adding fragments: {error}'
+                )
 
             if 'options' in doc and doc['options'] is None:
                 self.reporter.error(
@@ -1130,6 +1131,16 @@ class ModuleValidator(Validator):
                     self.collection_name,
                     is_module=self.plugin_type == 'module',
                     return_docs=True)
+                try:
+                    add_fragments(returns, os.path.abspath(self.object_path), fragment_loader=fragment_loader,
+                                  is_module=self.plugin_type == 'module', section='RETURN')
+                except AnsibleFragmentError as exc:
+                    error = str(exc).replace(os.path.abspath(self.object_path), self.object_path)
+                    self.reporter.error(
+                        path=self.object_path,
+                        code='return-fragment-error',
+                        msg=f'Error while adding fragments: {error}'
+                    )
             self._validate_docs_schema(
                 returns,
                 return_schema(for_collection=bool(self.collection), plugin_type=self.plugin_type),
@@ -1268,16 +1279,18 @@ class ModuleValidator(Validator):
         if not isinstance(options, dict):
             return
         for key, value in options.items():
-            self._validate_semantic_markup(value.get('description'))
-            self._validate_semantic_markup_options(value.get('suboptions'))
+            if isinstance(value, dict):
+                self._validate_semantic_markup(value.get('description'))
+                self._validate_semantic_markup_options(value.get('suboptions'))
 
     def _validate_semantic_markup_return_values(self, return_vars):
         if not isinstance(return_vars, dict):
             return
         for key, value in return_vars.items():
-            self._validate_semantic_markup(value.get('description'))
-            self._validate_semantic_markup(value.get('returned'))
-            self._validate_semantic_markup_return_values(value.get('contains'))
+            if isinstance(value, dict):
+                self._validate_semantic_markup(value.get('description'))
+                self._validate_semantic_markup(value.get('returned'))
+                self._validate_semantic_markup_return_values(value.get('contains'))
 
     def _validate_all_semantic_markup(self, docs, return_docs):
         if not isinstance(docs, dict):
@@ -1617,7 +1630,7 @@ class ModuleValidator(Validator):
         try:
             if not context:
                 add_fragments(docs, os.path.abspath(self.object_path), fragment_loader=fragment_loader,
-                              is_module=self.plugin_type == 'module')
+                              is_module=self.plugin_type == 'module', section='DOCUMENTATION')
         except Exception:
             # Cannot merge fragments
             return