Replace validate-modules's semantic markup parser with antsibull-docs-parser (#80406)

changelogs/fragments/80406-validate-modules-semantic-markup.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - "validate-modules sanity test - replace semantic markup parsing and validating code with the code from `antsibull-docs-parser 0.2.0 <https://github.com/ansible-community/antsibull-docs-parser/releases/tag/0.2.0>`__ (https://github.com/ansible/ansible/pull/80406)."

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

@@ -75,6 +75,23 @@ options:
   a10:
     description: O(foo.bar=1).
     type: str
+
+  a11:
+    description: Something with suboptions.
+    type: dict
+    suboptions:
+      b1:
+        description:
+          - V(C\(foo\)).
+          - RV(bam).
+          - P(foo.bar#baz).
+          - P(foo.bar.baz).
+          - P(foo.bar.baz#woof).
+          - E(foo\(bar).
+          - O(bar).
+          - O(bar=bam).
+          - O(foo.bar=1).
+        type: str
 '''
 
 EXAMPLES = '''#'''
@@ -103,5 +120,8 @@ if __name__ == '__main__':
         a8=dict(),
         a9=dict(),
         a10=dict(),
+        a11=dict(type='dict', options=dict(
+            b1=dict(),
+        ))
     ))
     module.exit_json()

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

@@ -9,11 +9,16 @@ plugins/modules/invalid_yaml_syntax.py:0:0: missing-documentation: No DOCUMENTAT
 plugins/modules/invalid_yaml_syntax.py:8:15: documentation-syntax-error: DOCUMENTATION is not valid YAML
 plugins/modules/invalid_yaml_syntax.py:12:15: invalid-examples: EXAMPLES is not valid YAML
 plugins/modules/invalid_yaml_syntax.py:16:15: return-syntax-error: RETURN is not valid YAML
-plugins/modules/semantic_markup.py:0:0: invalid-documentation-markup: DOCUMENTATION.options.a2.description: Directive "V(C\(foo\))" contains unnecessarily quoted "(" for dictionary value @ data['options']['a2']['description']. Got 'V(C\\(foo\\)).'
-plugins/modules/semantic_markup.py:0:0: invalid-documentation-markup: DOCUMENTATION.options.a4.description: Directive "P(foo.bar#baz)" must contain a FQCN; found "foo.bar" for dictionary value @ data['options']['a4']['description']. Got 'P(foo.bar#baz).'
-plugins/modules/semantic_markup.py:0:0: invalid-documentation-markup: DOCUMENTATION.options.a5.description: Directive "P(foo.bar.baz)" must contain a "#" for dictionary value @ data['options']['a5']['description']. Got 'P(foo.bar.baz).'
+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).'
+plugins/modules/semantic_markup.py:0:0: invalid-documentation-markup: DOCUMENTATION.options.a11.suboptions.b1.description.4: Directive "P(foo.bar.baz#woof)" must contain a valid plugin type; found "woof" @ data['options']['a11']['suboptions']['b1']['description'][4]. Got 'P(foo.bar.baz#woof).'
+plugins/modules/semantic_markup.py:0:0: invalid-documentation-markup: DOCUMENTATION.options.a11.suboptions.b1.description.5: While parsing "E(foo\(" at index 1: Unnecessarily escaped "(" @ data['options']['a11']['suboptions']['b1']['description'][5]. Got 'E(foo\\(bar).'
+plugins/modules/semantic_markup.py:0:0: invalid-documentation-markup: DOCUMENTATION.options.a2.description: While parsing "V(C\(" at index 1: Unnecessarily escaped "(" for dictionary value @ data['options']['a2']['description']. Got 'V(C\\(foo\\)).'
+plugins/modules/semantic_markup.py:0:0: invalid-documentation-markup: DOCUMENTATION.options.a4.description: While parsing "P(foo.bar#baz)" at index 1: Plugin name "foo.bar" is not a FQCN for dictionary value @ data['options']['a4']['description']. Got 'P(foo.bar#baz).'
+plugins/modules/semantic_markup.py:0:0: invalid-documentation-markup: DOCUMENTATION.options.a5.description: While parsing "P(foo.bar.baz)" at index 1: Parameter "foo.bar.baz" is not of the form FQCN#type for dictionary value @ data['options']['a5']['description']. Got 'P(foo.bar.baz).'
 plugins/modules/semantic_markup.py:0:0: invalid-documentation-markup: DOCUMENTATION.options.a6.description: Directive "P(foo.bar.baz#woof)" must contain a valid plugin type; found "woof" for dictionary value @ data['options']['a6']['description']. Got 'P(foo.bar.baz#woof).'
-plugins/modules/semantic_markup.py:0:0: invalid-documentation-markup: DOCUMENTATION.options.a7.description: Directive "E(foo\(bar)" contains unnecessarily quoted "(" for dictionary value @ data['options']['a7']['description']. Got 'E(foo\\(bar).'
+plugins/modules/semantic_markup.py:0:0: invalid-documentation-markup: DOCUMENTATION.options.a7.description: While parsing "E(foo\(" at index 1: Unnecessarily escaped "(" for dictionary value @ data['options']['a7']['description']. Got 'E(foo\\(bar).'
 plugins/modules/semantic_markup.py:0:0: invalid-documentation-markup: Directive "O(bar)" contains a non-existing option "bar"
 plugins/modules/semantic_markup.py:0:0: invalid-documentation-markup: Directive "O(bar=bam)" contains a non-existing option "bar"
 plugins/modules/semantic_markup.py:0:0: invalid-documentation-markup: Directive "O(foo.bar=1)" contains a non-existing option "foo.bar"

test/lib/ansible_test/_data/requirements/sanity.validate-modules.in

@@ -1,3 +1,4 @@
 jinja2  # ansible-core requirement
 pyyaml  # needed for collection_detail.py
 voluptuous
+antsibull-docs-parser==0.2.0

test/lib/ansible_test/_data/requirements/sanity.validate-modules.txt

@@ -1,4 +1,5 @@
 # edit "sanity.validate-modules.in" and generate with: hacking/update-sanity-requirements.py --test validate-modules
+antsibull-docs-parser==0.2.0
 Jinja2==3.1.2
 MarkupSafe==2.1.2
 PyYAML==6.0

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

@@ -33,6 +33,9 @@ from collections.abc import Mapping
 from contextlib import contextmanager
 from fnmatch import fnmatch
 
+from antsibull_docs_parser import dom
+from antsibull_docs_parser.parser import parse, Context
+
 import yaml
 
 from voluptuous.humanize import humanize_error
@@ -79,10 +82,6 @@ from .schema import (
     ansible_module_kwargs_schema,
     doc_schema,
     return_schema,
-    _SEM_OPTION_NAME,
-    _SEM_RET_VALUE,
-    _check_sem_quoting,
-    _parse_prefix,
 )
 
 from .utils import CaptureStd, NoArgsAnsibleModule, compare_unordered_lists, parse_yaml, parse_isodate
@@ -1164,39 +1163,31 @@ class ModuleValidator(Validator):
 
         return doc_info, doc
 
-    def _check_sem_option(self, directive, content):
-        try:
-            content = _check_sem_quoting(directive, content)
-            plugin_fqcn, plugin_type, option_link, option, value = _parse_prefix(directive, content)
-        except Exception:
-            # Validation errors have already been covered in the schema check
+    def _check_sem_option(self, part: dom.OptionNamePart, current_plugin: dom.PluginIdentifier) -> None:
+        if part.plugin is None or part.plugin != current_plugin:
             return
-        if plugin_fqcn is not None:
+        if part.entrypoint is not None:
             return
-        if tuple(option_link) not in self._all_options:
+        if tuple(part.link) not in self._all_options:
             self.reporter.error(
                 path=self.object_path,
                 code='invalid-documentation-markup',
-                msg='Directive "%s" contains a non-existing option "%s"' % (directive, option)
+                msg='Directive "%s" contains a non-existing option "%s"' % (part.source, part.name)
             )
 
-    def _check_sem_return_value(self, directive, content):
-        try:
-            content = _check_sem_quoting(directive, content)
-            plugin_fqcn, plugin_type, rv_link, rv, value = _parse_prefix(directive, content)
-        except Exception:
-            # Validation errors have already been covered in the schema check
+    def _check_sem_return_value(self, part: dom.ReturnValuePart, current_plugin: dom.PluginIdentifier) -> None:
+        if part.plugin is None or part.plugin != current_plugin:
             return
-        if plugin_fqcn is not None:
+        if part.entrypoint is not None:
             return
-        if tuple(rv_link) not in self._all_return_values:
+        if tuple(part.link) not in self._all_return_values:
             self.reporter.error(
                 path=self.object_path,
                 code='invalid-documentation-markup',
-                msg='Directive "%s" contains a non-existing return value "%s"' % (directive, rv)
+                msg='Directive "%s" contains a non-existing return value "%s"' % (part.source, part.name)
             )
 
-    def _validate_semantic_markup(self, object):
+    def _validate_semantic_markup(self, object) -> None:
         # Make sure we operate on strings
         if is_iterable(object):
             for entry in object:
@@ -1205,10 +1196,19 @@ class ModuleValidator(Validator):
         if not isinstance(object, string_types):
             return
 
-        for m in _SEM_OPTION_NAME.finditer(object):
-            self._check_sem_option(m.group(0), m.group(1))
-        for m in _SEM_RET_VALUE.finditer(object):
-            self._check_sem_return_value(m.group(0), m.group(1))
+        if self.collection:
+            fqcn = f'{self.collection_name}.{self.name}'
+        else:
+            fqcn = f'ansible.builtin.{self.name}'
+        current_plugin = dom.PluginIdentifier(fqcn=fqcn, type=self.plugin_type)
+        for par in parse(object, Context(current_plugin=current_plugin), errors='message', add_source=True):
+            for part in par:
+                # Errors are already covered during schema validation, we only check for option and
+                # return value references
+                if part.type == dom.PartType.OPTION_NAME:
+                    self._check_sem_option(part, current_plugin)
+                if part.type == dom.PartType.RETURN_VALUE:
+                    self._check_sem_return_value(part, current_plugin)
 
     def _validate_semantic_markup_collect(self, destination, sub_key, data, all_paths):
         if not isinstance(data, dict):

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

@@ -11,7 +11,7 @@ from ansible.module_utils.compat.version import StrictVersion
 from functools import partial
 from urllib.parse import urlparse
 
-from voluptuous import ALLOW_EXTRA, PREVENT_EXTRA, All, Any, Invalid, Length, Required, Schema, Self, ValueInvalid, Exclusive
+from voluptuous import ALLOW_EXTRA, PREVENT_EXTRA, All, Any, Invalid, Length, MultipleInvalid, Required, Schema, Self, ValueInvalid, Exclusive
 from ansible.constants import DOCUMENTABLE_PLUGINS
 from ansible.module_utils.six import string_types
 from ansible.module_utils.common.collections import is_iterable
@@ -20,6 +20,9 @@ from ansible.parsing.quoting import unquote
 from ansible.utils.version import SemanticVersion
 from ansible.release import __version__
 
+from antsibull_docs_parser import dom
+from antsibull_docs_parser.parser import parse, Context
+
 from .utils import parse_isodate
 
 list_string_types = list(string_types)
@@ -81,57 +84,8 @@ def date(error_code=None):
     return Any(isodate, error_code=error_code)
 
 
-_MODULE = re.compile(r"\bM\(([^)]+)\)")
-_PLUGIN = re.compile(r"\bP\(([^)]+)\)")
-_LINK = re.compile(r"\bL\(([^)]+)\)")
-_URL = re.compile(r"\bU\(([^)]+)\)")
-_REF = re.compile(r"\bR\(([^)]+)\)")
-
-_SEM_PARAMETER_STRING = r"\(((?:[^\\)]+|\\.)+)\)"
-_SEM_OPTION_NAME = re.compile(r"\bO" + _SEM_PARAMETER_STRING)
-_SEM_OPTION_VALUE = re.compile(r"\bV" + _SEM_PARAMETER_STRING)
-_SEM_ENV_VARIABLE = re.compile(r"\bE" + _SEM_PARAMETER_STRING)
-_SEM_RET_VALUE = re.compile(r"\bRV" + _SEM_PARAMETER_STRING)
-
-_UNESCAPE = re.compile(r"\\(.)")
-_CONTENT_LINK_SPLITTER_RE = re.compile(r'(?:\[[^\]]*\])?\.')
-_CONTENT_LINK_END_STUB_RE = re.compile(r'\[[^\]]*\]$')
-_FQCN_TYPE_PREFIX_RE = re.compile(r'^([^.]+\.[^.]+\.[^#]+)#([a-z]+):(.*)$')
-_IGNORE_MARKER = 'ignore:'
-_IGNORE_STRING = '(ignore)'
-
-_VALID_PLUGIN_TYPES = set(DOCUMENTABLE_PLUGINS)
-
-
-def _check_module_link(directive, content):
-    if not FULLY_QUALIFIED_COLLECTION_RESOURCE_RE.match(content):
-        raise _add_ansible_error_code(
-            Invalid('Directive "%s" must contain a FQCN' % directive), 'invalid-documentation-markup')
-
-
-def _check_plugin_link(directive, content):
-    if '#' not in content:
-        raise _add_ansible_error_code(
-            Invalid('Directive "%s" must contain a "#"' % directive), 'invalid-documentation-markup')
-    plugin_fqcn, plugin_type = content.split('#', 1)
-    if not FULLY_QUALIFIED_COLLECTION_RESOURCE_RE.match(plugin_fqcn):
-        raise _add_ansible_error_code(
-            Invalid('Directive "%s" must contain a FQCN; found "%s"' % (directive, plugin_fqcn)),
-            'invalid-documentation-markup')
-    if plugin_type not in _VALID_PLUGIN_TYPES:
-        raise _add_ansible_error_code(
-            Invalid('Directive "%s" must contain a valid plugin type; found "%s"' % (directive, plugin_type)),
-            'invalid-documentation-markup')
-
-
-def _check_link(directive, content):
-    if ',' not in content:
-        raise _add_ansible_error_code(
-            Invalid('Directive "%s" must contain a comma' % directive), 'invalid-documentation-markup')
-    idx = content.rindex(',')
-    title = content[:idx]
-    url = content[idx + 1:].lstrip(' ')
-    _check_url(directive, url)
+# Roles can also be referenced by semantic markup
+_VALID_PLUGIN_TYPES = set(DOCUMENTABLE_PLUGINS + ('role', ))
 
 
 def _check_url(directive, content):
@@ -139,67 +93,10 @@ def _check_url(directive, content):
         parsed_url = urlparse(content)
         if parsed_url.scheme not in ('', 'http', 'https'):
             raise ValueError('Schema must be HTTP, HTTPS, or not specified')
-    except ValueError as exc:
-        raise _add_ansible_error_code(
-            Invalid('Directive "%s" must contain an URL' % directive), 'invalid-documentation-markup')
-
-
-def _check_ref(directive, content):
-    if ',' not in content:
-        raise _add_ansible_error_code(
-            Invalid('Directive "%s" must contain a comma' % directive), 'invalid-documentation-markup')
-
-
-def _check_sem_quoting(directive, content):
-    for m in _UNESCAPE.finditer(content):
-        if m.group(1) not in ('\\', ')'):
-            raise _add_ansible_error_code(
-                Invalid('Directive "%s" contains unnecessarily quoted "%s"' % (directive, m.group(1))),
-                'invalid-documentation-markup')
-    return _UNESCAPE.sub(r'\1', content)
-
-
-def _parse_prefix(directive, content):
-    value = None
-    if '=' in content:
-        content, value = content.split('=', 1)
-    m = _FQCN_TYPE_PREFIX_RE.match(content)
-    if m:
-        plugin_fqcn = m.group(1)
-        plugin_type = m.group(2)
-        content = m.group(3)
-        if not FULLY_QUALIFIED_COLLECTION_RESOURCE_RE.match(plugin_fqcn):
-            raise _add_ansible_error_code(
-                Invalid('Directive "%s" must contain a FQCN; found "%s"' % (directive, plugin_fqcn)),
-                'invalid-documentation-markup')
-        if plugin_type not in _VALID_PLUGIN_TYPES:
-            raise _add_ansible_error_code(
-                Invalid('Directive "%s" must contain a valid plugin type; found "%s"' % (directive, plugin_type)),
-                'invalid-documentation-markup')
-    elif content.startswith(_IGNORE_MARKER):
-        content = content[len(_IGNORE_MARKER):]
-        plugin_fqcn = plugin_type = _IGNORE_STRING
-    else:
-        plugin_fqcn = plugin_type = None
-    if ':' in content or '#' in content:
-        raise _add_ansible_error_code(
-            Invalid('Directive "%s" contains wrongly specified FQCN/plugin type' % directive),
-            'invalid-documentation-markup')
-    content_link = _CONTENT_LINK_SPLITTER_RE.split(content)
-    for i, part in enumerate(content_link):
-        if i == len(content_link) - 1:
-            part = _CONTENT_LINK_END_STUB_RE.sub('', part)
-            content_link[i] = part
-        if '.' in part or '[' in part or ']' in part:
-            raise _add_ansible_error_code(
-                Invalid('Directive "%s" contains invalid name "%s"' % (directive, content)),
-                'invalid-documentation-markup')
-    return plugin_fqcn, plugin_type, content_link, content, value
-
-
-def _check_sem_option_return_value(directive, content):
-    content = _check_sem_quoting(directive, content)
-    _parse_prefix(directive, content)
+        return []
+    except ValueError:
+        return [_add_ansible_error_code(
+            Invalid('Directive %s must contain a valid URL' % directive), 'invalid-documentation-markup')]
 
 
 def doc_string(v):
@@ -207,35 +104,55 @@ def doc_string(v):
     if not isinstance(v, string_types):
         raise _add_ansible_error_code(
             Invalid('Must be a string'), 'invalid-documentation')
-    for m in _MODULE.finditer(v):
-        _check_module_link(m.group(0), m.group(1))
-    for m in _PLUGIN.finditer(v):
-        _check_plugin_link(m.group(0), m.group(1))
-    for m in _LINK.finditer(v):
-        _check_link(m.group(0), m.group(1))
-    for m in _URL.finditer(v):
-        _check_url(m.group(0), m.group(1))
-    for m in _REF.finditer(v):
-        _check_ref(m.group(0), m.group(1))
-    for m in _SEM_OPTION_NAME.finditer(v):
-        _check_sem_option_return_value(m.group(0), m.group(1))
-    for m in _SEM_OPTION_VALUE.finditer(v):
-        _check_sem_quoting(m.group(0), m.group(1))
-    for m in _SEM_ENV_VARIABLE.finditer(v):
-        _check_sem_quoting(m.group(0), m.group(1))
-    for m in _SEM_RET_VALUE.finditer(v):
-        _check_sem_option_return_value(m.group(0), m.group(1))
+    errors = []
+    for par in parse(v, Context(), errors='message', strict=True, add_source=True):
+        for part in par:
+            if part.type == dom.PartType.ERROR:
+                errors.append(_add_ansible_error_code(Invalid(part.message), 'invalid-documentation-markup'))
+            if part.type == dom.PartType.URL:
+                errors.extend(_check_url('U()', part.url))
+            if part.type == dom.PartType.LINK:
+                errors.extend(_check_url('L()', part.url))
+            if part.type == dom.PartType.MODULE:
+                if not FULLY_QUALIFIED_COLLECTION_RESOURCE_RE.match(part.fqcn):
+                    errors.append(_add_ansible_error_code(Invalid(
+                        'Directive "%s" must contain a FQCN; found "%s"' % (part.source, part.fqcn)),
+                        'invalid-documentation-markup'))
+            if part.type == dom.PartType.PLUGIN:
+                if not FULLY_QUALIFIED_COLLECTION_RESOURCE_RE.match(part.plugin.fqcn):
+                    errors.append(_add_ansible_error_code(Invalid(
+                        'Directive "%s" must contain a FQCN; found "%s"' % (part.source, part.plugin.fqcn)),
+                        'invalid-documentation-markup'))
+                if part.plugin.type not in _VALID_PLUGIN_TYPES:
+                    errors.append(_add_ansible_error_code(Invalid(
+                        'Directive "%s" must contain a valid plugin type; found "%s"' % (part.source, part.plugin.type)),
+                        'invalid-documentation-markup'))
+            if part.type == dom.PartType.OPTION_NAME:
+                if part.plugin is not None and not FULLY_QUALIFIED_COLLECTION_RESOURCE_RE.match(part.plugin.fqcn):
+                    errors.append(_add_ansible_error_code(Invalid(
+                        'Directive "%s" must contain a FQCN; found "%s"' % (part.source, part.plugin.fqcn)),
+                        'invalid-documentation-markup'))
+                if part.plugin is not None and part.plugin.type not in _VALID_PLUGIN_TYPES:
+                    errors.append(_add_ansible_error_code(Invalid(
+                        'Directive "%s" must contain a valid plugin type; found "%s"' % (part.source, part.plugin.type)),
+                        'invalid-documentation-markup'))
+            if part.type == dom.PartType.RETURN_VALUE:
+                if part.plugin is not None and not FULLY_QUALIFIED_COLLECTION_RESOURCE_RE.match(part.plugin.fqcn):
+                    errors.append(_add_ansible_error_code(Invalid(
+                        'Directive "%s" must contain a FQCN; found "%s"' % (part.source, part.plugin.fqcn)),
+                        'invalid-documentation-markup'))
+                if part.plugin is not None and part.plugin.type not in _VALID_PLUGIN_TYPES:
+                    errors.append(_add_ansible_error_code(Invalid(
+                        'Directive "%s" must contain a valid plugin type; found "%s"' % (part.source, part.plugin.type)),
+                        'invalid-documentation-markup'))
+    if len(errors) == 1:
+        raise errors[0]
+    if errors:
+        raise MultipleInvalid(errors)
     return v
 
 
-def doc_string_or_strings(v):
-    """Match a documentation string, or list of strings."""
-    if isinstance(v, string_types):
-        return doc_string(v)
-    if isinstance(v, (list, tuple)):
-        return [doc_string(vv) for vv in v]
-    raise _add_ansible_error_code(
-        Invalid('Must be a string or list of strings'), 'invalid-documentation')
+doc_string_or_strings = Any(doc_string, [doc_string])
 
 
 def is_callable(v):

test/units/ansible_test/test_validate_modules.py

@@ -18,6 +18,11 @@ def validate_modules() -> None:
     sys.modules['voluptuous'] = voluptuous = mock.MagicMock()
     sys.modules['voluptuous.humanize'] = voluptuous.humanize = mock.MagicMock()
 
+    # Mock out antsibull_docs_parser to facilitate testing without it, since tests aren't covering anything that uses it.
+
+    sys.modules['antsibull_docs_parser'] = antsibull_docs_parser = mock.MagicMock()
+    sys.modules['antsibull_docs_parser.parser'] = antsibull_docs_parser.parser = mock.MagicMock()
+
 
 @pytest.mark.parametrize('cstring,cexpected', [
     ['if type(foo) is Bar', True],