Add a new check to ensure that module arguments are valid python identifiers (#53278)

* Add a new check to ensure that module arguments are valid python identifiers

* Move the check up higher, out of the if docs block

* Skip validate-modules on py2

* Remove errant alias

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

@@ -128,7 +128,7 @@ Errors
   333       ``ANSIBLE_METADATA.status`` of deprecated or removed can't include other statuses
   334       ``ANSIBLE_METADATA`` cannot be changed in a point release for a stable branch
   335       argument_spec implies type="str" but documentation defines it as different data type
-
+  336       argument in argument_spec is not a valid python identifier
   ..
 ---------   -------------------
   **4xx**   **Syntax**

lib/ansible/modules/cloud/docker/docker_login.py

@@ -64,7 +64,6 @@ options:
     type: path
     default: ~/.docker/config.json
     aliases:
-      - self.config_path
       - dockercfg_path
   state:
     version_added: '2.3'
@@ -299,7 +298,7 @@ def main():
         email=dict(type='str'),
         reauthorize=dict(type='bool', default=False, aliases=['reauth']),
         state=dict(type='str', default='present', choices=['present', 'absent']),
-        config_path=dict(type='path', default='~/.docker/config.json', aliases=['self.config_path', 'dockercfg_path']),
+        config_path=dict(type='path', default='~/.docker/config.json', aliases=['dockercfg_path']),
     )
 
     required_if = [

test/runner/lib/sanity/validate_modules.py

@@ -36,6 +36,11 @@ from lib.test import (
 VALIDATE_SKIP_PATH = 'test/sanity/validate-modules/skip.txt'
 VALIDATE_IGNORE_PATH = 'test/sanity/validate-modules/ignore.txt'
 
+UNSUPPORTED_PYTHON_VERSIONS = (
+    '2.6',
+    '2.7',
+)
+
 
 class ValidateModulesTest(SanitySingleVersion):
     """Sanity test using validate-modules."""
@@ -45,6 +50,10 @@ class ValidateModulesTest(SanitySingleVersion):
         :type targets: SanityTargets
         :rtype: TestResult
         """
+        if args.python_version in UNSUPPORTED_PYTHON_VERSIONS:
+            display.warning('Skipping validate-modules on unsupported Python version %s.' % args.python_version)
+            return SanitySkipped(self.name)
+
         skip_paths = read_lines_without_comments(VALIDATE_SKIP_PATH)
         skip_paths_set = set(skip_paths)
 

test/sanity/validate-modules/ignore.txt

@@ -123,10 +123,10 @@ lib/ansible/modules/cloud/digital_ocean/digital_ocean_sshkey.py E322
 lib/ansible/modules/cloud/digital_ocean/digital_ocean_sshkey.py E324
 lib/ansible/modules/cloud/dimensiondata/dimensiondata_network.py E326
 lib/ansible/modules/cloud/dimensiondata/dimensiondata_vlan.py E326
+lib/ansible/modules/cloud/google/_gce.py E326
 lib/ansible/modules/cloud/google/gc_storage.py E322
 lib/ansible/modules/cloud/google/gc_storage.py E324
 lib/ansible/modules/cloud/google/gc_storage.py E326
-lib/ansible/modules/cloud/google/_gce.py E326
 lib/ansible/modules/cloud/google/gce_eip.py E322
 lib/ansible/modules/cloud/google/gce_instance_template.py E322
 lib/ansible/modules/cloud/google/gce_instance_template.py E324
@@ -392,6 +392,7 @@ lib/ansible/modules/files/lineinfile.py E323
 lib/ansible/modules/files/lineinfile.py E324
 lib/ansible/modules/files/lineinfile.py E326
 lib/ansible/modules/files/replace.py E323
+lib/ansible/modules/files/stat.py E336
 lib/ansible/modules/files/synchronize.py E322
 lib/ansible/modules/files/synchronize.py E323
 lib/ansible/modules/files/synchronize.py E324
@@ -689,6 +690,7 @@ lib/ansible/modules/notification/sendgrid.py E322
 lib/ansible/modules/notification/slack.py E324
 lib/ansible/modules/notification/snow_record.py E317
 lib/ansible/modules/packaging/language/bundler.py E324
+lib/ansible/modules/packaging/language/composer.py E336
 lib/ansible/modules/packaging/language/easy_install.py E324
 lib/ansible/modules/packaging/language/maven_artifact.py E324
 lib/ansible/modules/packaging/language/pear.py E322
@@ -699,25 +701,33 @@ lib/ansible/modules/packaging/os/apk.py E326
 lib/ansible/modules/packaging/os/apt.py E322
 lib/ansible/modules/packaging/os/apt.py E324
 lib/ansible/modules/packaging/os/apt.py E326
+lib/ansible/modules/packaging/os/apt.py E336
 lib/ansible/modules/packaging/os/apt_key.py E322
 lib/ansible/modules/packaging/os/apt_repository.py E322
 lib/ansible/modules/packaging/os/apt_repository.py E324
+lib/ansible/modules/packaging/os/apt_repository.py E336
 lib/ansible/modules/packaging/os/apt_rpm.py E322
 lib/ansible/modules/packaging/os/apt_rpm.py E324
 lib/ansible/modules/packaging/os/apt_rpm.py E326
+lib/ansible/modules/packaging/os/apt_rpm.py E336
+lib/ansible/modules/packaging/os/dnf.py E336
 lib/ansible/modules/packaging/os/flatpak.py E210
 lib/ansible/modules/packaging/os/flatpak_remote.py E210
 lib/ansible/modules/packaging/os/homebrew.py E326
+lib/ansible/modules/packaging/os/homebrew.py E336
 lib/ansible/modules/packaging/os/homebrew_cask.py E326
+lib/ansible/modules/packaging/os/homebrew_cask.py E336
 lib/ansible/modules/packaging/os/layman.py E322
 lib/ansible/modules/packaging/os/macports.py E326
 lib/ansible/modules/packaging/os/openbsd_pkg.py E326
 lib/ansible/modules/packaging/os/opkg.py E322
 lib/ansible/modules/packaging/os/opkg.py E324
 lib/ansible/modules/packaging/os/opkg.py E326
+lib/ansible/modules/packaging/os/opkg.py E336
 lib/ansible/modules/packaging/os/package_facts.py E324
 lib/ansible/modules/packaging/os/package_facts.py E326
 lib/ansible/modules/packaging/os/pacman.py E326
+lib/ansible/modules/packaging/os/pacman.py E336
 lib/ansible/modules/packaging/os/pkg5.py E326
 lib/ansible/modules/packaging/os/pkgin.py E322
 lib/ansible/modules/packaging/os/pkgng.py E322
@@ -731,14 +741,18 @@ lib/ansible/modules/packaging/os/rhsm_repository.py E324
 lib/ansible/modules/packaging/os/slackpkg.py E322
 lib/ansible/modules/packaging/os/slackpkg.py E324
 lib/ansible/modules/packaging/os/slackpkg.py E326
+lib/ansible/modules/packaging/os/slackpkg.py E336
 lib/ansible/modules/packaging/os/swdepot.py E322
 lib/ansible/modules/packaging/os/urpmi.py E322
 lib/ansible/modules/packaging/os/urpmi.py E324
 lib/ansible/modules/packaging/os/urpmi.py E326
+lib/ansible/modules/packaging/os/urpmi.py E336
 lib/ansible/modules/packaging/os/xbps.py E322
 lib/ansible/modules/packaging/os/xbps.py E326
+lib/ansible/modules/packaging/os/xbps.py E336
 lib/ansible/modules/packaging/os/yum.py E322
 lib/ansible/modules/packaging/os/yum.py E324
+lib/ansible/modules/packaging/os/yum.py E336
 lib/ansible/modules/packaging/os/yum_repository.py E322
 lib/ansible/modules/packaging/os/yum_repository.py E324
 lib/ansible/modules/packaging/os/zypper.py E326
@@ -774,6 +788,7 @@ lib/ansible/modules/remote_management/ucs/ucs_vnic_template.py E326
 lib/ansible/modules/remote_management/ucs/ucs_vsans.py E322
 lib/ansible/modules/remote_management/ucs/ucs_wwn_pool.py E322
 lib/ansible/modules/remote_management/ucs/ucs_wwn_pool.py E323
+lib/ansible/modules/source_control/github_deploy_key.py E336
 lib/ansible/modules/source_control/github_issue.py E324
 lib/ansible/modules/source_control/github_issue.py E326
 lib/ansible/modules/source_control/subversion.py E322
@@ -785,6 +800,9 @@ lib/ansible/modules/storage/netapp/_na_cdot_volume.py E317
 lib/ansible/modules/storage/netapp/_na_cdot_volume.py E322
 lib/ansible/modules/storage/netapp/_na_cdot_volume.py E324
 lib/ansible/modules/storage/netapp/_sf_volume_manager.py E322
+lib/ansible/modules/storage/netapp/_sf_volume_manager.py E336
+lib/ansible/modules/storage/netapp/na_elementsw_volume.py E336
+lib/ansible/modules/storage/netapp/na_ontap_nfs.py E336
 lib/ansible/modules/storage/netapp/netapp_e_amg.py E322
 lib/ansible/modules/storage/netapp/netapp_e_amg_role.py E322
 lib/ansible/modules/storage/netapp/netapp_e_flashcache.py E322
@@ -814,10 +832,12 @@ lib/ansible/modules/storage/zfs/zfs_facts.py E323
 lib/ansible/modules/storage/zfs/zpool_facts.py E323
 lib/ansible/modules/system/known_hosts.py E324
 lib/ansible/modules/system/puppet.py E322
+lib/ansible/modules/system/puppet.py E336
 lib/ansible/modules/system/runit.py E322
 lib/ansible/modules/system/runit.py E324
 lib/ansible/modules/system/service.py E210
 lib/ansible/modules/system/service.py E323
+lib/ansible/modules/system/systemd.py E336
 lib/ansible/modules/system/user.py E210
 lib/ansible/modules/system/user.py E324
 lib/ansible/modules/system/user.py E327

test/sanity/validate-modules/main.py

@@ -1301,6 +1301,14 @@ class ModuleValidator(Validator):
                          "but documentation defines choices as (%r)" % (arg, arg_choices, doc_choices))
                 )
 
+        for arg in args_from_argspec:
+            if not str(arg).isidentifier():
+                self.reporter.error(
+                    path=self.object_path,
+                    code=336,
+                    msg="Argument '%s' is not a valid python identifier" % arg
+                )
+
         if docs:
             file_common_arguments = set()
             for arg, data in FILE_COMMON_ARGUMENTS.items():