From 5b5cba5e50089a5879d4136348ec9ce2622b185f Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Thu, 22 Feb 2018 08:08:29 -0800 Subject: [PATCH] Update more code-smell tests. (#36570) * Enhance use-compat-six code-smell test. * Enhance use-argspec-type-path code-smell test. * Enhance replace-urlopen code-smell test. * Enhance boilerplate code-smell test. * Enhance no-underscore-variable code-smell test. --- test/sanity/code-smell/boilerplate.json | 6 + test/sanity/code-smell/boilerplate.py | 72 +++++++++ test/sanity/code-smell/boilerplate.sh | 94 ------------ .../code-smell/no-underscore-variable.json | 6 + .../code-smell/no-underscore-variable.py | 140 +++++++++++++++++ .../code-smell/no-underscore-variable.sh | 142 ------------------ test/sanity/code-smell/replace-urlopen.json | 6 + test/sanity/code-smell/replace-urlopen.py | 29 ++++ test/sanity/code-smell/replace-urlopen.sh | 15 -- .../code-smell/use-argspec-type-path.json | 9 ++ .../code-smell/use-argspec-type-path.py | 44 ++++++ .../code-smell/use-argspec-type-path.sh | 42 ------ test/sanity/code-smell/use-compat-six.json | 6 + test/sanity/code-smell/use-compat-six.py | 52 +++++++ test/sanity/code-smell/use-compat-six.sh | 21 --- 15 files changed, 370 insertions(+), 314 deletions(-) create mode 100644 test/sanity/code-smell/boilerplate.json create mode 100755 test/sanity/code-smell/boilerplate.py delete mode 100755 test/sanity/code-smell/boilerplate.sh create mode 100644 test/sanity/code-smell/no-underscore-variable.json create mode 100755 test/sanity/code-smell/no-underscore-variable.py delete mode 100755 test/sanity/code-smell/no-underscore-variable.sh create mode 100644 test/sanity/code-smell/replace-urlopen.json create mode 100755 test/sanity/code-smell/replace-urlopen.py delete mode 100755 test/sanity/code-smell/replace-urlopen.sh create mode 100644 test/sanity/code-smell/use-argspec-type-path.json create mode 100755 test/sanity/code-smell/use-argspec-type-path.py delete mode 100755 test/sanity/code-smell/use-argspec-type-path.sh create mode 100644 test/sanity/code-smell/use-compat-six.json create mode 100755 test/sanity/code-smell/use-compat-six.py delete mode 100755 test/sanity/code-smell/use-compat-six.sh diff --git a/test/sanity/code-smell/boilerplate.json b/test/sanity/code-smell/boilerplate.json new file mode 100644 index 00000000000..6f1edb783ce --- /dev/null +++ b/test/sanity/code-smell/boilerplate.json @@ -0,0 +1,6 @@ +{ + "extensions": [ + ".py" + ], + "output": "path-message" +} diff --git a/test/sanity/code-smell/boilerplate.py b/test/sanity/code-smell/boilerplate.py new file mode 100755 index 00000000000..b0e2e430d9c --- /dev/null +++ b/test/sanity/code-smell/boilerplate.py @@ -0,0 +1,72 @@ +#!/usr/bin/env python + +import sys + + +def main(): + skip = set([ + 'lib/ansible/compat/selectors/_selectors2.py', + 'lib/ansible/module_utils/six/_six.py', + 'setup.py', + ]) + + prune = [ + 'contrib/inventory/', + 'contrib/vault/', + 'docs/', + 'examples/', + 'hacking/', + 'lib/ansible/module_utils/', + 'lib/ansible/modules/cloud/amazon/', + 'lib/ansible/modules/cloud/cloudstack/', + 'lib/ansible/modules/cloud/ovirt/', + 'lib/ansible/modules/network/aos/', + 'lib/ansible/modules/network/avi/', + 'lib/ansible/modules/network/cloudengine/', + 'lib/ansible/modules/network/eos/', + 'lib/ansible/modules/network/ios/', + 'lib/ansible/modules/network/netvisor/', + 'lib/ansible/modules/network/nxos/', + 'lib/ansible/modules/network/panos/', + 'lib/ansible/modules/network/vyos/', + 'lib/ansible/modules/windows/', + 'lib/ansible/utils/module_docs_fragments/', + 'test/' + ] + + for path in sys.argv[1:]: + if path in skip: + continue + + if any(path.startswith(p) for p in prune): + continue + + with open(path, 'r') as path_fd: + future_ok = None + metaclass_ok = None + + lines = path_fd.read().splitlines() + + if not lines: + continue + + for line, text in enumerate(lines): + if text in ('from __future__ import (absolute_import, division, print_function)', + 'from __future__ import absolute_import, division, print_function'): + future_ok = line + + if text == '__metaclass__ = type': + metaclass_ok = line + + if future_ok and metaclass_ok: + break + + if future_ok is None: + print('%s: missing: from __future__ import (absolute_import, division, print_function)' % path) + + if metaclass_ok is None: + print('%s: missing: __metaclass__ = type' % path) + + +if __name__ == '__main__': + main() diff --git a/test/sanity/code-smell/boilerplate.sh b/test/sanity/code-smell/boilerplate.sh deleted file mode 100755 index 63ee2114dc0..00000000000 --- a/test/sanity/code-smell/boilerplate.sh +++ /dev/null @@ -1,94 +0,0 @@ -#!/bin/sh - -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 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 \ - -o -path ./lib/ansible/compat/selectors/_selectors2.py -prune \ - -o -path ./lib/ansible/utils/module_docs_fragments -prune \ - -o -name '*.py' -type f -size +0c -exec grep -HL '__metaclass__ = type' '{}' '+') - -future2=$(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 \ - -o -path ./lib/ansible/compat/selectors/_selectors2.py -prune \ - -o -path ./lib/ansible/utils/module_docs_fragments -prune \ - -o -name '*.py' -type f -size +0c -exec grep -HL 'from __future__ import (absolute_import, division, print_function)' '{}' '+') - -# Eventually we want metaclass3 and future3 to get down to 0 -metaclass3=$(find ./lib/ansible/modules -path ./lib/ansible/modules/windows -prune \ - -o -path ./lib/ansible/modules/cloud/ovirt -prune \ - -o -path ./lib/ansible/modules/cloud/cloudstack -prune \ - -o -path ./lib/ansible/modules/cloud/amazon -prune \ - -o -path ./lib/ansible/modules/network/aos -prune \ - -o -path ./lib/ansible/modules/network/avi -prune \ - -o -path ./lib/ansible/modules/network/cloudengine -prune \ - -o -path ./lib/ansible/modules/network/eos -prune \ - -o -path ./lib/ansible/modules/network/ios -prune \ - -o -path ./lib/ansible/modules/network/netvisor -prune \ - -o -path ./lib/ansible/modules/network/nxos -prune \ - -o -path ./lib/ansible/modules/network/panos -prune \ - -o -path ./lib/ansible/modules/network/vyos -prune \ - -o -name '*.py' -type f -size +0c -exec grep -HL '__metaclass__ = type' '{}' '+') - -future3=$(find ./lib/ansible/modules -path ./lib/ansible/modules/windows -prune \ - -o -path ./lib/ansible/modules/cloud/ovirt -prune \ - -o -path ./lib/ansible/modules/cloud/cloudstack -prune \ - -o -path ./lib/ansible/modules/cloud/amazon -prune \ - -o -path ./lib/ansible/modules/network/aos -prune \ - -o -path ./lib/ansible/modules/network/avi -prune \ - -o -path ./lib/ansible/modules/network/cloudengine -prune \ - -o -path ./lib/ansible/modules/network/eos -prune \ - -o -path ./lib/ansible/modules/network/ios -prune \ - -o -path ./lib/ansible/modules/network/netvisor -prune \ - -o -path ./lib/ansible/modules/network/nxos -prune \ - -o -path ./lib/ansible/modules/network/panos -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)?' '{}' '+') - -### -### 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 -### have to construct the test file by file -### - contrib/ <=== Not a priority as these will move to inventory plugins over time - - -if test -n "$metaclass1" -o -n "$metaclass2" -o -n "$metaclass3" ; then -printf "\n== Missing __metaclass__ = type ==\n" -fi - -if test -n "$metaclass1" ; then - printf "%s\n" "$metaclass1" -fi -if test -n "$metaclass2" ; then - printf "%s\n" "$metaclass2" -fi -if test -n "$metaclass3" ; then - printf "%s\n" "$metaclass3" -fi - -if test -n "$future1" -o -n "$future2" -o -n "$future3" ; then - printf "\n== Missing from __future__ import (absolute_import, division, print_function) ==\n" -fi - -if test -n "$future1" ; then - printf "%s\n" "$future1" -fi -if test -n "$future2" ; then - printf "%s\n" "$future2" -fi -if test -n "$future3" ; then - printf "%s\n" "$future3" -fi - -if test -n "$future1$future2$future3$metaclass1$metaclass2$metaclass3" ; then - exit 2 -fi -exit 0 diff --git a/test/sanity/code-smell/no-underscore-variable.json b/test/sanity/code-smell/no-underscore-variable.json new file mode 100644 index 00000000000..776590b74d2 --- /dev/null +++ b/test/sanity/code-smell/no-underscore-variable.json @@ -0,0 +1,6 @@ +{ + "extensions": [ + ".py" + ], + "output": "path-line-column-message" +} diff --git a/test/sanity/code-smell/no-underscore-variable.py b/test/sanity/code-smell/no-underscore-variable.py new file mode 100755 index 00000000000..5ac713098ea --- /dev/null +++ b/test/sanity/code-smell/no-underscore-variable.py @@ -0,0 +1,140 @@ +#!/usr/bin/env python + +# Only needed until we can enable a pylint test for this. We may have to write +# one or add it to another existing test (like the one to warn on inappropriate +# variable names). Adding to an existing test may be hard as we may have many +# other things that are not compliant with that test. + +import os +import re +import sys + + +def main(): + skip = set([ + 'test/sanity/code-smell/%s' % os.path.basename(__file__), + # These files currently use _ as a variable. Fix them and then remove them + # from this list. Note that we're not sure if we'll translate module return + # values. If we decide never to do that, then we can stop checking for those. + 'contrib/inventory/gce.py', + 'lib/ansible/cli/console.py', + 'lib/ansible/compat/selectors/_selectors2.py', + 'lib/ansible/executor/playbook_executor.py', + 'lib/ansible/executor/task_queue_manager.py', + 'lib/ansible/module_utils/facts/network/linux.py', + 'lib/ansible/module_utils/urls.py', + 'lib/ansible/modules/cloud/amazon/data_pipeline.py', + 'lib/ansible/modules/cloud/amazon/ec2_group_facts.py', + 'lib/ansible/modules/cloud/amazon/ec2_vpc_nat_gateway.py', + 'lib/ansible/modules/cloud/amazon/ec2_vpc_vpn.py', + 'lib/ansible/modules/cloud/amazon/efs.py', + 'lib/ansible/modules/cloud/amazon/efs_facts.py', + 'lib/ansible/modules/cloud/amazon/kinesis_stream.py', + 'lib/ansible/modules/cloud/amazon/route53_zone.py', + 'lib/ansible/modules/cloud/amazon/s3_sync.py', + 'lib/ansible/modules/cloud/azure/azure_rm_loadbalancer.py', + 'lib/ansible/modules/cloud/docker/docker_container.py', + 'lib/ansible/modules/cloud/docker/docker_service.py', + 'lib/ansible/modules/cloud/google/gce.py', + 'lib/ansible/modules/cloud/google/gce_eip.py', + 'lib/ansible/modules/cloud/google/gce_img.py', + 'lib/ansible/modules/cloud/google/gce_instance_template.py', + 'lib/ansible/modules/cloud/google/gce_lb.py', + 'lib/ansible/modules/cloud/google/gce_mig.py', + 'lib/ansible/modules/cloud/google/gce_net.py', + 'lib/ansible/modules/cloud/google/gce_pd.py', + 'lib/ansible/modules/cloud/google/gce_snapshot.py', + 'lib/ansible/modules/cloud/google/gce_tag.py', + 'lib/ansible/modules/cloud/google/gcp_backend_service.py', + 'lib/ansible/modules/cloud/google/gcp_healthcheck.py', + 'lib/ansible/modules/cloud/lxc/lxc_container.py', + 'lib/ansible/modules/files/copy.py', + 'lib/ansible/modules/files/patch.py', + 'lib/ansible/modules/files/synchronize.py', + 'lib/ansible/modules/monitoring/statusio_maintenance.py', + 'lib/ansible/modules/monitoring/zabbix/zabbix_maintenance.py', + 'lib/ansible/modules/net_tools/basics/uri.py', + 'lib/ansible/modules/network/cloudengine/ce_acl.py', + 'lib/ansible/modules/network/cloudengine/ce_command.py', + 'lib/ansible/modules/network/cloudengine/ce_dldp_interface.py', + 'lib/ansible/modules/network/cloudengine/ce_mlag_interface.py', + 'lib/ansible/modules/network/cloudvision/cv_server_provision.py', + 'lib/ansible/modules/network/f5/bigip_remote_syslog.py', + 'lib/ansible/modules/network/illumos/dladm_etherstub.py', + 'lib/ansible/modules/network/illumos/dladm_iptun.py', + 'lib/ansible/modules/network/illumos/dladm_linkprop.py', + 'lib/ansible/modules/network/illumos/dladm_vlan.py', + 'lib/ansible/modules/network/illumos/dladm_vnic.py', + 'lib/ansible/modules/network/illumos/flowadm.py', + 'lib/ansible/modules/network/illumos/ipadm_addr.py', + 'lib/ansible/modules/network/illumos/ipadm_addrprop.py', + 'lib/ansible/modules/network/illumos/ipadm_if.py', + 'lib/ansible/modules/network/illumos/ipadm_ifprop.py', + 'lib/ansible/modules/network/illumos/ipadm_prop.py', + 'lib/ansible/modules/network/vyos/vyos_command.py', + 'lib/ansible/modules/packaging/language/pip.py', + 'lib/ansible/modules/packaging/os/yum.py', + 'lib/ansible/modules/source_control/git.py', + 'lib/ansible/modules/system/alternatives.py', + 'lib/ansible/modules/system/beadm.py', + 'lib/ansible/modules/system/cronvar.py', + 'lib/ansible/modules/system/dconf.py', + 'lib/ansible/modules/system/filesystem.py', + 'lib/ansible/modules/system/gconftool2.py', + 'lib/ansible/modules/system/interfaces_file.py', + 'lib/ansible/modules/system/iptables.py', + 'lib/ansible/modules/system/java_cert.py', + 'lib/ansible/modules/system/lvg.py', + 'lib/ansible/modules/system/lvol.py', + 'lib/ansible/modules/system/parted.py', + 'lib/ansible/modules/system/timezone.py', + 'lib/ansible/modules/system/ufw.py', + 'lib/ansible/modules/utilities/logic/wait_for.py', + 'lib/ansible/modules/web_infrastructure/rundeck_acl_policy.py', + 'lib/ansible/parsing/vault/__init__.py', + 'lib/ansible/playbook/base.py', + 'lib/ansible/playbook/helpers.py', + 'lib/ansible/playbook/role/__init__.py', + 'lib/ansible/playbook/taggable.py', + 'lib/ansible/plugins/callback/hipchat.py', + 'lib/ansible/plugins/connection/lxc.py', + 'lib/ansible/plugins/filter/core.py', + 'lib/ansible/plugins/lookup/sequence.py', + 'lib/ansible/plugins/strategy/__init__.py', + 'lib/ansible/plugins/strategy/linear.py', + 'test/legacy/cleanup_gce.py', + 'test/legacy/gce_credentials.py', + 'test/runner/lib/cloud/cs.py', + 'test/runner/lib/core_ci.py', + 'test/runner/lib/delegation.py', + 'test/runner/lib/docker_util.py', + 'test/runner/lib/executor.py', + 'test/runner/lib/http.py', + 'test/runner/lib/import_analysis.py', + 'test/runner/lib/manage_ci.py', + 'test/runner/lib/target.py', + 'test/runner/lib/util.py', + 'test/sanity/import/importer.py', + 'test/sanity/validate-modules/main.py', + 'test/units/executor/test_play_iterator.py', + 'test/units/module_utils/basic/test_run_command.py', + 'test/units/modules/cloud/amazon/test_ec2_vpc_nat_gateway.py', + 'test/units/modules/cloud/amazon/test_ec2_vpc_vpn.py', + 'test/units/modules/system/interfaces_file/test_interfaces_file.py', + ]) + + for path in sys.argv[1:]: + if path in skip: + continue + + with open(path, 'r') as path_fd: + for line, text in enumerate(path_fd.readlines()): + match = re.search(r'(?: |[^C]\()(_)(?: |,|\))', text) + + if match: + print('%s:%d:%d: use `dummy` instead of `_` for a variable name' % ( + path, line + 1, match.start(1) + 1)) + + +if __name__ == '__main__': + main() diff --git a/test/sanity/code-smell/no-underscore-variable.sh b/test/sanity/code-smell/no-underscore-variable.sh deleted file mode 100755 index d058f46b331..00000000000 --- a/test/sanity/code-smell/no-underscore-variable.sh +++ /dev/null @@ -1,142 +0,0 @@ -#!/bin/sh - -# Only needed until we can enable a pylint test for this. We may have to write -# one or add it to another existing test (like the one to warn on inappropriate -# variable names). Adding to an existing test may be hard as we may have many -# other things that are not compliant with that test. - -# These files currently use _ as a variable. Fix them and then remove them -# from this list note that we're not sure if we'll translate module return -# values. If we decide never to do that, then we can stop checking for those. - -TO_BE_FIXED=' -./contrib/inventory/gce.py -./lib/ansible/cli/console.py -./lib/ansible/compat/selectors/_selectors2.py -./lib/ansible/executor/playbook_executor.py -./lib/ansible/executor/task_queue_manager.py -./lib/ansible/module_utils/facts/network/linux.py -./lib/ansible/module_utils/urls.py -./lib/ansible/modules/cloud/amazon/data_pipeline.py -./lib/ansible/modules/cloud/amazon/ec2_group_facts.py -./lib/ansible/modules/cloud/amazon/ec2_vpc_nat_gateway.py -./lib/ansible/modules/cloud/amazon/ec2_vpc_vpn.py -./lib/ansible/modules/cloud/amazon/efs.py -./lib/ansible/modules/cloud/amazon/efs_facts.py -./lib/ansible/modules/cloud/amazon/kinesis_stream.py -./lib/ansible/modules/cloud/amazon/route53_zone.py -./lib/ansible/modules/cloud/amazon/s3_sync.py -./lib/ansible/modules/cloud/azure/azure_rm_loadbalancer.py -./lib/ansible/modules/cloud/docker/_docker.py -./lib/ansible/modules/cloud/docker/docker_container.py -./lib/ansible/modules/cloud/docker/docker_service.py -./lib/ansible/modules/cloud/google/gce.py -./lib/ansible/modules/cloud/google/gce_eip.py -./lib/ansible/modules/cloud/google/gce_img.py -./lib/ansible/modules/cloud/google/gce_instance_template.py -./lib/ansible/modules/cloud/google/gce_lb.py -./lib/ansible/modules/cloud/google/gce_mig.py -./lib/ansible/modules/cloud/google/gce_net.py -./lib/ansible/modules/cloud/google/gce_pd.py -./lib/ansible/modules/cloud/google/gce_snapshot.py -./lib/ansible/modules/cloud/google/gce_tag.py -./lib/ansible/modules/cloud/google/gcp_backend_service.py -./lib/ansible/modules/cloud/google/gcp_healthcheck.py -./lib/ansible/modules/cloud/lxc/lxc_container.py -./lib/ansible/modules/clustering/oc.py -./lib/ansible/modules/files/copy.py -./lib/ansible/modules/files/patch.py -./lib/ansible/modules/files/synchronize.py -./lib/ansible/modules/monitoring/statusio_maintenance.py -./lib/ansible/modules/monitoring/zabbix/zabbix_maintenance.py -./lib/ansible/modules/net_tools/basics/uri.py -./lib/ansible/modules/network/cloudengine/ce_acl.py -./lib/ansible/modules/network/cloudengine/ce_command.py -./lib/ansible/modules/network/cloudengine/ce_dldp_interface.py -./lib/ansible/modules/network/cloudengine/ce_mlag_interface.py -./lib/ansible/modules/network/cloudvision/cv_server_provision.py -./lib/ansible/modules/network/f5/bigip_remote_syslog.py -./lib/ansible/modules/network/illumos/dladm_etherstub.py -./lib/ansible/modules/network/illumos/dladm_iptun.py -./lib/ansible/modules/network/illumos/dladm_linkprop.py -./lib/ansible/modules/network/illumos/dladm_vlan.py -./lib/ansible/modules/network/illumos/dladm_vnic.py -./lib/ansible/modules/network/illumos/flowadm.py -./lib/ansible/modules/network/illumos/ipadm_addr.py -./lib/ansible/modules/network/illumos/ipadm_addrprop.py -./lib/ansible/modules/network/illumos/ipadm_if.py -./lib/ansible/modules/network/illumos/ipadm_ifprop.py -./lib/ansible/modules/network/illumos/ipadm_prop.py -./lib/ansible/modules/network/vyos/vyos_command.py -./lib/ansible/modules/packaging/language/pip.py -./lib/ansible/modules/packaging/os/yum.py -./lib/ansible/modules/source_control/git.py -./lib/ansible/modules/system/alternatives.py -./lib/ansible/modules/system/beadm.py -./lib/ansible/modules/system/cronvar.py -./lib/ansible/modules/system/dconf.py -./lib/ansible/modules/system/filesystem.py -./lib/ansible/modules/system/gconftool2.py -./lib/ansible/modules/system/interfaces_file.py -./lib/ansible/modules/system/iptables.py -./lib/ansible/modules/system/java_cert.py -./lib/ansible/modules/system/lvg.py -./lib/ansible/modules/system/lvol.py -./lib/ansible/modules/system/parted.py -./lib/ansible/modules/system/timezone.py -./lib/ansible/modules/system/ufw.py -./lib/ansible/modules/utilities/logic/wait_for.py -./lib/ansible/modules/web_infrastructure/letsencrypt.py -./lib/ansible/modules/web_infrastructure/rundeck_acl_policy.py -./lib/ansible/parsing/vault/__init__.py -./lib/ansible/playbook/base.py -./lib/ansible/playbook/helpers.py -./lib/ansible/playbook/role/__init__.py -./lib/ansible/playbook/taggable.py -./lib/ansible/plugins/callback/hipchat.py -./lib/ansible/plugins/connection/lxc.py -./lib/ansible/plugins/filter/core.py -./lib/ansible/plugins/lookup/sequence.py -./lib/ansible/plugins/strategy/__init__.py -./lib/ansible/plugins/strategy/linear.py -./test/legacy/cleanup_gce.py -./test/legacy/gce_credentials.py -./test/sanity/import/importer.py -./test/runner/lib/cloud/cs.py -./test/runner/lib/core_ci.py -./test/runner/lib/delegation.py -./test/runner/lib/docker_util.py -./test/runner/lib/executor.py -./test/runner/lib/http.py -./test/runner/lib/import_analysis.py -./test/runner/lib/manage_ci.py -./test/runner/lib/target.py -./test/runner/lib/util.py -./test/sanity/validate-modules/main.py -./test/units/executor/test_play_iterator.py -./test/units/module_utils/basic/test_run_command.py -./test/units/modules/cloud/amazon/test_ec2_vpc_nat_gateway.py -./test/units/modules/cloud/amazon/test_ec2_vpc_vpn.py -./test/units/modules/system/interfaces_file/test_interfaces_file.py -' - -for FILE in $TO_BE_FIXED ; do - GREP_FORMAT_WHITELIST="$GREP_FORMAT_WHITELIST -e $FILE" -done - -# GREP_FORMAT_WHITELIST has been formatted so that wordsplitting is wanted. Therefore no double quotes around the var -# shellcheck disable=SC2086 -underscore_as_variable=$(find . -path ./test/runner/.tox -prune \ - -path ./contrib/inventory/gce.py \ - -o -name '*.py' -type f -exec egrep -H '( |[^C]\()_( |,|\))' \{\} \+ | egrep -v $GREP_FORMAT_WHITELIST ) - - -if test -n "$underscore_as_variable" ; then - printf "\n== Underscore used as a variable ==\n" - printf "%s" "$underscore_as_variable" - failures=$(printf "%s" "$underscore_as_variable"| wc -l) - failures=$((failures + 2)) - exit "$failures" -fi - -exit 0 diff --git a/test/sanity/code-smell/replace-urlopen.json b/test/sanity/code-smell/replace-urlopen.json new file mode 100644 index 00000000000..776590b74d2 --- /dev/null +++ b/test/sanity/code-smell/replace-urlopen.json @@ -0,0 +1,6 @@ +{ + "extensions": [ + ".py" + ], + "output": "path-line-column-message" +} diff --git a/test/sanity/code-smell/replace-urlopen.py b/test/sanity/code-smell/replace-urlopen.py new file mode 100755 index 00000000000..91d9175f6b8 --- /dev/null +++ b/test/sanity/code-smell/replace-urlopen.py @@ -0,0 +1,29 @@ +#!/usr/bin/env python + +import os +import re +import sys + + +def main(): + skip = set([ + 'test/sanity/code-smell/%s' % os.path.basename(__file__), + 'lib/ansible/module_utils/six/__init__.py', + 'lib/ansible/module_utils/urls.py', + ]) + + for path in sys.argv[1:]: + if path in skip: + continue + + with open(path, 'r') as path_fd: + for line, text in enumerate(path_fd.readlines()): + match = re.search(r'^(?:[^#]*?)(urlopen)', text) + + if match: + print('%s:%d:%d: use `ansible.module_utils.urls.open_url` instead of `urlopen`' % ( + path, line + 1, match.start(1) + 1)) + + +if __name__ == '__main__': + main() diff --git a/test/sanity/code-smell/replace-urlopen.sh b/test/sanity/code-smell/replace-urlopen.sh deleted file mode 100755 index a5917b018ba..00000000000 --- a/test/sanity/code-smell/replace-urlopen.sh +++ /dev/null @@ -1,15 +0,0 @@ -#!/bin/sh - -urllib_users=$(find . -name '*.py' -exec grep -H urlopen '{}' '+' | grep -v \ - -e '^[^:]*/.tox/' \ - -e '^\./lib/ansible/module_utils/urls.py:' \ - -e '^\./lib/ansible/module_utils/six/__init__.py:' \ - -e '^[^:]*:#' -) - -if [ "${urllib_users}" ]; then - echo "${urllib_users}" - echo "One or more file(s) listed above use urlopen." - echo "Use open_url from module_utils instead of urlopen." - exit 1 -fi diff --git a/test/sanity/code-smell/use-argspec-type-path.json b/test/sanity/code-smell/use-argspec-type-path.json new file mode 100644 index 00000000000..aa531d39e7a --- /dev/null +++ b/test/sanity/code-smell/use-argspec-type-path.json @@ -0,0 +1,9 @@ +{ + "prefixes": [ + "lib/ansible/modules/" + ], + "extensions": [ + ".py" + ], + "output": "path-line-column-message" +} diff --git a/test/sanity/code-smell/use-argspec-type-path.py b/test/sanity/code-smell/use-argspec-type-path.py new file mode 100755 index 00000000000..36871d3f4e9 --- /dev/null +++ b/test/sanity/code-smell/use-argspec-type-path.py @@ -0,0 +1,44 @@ +#!/usr/bin/env python + +import re +import sys + + +def main(): + skip = set([ + # add legitimate uses of expanduser to the following list + 'lib/ansible/modules/cloud/lxc/lxc_container.py', + 'lib/ansible/modules/cloud/rackspace/rax_files_objects.py', + 'lib/ansible/modules/database/mongodb/mongodb_parameter.py', + 'lib/ansible/modules/database/mongodb/mongodb_user.py', + 'lib/ansible/modules/database/postgresql/postgresql_db.py', + 'lib/ansible/modules/files/synchronize.py', + 'lib/ansible/modules/source_control/git.py', + 'lib/ansible/modules/system/puppet.py', + 'lib/ansible/modules/utilities/logic/async_status.py', + 'lib/ansible/modules/utilities/logic/async_wrapper.py', + 'lib/ansible/modules/web_infrastructure/ansible_tower/tower_host.py', + 'lib/ansible/modules/web_infrastructure/ansible_tower/tower_group.py', + 'lib/ansible/modules/web_infrastructure/jenkins_plugin.py', + # fix uses of expanduser in the following modules and remove them from the following list + 'lib/ansible/modules/cloud/rackspace/rax.py', + 'lib/ansible/modules/cloud/rackspace/rax_scaling_group.py', + 'lib/ansible/modules/files/archive.py', + 'lib/ansible/modules/files/find.py', + ]) + + for path in sys.argv[1:]: + if path in skip: + continue + + with open(path, 'r') as path_fd: + for line, text in enumerate(path_fd.readlines()): + match = re.search(r'(expanduser)', text) + + if match: + print('%s:%d:%d: use argspec type="path" instead of type="str" to avoid use of `expanduser`' % ( + path, line + 1, match.start(1) + 1)) + + +if __name__ == '__main__': + main() diff --git a/test/sanity/code-smell/use-argspec-type-path.sh b/test/sanity/code-smell/use-argspec-type-path.sh deleted file mode 100755 index 2b3fa286c06..00000000000 --- a/test/sanity/code-smell/use-argspec-type-path.sh +++ /dev/null @@ -1,42 +0,0 @@ -#!/bin/sh - -# Add valid uses of expanduser to this list -WHITELIST='cloud/lxc/lxc_container.py -cloud/rackspace/rax_files_objects.py -database/mongodb/mongodb_parameter.py -database/mongodb/mongodb_user.py -database/postgresql/postgresql_db.py -files/synchronize.py -source_control/git.py -system/puppet.py -utilities/logic/async_status.py -utilities/logic/async_wrapper.py -web_infrastructure/ansible_tower/tower_host.py -web_infrastructure/ansible_tower/tower_group.py -web_infrastructure/jenkins_plugin.py' - -# Modules which need to be ported to get rid of expanduser. See: https://github.com/ansible/ansible/projects/12 -GRANDFATHERED_NEED_PORTING='cloud/rackspace/rax.py -cloud/rackspace/rax_scaling_group.py -files/find.py -files/archive.py' - -for FILE in $WHITELIST ; do - GREP_FORMAT_WHITELIST="$GREP_FORMAT_WHITELIST -e $FILE" -done - -for FILE in $GRANDFATHERED_NEED_PORTING ; do - GREP_FORMAT_WHITELIST="$GREP_FORMAT_WHITELIST -e $FILE" -done - -# GREP_FORMAT_WHITELIST has been formatted so that wordsplitting is wanted. Therefore no double quotes around the var -# shellcheck disable=SC2086 -egrep -r 'expanduser' lib/ansible/modules | egrep '\.py:' | egrep -v $GREP_FORMAT_WHITELIST - -if [ $? -ne 1 ]; then - printf 'The module(s) listed above use expanduser.\n' - printf 'This may indicate the module should be using an argpsec type="path" instead of type="str"\n' - printf 'If this is a false positive, add to the whitelist in:\n' - printf ' test/sanity/code-smell/use-argspec-type-path.sh\n' - exit 1 -fi diff --git a/test/sanity/code-smell/use-compat-six.json b/test/sanity/code-smell/use-compat-six.json new file mode 100644 index 00000000000..776590b74d2 --- /dev/null +++ b/test/sanity/code-smell/use-compat-six.json @@ -0,0 +1,6 @@ +{ + "extensions": [ + ".py" + ], + "output": "path-line-column-message" +} diff --git a/test/sanity/code-smell/use-compat-six.py b/test/sanity/code-smell/use-compat-six.py new file mode 100755 index 00000000000..a7fe13fb0a2 --- /dev/null +++ b/test/sanity/code-smell/use-compat-six.py @@ -0,0 +1,52 @@ +#!/usr/bin/env python + +import os +import re +import sys + + +def main(): + skip = set([ + 'test/sanity/code-smell/%s' % os.path.basename(__file__), + # digital_ocean is checking for six because dopy doesn't specify the + # requirement on six so it needs to try importing six to give the correct error message + 'lib/ansible/modules/cloud/digital_ocean/digital_ocean.py', + # correct imports in the following files and remove them from this list + 'contrib/inventory/apache-libcloud.py', + 'contrib/inventory/cobbler.py', + 'contrib/inventory/collins.py', + 'contrib/inventory/consul_io.py', + 'contrib/inventory/ec2.py', + 'contrib/inventory/freeipa.py', + 'contrib/inventory/lxd.py', + 'contrib/inventory/nova.py', + 'contrib/inventory/nsot.py', + 'contrib/inventory/packet_net.py', + 'contrib/inventory/proxmox.py', + 'contrib/inventory/rax.py', + 'contrib/inventory/rudder.py', + 'contrib/inventory/scaleway.py', + 'contrib/inventory/spacewalk.py', + 'contrib/inventory/vmware.py', + 'contrib/inventory/vmware_inventory.py', + 'docs/bin/plugin_formatter.py', + ]) + + for path in sys.argv[1:]: + if path in skip: + continue + + if path.startswith('test/sanity/pylint/plugins/'): + continue + + with open(path, 'r') as path_fd: + for line, text in enumerate(path_fd.readlines()): + match = re.search(r'((^\s*import\s+six\b)|(^\s*from\s+six\b))', text) + + if match: + print('%s:%d:%d: use `ansible.module_utils.six` instead of `six`' % ( + path, line + 1, match.start(1) + 1)) + + +if __name__ == '__main__': + main() diff --git a/test/sanity/code-smell/use-compat-six.sh b/test/sanity/code-smell/use-compat-six.sh deleted file mode 100755 index d81b8f4ca48..00000000000 --- a/test/sanity/code-smell/use-compat-six.sh +++ /dev/null @@ -1,21 +0,0 @@ -#!/bin/sh - -# Do we want to check dynamic inventory, bin, etc? -BASEDIR=${1-"lib"} -# REGEX of individual files where the import of six is not a problem -# digital_ocean is checking for six because dopy doesn't specify the -# requirement on six so it needs to try importing six to give the correct error -# message -WHITELIST='(lib/ansible/modules/cloud/digital_ocean/digital_ocean.py)' - -SIX_USERS=$(find "$BASEDIR" -name '*.py' -exec grep -wH six '{}' '+' \ - | grep import \ - | grep -v ansible.module_utils.six \ - | grep -v 'ansible.module_utils import six' \ - | egrep -v "^$WHITELIST:" -) - -if [ "${SIX_USERS}" ]; then - echo "${SIX_USERS}" - exit 1 -fi