Change validate-modules for removed modules

Removed modules now don't have documentation.  Need to account for that
when checking them in validte-modules

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

@@ -106,7 +106,9 @@ Errors
   316       Invalid ``ANSIBLE_METADATA`` schema
   317       option is marked as required but specifies a default.
             Arguments with a default should not be marked as required
-  318       Module deprecated, but DOCUMENTATION.deprecated is missing
+  318       Module marked as deprecated or removed in at least one of the filename, its metadata, or
+            in DOCUMENTATION (setting DOCUMENTATION.deprecated for deprecation or removing all
+            documentation for removed) but not in all three places.
   319       ``RETURN`` fragments missing  or invalid
   320       ``DOCUMENTATION.options`` must be a dictionary/hash when used
   321       ``Exception`` attempting to import module for ``argument_spec`` introspection
@@ -121,6 +123,8 @@ Errors
   330       Choices value from the argument_spec is not compatible with type defined in the argument_spec
   331       argument in argument_spec must be a dictionary/hash when used
   332       ``AnsibleModule`` schema validation error
+  333       ``ANSIBLE_METADATA.status`` of deprecated or removed can't include other statuses
+
   ..
 ---------   -------------------
   **4xx**   **Syntax**

test/sanity/validate-modules/main.py

@@ -831,8 +831,79 @@ class ModuleValidator(Validator):
 
     def _validate_docs(self):
         doc_info = self._get_docs()
-        deprecated = False
         doc = None
+        documentation_exists = False
+        examples_exist = False
+        returns_exist = False
+        # We have three ways of marking deprecated/removed files.  Have to check each one
+        # individually and then make sure they all agree
+        filename_deprecated_or_removed = False
+        deprecated = False
+        removed = False
+        doc_deprecated = None  # doc legally might not exist
+
+        if self.object_name.startswith('_') and not os.path.islink(self.object_path):
+            filename_deprecated_or_removed = True
+
+        # Have to check the metadata first so that we know if the module is removed or deprecated
+        if not bool(doc_info['ANSIBLE_METADATA']['value']):
+            self.reporter.error(
+                path=self.object_path,
+                code=314,
+                msg='No ANSIBLE_METADATA provided'
+            )
+        else:
+            metadata = None
+            if isinstance(doc_info['ANSIBLE_METADATA']['value'], ast.Dict):
+                metadata = ast.literal_eval(
+                    doc_info['ANSIBLE_METADATA']['value']
+                )
+            else:
+                # ANSIBLE_METADATA doesn't properly support YAML
+                # we should consider removing it from the spec
+                # Below code kept, incase we change our minds
+
+                # metadata, errors, traces = parse_yaml(
+                #     doc_info['ANSIBLE_METADATA']['value'].s,
+                #     doc_info['ANSIBLE_METADATA']['lineno'],
+                #     self.name, 'ANSIBLE_METADATA'
+                # )
+                # for error in errors:
+                #     self.reporter.error(
+                #         path=self.object_path,
+                #         code=315,
+                #         **error
+                #     )
+                # for trace in traces:
+                #     self.reporter.trace(
+                #         path=self.object_path,
+                #         tracebk=trace
+                #     )
+
+                self.reporter.error(
+                    path=self.object_path,
+                    code=315,
+                    msg='ANSIBLE_METADATA was not provided as a dict, YAML not supported'
+                )
+
+            if metadata:
+                self._validate_docs_schema(metadata, metadata_1_1_schema(),
+                                           'ANSIBLE_METADATA', 316)
+                # We could validate these via the schema if we knew what the values are ahead of
+                # time.  We can figure that out for deprecated but we can't for removed.  Only the
+                # metadata has that information.
+                if 'removed' in metadata['status']:
+                    removed = True
+                if 'deprecated' in metadata['status']:
+                    deprecated = True
+                if (deprecated or removed) and len(metadata['status']) > 1:
+                    self.reporter.error(
+                        path=self.object_path,
+                        code=333,
+                        msg='ANSIBLE_METADATA.status  must be exactly one of "deprecated" or "removed"'
+                    )
+
+        if not removed:
             if not bool(doc_info['DOCUMENTATION']['value']):
                 self.reporter.error(
                     path=self.object_path,
@@ -840,6 +911,7 @@ class ModuleValidator(Validator):
                     msg='No DOCUMENTATION provided'
                 )
             else:
+                documentation_exists = True
                 doc, errors, traces = parse_yaml(
                     doc_info['DOCUMENTATION']['value'],
                     doc_info['DOCUMENTATION']['lineno'],
@@ -885,14 +957,10 @@ class ModuleValidator(Validator):
                             msg='DOCUMENTATION.options must be a dictionary/hash when used',
                         )
 
-                if self.object_name.startswith('_') and not os.path.islink(self.object_path):
+                    if 'deprecated' in doc and doc.get('deprecated'):
-                    deprecated = True
+                        doc_deprecated = True
-                    if 'deprecated' not in doc or not doc.get('deprecated'):
+                    else:
-                        self.reporter.error(
+                        doc_deprecated = False
-                            path=self.object_path,
-                            code=318,
-                            msg='Module deprecated, but DOCUMENTATION.deprecated is missing'
-                        )
 
                     if os.path.islink(self.object_path):
                         # This module has an alias, which we can tell as it's a symlink
@@ -912,6 +980,7 @@ class ModuleValidator(Validator):
                     msg='No EXAMPLES provided'
                 )
             else:
+                examples_exists = True
                 _, errors, traces = parse_yaml(doc_info['EXAMPLES']['value'],
                                                doc_info['EXAMPLES']['lineno'],
                                                self.name, 'EXAMPLES', load_all=True)
@@ -928,6 +997,7 @@ class ModuleValidator(Validator):
                     )
 
             if not bool(doc_info['RETURN']['value']):
+                returns_exists = True
                 if self._is_new_module():
                     self.reporter.error(
                         path=self.object_path,
@@ -960,50 +1030,25 @@ class ModuleValidator(Validator):
                         tracebk=trace
                     )
 
-        if not bool(doc_info['ANSIBLE_METADATA']['value']):
+        # Check for mismatched deprecation
-            self.reporter.error(
+        mismatched_deprecation = True
-                path=self.object_path,
+        if not (filename_deprecated_or_removed or removed or deprecated or doc_deprecated):
-                code=314,
+            mismatched_deprecation = False
-                msg='No ANSIBLE_METADATA provided'
-            )
-        else:
-            metadata = None
-            if isinstance(doc_info['ANSIBLE_METADATA']['value'], ast.Dict):
-                metadata = ast.literal_eval(
-                    doc_info['ANSIBLE_METADATA']['value']
-                )
         else:
-                # ANSIBLE_METADATA doesn't properly support YAML
+            if (filename_deprecated_or_removed and deprecated and doc_deprecated):
-                # we should consider removing it from the spec
+                mismatched_deprecation = False
-                # Below code kept, incase we change our minds
+            if (filename_deprecated_or_removed and removed and not (documentation_exists or examples_exist or returns_exist)):
-
+                mismatched_deprecation = False
-                # metadata, errors, traces = parse_yaml(
-                #     doc_info['ANSIBLE_METADATA']['value'].s,
-                #     doc_info['ANSIBLE_METADATA']['lineno'],
-                #     self.name, 'ANSIBLE_METADATA'
-                # )
-                # for error in errors:
-                #     self.reporter.error(
-                #         path=self.object_path,
-                #         code=315,
-                #         **error
-                #     )
-                # for trace in traces:
-                #     self.reporter.trace(
-                #         path=self.object_path,
-                #         tracebk=trace
-                #     )
 
+        if mismatched_deprecation:
             self.reporter.error(
                 path=self.object_path,
-                    code=315,
+                code=318,
-                    msg='ANSIBLE_METADATA was not provided as a dict, YAML not supported'
+                msg='Module deprecation/removed must agree in Metadata, by prepending filename with'
+                    ' "_", and setting DOCUMENTATION.deprecated for deprecation or by removing all'
+                    ' documentation for removed'
             )
 
-            if metadata:
-                self._validate_docs_schema(metadata, metadata_1_1_schema(deprecated),
-                                           'ANSIBLE_METADATA', 316)
-
         return doc_info, doc
 
     def _check_version_added(self, doc):
@@ -1368,22 +1413,23 @@ class ModuleValidator(Validator):
                 )
             return
 
-        end_of_deprecation_should_be_docs_only = False
+        end_of_deprecation_should_be_removed_only = False
         if self._python_module():
             doc_info, docs = self._validate_docs()
 
             # See if current version => deprecated.removed_in, ie, should be docs only
-            if docs and 'deprecated' in docs and docs['deprecated'] is not None:
+            if 'removed' in ast.literal_eval(doc_info['ANSIBLE_METADATA']['value'])['status']:
+                end_of_deprecation_should_be_removed_only = True
+            elif docs and 'deprecated' in docs and docs['deprecated'] is not None:
                 try:
                     removed_in = StrictVersion(str(docs.get('deprecated')['removed_in']))
                 except ValueError:
-                    end_of_deprecation_should_be_docs_only = False
+                    end_of_deprecation_should_be_removed_only = False
                 else:
                     strict_ansible_version = StrictVersion('.'.join(ansible_version.split('.')[:2]))
-                    end_of_deprecation_should_be_docs_only = strict_ansible_version >= removed_in
+                    end_of_deprecation_should_be_removed_only = strict_ansible_version >= removed_in
-                    # FIXME if +2 then file should be empty? - maybe add this only in the future
 
-        if self._python_module() and not self._just_docs() and not end_of_deprecation_should_be_docs_only:
+        if self._python_module() and not self._just_docs() and not end_of_deprecation_should_be_removed_only:
             self._validate_ansible_module_call(docs)
             self._check_for_sys_exit()
             self._find_blacklist_imports()
@@ -1400,14 +1446,17 @@ class ModuleValidator(Validator):
             self._find_ps_docs_py_file()
 
         self._check_gpl3_header()
-        if not self._just_docs() and not end_of_deprecation_should_be_docs_only:
+        if not self._just_docs() and not end_of_deprecation_should_be_removed_only:
             self._check_interpreter(powershell=self._powershell_module())
             self._check_type_instead_of_isinstance(
                 powershell=self._powershell_module()
             )
-        if end_of_deprecation_should_be_docs_only:
+        if end_of_deprecation_should_be_removed_only:
             # Ensure that `if __name__ == '__main__':` calls `removed_module()` which ensure that the module has no code in
             main = self._find_main_call('removed_module')
+            # FIXME: Ensure that the version in the call to removed_module is less than +2.
+            # Otherwise it's time to remove the file (This may need to be done in another test to
+            # avoid breaking whenever the Ansible version bumps)
 
 
 class PythonPackageValidator(Validator):

test/sanity/validate-modules/schema.py

@@ -179,10 +179,8 @@ def metadata_1_0_schema(deprecated):
     )
 
 
-def metadata_1_1_schema(deprecated):
+def metadata_1_1_schema():
     valid_status = Any('stableinterface', 'preview', 'deprecated', 'removed')
-    if deprecated:
-        valid_status = Any('deprecated')
 
     return Schema(
         {