Rename pylint plugin and add tests. (#70225)

* Renamed custom pylint plugin for unwanted names.
* Add integration tests for sanity test failures.

changelogs/fragments/ansible-test-pylint-plugin-name.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - ansible-test - Changed the internal name of the custom plugin used to identify use of unwanted imports and functions.

test/integration/targets/ansible-test/ansible_collections/ns/col/plugins/modules/bad.py

@@ -0,0 +1,34 @@
+#!/usr/bin/python
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+from __future__ import absolute_import, division, print_function
+__metaclass__ = type
+
+DOCUMENTATION = '''
+module: bad
+short_description: Bad test module
+description: Bad test module.
+author:
+  - Ansible Core Team
+'''
+
+EXAMPLES = '''
+- bad:
+'''
+
+RETURN = ''''''
+
+from ansible.module_utils.basic import AnsibleModule
+from ansible import constants  # intentionally trigger pylint ansible-bad-module-import error
+
+
+def main():
+    module = AnsibleModule(
+        argument_spec=dict(),
+    )
+
+    module.exit_json()
+
+
+if __name__ == '__main__':
+    main()

test/integration/targets/ansible-test/ansible_collections/ns/col/tests/integration/targets/hello/files/bad.py

@@ -0,0 +1,16 @@
+from __future__ import absolute_import, division, print_function
+__metaclass__ = type
+
+import tempfile
+
+try:
+    import urllib2  # intentionally trigger pylint ansible-bad-import error
+except ImportError:
+    urllib2 = None
+
+try:
+    from urllib2 import Request  # intentionally trigger pylint ansible-bad-import-from error
+except ImportError:
+    Request = None
+
+tempfile.mktemp()  # intentionally trigger pylint ansible-bad-function error

test/integration/targets/ansible-test/ansible_collections/ns/col/tests/sanity/ignore.txt

@@ -0,0 +1,5 @@
+plugins/modules/bad.py import
+plugins/modules/bad.py pylint:ansible-bad-module-import
+tests/integration/targets/hello/files/bad.py pylint:ansible-bad-function
+tests/integration/targets/hello/files/bad.py pylint:ansible-bad-import
+tests/integration/targets/hello/files/bad.py pylint:ansible-bad-import-from

test/integration/targets/ansible-test/collection-tests/venv.sh

@@ -5,6 +5,10 @@ set -eux -o pipefail
 cp -a "${TEST_DIR}/ansible_collections" "${WORK_DIR}"
 cd "${WORK_DIR}/ansible_collections/ns/col"
 
+# rename the sanity ignore file to match the current ansible version and update import ignores with the python version
+ansible_version="$(python -c 'import ansible.release; print(".".join(ansible.release.__version__.split(".")[:2]))')"
+sed "s/ import$/ import-${ANSIBLE_TEST_PYTHON_VERSION}/;" < "tests/sanity/ignore.txt" > "tests/sanity/ignore-${ansible_version}.txt"
+
 # common args for all tests
 # each test will be run in a separate venv to verify that requirements have been properly specified
 common=(--venv --python "${ANSIBLE_TEST_PYTHON_VERSION}" --color --truncate 0 "${@}")

test/lib/ansible_test/_data/sanity/pylint/plugins/unwanted.py

@@ -14,8 +14,8 @@ ANSIBLE_TEST_MODULES_PATH = os.environ['ANSIBLE_TEST_MODULES_PATH']
 ANSIBLE_TEST_MODULE_UTILS_PATH = os.environ['ANSIBLE_TEST_MODULE_UTILS_PATH']
 
 
-class BlacklistEntry:
-    """Defines a import blacklist entry."""
+class UnwantedEntry:
+    """Defines an unwanted import."""
     def __init__(self, alternative, modules_only=False, names=None, ignore_paths=None):
         """
         :type alternative: str
@@ -58,11 +58,11 @@ def is_module_path(path):
     return path.startswith(ANSIBLE_TEST_MODULES_PATH) or path.startswith(ANSIBLE_TEST_MODULE_UTILS_PATH)
 
 
-class AnsibleBlacklistChecker(BaseChecker):
-    """Checker for blacklisted imports and functions."""
+class AnsibleUnwantedChecker(BaseChecker):
+    """Checker for unwanted imports and functions."""
     __implements__ = (IAstroidChecker,)
 
-    name = 'blacklist'
+    name = 'unwanted'
 
     BAD_IMPORT = 'ansible-bad-import'
     BAD_IMPORT_FROM = 'ansible-bad-import-from'
@@ -84,58 +84,58 @@ class AnsibleBlacklistChecker(BaseChecker):
                'Identifies imports which should not be used.'),
     )
 
-    blacklist_imports = dict(
+    unwanted_imports = dict(
         # Additional imports that we may want to start checking:
-        # boto=BlacklistEntry('boto3', modules_only=True),
-        # requests=BlacklistEntry('ansible.module_utils.urls', modules_only=True),
-        # urllib=BlacklistEntry('ansible.module_utils.urls', modules_only=True),
+        # boto=UnwantedEntry('boto3', modules_only=True),
+        # requests=UnwantedEntry('ansible.module_utils.urls', modules_only=True),
+        # urllib=UnwantedEntry('ansible.module_utils.urls', modules_only=True),
 
         # see https://docs.python.org/2/library/urllib2.html
-        urllib2=BlacklistEntry('ansible.module_utils.urls',
-                               ignore_paths=(
-                                   '/lib/ansible/module_utils/urls.py',
-                               )),
+        urllib2=UnwantedEntry('ansible.module_utils.urls',
+                              ignore_paths=(
+                                  '/lib/ansible/module_utils/urls.py',
+                              )),
 
         # see https://docs.python.org/3.7/library/collections.abc.html
-        collections=BlacklistEntry('ansible.module_utils.common._collections_compat',
-                                   ignore_paths=(
-                                       '/lib/ansible/module_utils/common/_collections_compat.py',
-                                   ),
-                                   names=(
-                                       'MappingView',
-                                       'ItemsView',
-                                       'KeysView',
-                                       'ValuesView',
-                                       'Mapping', 'MutableMapping',
-                                       'Sequence', 'MutableSequence',
-                                       'Set', 'MutableSet',
-                                       'Container',
-                                       'Hashable',
-                                       'Sized',
-                                       'Callable',
-                                       'Iterable',
-                                       'Iterator',
-                                   )),
+        collections=UnwantedEntry('ansible.module_utils.common._collections_compat',
+                                  ignore_paths=(
+                                      '/lib/ansible/module_utils/common/_collections_compat.py',
+                                  ),
+                                  names=(
+                                      'MappingView',
+                                      'ItemsView',
+                                      'KeysView',
+                                      'ValuesView',
+                                      'Mapping', 'MutableMapping',
+                                      'Sequence', 'MutableSequence',
+                                      'Set', 'MutableSet',
+                                      'Container',
+                                      'Hashable',
+                                      'Sized',
+                                      'Callable',
+                                      'Iterable',
+                                      'Iterator',
+                                  )),
     )
 
-    blacklist_functions = {
+    unwanted_functions = {
         # see https://docs.python.org/2/library/tempfile.html#tempfile.mktemp
-        'tempfile.mktemp': BlacklistEntry('tempfile.mkstemp'),
-
-        'sys.exit': BlacklistEntry('exit_json or fail_json',
-                                   ignore_paths=(
-                                       '/lib/ansible/module_utils/basic.py',
-                                       '/lib/ansible/modules/async_wrapper.py',
-                                       '/lib/ansible/module_utils/common/removed.py',
-                                   ),
-                                   modules_only=True),
-
-        'builtins.print': BlacklistEntry('module.log or module.debug',
-                                         ignore_paths=(
-                                             '/lib/ansible/module_utils/basic.py',
-                                             '/lib/ansible/module_utils/common/removed.py',
-                                         ),
-                                         modules_only=True),
+        'tempfile.mktemp': UnwantedEntry('tempfile.mkstemp'),
+
+        'sys.exit': UnwantedEntry('exit_json or fail_json',
+                                  ignore_paths=(
+                                      '/lib/ansible/module_utils/basic.py',
+                                      '/lib/ansible/modules/async_wrapper.py',
+                                      '/lib/ansible/module_utils/common/removed.py',
+                                  ),
+                                  modules_only=True),
+
+        'builtins.print': UnwantedEntry('module.log or module.debug',
+                                        ignore_paths=(
+                                            '/lib/ansible/module_utils/basic.py',
+                                            '/lib/ansible/module_utils/common/removed.py',
+                                        ),
+                                        modules_only=True),
     }
 
     def visit_import(self, node):
@@ -163,7 +163,7 @@ class AnsibleBlacklistChecker(BaseChecker):
 
         module = last_child.name
 
-        entry = self.blacklist_imports.get(module)
+        entry = self.unwanted_imports.get(module)
 
         if entry and entry.names:
             if entry.applies_to(self.linter.current_file, node.attrname):
@@ -183,7 +183,7 @@ class AnsibleBlacklistChecker(BaseChecker):
                 if not func:
                     continue
 
-                entry = self.blacklist_functions.get(func)
+                entry = self.unwanted_functions.get(func)
 
                 if entry and entry.applies_to(self.linter.current_file):
                     self.add_message(self.BAD_FUNCTION, args=(entry.alternative, func), node=node)
@@ -197,7 +197,7 @@ class AnsibleBlacklistChecker(BaseChecker):
         """
         self._check_module_import(node, modname)
 
-        entry = self.blacklist_imports.get(modname)
+        entry = self.unwanted_imports.get(modname)
 
         if not entry:
             return
@@ -213,7 +213,7 @@ class AnsibleBlacklistChecker(BaseChecker):
         """
         self._check_module_import(node, modname)
 
-        entry = self.blacklist_imports.get(modname)
+        entry = self.unwanted_imports.get(modname)
 
         if not entry:
             return
@@ -239,4 +239,4 @@ class AnsibleBlacklistChecker(BaseChecker):
 
 def register(linter):
     """required method to auto register this checker """
-    linter.register_checker(AnsibleBlacklistChecker(linter))
+    linter.register_checker(AnsibleUnwantedChecker(linter))

test/sanity/ignore.txt

@@ -190,6 +190,9 @@ lib/ansible/plugins/strategy/__init__.py pylint:ansible-deprecated-version
 lib/ansible/plugins/strategy/__init__.py pylint:blacklisted-name
 lib/ansible/plugins/strategy/linear.py pylint:blacklisted-name
 lib/ansible/vars/hostvars.py pylint:blacklisted-name
+test/integration/targets/ansible-test/ansible_collections/ns/col/tests/integration/targets/hello/files/bad.py pylint:ansible-bad-import
+test/integration/targets/ansible-test/ansible_collections/ns/col/tests/integration/targets/hello/files/bad.py pylint:ansible-bad-import-from
+test/integration/targets/ansible-test/ansible_collections/ns/col/tests/integration/targets/hello/files/bad.py pylint:ansible-bad-function
 test/integration/targets/ansible-test/ansible_collections/ns/col/plugins/modules/hello.py pylint:relative-beyond-top-level
 test/integration/targets/ansible-test/ansible_collections/ns/col/tests/unit/plugins/module_utils/test_my_util.py pylint:relative-beyond-top-level
 test/integration/targets/ansible-test/ansible_collections/ns/col/tests/unit/plugins/modules/test_hello.py pylint:relative-beyond-top-level