Check for imports to be between last doc line and first callable (#21863)

* Check for imports to be between last doc line and first callable

* Small readme update

test/sanity/validate-modules/README.rst

@@ -61,6 +61,8 @@ Errors
 +---------+--------------------------------------------------------------------------------------------------------------------------------------------+
 | 106     | Import found before documentation variables. All imports must appear below ``DOCUMENTATION``/``EXAMPLES``/``RETURN``/``ANSIBLE_METADATA``  |
 +---------+--------------------------------------------------------------------------------------------------------------------------------------------+
+| 107     | Imports should be directly below ``DOCUMENTATION``/``EXAMPLES``/``RETURN``/``ANSIBLE_METADATA``                                            |
++---------+--------------------------------------------------------------------------------------------------------------------------------------------+
 +---------+--------------------------------------------------------------------------------------------------------------------------------------------+
 | **2xx** | **Imports**                                                                                                                                |
 +---------+--------------------------------------------------------------------------------------------------------------------------------------------+
@@ -140,6 +142,11 @@ Warnings
 +---------+--------------------------------------------------------------------------------------------------------------------------------------------+
 | code    | sample message                                                                                                                             |
 +=========+============================================================================================================================================+
+| **1xx** | **Locations**                                                                                                                              |
++---------+--------------------------------------------------------------------------------------------------------------------------------------------+
+| 107     | Imports should be directly below ``DOCUMENTATION``/``EXAMPLES``/``RETURN``/``ANSIBLE_METADATA`` for legacy modules                         |
++---------+--------------------------------------------------------------------------------------------------------------------------------------------+
++---------+--------------------------------------------------------------------------------------------------------------------------------------------+
 | **2xx** | **Imports**                                                                                                                                |
 +---------+--------------------------------------------------------------------------------------------------------------------------------------------+
 | 208     | ``module_utils`` imports should import specific components for legacy module, not ``*``                                                    |

test/sanity/validate-modules/validate-modules

@@ -393,6 +393,14 @@ class ModuleValidator(Validator):
 
         return linenos
 
+    def _get_first_callable(self):
+        linenos = []
+        for child in self.ast.body:
+            if isinstance(child, (ast.FunctionDef, ast.ClassDef)):
+                linenos.append(child.lineno)
+
+        return min(linenos)
+
     def _find_main_call(self):
         lineno = False
         if_bodies = []
@@ -445,38 +453,58 @@ class ModuleValidator(Validator):
                     'Found Try/Except block without HAS_ assginment'
                 ))
 
-    def _ensure_imports_below_docs(self, doc_info):
+    def _ensure_imports_below_docs(self, doc_info, first_callable):
-        doc_lines = [doc_info[key]['lineno'] for key in doc_info]
+        min_doc_line = min(
+            [doc_info[key]['lineno'] for key in doc_info if doc_info[key]['lineno']]
+        )
+        max_doc_line = max(
+            [doc_info[key]['end_lineno'] for key in doc_info if doc_info[key]['end_lineno']]
+        )
+
+        import_lines = []
 
         for child in self.ast.body:
             if isinstance(child, (ast.Import, ast.ImportFrom)):
-                for lineno in doc_lines:
+                import_lines.append(child.lineno)
-                    if child.lineno < lineno:
+                if child.lineno < min_doc_line:
-                        self.errors.append((
+                    self.errors.append((
-                            106,
+                        106,
-                            ('Import found before documentation variables. '
+                        ('Import found before documentation variables. '
-                             'All imports must appear below '
+                         'All imports must appear below '
-                             'DOCUMENTATION/EXAMPLES/RETURN/ANSIBLE_METADATA. '
+                         'DOCUMENTATION/EXAMPLES/RETURN/ANSIBLE_METADATA. '
-                             'line %d' % (child.lineno,))
+                         'line %d' % (child.lineno,))
-                        ))
+                    ))
-                        break
+                    break
             elif isinstance(child, ast.TryExcept):
                 bodies = child.body
                 for handler in child.handlers:
                     bodies.extend(handler.body)
                 for grandchild in bodies:
                     if isinstance(grandchild, (ast.Import, ast.ImportFrom)):
-                        for lineno in doc_lines:
+                        import_lines.append(grandchild.lineno)
-                            if child.lineno < lineno:
+                        if grandchild.lineno < min_doc_line:
-                                self.errors.append((
+                            self.errors.append((
-                                    106,
+                                106,
-                                    ('Import found before documentation '
+                                ('Import found before documentation '
-                                     'variables. All imports must appear below '
+                                 'variables. All imports must appear below '
-                                     'DOCUMENTATION/EXAMPLES/RETURN/'
+                                 'DOCUMENTATION/EXAMPLES/RETURN/'
-                                     'ANSIBLE_METADATA. line %d' %
+                                 'ANSIBLE_METADATA. line %d' %
-                                     (child.lineno,))
+                                 (child.lineno,))
-                                ))
+                            ))
-                                break
+                            break
+
+        for import_line in import_lines:
+            if not (max_doc_line < import_line < first_callable):
+                msg = (
+                    107,
+                    ('Imports should be directly below DOCUMENTATION/EXAMPLES/'
+                     'RETURN/ANSIBLE_METADATA. line %d' % import_line)
+                )
+                if self._is_new_module():
+                    self.errors.append(msg)
+                else:
+                    self.warnings.append(msg)
+
 
     def _find_ps_replacers(self):
         if 'WANT_JSON' not in self.text:
@@ -496,19 +524,23 @@ class ModuleValidator(Validator):
         docs = {
             'DOCUMENTATION': {
                 'value': None,
-                'lineno': 0
+                'lineno': 0,
+                'end_lineno': 0,
             },
             'EXAMPLES': {
                 'value': None,
-                'lineno': 0
+                'lineno': 0,
+                'end_lineno': 0,
             },
             'RETURN': {
                 'value': None,
-                'lineno': 0
+                'lineno': 0,
+                'end_lineno': 0,
             },
             'ANSIBLE_METADATA': {
                 'value': None,
-                'lineno': 0
+                'lineno': 0,
+                'end_lineno': 0,
             }
         }
         for child in self.ast.body:
@@ -517,15 +549,32 @@ class ModuleValidator(Validator):
                     if grandchild.id == 'DOCUMENTATION':
                         docs['DOCUMENTATION']['value'] = child.value.s
                         docs['DOCUMENTATION']['lineno'] = child.lineno
+                        docs['DOCUMENTATION']['end_lineno'] = (
+                            child.lineno + len(child.value.s.splitlines())
+                        )
                     elif grandchild.id == 'EXAMPLES':
-                        docs['EXAMPLES']['value'] = child.value.s[1:]
+                        docs['EXAMPLES']['value'] = child.value.s
                         docs['EXAMPLES']['lineno'] = child.lineno
+                        docs['EXAMPLES']['end_lineno'] = (
+                            child.lineno + len(child.value.s.splitlines())
+                        )
                     elif grandchild.id == 'RETURN':
                         docs['RETURN']['value'] = child.value.s
                         docs['RETURN']['lineno'] = child.lineno
+                        docs['RETURN']['end_lineno'] = (
+                            child.lineno + len(child.value.s.splitlines())
+                        )
                     elif grandchild.id == 'ANSIBLE_METADATA':
                         docs['ANSIBLE_METADATA']['value'] = child.value
                         docs['ANSIBLE_METADATA']['lineno'] = child.lineno
+                        try:
+                            docs['ANSIBLE_METADATA']['end_lineno'] = (
+                                child.lineno + len(child.value.s.splitlines())
+                            )
+                        except AttributeError:
+                            docs['ANSIBLE_METADATA']['end_lineno'] = (
+                                child.value.values[-1].lineno
+                            )
 
         return docs
 
@@ -796,7 +845,8 @@ class ModuleValidator(Validator):
             self._find_module_utils(main)
             self._find_has_import()
             self._check_for_tabs()
-            self._ensure_imports_below_docs(doc_info)
+            first_callable = self._get_first_callable()
+            self._ensure_imports_below_docs(doc_info, first_callable)
 
         if self._powershell_module():
             self._find_ps_replacers()