code-smell test changes

* Create get_exception and wildcard import code-smell tests
* Add more detail to boilerplate and no-basestring descriptions
* Remove the no-list-cmp test as the pylint undefined-variable test covers it

docs/docsite/rst/dev_guide/testing/sanity/boilerplate.rst

@@ -1,11 +1,10 @@
 Sanity Tests » boilerplate
 ==========================
 
-Most Python files other than those under ``lib/ansible/modules/`` should include the following boilerplate:
+Most Python files should include the following boilerplate:
 
 .. code-block:: python
 
     from __future__ import (absolute_import, division, print_function)
-
     __metaclass__ = type
 

docs/docsite/rst/dev_guide/testing/sanity/no-basestring.rst

@@ -1,4 +1,11 @@
 Sanity Tests » no-basestring
 ============================
 
-Do not use ``isinstance(s, basestring)``.
+Do not use ``isinstance(s, basestring)`` as basestring has been removed in
+Python3.  You can import ``string_types``, ``binary_type``, or ``text_type``
+from ``ansible.module_utils.six`` and then use ``isinstance(s, string_types)``
+or ``isinstance(s, (binary_type, text_type))`` instead.
+
+If this is part of code to convert a string to a particular type,
+``ansible.module_utils._text`` contains several functions that may be even
+better for you: ``to_text``, ``to_bytes``, and ``to_native``.

docs/docsite/rst/dev_guide/testing/sanity/no-get-exception.rst

@@ -0,0 +1,28 @@
+Sanity Tests » no-get-exception
+===============================
+
+We created a function, ``ansible.module_utils.pycompat24.get_exception`` to
+help retrieve exceptions in a manner compatible with Python-2.4 through
+Python-3.6.  We no longer support Python-2.4 and Python-2.5 so this is
+extraneous and we want to deprecate the function.  Porting code should look
+something like this:
+
+.. code-block:: python
+
+    # Unfixed code:
+    try:
+        raise IOError('test')
+    except IOError:
+        e = get_excetion()
+        do_something(e)
+    except:
+        e = get_exception()
+        do_something_else(e)
+
+    # After fixing:
+    try:
+        raise IOError('test')
+    except IOErrors as e:
+        do_something(e)
+    except Exception as e:
+        do_something_else(e)

docs/docsite/rst/dev_guide/testing/sanity/no-list-cmp.rst

@@ -1,8 +0,0 @@
-Sanity Tests » no-list-cmp
-==========================
-
-The ``cmp`` function has been removed in Python 3.
-
-See `When sorting, use key instead of cmp`_.
-
-.. _When sorting, use key instead of cmp: http://python3porting.com/preparing.html#when-sorting-use-key-instead-of-cmp

docs/docsite/rst/dev_guide/testing/sanity/no-wildcard-import.rst

@@ -0,0 +1,29 @@
+Sanity Tests » no-wildcard-import
+=================================
+
+Using :code:`import *` is a bad habit which pollutes your namespace, hinders
+debugging, and interferes with static analysis of code.  For those reasons, we
+do want to limit the use of :code:`import *` in the ansible code.  Change our
+code to import the specific names that you need instead.
+
+Examples of unfixed code:
+
+.. code-block:: python
+
+    from ansible.module_utils.six import *
+    if isinstance(variable, string_types):
+        do_something(variable)
+
+    from ansible.module_utils.basic import *
+    module = AnsibleModule([...])
+
+Examples of fixed code:
+
+.. code-block:: python
+
+    from ansible.module_utils import six
+    if isinstance(variable, six.string_types):
+        do_something(variable)
+
+    from ansible.module_utils.basic import AnsibleModule
+    module = AnsibleModule([...])

test/sanity/code-smell/boilerplate.sh

@@ -3,6 +3,7 @@
 metaclass1=$(find ./bin -type f -exec grep -HL '__metaclass__ = type' '{}' '+')
 future1=$(find ./bin -type f -exec grep -HL 'from __future__ import (absolute_import, division, print_function)' '{}' '+')
 
+# We eventually want to remove the module_utils and modules pruning from metaclass2 and future2
 metaclass2=$(find ./lib/ansible -path ./lib/ansible/modules -prune \
         -o -path ./lib/ansible/module_utils -prune \
         -o -path ./lib/ansible/module_utils/six/_six.py -prune \
@@ -60,33 +61,10 @@ future3=$(find ./lib/ansible/modules -path ./lib/ansible/modules/windows -prune
         -o -path ./lib/ansible/modules/network/vyos -prune \
         -o -name '*.py' -type f -size +0c -exec egrep -HL 'from __future__ import (?absolute_import, division, print_function)?' '{}' '+')
 
-# Ordered by approximate work, lowest to highest
-# Key:
-#     [*]: import * fixes
-#     [!]: many get_exception fixes
-#     [i]: a few get_exception fixes
-# (everything below needs boilerplate added)
-# Priorities: import*, get_exception, then boilerplate-only
-#
-# network/ios
-# network/eos [i]
-# network/netvisor
-# network/aos [!]
-# network/vyos [i]
-# network/lenovo
-# network/panos [!]
-# network/junos [i]
-# files [!]
-# network/avi
-# network/f5 [*][i]
-# monitoring [*][!]
-# packaging/os [*][i]
-# cloud/cloudstack [*]
-# cloud/openstack [*]
-# cloud/ovirt
-# network/cloudengine [i]
-# network/nxos [*][i]
-# cloud/amazon [*]
+###
+### Important: If you're looking for a list of files to cleanup for boilerplate 
+### Look at this wikipage instead: https://github.com/ansible/community/wiki/Testing:-boilerplate,-wildcard-imports,-and-get_exception
+###
 
 ### TODO:
 ### - module_utils  <=== these are important but not well organized so we'd

test/sanity/code-smell/no-get-exception.sh

@@ -0,0 +1,36 @@
+#!/bin/sh
+
+# We're getting rid of get_exception in our code so that we can deprecate it.
+# get_exception is no longer needed as we no longer support python-2.4 on the controller.
+
+# We eventually want this list to be empty
+get_exception=$(find . -path ./test/runner/.tox -prune \
+        -o -path ./lib/ansible/module_utils -prune \
+        -o -path ./lib/ansible/modules/storage/netapp -prune \
+        -o -path ./lib/ansible/modules/packaging/os -prune \
+        -o -path ./lib/ansible/modules/monitoring -prune \
+        -o -path ./lib/ansible/modules/network/panos -prune \
+        -o -path ./lib/ansible/modules/network/nxos -prune \
+        -o -path ./lib/ansible/modules/network/junos -prune \
+        -o -path ./lib/ansible/modules/network/vyos -prune \
+        -o -path ./lib/ansible/modules/network/fortios -prune \
+        -o -path ./lib/ansible/modules/network/f5 -prune \
+        -o -path ./lib/ansible/modules/network/cloudengine -prune \
+        -o -path ./lib/ansible/modules/network/aos -prune \
+        -o -path ./lib/ansible/modules/network/eos -prune \
+        -o -path ./lib/ansible/modules/files -prune \
+        -o -path ./lib/ansible/modules/system -prune \
+        -o -path ./lib/ansible/modules/web_infrastructure -prune \
+        -o -path ./lib/ansible/plugins/action/wait_for_connection.py -prune \
+        -o -name '*.py' -type f -exec grep -H 'get_exception' '{}' '+')
+
+
+if test -n "$get_exception" ; then
+  printf "\n== Contains get_exception() calls ==\n"
+  printf "%s" "$get_exception"
+  failures=$(printf "%s" "$get_exception"| wc -l)
+  failures=$((failures + 2))
+  exit "$failures"
+fi
+
+exit 0

test/sanity/code-smell/no-list-cmp.sh

@@ -1,18 +0,0 @@
-#!/bin/sh
-
-CMP_USERS=$(grep -rI ' cmp[^a-zA-Z0-9_:=]' . \
-	--exclude-dir .tox \
-	| grep -v \
-	-e lib/ansible/module_utils/six/_six.py \
-	-e .git \
-	-e docs/docsite/_build/ \
-	-e docs/docsite/rst/dev_guide/testing/sanity/ \
-	-e test/sanity/code-smell/no-list-cmp.sh
-	)
-
-if [ "${CMP_USERS}" ]; then
-	echo 'cmp has been removed in python3.  Alternatives:'
-	echo '    http://python3porting.com/preparing.html#when-sorting-use-key-instead-of-cmp'
-	echo "${CMP_USERS}"
-	exit 1
-fi

test/sanity/code-smell/no-wildcard-import.sh

@@ -0,0 +1,42 @@
+#!/bin/sh
+
+# Only needed until we enable pylint test for wildcard imports
+
+
+# The first three paths here are valid uses of wildcard imports
+# unsafe_proxy is backwards compat (pylint disabled added)
+# module_common.py is picked up from static strings, not from actual imports (pylint won't detect)
+# test_action.py is picked up from static strings, not from actual imports (pylint won't detect)
+# mock.py is importing code for an installed library for compat (pylint disabled added)
+# unittest.py is importing code for an installed library for compat (pylint disabled added)
+#
+# Everything else needs to be fixed
+wildcard_imports=$(find . -path ./test/runner/.tox -prune \
+        -o -path ./lib/ansible/vars/unsafe_proxy.py -prune \
+        -o -path ./lib/ansible/executor/module_common.py -prune \
+        -o -path ./test/units/plugins/action/test_action.py \
+        -o -path ./lib/ansible/compat/tests/mock.py -prune \
+        -o -path ./lib/ansible/compat/tests/unittest.py \
+        -o -path ./lib/ansible/module_utils/api.py -prune \
+        -o -path ./lib/ansible/module_utils/cloud.py -prune \
+        -o -path ./lib/ansible/module_utils/aos.py -prune \
+        -o -path ./test/units/modules/network/cumulus/test_nclu.py -prune \
+        -o -path ./lib/ansible/modules/cloud/amazon -prune \
+        -o -path ./lib/ansible/modules/cloud/openstack -prune \
+        -o -path ./lib/ansible/modules/cloud/cloudstack -prune \
+        -o -path ./lib/ansible/modules/monitoring -prune \
+        -o -path ./lib/ansible/modules/network/f5 -prune \
+        -o -path ./lib/ansible/modules/network/nxos -prune \
+        -o -path ./lib/ansible/modules/packaging/os -prune \
+        -o -name '*.py' -type f -exec grep -H 'import \*' '{}' '+')
+
+
+if test -n "$wildcard_imports" ; then
+  printf "\n== Wildcard imports detected ==\n"
+  printf "%s" "$wildcard_imports"
+  failures=$(printf "%s" "$wildcard_imports"| wc -l)
+  failures=$((failures + 2))
+  exit "$failures"
+fi
+
+exit 0