Check loader modules __virtual__ and salt/modules/*.py with pre-commit

Fixes #58537

.github/workflows/pre-commit.yml

@@ -47,7 +47,7 @@ jobs:
     - name: Check ALL Files On Branch
       if: github.event_name != 'pull_request'
       env:
-        SKIP: lint-salt,lint-tests,pyupgrade,remove-import-headers,rstcheck
+        SKIP: lint-salt,lint-tests,pyupgrade,remove-import-headers,rstcheck,loader-check-virtual,loader-check-module-docstrings
       run: |
         pre-commit run --show-diff-on-failure --color=always --all-files
 

.pre-commit-config.yaml

@@ -1215,6 +1215,40 @@ repos:
           - jinja2
           - msgpack
 
+  - repo: https://github.com/saltstack/invoke-pre-commit
+    rev: v1.5.0
+    hooks:
+      - id: invoke
+        alias: loader-check-virtual
+        name: Check modules __virtual__
+        files: salt/.*\.py$
+        exclude: >
+            (?x)^(
+                templates/.*|
+                salt/ext/.*|
+            )$
+        args:
+          - loader.check-virtual
+        additional_dependencies:
+          - blessings
+          - pyyaml
+          - distro
+          - jinja2
+          - msgpack
+
+  - repo: https://github.com/saltstack/invoke-pre-commit
+    rev: v1.5.0
+    hooks:
+      - id: invoke
+        alias: loader-check-module-docstrings
+        name: Check salt/modules/*.py docstrings
+        files: salt/modules/.*\.py$
+        args:
+          - loader.check-module-docstrings
+        additional_dependencies:
+          - blessings
+          - pyyaml
+
   - repo: https://github.com/saltstack/mirrors-nox
     rev: v2020.8.22
     hooks:

cicd/jenkins/pr-pre-commit

@@ -5,7 +5,7 @@ if (env.CHANGE_ID) {
     pre_commit_skips = ''
 } else {
     // This is a branch build
-    pre_commit_skips = 'pyupgrade,remove-import-headers,rstcheck'
+    pre_commit_skips = 'pyupgrade,remove-import-headers,rstcheck,loader-check-virtual,loader-check-module-docstrings'
 }
 
 runPreCommit(

tasks/loader.py

@@ -7,6 +7,7 @@
 
 import ast
 import pathlib
+import re
 
 from invoke import task  # pylint: disable=3rd-party-module-not-gated
 from tasks import utils
@@ -86,7 +87,7 @@ def check_virtual(ctx, files):
     exitcode = 0
     for path in _files:
         strpath = str(path)
-        if strpath.endswith("__init__.py"):
+        if path.name == "__init__.py":
             continue
         for loader in salt_loaders:
             try:
@@ -95,7 +96,7 @@ def check_virtual(ctx, files):
             except ValueError:
                 # Path doesn't start with the loader path, carry on
                 continue
-        module = ast.parse(path.read_text(), filename=strpath)
+        module = ast.parse(path.read_text(), filename=str(path))
         found_virtual_func = False
         for funcdef in [
             node for node in module.body if isinstance(node, ast.FunctionDef)
@@ -139,3 +140,102 @@ def check_virtual(ctx, files):
     if exitcode:
         utils.error("Found {} errors", errors)
     utils.exit_invoke(exitcode)
+
+
+@task(iterable=["files"], positional=["files"])
+def check_module_docstrings(ctx, files, check_proper_formatting=False):
+    # CD into Salt's repo root directory
+    ctx.cd(CODE_DIR)
+
+    # Unfortunately invoke does not support nargs.
+    # We migth have been passed --files="foo.py bar.py"
+    # Turn that into a list of paths
+    _files = []
+    for path in files:
+        if not path:
+            continue
+        _files.extend(path.split())
+    if not _files:
+        _files = (SALT_CODE_DIR / "modules").rglob("*.py")
+    else:
+        _files = [pathlib.Path(fname) for fname in _files]
+
+    _files = [path.resolve() for path in _files]
+
+    _files_to_check = []
+    salt_modules_path = SALT_CODE_DIR / "modules"
+    for path in _files:
+        try:
+            relpath = path.relative_to(salt_modules_path)
+            if str(relpath.parent) != ".":
+                # We don't want to check nested packages
+                continue
+            _files_to_check.append(path)
+        except ValueError:
+            # This is not a salt/modules/*.py module. Carry on.
+            continue
+
+    errors = 0
+    exitcode = 0
+    cli_example_regex = re.compile(r"CLI Example(?:s)?:")
+    good_cli_example_regex = re.compile(
+        r"CLI Example(?:s)?:\n\n.. code-block:: bash\n\n    salt (.*) '*'", re.MULTILINE
+    )
+    for path in _files_to_check:
+        if path.name == "__init__.py":
+            continue
+        module = ast.parse(path.read_text(), filename=str(path))
+        for funcdef in [
+            node for node in module.body if isinstance(node, ast.FunctionDef)
+        ]:
+            if funcdef.name in ("__virtual__", "__init__"):
+                # We're not interested in these
+                continue
+            if funcdef.name.startswith("_"):
+                # We're not interested in private functions
+                continue
+            docstring = ast.get_docstring(funcdef)
+            if not docstring:
+                errors += 1
+                exitcode = 1
+                utils.error(
+                    "The function {!r} on {} does not have a docstring",
+                    funcdef.name,
+                    path.relative_to(CODE_DIR),
+                )
+                continue
+
+            if not cli_example_regex.search(docstring):
+                errors += 1
+                exitcode = 1
+                utils.error(
+                    "The function {!r} on {} does not have a 'CLI Example:' in it's docstring",
+                    funcdef.name,
+                    path.relative_to(CODE_DIR),
+                )
+                continue
+
+            if check_proper_formatting is False:
+                continue
+
+            # By now we now this function has a docstring and it has a CLI Example section
+            # Let's now check if it's properly formatted
+            if not good_cli_example_regex.search(docstring):
+                errors += 1
+                exitcode = 1
+                utils.error(
+                    "The function {!r} on {} does not have a proper 'CLI Example:' section in "
+                    "it's docstring. The proper format is:\n"
+                    "CLI Example:\n"
+                    "\n"
+                    ".. code-block:: bash\n"
+                    "\n"
+                    "    salt '*' <insert example here>\n",
+                    funcdef.name,
+                    path.relative_to(CODE_DIR),
+                )
+                continue
+
+    if exitcode:
+        utils.error("Found {} errors", errors)
+    utils.exit_invoke(exitcode)