From 912d6ed8cc261ac02fc514c89e04b3f48a7e6d68 Mon Sep 17 00:00:00 2001
From: Matt Clay <matt@mystile.com>
Date: Mon, 7 Nov 2016 16:46:33 -0800
Subject: [PATCH] Clean up code-smell sanity scripts. (#18407)

- Replace find ';' with '+' for faster execution.
- Replace grep -R with -r to avoid recursive warnings.
- Exclude .git and .tox directories from recursive grep.
- Improve messaging on failed sanity checks.
- Add no-basestring check to Shippable.
---
 test/sanity/code-smell/boilerplate.sh     |  8 ++++----
 test/sanity/code-smell/line-endings.sh    |  5 ++---
 test/sanity/code-smell/no-basestring.sh   | 24 +++++++++--------------
 test/sanity/code-smell/replace-urlopen.sh | 21 +++++++++++---------
 test/sanity/code-smell/shebang.sh         |  6 ++++--
 test/sanity/code-smell/use-compat-six.sh  | 16 +++++++++------
 test/utils/shippable/code-smell.sh        |  5 +++--
 7 files changed, 44 insertions(+), 41 deletions(-)

diff --git a/test/sanity/code-smell/boilerplate.sh b/test/sanity/code-smell/boilerplate.sh
index 87fb8c92d21..213893901a3 100755
--- a/test/sanity/code-smell/boilerplate.sh
+++ b/test/sanity/code-smell/boilerplate.sh
@@ -1,7 +1,7 @@
 #!/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)' \{\} \;)
+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)' '{}' '+')
 
 metaclass2=$(find ./lib/ansible -path ./lib/ansible/modules/core -prune \
         -o -path ./lib/ansible/modules/__init__.py \
@@ -9,7 +9,7 @@ metaclass2=$(find ./lib/ansible -path ./lib/ansible/modules/core -prune \
         -o -path ./lib/ansible/module_utils -prune \
         -o -path ./lib/ansible/compat/six/_six.py -prune \
         -o -path ./lib/ansible/utils/module_docs_fragments -prune \
-        -o -name '*.py' -exec grep -HL '__metaclass__ = type' \{\} \;)
+        -o -name '*.py' -exec grep -HL '__metaclass__ = type' '{}' '+')
 
 future2=$(find ./lib/ansible -path ./lib/ansible/modules/core -prune \
         -o -path ./lib/ansible/modules/__init__.py \
@@ -17,7 +17,7 @@ future2=$(find ./lib/ansible -path ./lib/ansible/modules/core -prune \
         -o -path ./lib/ansible/module_utils -prune \
         -o -path ./lib/ansible/compat/six/_six.py -prune \
         -o -path ./lib/ansible/utils/module_docs_fragments -prune \
-        -o -name '*.py' -exec grep -HL 'from __future__ import (absolute_import, division, print_function)' \{\} \;)
+        -o -name '*.py' -exec grep -HL 'from __future__ import (absolute_import, division, print_function)' '{}' '+')
 
 ### TODO:
 ### - contrib/
diff --git a/test/sanity/code-smell/line-endings.sh b/test/sanity/code-smell/line-endings.sh
index d97b1143dc2..fc8d65a1aeb 100755
--- a/test/sanity/code-smell/line-endings.sh
+++ b/test/sanity/code-smell/line-endings.sh
@@ -1,9 +1,8 @@
 #!/bin/sh
 
-grep -RIPl '\r' . 2>/dev/null \
+grep -rIPl '\r' . \
     --exclude-dir .git \
-    | grep -v -E \
-    -e '/.tox/' \
+    --exclude-dir .tox \
     | grep -v -F \
     -e './test/integration/targets/win_regmerge/templates/win_line_ending.j2'
 
diff --git a/test/sanity/code-smell/no-basestring.sh b/test/sanity/code-smell/no-basestring.sh
index 59a6c9bf557..27757a9c447 100755
--- a/test/sanity/code-smell/no-basestring.sh
+++ b/test/sanity/code-smell/no-basestring.sh
@@ -1,24 +1,18 @@
 #!/bin/sh
 
-BASEDIR=${1-"."}
-
-# Not entirely correct but
-# * basestring is still present and harmless in comments
-# * basestring is also currently present in modules.  Porting of modules is more
-#   of an Ansible 2.3 or greater goal.
-BASESTRING_USERS=$(grep -r basestring "${BASEDIR}" \
+BASESTRING_USERS=$(grep -r basestring . \
+    --exclude-dir .git \
+    --exclude-dir .tox \
     | grep isinstance \
     | grep -v \
     -e lib/ansible/compat/six/_six.py \
     -e lib/ansible/module_utils/six.py \
-    -e lib/ansible/modules/core \
-    -e lib/ansible/modules/extras \
-    -e '/.tox/' \
+    -e lib/ansible/modules/core/ \
+    -e lib/ansible/modules/extras/ \
+    -e '^[^:]*:#'
     )
 
-if test -n "${BASESTRING_USERS}"; then
-  printf "%s" "${BASESTRING_USERS}"
-  exit 1
-else
-  exit 0
+if [ "${BASESTRING_USERS}" ]; then
+    echo "${BASESTRING_USERS}"
+    exit 1
 fi
diff --git a/test/sanity/code-smell/replace-urlopen.sh b/test/sanity/code-smell/replace-urlopen.sh
index 5bef3bc21e3..6bd0f6ee8cb 100755
--- a/test/sanity/code-smell/replace-urlopen.sh
+++ b/test/sanity/code-smell/replace-urlopen.sh
@@ -1,13 +1,16 @@
 #!/bin/sh
 
-BASEDIR=${1-"."}
+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.py:' \
+    -e '^\./lib/ansible/compat/six/_six.py:' \
+    -e '^[^:]*:#'
+)
 
-URLLIB_USERS=$(find "$BASEDIR" -name '*.py' -exec grep -H urlopen \{\} \;)
-URLLIB_USERS=$(echo "$URLLIB_USERS" | sed '/\(\n\|lib\/ansible\/module_utils\/urls.py\|lib\/ansible\/module_utils\/six.py\|lib\/ansible\/compat\/six\/_six.py\|.tox\)/d')
-URLLIB_USERS=$(echo "$URLLIB_USERS" | sed '/^[^:]\+:#/d')
-if test -n "$URLLIB_USERS" ; then
-  printf "%s" "$URLLIB_USERS"
-  exit 1
-else
-  exit 0
+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/shebang.sh b/test/sanity/code-smell/shebang.sh
index b392e8f9816..9e473810505 100755
--- a/test/sanity/code-smell/shebang.sh
+++ b/test/sanity/code-smell/shebang.sh
@@ -1,7 +1,9 @@
 #!/bin/sh
 
-grep '^#!' -RIn . 2>/dev/null | grep ':1:' | sed 's/:1:/:/' | grep -v -E \
-    -e '/.tox/' \
+grep '^#!' -rIn . \
+    --exclude-dir .git \
+    --exclude-dir .tox \
+    | grep ':1:' | sed 's/:1:/:/' | grep -v -E \
     -e '^\./lib/ansible/modules/' \
     -e '^\./test/integration/targets/[^/]*/library/[^/]*:#!powershell$' \
     -e '^\./test/sanity/validate-modules/validate-modules:#!/usr/bin/env python2$' \
diff --git a/test/sanity/code-smell/use-compat-six.sh b/test/sanity/code-smell/use-compat-six.sh
index cdce0ec0e35..0fa120e82ac 100755
--- a/test/sanity/code-smell/use-compat-six.sh
+++ b/test/sanity/code-smell/use-compat-six.sh
@@ -8,10 +8,14 @@ BASEDIR=${1-"lib"}
 #   message
 WHITELIST='(lib/ansible/modules/core/cloud/digital_ocean/digital_ocean.py)'
 
-SIX_USERS=$(find "$BASEDIR" -name '*.py' -exec grep -wH six \{\} \;|grep import |grep -v ansible.compat| grep -v ansible.module_utils.six| egrep -v "^$WHITELIST:")
-if test -n "$SIX_USERS" ; then
-  printf "%s" "$SIX_USERS"
-  exit 1
-else
-  exit 0
+SIX_USERS=$(find "$BASEDIR" -name '*.py' -exec grep -wH six '{}' '+' \
+    | grep import \
+    | grep -v ansible.compat \
+    | grep -v ansible.module_utils.six \
+    | egrep -v "^$WHITELIST:"
+)
+
+if [ "${SIX_USERS}" ]; then
+    echo "${SIX_USERS}"
+    exit 1
 fi
diff --git a/test/utils/shippable/code-smell.sh b/test/utils/shippable/code-smell.sh
index a8407d45403..dbcf81c7ba0 100755
--- a/test/utils/shippable/code-smell.sh
+++ b/test/utils/shippable/code-smell.sh
@@ -16,13 +16,14 @@ if [ "${install_deps}" != "" ]; then
 fi
 
 yamllint .
-test/sanity/code-smell/replace-urlopen.sh .
-test/sanity/code-smell/use-compat-six.sh lib
+test/sanity/code-smell/replace-urlopen.sh
+test/sanity/code-smell/use-compat-six.sh
 test/sanity/code-smell/boilerplate.sh
 test/sanity/code-smell/required-and-default-attributes.sh
 test/sanity/code-smell/shebang.sh
 test/sanity/code-smell/line-endings.sh
 test/sanity/code-smell/empty-init.sh
+test/sanity/code-smell/no-basestring.sh
 
 shellcheck \
     test/integration/targets/*/*.sh \