Fixes to reports.py -- don't crash on control characters, ensure directories earlier (#5885)

Fixes #5884 (both parts). Also fixes testcmdline.py to report unexpected stderr or exit code even when there's an expected output file.

Author: gvanrossum

mypy/report.py

@@ -90,6 +90,8 @@ def finish(self) -> None:
 class AbstractReporter(metaclass=ABCMeta):
     def __init__(self, reports: Reports, output_dir: str) -> None:
         self.output_dir = output_dir
+        if output_dir != '<memory>':
+            stats.ensure_dir_exists(output_dir)
 
     @abstractmethod
     def on_file(self, tree: MypyFile, type_map: Dict[Expression, Type], options: Options) -> None:
@@ -124,8 +126,6 @@ def __init__(self, reports: Reports, output_dir: str) -> None:
         super().__init__(reports, output_dir)
         self.counts = {}  # type: Dict[str, Tuple[int, int, int, int]]
 
-        stats.ensure_dir_exists(output_dir)
-
     def on_file(self,
                 tree: MypyFile,
                 type_map: Dict[Expression, Type],
@@ -170,7 +170,6 @@ def __init__(self, reports: Reports, output_dir: str) -> None:
         super().__init__(reports, output_dir)
         self.counts = {}  # type: Dict[str, Tuple[int, int]]
         self.any_types_counter = {}  # type: Dict[str, typing.Counter[int]]
-        stats.ensure_dir_exists(output_dir)
 
     def on_file(self,
                 tree: MypyFile,
@@ -361,8 +360,6 @@ def __init__(self, reports: Reports, output_dir: str) -> None:
         super().__init__(reports, output_dir)
         self.lines_covered = {}  # type: Dict[str, List[int]]
 
-        stats.ensure_dir_exists(output_dir)
-
     def on_file(self,
                 tree: MypyFile,
                 type_map: Dict[Expression, Type],
@@ -418,6 +415,10 @@ def __init__(self, reports: Reports, output_dir: str) -> None:
         self.last_xml = None  # type: Optional[Any]
         self.files = []  # type: List[FileInfo]
 
+    # XML doesn't like control characters, but they are sometimes
+    # legal in source code (e.g. comments, string literals).
+    control_fixer = str.maketrans(''.join(chr(i) for i in range(32)), '?' * 32)
+
     def on_file(self,
                 tree: MypyFile,
                 type_map: Dict[Expression, Type],
@@ -446,7 +447,7 @@ def on_file(self,
                 etree.SubElement(root, 'line',
                                  number=str(lineno),
                                  precision=stats.precision_names[status],
-                                 content=line_text.rstrip('\n'),
+                                 content=line_text.rstrip('\n').translate(self.control_fixer),
                                  any_info=self._get_any_info_for_line(visitor, lineno))
         # Assumes a layout similar to what XmlReporter uses.
         xslt_path = os.path.relpath('mypy-html.xslt', path)
@@ -737,7 +738,6 @@ def on_finish(self) -> None:
         last_xml = self.memory_xml.last_xml
         assert last_xml is not None
         out_path = os.path.join(self.output_dir, 'index.txt')
-        stats.ensure_dir_exists(os.path.dirname(out_path))
         transformed_txt = bytes(self.xslt_txt(last_xml))
         with open(out_path, 'wb') as out_file:
             out_file.write(transformed_txt)

mypy/test/testcmdline.py

@@ -52,24 +52,31 @@ def test_python_cmdline(testcase: DataDrivenTestCase) -> None:
     env['PYTHONPATH'] = PREFIX
     process = subprocess.Popen(fixed + args,
                                stdout=subprocess.PIPE,
-                               stderr=subprocess.STDOUT,
+                               stderr=subprocess.PIPE,
                                cwd=test_temp_dir,
                                env=env)
-    outb = process.stdout.read()
+    outb, errb = process.communicate()
+    result = process.returncode
     # Split output into lines.
     out = [s.rstrip('\n\r') for s in str(outb, 'utf8').splitlines()]
+    err = [s.rstrip('\n\r') for s in str(errb, 'utf8').splitlines()]
 
     if "PYCHARM_HOSTED" in os.environ:
-        pos = next((p for p, i in enumerate(out) if i.startswith('pydev debugger: ')), None)
-        if pos is not None:
-            del out[pos]  # the attaching debugger message itself
-            del out[pos]  # plus the extra new line added
+        for pos, line in enumerate(err):
+            if line.startswith('pydev debugger: '):
+                # Delete the attaching debugger message itself, plus the extra newline added.
+                del err[pos:pos + 2]
+                break
 
-    result = process.wait()
     # Remove temp file.
     os.remove(program_path)
     # Compare actual output to expected.
     if testcase.output_files:
+        # Ignore stdout, but we insist on empty stderr and zero status.
+        if err or result:
+            raise AssertionError(
+                'Expected zero status and empty stderr, got %d and\n%s' %
+                (result, '\n'.join(err + out)))
         for path, expected_content in testcase.output_files:
             if not os.path.exists(path):
                 raise AssertionError(
@@ -85,7 +92,7 @@ def test_python_cmdline(testcase: DataDrivenTestCase) -> None:
                                        'Output file {} did not match its expected output'.format(
                                            path))
     else:
-        out = normalize_error_messages(out)
+        out = normalize_error_messages(err + out)
         obvious_result = 1 if out else 0
         if obvious_result != result:
             out.append('== Return code: {}'.format(result))

test-data/unit/reports.test

@@ -77,7 +77,7 @@ def untyped_function():
 # cmd: mypy --html-report report n.py
 [file n.py]
 class A(object):
-    pass
+    pass  # hex 1f here: ()
 
 [file report/mypy-html.css]
 [file report/index.html]
@@ -96,7 +96,7 @@ class A(object):
 <span id="L2" class="lineno"><a class="lineno" href="#L2">2</a></span>
 </pre></td>
 <td class="table-code"><pre><span class="line-empty" title="No Anys on this line!">class A(object):</span>
-<span class="line-empty" title="No Anys on this line!">    pass</span>
+<span class="line-empty" title="No Anys on this line!">    pass  # hex 1f here: (?)</span>
 </pre></td>
 </tr></tbody>
 </table>