From 2fbfba0ef3d6056ee0bbcf50d317f38490aad1cf Mon Sep 17 00:00:00 2001
From: Matt Martz <matt@sivel.net>
Date: Wed, 3 May 2017 10:25:08 -0500
Subject: [PATCH] Make Reporter class hold all results, move line/col into
 results, and out of message (#24127)

* Make Reporter class hold all results, move line/col into results, and out of message

* Move line/col out of message for YAML parser errors

* We have lineno for the DOC var, use it for YAML parser errors

* Remove valdiate-modules files from legacy-files

* pep8 indentation fixes

* Add todo for line/col in _validate_docs_schema
---
 test/sanity/pep8/legacy-files.txt             |   2 -
 test/sanity/validate-modules/module_args.py   |   2 +-
 test/sanity/validate-modules/schema.py        |   2 +-
 test/sanity/validate-modules/utils.py         |  22 +-
 test/sanity/validate-modules/validate-modules | 583 +++++++++++-------
 5 files changed, 385 insertions(+), 226 deletions(-)

diff --git a/test/sanity/pep8/legacy-files.txt b/test/sanity/pep8/legacy-files.txt
index 3a462537ec6..63062f09188 100644
--- a/test/sanity/pep8/legacy-files.txt
+++ b/test/sanity/pep8/legacy-files.txt
@@ -965,8 +965,6 @@ test/integration/setup_gce.py
 test/integration/targets/async/library/async_test.py
 test/integration/targets/uri/files/testserver.py
 test/sanity/code-smell/ansible-var-precedence-check.py
-test/sanity/validate-modules/module_args.py
-test/sanity/validate-modules/schema.py
 test/units/cli/test_galaxy.py
 test/units/contrib/inventory/test_vmware_inventory.py
 test/units/errors/test_errors.py
diff --git a/test/sanity/validate-modules/module_args.py b/test/sanity/validate-modules/module_args.py
index 473e69d42f8..2d8a6909b59 100644
--- a/test/sanity/validate-modules/module_args.py
+++ b/test/sanity/validate-modules/module_args.py
@@ -70,7 +70,7 @@ def add_mocks(filename):
         if [s for s in sources if s[:7] in ['ansible', '__main_']]:
             parts = module.split('.')
             for i in range(len(parts)):
-                dotted = '.'.join(parts[:i+1])
+                dotted = '.'.join(parts[:i + 1])
                 # Never mock out ansible or ansible.module_utils
                 # we may get here if a specific module_utils file needed mocked
                 if dotted in ('ansible', 'ansible.module_utils',):
diff --git a/test/sanity/validate-modules/schema.py b/test/sanity/validate-modules/schema.py
index 008e53bc069..42ce90c45bc 100644
--- a/test/sanity/validate-modules/schema.py
+++ b/test/sanity/validate-modules/schema.py
@@ -81,6 +81,7 @@ def return_schema(data):
         extra=PREVENT_EXTRA
     )
 
+
 def doc_schema(module_name):
     if module_name.startswith('_'):
         module_name = module_name[1:]
@@ -116,7 +117,6 @@ def metadata_schema(deprecated):
     )
 
 
-
 # Things to add soon
 ####################
 # 1) Recursively validate `type: complex` fields
diff --git a/test/sanity/validate-modules/utils.py b/test/sanity/validate-modules/utils.py
index c997de64a69..280cb48e64d 100644
--- a/test/sanity/validate-modules/utils.py
+++ b/test/sanity/validate-modules/utils.py
@@ -85,17 +85,25 @@ def parse_yaml(value, lineno, module, name, load_all=False):
     except yaml.MarkedYAMLError as e:
         e.problem_mark.line += lineno - 1
         e.problem_mark.name = '%s.%s' % (module, name)
-        errors.append('%s is not valid YAML. Line %d column %d' %
-                      (name, e.problem_mark.line + 1,
-                       e.problem_mark.column + 1))
+        errors.append({
+            'msg': '%s is not valid YAML' % name,
+            'line': e.problem_mark.line + 1,
+            'column': e.problem_mark.column + 1
+        })
         traces.append(e)
     except yaml.reader.ReaderError as e:
         traces.append(e)
-        errors.append('%s is not valid YAML. Character '
-                      '0x%x at position %d.' %
-                      (name, e.character, e.position))
+        # TODO: Better line/column detection
+        errors.append({
+            'msg': ('%s is not valid YAML. Character '
+                    '0x%x at position %d.' % (name, e.character, e.position)),
+            'line': lineno
+        })
     except yaml.YAMLError as e:
         traces.append(e)
-        errors.append('%s is not valid YAML: %s: %s' % (name, type(e), e))
+        errors.append({
+            'msg': '%s is not valid YAML: %s: %s' % (name, type(e), e),
+            'line': lineno
+        })
 
     return data, errors, traces
diff --git a/test/sanity/validate-modules/validate-modules b/test/sanity/validate-modules/validate-modules
index 717b7b95376..399378d57cd 100755
--- a/test/sanity/validate-modules/validate-modules
+++ b/test/sanity/validate-modules/validate-modules
@@ -41,7 +41,7 @@ from ansible.utils.plugin_docs import BLACKLIST, get_docstring
 
 from module_args import get_argument_spec
 
-from schema import doc_schema, option_schema, metadata_schema, return_schema
+from schema import doc_schema, metadata_schema, return_schema
 
 from utils import CaptureStd, parse_yaml
 from voluptuous.humanize import humanize_error
@@ -63,18 +63,18 @@ TYPE_REGEX = re.compile(r'.*(if|or)(\s+[^"\']*|\s+)(?<!_)(?<!str\()type\(.*')
 BLACKLIST_IMPORTS = {
     'requests': {
         'new_only': True,
-        'msg': (
-            203,
-            ('requests import found, should use '
-             'ansible.module_utils.urls instead')
-        )
+        'error': {
+            'code': 203,
+            'msg': ('requests import found, should use '
+                    'ansible.module_utils.urls instead')
+        }
     },
     'boto(?:\.|$)': {
         'new_only': True,
-        'msg': (
-            204,
-            ('boto import found, new modules should use boto3')
-        )
+        'error': {
+            'code': 204,
+            'msg': 'boto import found, new modules should use boto3'
+        }
     },
 }
 
@@ -88,6 +88,43 @@ class ReporterEncoder(json.JSONEncoder):
 
 
 class Reporter(object):
+    def __init__(self):
+        self.files = OrderedDict()
+
+    def _ensure_default_entry(self, path):
+        try:
+            self.files[path]
+        except KeyError:
+            self.files[path] = {
+                'errors': [],
+                'warnings': [],
+                'traces': [],
+                'warning_traces': []
+            }
+
+    def _log(self, path, code, msg, level='error', line=0, column=0):
+        self._ensure_default_entry(path)
+        lvl_dct = self.files[path]['%ss' % level]
+        lvl_dct.append({
+            'code': code,
+            'msg': msg,
+            'line': line,
+            'column': column
+        })
+
+    def error(self, *args, **kwargs):
+        self._log(*args, level='error', **kwargs)
+
+    def warning(self, *args, **kwargs):
+        self._log(*args, level='warning', **kwargs)
+
+    def trace(self, path, tracebk):
+        self._ensure_default_entry(path)
+        self.files[path]['traces'].append(tracebk)
+
+    def warning_trace(self, path, tracebk):
+        self._ensure_default_entry(path)
+        self.files[path]['warning_traces'].append(tracebk)
 
     @staticmethod
     @contextmanager
@@ -111,20 +148,14 @@ class Reporter(object):
 
         return temp_reports
 
-    @staticmethod
-    def plain(reports, warnings=False, output='-'):
+    def plain(self, warnings=False, output='-'):
         """Print out the test results in plain format
 
         output is ignored here for now
         """
         ret = []
 
-        for path, report in Reporter._filter_out_ok(reports).items():
-            if report['errors'] or (warnings and report['warnings']):
-                print('=' * 76)
-                print(path)
-                print('=' * 76)
-
+        for path, report in Reporter._filter_out_ok(self.files).items():
             traces = report['traces'][:]
             if warnings and report['warnings']:
                 traces.extend(report['warning_traces'])
@@ -133,28 +164,25 @@ class Reporter(object):
                 print('TRACE:')
                 print('\n    '.join(('    %s' % trace).splitlines()))
             for error in report['errors']:
-                print('ERROR:%(code)d:%(msg)s' % error)
+                error['path'] = path
+                print('%(path)s:%(line)d:%(column)d: E%(code)d %(msg)s' % error)
                 ret.append(1)
             if warnings:
                 for warning in report['warnings']:
-                    print('WARNING:%(code)d:%(msg)s' % warning)
-                    # ret.append(1)  # Don't incrememt exit status for warnings
-
-            if report['errors'] or (warnings and report['warnings']):
-                print()
+                    warning['path'] = path
+                    print('%(path)s:%(line)d:%(column)d: W%(code)d %(msg)s' % warning)
 
         return 3 if ret else 0
 
-    @staticmethod
-    def json(reports, warnings=False, output='-'):
+    def json(self, warnings=False, output='-'):
         """Print out the test results in json format
 
         warnings is not respected in this output
         """
-        ret = [len(r['errors']) for _, r in reports.items()]
+        ret = [len(r['errors']) for _, r in self.files.items()]
 
         with Reporter._output_handle(output) as handle:
-            print(json.dumps(Reporter._filter_out_ok(reports), indent=4, cls=ReporterEncoder), file=handle)
+            print(json.dumps(Reporter._filter_out_ok(self.files), indent=4, cls=ReporterEncoder), file=handle)
 
         return 3 if sum(ret) else 0
 
@@ -164,15 +192,8 @@ class Validator(with_metaclass(abc.ABCMeta, object)):
     are scanning multiple objects for problems, you'll want to have a separate
     Validator for each one."""
 
-    def __init__(self):
-        self.reset()
-
-    def reset(self):
-        """Reset the test results"""
-        self.errors = []
-        self.warnings = []
-        self.traces = []
-        self.warning_traces = []
+    def __init__(self, reporter=None):
+        self.reporter = reporter
 
     @abc.abstractproperty
     def object_name(self):
@@ -185,20 +206,9 @@ class Validator(with_metaclass(abc.ABCMeta, object)):
         pass
 
     @abc.abstractmethod
-    def validate(self, reset=True):
+    def validate(self):
         """Run this method to generate the test results"""
-        if reset:
-            self.reset()
-
-    def report(self):
-        return {
-            self.object_path: OrderedDict([
-                ('errors', [{'code': code, 'msg': msg} for code, msg in self.errors]),
-                ('traces', self.traces[:]),
-                ('warnings', [{'code': code, 'msg': msg} for code, msg in self.warnings]),
-                ('warning_traces', self.warning_traces[:])
-            ])
-        }
+        pass
 
 
 class ModuleValidator(Validator):
@@ -215,8 +225,8 @@ class ModuleValidator(Validator):
         'setup.ps1'
     ))
 
-    def __init__(self, path, analyze_arg_spec=False, base_branch=None, git_cache=None):
-        super(ModuleValidator, self).__init__()
+    def __init__(self, path, analyze_arg_spec=False, base_branch=None, git_cache=None, reporter=None):
+        super(ModuleValidator, self).__init__(reporter=reporter or Reporter())
 
         self.path = path
         self.basename = os.path.basename(self.path)
@@ -315,11 +325,19 @@ class ModuleValidator(Validator):
     def _check_interpreter(self, powershell=False):
         if powershell:
             if not self.text.startswith('#!powershell\n'):
-                self.errors.append((102, 'Interpreter line is not "#!powershell"'))
+                self.reporter.error(
+                    path=self.object_path,
+                    code=102,
+                    msg='Interpreter line is not "#!powershell"'
+                )
             return
 
         if not self.text.startswith('#!/usr/bin/python'):
-            self.errors.append((101, 'Interpreter line is not "#!/usr/bin/python"'))
+            self.reporter.error(
+                path=self.object_path,
+                code=101,
+                msg='Interpreter line is not "#!/usr/bin/python"'
+            )
 
     def _check_type_instead_of_isinstance(self, powershell=False):
         if powershell:
@@ -327,35 +345,45 @@ class ModuleValidator(Validator):
         for line_no, line in enumerate(self.text.splitlines()):
             typekeyword = TYPE_REGEX.match(line)
             if typekeyword:
-                self.errors.append((
-                    403,
-                    ('Type comparison using type() found on '
-                     'line %d. Use isinstance() instead' % (line_no + 1))
-                ))
+                # TODO: add column
+                self.reporter.error(
+                    path=self.object_path,
+                    code=403,
+                    msg=('Type comparison using type() found. '
+                         'Use isinstance() instead'),
+                    line=line_no + 1
+                )
 
     def _check_for_sys_exit(self):
         if 'sys.exit(' in self.text:
-            self.errors.append(
-                (
-                    205,
-                    'sys.exit() call found. Should be exit_json/fail_json'
-                )
+            # TODO: Add line/col
+            self.reporter.error(
+                path=self.object_path,
+                code=205,
+                msg='sys.exit() call found. Should be exit_json/fail_json'
             )
 
     def _check_for_gpl3_header(self):
         if ('GNU General Public License' not in self.text and
                 'version 3' not in self.text):
-            self.errors.append((105, 'GPLv3 license header not found'))
+            self.reporter.error(
+                path=self.object_path,
+                code=105,
+                msg='GPLv3 license header not found'
+            )
 
     def _check_for_tabs(self):
         for line_no, line in enumerate(self.text.splitlines()):
             indent = INDENT_REGEX.search(line)
             if indent and '\t' in line:
                 index = line.index('\t')
-                self.errors.append((
-                    402,
-                    'indentation contains tabs. line %d column %d' % (line_no + 1, index)
-                ))
+                self.reporter.error(
+                    path=self.object_path,
+                    code=402,
+                    msg='indentation contains tabs',
+                    line=line_no + 1,
+                    column=index
+                )
 
     def _find_blacklist_imports(self):
         for child in self.ast.body:
@@ -370,14 +398,20 @@ class ModuleValidator(Validator):
                     if isinstance(grandchild, ast.Import):
                         names.extend(grandchild.names)
             for name in names:
+                # TODO: Add line/col
                 for blacklist_import, options in BLACKLIST_IMPORTS.items():
                     if re.search(blacklist_import, name.name):
-                        msg = options['msg']
                         new_only = options['new_only']
                         if self._is_new_module() and new_only:
-                            self.errors.append(msg)
+                            self.reporter.error(
+                                path=self.object_path,
+                                **options['error']
+                            )
                         elif not new_only:
-                            self.errors.append(msg)
+                            self.reporter.error(
+                                path=self.object_path,
+                                **options['error']
+                            )
 
     def _find_module_utils(self, main):
         linenos = []
@@ -403,25 +437,38 @@ class ModuleValidator(Validator):
                             msg = (
                                 208,
                                 ('module_utils imports should import specific '
-                                 'components, not "*". line %d' % child.lineno)
+                                 'components, not "*"')
                             )
                             if self._is_new_module():
-                                self.errors.append(msg)
+                                self.reporter.error(
+                                    path=self.object_path,
+                                    code=msg[0],
+                                    msg=msg[1],
+                                    line=child.lineno
+                                )
                             else:
-                                self.warnings.append(msg)
+                                self.reporter.warning(
+                                    path=self.object_path,
+                                    code=msg[0],
+                                    msg=msg[1],
+                                    line=child.lineno
+                                )
 
                         if (isinstance(name, ast.alias) and
                                 name.name == 'basic'):
                             found_basic = True
 
         if not linenos:
-            self.errors.append((201, 'Did not find a module_utils import'))
+            self.reporter.error(
+                path=self.object_path,
+                code=201,
+                msg='Did not find a module_utils import'
+            )
         elif not found_basic:
-            self.warnings.append(
-                (
-                    292,
-                    'Did not find "ansible.module_utils.basic" import'
-                )
+            self.reporter.warning(
+                path=self.object_path,
+                code=292,
+                msg='Did not find "ansible.module_utils.basic" import'
             )
 
         return linenos
@@ -455,13 +502,19 @@ class ModuleValidator(Validator):
                             child.value.func.id == 'main'):
                         lineno = child.lineno
                         if lineno < self.length - 1:
-                            self.errors.append((
-                                104,
-                                'Call to main() not the last line'
-                            ))
+                            self.reporter.error(
+                                path=self.object_path,
+                                code=104,
+                                msg='Call to main() not the last line',
+                                line=lineno
+                            )
 
         if not lineno:
-            self.errors.append((103, 'Did not find a call to main'))
+            self.reporter.error(
+                path=self.object_path,
+                code=103,
+                msg='Did not find a call to main'
+            )
 
         return lineno or 0
 
@@ -481,10 +534,12 @@ class ModuleValidator(Validator):
                             if target.id.lower().startswith('has_'):
                                 found_has = True
             if found_try_except_import and not found_has:
-                self.warnings.append((
-                    291,
-                    'Found Try/Except block without HAS_ assginment'
-                ))
+                # TODO: Add line/col
+                self.reporter.warning(
+                    path=self.object_path,
+                    code=291,
+                    msg='Found Try/Except block without HAS_ assginment'
+                )
 
     def _ensure_imports_below_docs(self, doc_info, first_callable):
         min_doc_line = min(
@@ -500,13 +555,14 @@ class ModuleValidator(Validator):
             if isinstance(child, (ast.Import, ast.ImportFrom)):
                 import_lines.append(child.lineno)
                 if child.lineno < min_doc_line:
-                    self.errors.append((
-                        106,
-                        ('Import found before documentation variables. '
-                         'All imports must appear below '
-                         'DOCUMENTATION/EXAMPLES/RETURN/ANSIBLE_METADATA. '
-                         'line %d' % (child.lineno,))
-                    ))
+                    self.reporter.error(
+                        path=self.object_path,
+                        code=106,
+                        msg=('Import found before documentation variables. '
+                             'All imports must appear below '
+                             'DOCUMENTATION/EXAMPLES/RETURN/ANSIBLE_METADATA.'),
+                        line=child.lineno
+                    )
                     break
             elif isinstance(child, TRY_EXCEPT):
                 bodies = child.body
@@ -516,14 +572,15 @@ class ModuleValidator(Validator):
                     if isinstance(grandchild, (ast.Import, ast.ImportFrom)):
                         import_lines.append(grandchild.lineno)
                         if grandchild.lineno < min_doc_line:
-                            self.errors.append((
-                                106,
-                                ('Import found before documentation '
-                                 'variables. All imports must appear below '
-                                 'DOCUMENTATION/EXAMPLES/RETURN/'
-                                 'ANSIBLE_METADATA. line %d' %
-                                 (child.lineno,))
-                            ))
+                            self.reporter.error(
+                                path=self.object_path,
+                                code=106,
+                                msg=('Import found before documentation '
+                                     'variables. All imports must appear below '
+                                     'DOCUMENTATION/EXAMPLES/RETURN/'
+                                     'ANSIBLE_METADATA.'),
+                                line=child.lineno
+                            )
                             break
 
         for import_line in import_lines:
@@ -531,26 +588,48 @@ class ModuleValidator(Validator):
                 msg = (
                     107,
                     ('Imports should be directly below DOCUMENTATION/EXAMPLES/'
-                     'RETURN/ANSIBLE_METADATA. line %d' % import_line)
+                     'RETURN/ANSIBLE_METADATA.')
                 )
                 if self._is_new_module():
-                    self.errors.append(msg)
+                    self.reporter.error(
+                        path=self.object_path,
+                        code=msg[0],
+                        msg=msg[1],
+                        line=import_line
+                    )
                 else:
-                    self.warnings.append(msg)
+                    self.reporter.warning(
+                        path=self.object_path,
+                        code=msg[0],
+                        msg=msg[1],
+                        line=import_line
+                    )
 
     def _find_ps_replacers(self):
         if 'WANT_JSON' not in self.text:
-            self.errors.append((206, 'WANT_JSON not found in module'))
+            self.reporter.error(
+                path=self.object_path,
+                code=206,
+                msg='WANT_JSON not found in module'
+            )
 
         if REPLACER_WINDOWS not in self.text:
-            self.errors.append((207, '"%s" not found in module' % REPLACER_WINDOWS))
+            self.reporter.error(
+                path=self.object_path,
+                code=207,
+                msg='"%s" not found in module' % REPLACER_WINDOWS
+            )
 
     def _find_ps_docs_py_file(self):
         if self.object_name in self.PS_DOC_BLACKLIST:
             return
         py_path = self.path.replace('.ps1', '.py')
         if not os.path.isfile(py_path):
-            self.errors.append((503, 'Missing python documentation file'))
+            self.reporter.error(
+                path=self.object_path,
+                code=503,
+                msg='Missing python documentation file'
+            )
 
     def _get_docs(self):
         docs = {
@@ -611,6 +690,7 @@ class ModuleValidator(Validator):
         return docs
 
     def _validate_docs_schema(self, doc, schema, name, error_code):
+        # TODO: Add line/col
         errors = []
         try:
             schema(doc)
@@ -627,87 +707,142 @@ class ModuleValidator(Validator):
             else:
                 error_message = error
 
-            self.errors.append((
-                error_code,
-                '%s.%s: %s' % (name, '.'.join(path), error_message)
-            ))
+            self.reporter.error(
+                path=self.object_path,
+                code=error_code,
+                msg='%s.%s: %s' % (name, '.'.join(path), error_message)
+            )
 
     def _validate_docs(self):
         doc_info = self._get_docs()
         deprecated = False
         if not bool(doc_info['DOCUMENTATION']['value']):
-            self.errors.append((301, 'No DOCUMENTATION provided'))
+            self.reporter.error(
+                path=self.object_path,
+                code=301,
+                msg='No DOCUMENTATION provided'
+            )
         else:
             doc, errors, traces = parse_yaml(
                 doc_info['DOCUMENTATION']['value'],
                 doc_info['DOCUMENTATION']['lineno'],
                 self.name, 'DOCUMENTATION'
             )
-            self.errors.extend([(302, e) for e in errors])
-            self.traces.extend(traces)
+            for error in errors:
+                self.reporter.error(
+                    path=self.object_path,
+                    code=302,
+                    **error
+                )
+            for trace in traces:
+                self.reporter.trace(
+                    path=self.object_path,
+                    tracebk=trace
+                )
             if not errors and not traces:
                 with CaptureStd():
                     try:
                         get_docstring(self.path, verbose=True)
                     except AssertionError:
                         fragment = doc['extends_documentation_fragment']
-                        self.errors.append((
-                            303,
-                            'DOCUMENTATION fragment missing: %s' % fragment
-                        ))
+                        self.reporter.error(
+                            path=self.object_path,
+                            code=303,
+                            msg='DOCUMENTATION fragment missing: %s' % fragment
+                        )
                     except Exception:
-                        self.traces.append(traceback.format_exc())
-                        self.errors.append((
-                            304,
-                            'Unknown DOCUMENTATION error, see TRACE'
-                        ))
+                        self.reporter.trace(
+                            path=self.object_path,
+                            tracebk=traceback.format_exc()
+                        )
+                        self.reporter.error(
+                            path=self.object_path,
+                            code=304,
+                            msg='Unknown DOCUMENTATION error, see TRACE'
+                        )
 
                 if 'options' in doc and doc['options'] is None and doc.get('extends_documentation_fragment'):
-                    self.errors.append((
-                        305,
-                        ('DOCUMENTATION.options must be a dictionary/hash when used '
-                         'with DOCUMENTATION.extends_documentation_fragment')
-                    ))
+                    self.reporter.error(
+                        path=self.object_path,
+                        code=304,
+                        msg=('DOCUMENTATION.options must be a dictionary/hash when used '
+                             'with DOCUMENTATION.extends_documentation_fragment')
+                    )
 
                 if self.object_name.startswith('_') and not os.path.islink(self.object_path):
                     deprecated = True
                     if 'deprecated' not in doc or not doc.get('deprecated'):
-                        self.errors.append((
-                            318,
-                            'Module deprecated, but DOCUMENTATION.deprecated is missing'
-                        ))
+                        self.reporter.error(
+                            path=self.object_path,
+                            code=318,
+                            msg='Module deprecated, but DOCUMENTATION.deprecated is missing'
+                        )
 
                 self._validate_docs_schema(doc, doc_schema(self.object_name.split('.')[0]), 'DOCUMENTATION', 305)
                 self._check_version_added(doc)
                 self._check_for_new_args(doc)
 
         if not bool(doc_info['EXAMPLES']['value']):
-            self.errors.append((310, 'No EXAMPLES provided'))
+            self.reporter.error(
+                path=self.object_path,
+                code=310,
+                msg='No EXAMPLES provided'
+            )
         else:
             _, errors, traces = parse_yaml(doc_info['EXAMPLES']['value'],
                                            doc_info['EXAMPLES']['lineno'],
                                            self.name, 'EXAMPLES', load_all=True)
-            self.errors.extend([(311, error) for error in errors])
-            self.traces.extend(traces)
+            for error in errors:
+                self.reporter.error(
+                    path=self.object_path,
+                    code=311,
+                    **error
+                )
+            for trace in traces:
+                self.reporter.trace(
+                    path=self.object_path,
+                    tracebk=trace
+                )
 
         if not bool(doc_info['RETURN']['value']):
             if self._is_new_module():
-                self.errors.append((312, 'No RETURN documentation provided'))
+                self.reporter.error(
+                    path=self.object_path,
+                    code=312,
+                    msg='No RETURN provided'
+                )
             else:
-                self.warnings.append((312, 'No RETURN provided'))
+                self.reporter.warning(
+                    path=self.object_path,
+                    code=312,
+                    msg='No RETURN provided'
+                )
         else:
             data, errors, traces = parse_yaml(doc_info['RETURN']['value'],
-                                           doc_info['RETURN']['lineno'],
-                                           self.name, 'RETURN')
-
+                                              doc_info['RETURN']['lineno'],
+                                              self.name, 'RETURN')
             if data:
                 for ret_key in data:
                     self._validate_docs_schema(data[ret_key], return_schema(data[ret_key]), 'RETURN.%s' % ret_key, 319)
-            self.errors.extend([(313, error) for error in errors])
-            self.traces.extend(traces)
+
+            for error in errors:
+                self.reporter.error(
+                    path=self.object_path,
+                    code=313,
+                    **error
+                )
+            for trace in traces:
+                self.reporter.trace(
+                    path=self.object_path,
+                    tracebk=trace
+                )
 
         if not bool(doc_info['ANSIBLE_METADATA']['value']):
-            self.errors.append((314, 'No ANSIBLE_METADATA provided'))
+            self.reporter.error(
+                path=self.object_path,
+                code=314,
+                msg='No ANSIBLE_METADATA provided'
+            )
         else:
             metadata = None
             if isinstance(doc_info['ANSIBLE_METADATA']['value'], ast.Dict):
@@ -720,8 +855,17 @@ class ModuleValidator(Validator):
                     doc_info['ANSIBLE_METADATA']['lineno'],
                     self.name, 'ANSIBLE_METADATA'
                 )
-                self.errors.extend([(315, error) for error in errors])
-                self.traces.extend(traces)
+                for error in errors:
+                    self.reporter.error(
+                        path=self.object_path,
+                        code=315,
+                        **error
+                    )
+                for trace in traces:
+                    self.reporter.trace(
+                        path=self.object_path,
+                        tracebk=trace
+                    )
 
             if metadata:
                 self._validate_docs_schema(metadata, metadata_schema(deprecated),
@@ -737,10 +881,11 @@ class ModuleValidator(Validator):
             version_added = StrictVersion(str(doc.get('version_added', '0.0')))
         except ValueError:
             version_added = doc.get('version_added', '0.0')
-            self.errors.append((
-                306,
-                'version_added is not a valid version number: %r' % version_added
-            ))
+            self.reporter.error(
+                path=self.object_path,
+                code=306,
+                msg='version_added is not a valid version number: %r' % version_added
+            )
             return
 
         should_be = '.'.join(ansible_version.split('.')[:2])
@@ -748,10 +893,11 @@ class ModuleValidator(Validator):
 
         if (version_added < strict_ansible_version or
                 strict_ansible_version < version_added):
-            self.errors.append((
-                307,
-                'version_added should be %s. Currently %s' % (should_be, version_added)
-            ))
+            self.reporter.error(
+                path=self.object_path,
+                code=307,
+                msg='version_added should be %s. Currently %s' % (should_be, version_added)
+            )
 
     def _validate_argument_spec(self):
         if not self.analyze_arg_spec:
@@ -759,12 +905,13 @@ class ModuleValidator(Validator):
         spec = get_argument_spec(self.path)
         for arg, data in spec.items():
             if data.get('required') and data.get('default', object) != object:
-                self.errors.append((
-                    317,
-                    ('"%s" is marked as required but specifies '
-                     'a default. Arguments with a default '
-                     'should not be marked as required' % arg)
-                ))
+                self.reporter.error(
+                    path=self.object_path,
+                    code=317,
+                    msg=('"%s" is marked as required but specifies '
+                         'a default. Arguments with a default '
+                         'should not be marked as required' % arg)
+                )
 
     def _check_for_new_args(self, doc):
         if not self.base_branch or self._is_new_module():
@@ -776,19 +923,24 @@ class ModuleValidator(Validator):
                 existing_options = existing_doc.get('options', {})
             except AssertionError:
                 fragment = doc['extends_documentation_fragment']
-                self.warnings.append((
-                    392,
-                    'Pre-existing DOCUMENTATION fragment missing: %s' % fragment
-                ))
+                self.reporter.warning(
+                    path=self.object_path,
+                    code=392,
+                    msg='Pre-existing DOCUMENTATION fragment missing: %s' % fragment
+                )
                 return
             except Exception as e:
-                self.warning_traces.append(e)
-                self.warnings.append((
-                    391,
-                    ('Unknown pre-existing DOCUMENTATION '
-                     'error, see TRACE. Submodule refs may '
-                     'need updated')
-                ))
+                self.reporter.warning_trace(
+                    path=self.object_path,
+                    tracebk=e
+                )
+                self.reporter.warning(
+                    path=self.object_path,
+                    code=391,
+                    msg=('Unknown pre-existing DOCUMENTATION '
+                         'error, see TRACE. Submodule refs may '
+                         'need updated')
+                )
                 return
 
         try:
@@ -815,12 +967,13 @@ class ModuleValidator(Validator):
                 )
             except ValueError:
                 version_added = details.get('version_added', '0.0')
-                self.errors.append((
-                    308,
-                    ('version_added for new option (%s) '
-                     'is not a valid version number: %r' %
-                     (option, version_added))
-                ))
+                self.reporter.error(
+                    path=self.object_path,
+                    code=308,
+                    msg=('version_added for new option (%s) '
+                         'is not a valid version number: %r' %
+                         (option, version_added))
+                )
                 continue
             except:
                 # If there is any other exception it should have been caught
@@ -831,12 +984,13 @@ class ModuleValidator(Validator):
             if (strict_ansible_version != mod_version_added and
                     (version_added < strict_ansible_version or
                      strict_ansible_version < version_added)):
-                self.errors.append((
-                    309,
-                    ('version_added for new option (%s) should '
-                     'be %s. Currently %s' %
-                     (option, should_be, version_added))
-                ))
+                self.reporter.error(
+                    path=self.object_path,
+                    code=309,
+                    msg=('version_added for new option (%s) should '
+                         'be %s. Currently %s' %
+                         (option, should_be, version_added))
+                )
 
     @staticmethod
     def is_blacklisted(path):
@@ -858,25 +1012,29 @@ class ModuleValidator(Validator):
     def validate(self):
         super(ModuleValidator, self).validate()
 
-#        if self._powershell_module():
-#            self.warnings.append('Cannot check powershell modules at this '
-#                                 'time.  Skipping')
-#            return
         if not self._python_module() and not self._powershell_module():
-            self.errors.append((
-                501,
-                ('Official Ansible modules must have a .py '
-                 'extension for python modules or a .ps1 '
-                 'for powershell modules')
-            ))
+            self.reporter.error(
+                path=self.object_path,
+                code=501,
+                msg=('Official Ansible modules must have a .py '
+                     'extension for python modules or a .ps1 '
+                     'for powershell modules')
+            )
             self._python_module_override = True
 
         if self._python_module() and self.ast is None:
-            self.errors.append((401, 'Python SyntaxError while parsing module'))
+            self.reporter.error(
+                path=self.object_path,
+                code=401,
+                msg='Python SyntaxError while parsing module'
+            )
             try:
                 compile(self.text, self.path, 'exec')
             except Exception:
-                self.traces.append(traceback.format_exc())
+                self.reporter.trace(
+                    path=self.object_path,
+                    tracebk=traceback.format_exc()
+                )
             return
 
         if self._python_module():
@@ -908,8 +1066,8 @@ class ModuleValidator(Validator):
 class PythonPackageValidator(Validator):
     BLACKLIST_FILES = frozenset(('__pycache__',))
 
-    def __init__(self, path):
-        super(PythonPackageValidator, self).__init__()
+    def __init__(self, path, reporter=None):
+        super(PythonPackageValidator, self).__init__(reporter=reporter or Reporter())
 
         self.path = path
         self.basename = os.path.basename(path)
@@ -930,11 +1088,10 @@ class PythonPackageValidator(Validator):
 
         init_file = os.path.join(self.path, '__init__.py')
         if not os.path.exists(init_file):
-            self.errors.append(
-                (
-                    502,
-                    'Ansible module subdirectories must contain an __init__.py'
-                )
+            self.reporter.error(
+                path=self.object_path,
+                code=502,
+                msg='Ansible module subdirectories must contain an __init__.py'
             )
 
 
@@ -975,8 +1132,7 @@ def main():
 
     args.modules[:] = [m.rstrip('/') for m in args.modules]
 
-    reports = OrderedDict()
-
+    reporter = Reporter()
     git_cache = GitCache(args.base_branch)
 
     for module in args.modules:
@@ -987,9 +1143,8 @@ def main():
             if ModuleValidator.is_blacklisted(path):
                 continue
             with ModuleValidator(path, analyze_arg_spec=args.arg_spec,
-                                 base_branch=args.base_branch, git_cache=git_cache) as mv:
+                                 base_branch=args.base_branch, git_cache=git_cache, reporter=reporter) as mv:
                 mv.validate()
-                reports.update(mv.report())
 
         for root, dirs, files in os.walk(module):
             basedir = root[len(module) + 1:].split('/', 1)[0]
@@ -1001,9 +1156,8 @@ def main():
                 path = os.path.join(root, dirname)
                 if args.exclude and args.exclude.search(path):
                     continue
-                pv = PythonPackageValidator(path)
+                pv = PythonPackageValidator(path, reporter=reporter)
                 pv.validate()
-                reports.update(pv.report())
 
             for filename in files:
                 path = os.path.join(root, filename)
@@ -1012,14 +1166,13 @@ def main():
                 if ModuleValidator.is_blacklisted(path):
                     continue
                 with ModuleValidator(path, analyze_arg_spec=args.arg_spec,
-                                     base_branch=args.base_branch, git_cache=git_cache) as mv:
+                                     base_branch=args.base_branch, git_cache=git_cache, reporter=reporter) as mv:
                     mv.validate()
-                    reports.update(mv.report())
 
     if args.format == 'plain':
-        sys.exit(Reporter.plain(reports, warnings=args.warnings, output=args.output))
+        sys.exit(reporter.plain(warnings=args.warnings, output=args.output))
     else:
-        sys.exit(Reporter.json(reports, warnings=args.warnings, output=args.output))
+        sys.exit(reporter.json(warnings=args.warnings, output=args.output))
 
 
 class GitCache(object):