Add a check for type() instead of isinstance() (#18439)

test/sanity/validate-modules/README.rst

@@ -55,6 +55,7 @@ Errors
 #. New arguments have the correct ``version_added``
 #. Modules should not import requests, instead use ``ansible.module_utils.urls``
 #. Missing ``RETURN`` for new modules
+#. Use of ``type()`` for type comparison instead of ``isinstance()``
 
 Warnings
 ^^^^^^^^

test/sanity/validate-modules/test_validate_modules_regex.py

@@ -0,0 +1,65 @@
+#!/usr/bin/env python
+
+# This is a standalone test for the regex inside validate-modules
+# It is not suitable to add to the make tests target because the
+# file under test is outside the test's sys.path AND has a hyphen
+# in the name making it unimportable.
+#
+# To execute this by hand:
+#   1) cd <checkoutdir>
+#   2) source hacking/env-setup
+#   3) PYTHONPATH=./lib nosetests -d -w test -v --nocapture sanity/validate-modules
+
+import os
+import re
+from ansible.compat.tests import unittest
+
+#TYPE_REGEX = re.compile(r'.*\stype\(.*')
+#TYPE_REGEX = re.compile(r'.*(if|or)\stype\(.*')
+#TYPE_REGEX = re.compile(r'.*(if|or)(\s+.*|\s+)type\(.*')
+#TYPE_REGEX = re.compile(r'.*(if|or)(\s+.*|\s+)type\(.*')
+#TYPE_REGEX = re.compile(r'.*(if|\sor)(\s+.*|\s+)type\(.*')
+#TYPE_REGEX =  re.compile(r'.*(if|\sor)(\s+.*|\s+)(?<!_)type\(.*')
+TYPE_REGEX =  re.compile(r'.*(if|or)(\s+.*|\s+)(?<!_)(?<!str\()type\(.*')
+
+class TestValidateModulesRegex(unittest.TestCase):
+
+    def test_type_regex(self):
+        # each of these examples needs to be matched or not matched
+        checks = [
+            ['if type(foo) is Bar', True],
+            ['if Bar is type(foo)', True],
+            ['if type(foo) is not Bar', True],
+            ['if Bar is not type(foo)', True],
+            ['if type(foo) == Bar', True],
+            ['if Bar == type(foo)', True],
+            ['if type(foo)==Bar', True],
+            ['if Bar==type(foo)', True],
+            ['if type(foo) != Bar', True],
+            ['if Bar != type(foo)', True],
+            ['if type(foo)!=Bar', True],
+            ['if Bar!=type(foo)', True],
+            ['if foo or type(bar) != Bar', True],
+            ['x = type(foo)', False],
+            ["error = err.message + ' ' + str(err) + ' - ' + str(type(err))", False],
+            #cloud/amazon/ec2_group.py
+            ["module.fail_json(msg='Invalid rule parameter type [%s].' % type(rule))", False],
+            #files/patch.py
+            ["p = type('Params', (), module.params)", False], #files/patch.py
+            #system/osx_defaults.py
+            ["if self.current_value is not None and not isinstance(self.current_value, type(self.value)):", True],
+            #system/osx_defaults.py
+            ['raise OSXDefaultsException("Type mismatch. Type in defaults: " + type(self.current_value).__name__)', False],
+            #network/nxos/nxos_interface.py
+            ["if get_interface_type(interface) == 'svi':", False],
+        ]
+        for idc,check in enumerate(checks):
+            cstring = check[0]
+            cexpected = check[1]
+
+            match = TYPE_REGEX.match(cstring)
+            if cexpected and not match:
+                assert False, "%s should have matched" % cstring
+            elif not cexpected and match:
+                assert False, "%s should not have matched" % cstring
+

test/sanity/validate-modules/validate-modules

@@ -46,6 +46,7 @@ import yaml.reader
 
 BLACKLIST_DIRS = frozenset(('.git', 'test', '.github', '.idea'))
 INDENT_REGEX = re.compile(r'([\t]*)')
+TYPE_REGEX =  re.compile(r'.*(if|or)(\s+.*|\s+)(?<!_)(?<!str\()type\(.*')
 BLACKLIST_IMPORTS = {
     'requests': {
         'new_only': True,
@@ -197,6 +198,14 @@ class ModuleValidator(Validator):
         if not self.text.startswith('#!/usr/bin/python'):
             self.errors.append('Interpreter line is not "#!/usr/bin/python"')
 
+    def _check_type_instead_of_isinstance(self, powershell=False):
+        if powershell:
+            return
+        for line_no, line in enumerate(self.text.splitlines()):
+            typekeyword = TYPE_REGEX.match(line)
+            if typekeyword:
+                self.errors.append('Type comparison using type() found on line %d. Use isinstance() instead' % (line_no + 1))
+
     def _check_for_sys_exit(self):
         if 'sys.exit(' in self.text:
             self.errors.append('sys.exit() call found. Should be '
@@ -592,6 +601,7 @@ class ModuleValidator(Validator):
         self._check_for_gpl3_header()
         if not self._just_docs():
             self._check_interpreter(powershell=self._powershell_module())
+            self._check_type_instead_of_isinstance(powershell=self._powershell_module())
 
 
 class PythonPackageValidator(Validator):