validate-modules: fix version_added validation for top-level, fix err…

…or codes (#70869) (#70947)

* Also validate top-level version_added.

* Fix error code.

* Produce same version_added validation error in schema than in code (and stop returning it twice).

* Return correct error codes for invalid version_added for options and return values.

* Add changelog.

* Fix forgotten closing braket.

* Accept 'historical' for some top-level version_added.

(cherry picked from commit 7e2cc7d)

changelogs/fragments/70869-ansible-test-validate-modules-version-added.yml

@@ -0,0 +1,3 @@
+bugfixes:
+- "ansible-test validate-modules - return correct error codes ``option-invalid-version-added`` resp. ``return-invalid-version-added`` instead of the wrong error ``deprecation-either-date-or-version`` when an invalid value of ``version_added`` is specified for an option or a return value (https://github.com/ansible/ansible/pull/70869)."
+- "ansible-test validate-modules - ``version_added`` on module level was not validated for modules in collections (https://github.com/ansible/ansible/pull/70869)."

docs/docsite/rst/dev_guide/testing_validate-modules.rst

@@ -120,7 +120,7 @@ Codes
   no-default-for-required-parameter                            Documentation        Error                  Option is marked as required but specifies a default. Arguments with a default should not be marked as required
   nonexistent-parameter-documented                             Documentation        Error                  Argument is listed in DOCUMENTATION.options, but not accepted by the module
   option-incorrect-version-added                               Documentation        Error                  ``version_added`` for new option is incorrect
-  option-invalid-version-added                                 Documentation        Error                  ``version_added`` for new option is not a valid version number
+  option-invalid-version-added                                 Documentation        Error                  ``version_added`` for option is not a valid version number
   parameter-invalid                                            Documentation        Error                  Argument in argument_spec is not a valid python identifier
   parameter-invalid-elements                                   Documentation        Error                  Value for "elements" is valid only when value of "type" is ``list``
   implied-parameter-type-mismatch                              Documentation        Error                  Argument_spec implies ``type="str"`` but documentation defines it as different data type
@@ -132,6 +132,7 @@ Codes
   parameter-state-invalid-choice                               Parameters           Error                  Argument ``state`` includes ``get``, ``list`` or ``info`` as a choice.  Functionality should be in an ``_info`` or (if further conditions apply) ``_facts`` module.
   python-syntax-error                                          Syntax               Error                  Python ``SyntaxError`` while parsing module
   return-syntax-error                                          Documentation        Error                  ``RETURN`` is not valid YAML, ``RETURN`` fragments missing  or invalid
+  return-invalid-version-added                                 Documentation        Error                  ``version_added`` for return value is not a valid version number
   subdirectory-missing-init                                    Naming               Error                  Ansible module subdirectories must contain an ``__init__.py``
   try-except-missing-has                                       Imports              Warning                Try/Except ``HAS_`` expression missing
   undocumented-parameter                                       Documentation        Error                  Argument is listed in the argument_spec, but not documented in the module

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

@@ -1160,16 +1160,18 @@ def _check_version_added(self, doc, existing_doc):
         try:
             collection_name = doc.get('version_added_collection')
             version_added = self._create_strict_version(
-                str(doc.get('version_added', '0.0') or '0.0'),
+                str(version_added_raw or '0.0'),
                 collection_name=collection_name)
         except ValueError as e:
-            version_added = doc.get('version_added', '0.0')
-            if version_added != 'historical' or self._is_new_module():
-                self.reporter.error(
-                    path=self.object_path,
-                    code='module-invalid-version-added',
-                    msg='version_added is not a valid version number: %r. Error: %s' % (version_added, e)
-                )
+            version_added = version_added_raw or '0.0'
+            if self._is_new_module() or version_added != 'historical':
+                # already reported during schema validation, except:
+                if version_added == 'historical':
+                    self.reporter.error(
+                        path=self.object_path,
+                        code='module-invalid-version-added',
+                        msg='version_added is not a valid version number: %r. Error: %s' % (version_added, e)
+                    )
                 return
 
         if existing_doc and str(version_added_raw) != str(existing_doc.get('version_added')):
@@ -2076,13 +2078,7 @@ def _check_for_new_args(self, doc):
                     str(details.get('version_added', '0.0')),
                     collection_name=collection_name)
             except ValueError as e:
-                self.reporter.error(
-                    path=self.object_path,
-                    code='option-invalid-version-added',
-                    msg=('version_added for option (%s) is not a valid '
-                         'version for %s. Currently %r. Error: %s' %
-                         (option, collection_name, details.get('version_added', '0.0'), e))
-                )
+                # already reported during schema validation
                 continue
 
             if collection_name != self.collection_name:

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

@@ -9,6 +9,7 @@
 import re
 
 from distutils.version import StrictVersion
+from functools import partial
 
 from voluptuous import ALLOW_EXTRA, PREVENT_EXTRA, All, Any, Invalid, Length, Required, Schema, Self, ValueInvalid
 from ansible.module_utils.six import string_types
@@ -226,22 +227,24 @@ def ansible_module_kwargs_schema(for_collection):
 ))
 
 
-def version_added(v):
+def version_added(v, error_code='version-added-invalid', accept_historical=False):
     if 'version_added' in v:
         version_added = v.get('version_added')
         if isinstance(version_added, string_types):
             # If it is not a string, schema validation will have already complained
             # - or we have a float and we are in ansible/ansible, in which case we're
             # also happy.
             if v.get('version_added_collection') == 'ansible.builtin':
+                if version_added == 'historical' and accept_historical:
+                    return v
                 try:
                     version = StrictVersion()
                     version.parse(version_added)
                 except ValueError as exc:
                     raise _add_ansible_error_code(
                         Invalid('version_added (%r) is not a valid ansible-base version: '
                                 '%s' % (version_added, exc)),
-                        error_code='deprecation-either-date-or-version')
+                        error_code=error_code)
             else:
                 try:
                     version = SemanticVersion()
@@ -251,7 +254,7 @@ def version_added(v):
                         Invalid('version_added (%r) is not a valid collection version '
                                 '(see specification at https://semver.org/): '
                                 '%s' % (version_added, exc)),
-                        error_code='deprecation-either-date-or-version')
+                        error_code=error_code)
     elif 'version_added_collection' in v:
         # Must have been manual intervention, since version_added_collection is only
         # added automatically when version_added is present
@@ -304,7 +307,7 @@ def list_dict_option_schema(for_collection):
     option_version_added = Schema(
         All({
             'suboptions': Any(None, *[{str_type: Self} for str_type in string_types]),
-        }, version_added),
+        }, partial(version_added, error_code='option-invalid-version-added')),
         extra=ALLOW_EXTRA
     )
 
@@ -343,7 +346,7 @@ def return_schema(for_collection):
                 }
             ),
             Schema(return_contains),
-            Schema(version_added),
+            Schema(partial(version_added, error_code='option-invalid-version-added')),
         ),
         Schema(type(None)),
     )
@@ -371,7 +374,7 @@ def return_schema(for_collection):
                 }
             ),
             Schema({any_string_types: return_contains}),
-            Schema({any_string_types: version_added}),
+            Schema({any_string_types: partial(version_added, error_code='option-invalid-version-added')}),
         ),
         Schema(type(None)),
     )
@@ -460,8 +463,13 @@ def doc_schema(module_name, for_collection=False, deprecated_module=False):
 
         doc_schema_dict.update(deprecation_required_scheme)
     return Schema(
-        doc_schema_dict,
-        extra=PREVENT_EXTRA
+        All(
+            Schema(
+                doc_schema_dict,
+                extra=PREVENT_EXTRA
+            ),
+            partial(version_added, error_code='module-invalid-version-added', accept_historical=not for_collection),
+        )
     )