Add support for enhanced code-smell tests. (#36332)

* Add support for enhanced code-smell tests:

- Path selection handled by ansible-test.
- Optional path filtering based on extension.
- Optional path filtering based on prefixes.
- Optional lint friendly output.

* Enhance no-assert code-smell test.
* Enhance no-tests-as-filters code-smell test.

test/runner/lib/sanity/__init__.py

@@ -3,6 +3,7 @@ from __future__ import absolute_import, print_function
 
 import abc
 import glob
+import json
 import os
 import re
 
@@ -13,6 +14,7 @@ from lib.util import (
     run_command,
     import_plugins,
     load_plugins,
+    parse_to_dict,
     ABC,
 )
 
@@ -94,7 +96,7 @@ def command_sanity(args):
             options = ''
 
             if isinstance(test, SanityCodeSmellTest):
-                result = test.test(args)
+                result = test.test(args, targets)
             elif isinstance(test, SanityMultipleVersion):
                 result = test.test(args, targets, python_version=version)
                 options = ' --python %s' % version
@@ -205,19 +207,52 @@ class SanityCodeSmellTest(SanityTest):
     """Sanity test script."""
     def __init__(self, path):
         name = os.path.splitext(os.path.basename(path))[0]
+        config = os.path.splitext(path)[0] + '.json'
 
         self.path = path
+        self.config = config if os.path.exists(config) else None
 
         super(SanityCodeSmellTest, self).__init__(name)
 
-    def test(self, args):
+    def test(self, args, targets):
         """
         :type args: SanityConfig
+        :type targets: SanityTargets
         :rtype: SanityResult
         """
         cmd = [self.path]
         env = ansible_environment(args, color=False)
 
+        pattern = None
+
+        if self.config:
+            with open(self.config, 'r') as config_fd:
+                config = json.load(config_fd)
+
+            output = config.get('output')
+            extensions = config.get('extensions')
+            prefixes = config.get('prefixes')
+
+            if output == 'path-line-column-message':
+                pattern = '^(?P<path>[^:]*):(?P<line>[0-9]+):(?P<column>[0-9]+): (?P<message>.*)$'
+            elif output == 'path-message':
+                pattern = '^(?P<path>[^:]*): (?P<message>.*)$'
+            else:
+                pattern = ApplicationError('Unsupported output type: %s' % output)
+
+            paths = sorted(i.path for i in targets.include)
+
+            if extensions:
+                paths = [p for p in paths if os.path.splitext(p)[1] in extensions or (p.startswith('bin/') and '.py' in extensions)]
+
+            if prefixes:
+                paths = [p for p in paths if any(p.startswith(pre) for pre in prefixes)]
+
+            if not paths:
+                return SanitySkipped(self.name)
+
+            cmd += paths
+
         try:
             stdout, stderr = run_command(args, cmd, env=env, capture=True)
             status = 0
@@ -226,6 +261,19 @@ class SanityCodeSmellTest(SanityTest):
             stderr = ex.stderr
             status = ex.status
 
+        if stdout and not stderr:
+            if pattern:
+                matches = [parse_to_dict(pattern, line) for line in stdout.splitlines()]
+
+                messages = [SanityMessage(
+                    message=m['message'],
+                    path=m['path'],
+                    line=int(m.get('line', 0)),
+                    column=int(m.get('column', 0)),
+                ) for m in matches]
+
+                return SanityFailure(self.name, messages=messages)
+
         if stderr or status:
             summary = u'%s' % SubprocessError(cmd=cmd, status=status, stderr=stderr, stdout=stdout)
             return SanityFailure(self.name, summary=summary)

test/sanity/code-smell/no-assert.json

@@ -0,0 +1,10 @@
+{
+    "extensions": [
+        ".py"
+    ],
+    "prefixes": [
+        "bin/",
+        "lib/ansible/"
+    ],
+    "output": "path-line-column-message"
+}

test/sanity/code-smell/no-assert.py

@@ -1,40 +1,29 @@
 #!/usr/bin/env python
 from __future__ import print_function
 
-import os
 import re
 import sys
 
-from collections import defaultdict
-
-PATH = 'lib/ansible'
 ASSERT_RE = re.compile(r'.*(?<![-:a-zA-Z#][ -])\bassert\b(?!:).*')
 
-all_matches = defaultdict(list)
 
-for dirpath, dirnames, filenames in os.walk(PATH):
-    for filename in filenames:
-        path = os.path.join(dirpath, filename)
-        if not os.path.isfile(path) or not path.endswith('.py'):
-            continue
+def main():
+    failed = False
 
+    for path in sys.argv[1:]:
         with open(path, 'r') as f:
             for i, line in enumerate(f.readlines()):
                 matches = ASSERT_RE.findall(line)
 
                 if matches:
-                    all_matches[path].append((i + 1, line.index('assert') + 1, matches))
+                    failed = True
+                    lineno = i + 1
+                    colno = line.index('assert') + 1
+                    print('%s:%d:%d: raise AssertionError instead of: %s' % (path, lineno, colno, matches[0][colno - 1:]))
+
+    if failed:
+        sys.exit(1)
 
 
-if all_matches:
-    print('Use of assert in production code is not recommended.')
-    print('Python will remove all assert statements if run with optimizations')
-    print('Alternatives:')
-    print('    if not isinstance(value, dict):')
-    print('        raise AssertionError("Expected a dict for value")')
-
-    for path, matches in all_matches.items():
-        for line_matches in matches:
-            for match in line_matches[2]:
-                print('%s:%d:%d: %s' % ((path,) + line_matches[:2] + (match,)))
-    sys.exit(1)
+if __name__ == '__main__':
+    main()

test/sanity/code-smell/no-tests-as-filters.json

@@ -0,0 +1,7 @@
+{
+    "extensions": [
+        ".yml",
+        ".yaml"
+    ],
+    "output": "path-line-column-message"
+}

test/sanity/code-smell/no-tests-as-filters.py

@@ -19,12 +19,9 @@
 
 from __future__ import print_function
 
-import os
 import re
 import sys
 
-from collections import defaultdict
-
 from ansible.plugins.test import core, files, mathstuff
 
 TESTS = list(core.TestModule().tests().keys()) + list(files.TestModule().tests().keys()) + list(mathstuff.TestModule().tests().keys())
@@ -48,41 +45,42 @@ TEST_MAP = {
 }
 
 
-FILTER_RE = re.compile(r'((.+?)\s*([\w \.\'"]+)(\s*)\|(\s*)(\w+))')
+FILTER_RE = re.compile(r'((.+?)\s*(?P<left>[\w \.\'"]+)(\s*)\|(\s*)(?P<filter>\w+))')
 
 
 def main():
-    all_matches = defaultdict(list)
+    failed = False
 
-    for root, dirs, filenames in os.walk('.'):
-        for name in filenames:
-            if os.path.splitext(name)[1] not in ('.yml', '.yaml'):
+    for path in sys.argv[1:]:
+        with open(path) as f:
+            text = f.read()
+
+        lines = text.splitlines(True)
+        previous = 0
+        offset = 0
+        lineno = 0
+
+        for match in FILTER_RE.finditer(text):
+            filter_name = match.group('filter')
+            test_name = TEST_MAP.get(filter_name, filter_name)
+
+            if test_name not in TESTS:
                 continue
-            path = os.path.join(root, name)
 
-            with open(path) as f:
-                text = f.read()
+            failed = True
+            left = match.group('left').strip()
+            start = match.start('left')
 
-            for match in FILTER_RE.findall(text):
-                filter_name = match[5]
+            while start >= offset:
+                previous = offset
+                offset += len(lines[lineno])
+                lineno += 1
 
-                try:
-                    test_name = TEST_MAP[filter_name]
-                except KeyError:
-                    test_name = filter_name
+            colno = start - previous + 1
 
-                if test_name not in TESTS:
-                    continue
+            print('%s:%d:%d: use `%s is %s` instead of `%s | %s`' % (path, lineno, colno, left, test_name, left, filter_name))
 
-                all_matches[path].append(match[0])
-
-    if all_matches:
-        print('Use of Ansible provided Jinja2 tests as filters is deprecated.')
-        print('Please update to use `is` syntax such as `result is failed`.')
-
-        for path, matches in all_matches.items():
-            for match in matches:
-                print('%s: %s' % (path, match,))
+    if failed:
         sys.exit(1)