-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add Error format support, and JSON output option (#11396)
### Description Resolves #10816 The changes this PR makes are relatively small. It currently: - Adds an `--output` option to mypy CLI - Adds a `ErrorFormatter` abstract base class, which can be subclassed to create new output formats - Adds a `MypyError` class that represents the external format of a mypy error. - Adds a check for `--output` being `'json'`, in which case the `JSONFormatter` is used to produce the reported output. #### Demo: ```console $ mypy mytest.py mytest.py:2: error: Incompatible types in assignment (expression has type "str", variable has type "int") mytest.py:3: error: Name "z" is not defined Found 2 errors in 1 file (checked 1 source file) $ mypy mytest.py --output=json {"file": "mytest.py", "line": 2, "column": 4, "severity": "error", "message": "Incompatible types in assignment (expression has type \"str\", variable has type \"int\")", "code": "assignment"} {"file": "mytest.py", "line": 3, "column": 4, "severity": "error", "message": "Name \"z\" is not defined", "code": "name-defined"} ``` --- A few notes regarding the changes: - I chose to re-use the intermediate `ErrorTuple`s created during error reporting, instead of using the more general `ErrorInfo` class, because a lot of machinery already exists in mypy for sorting and removing duplicate error reports, which produces `ErrorTuple`s at the end. The error sorting and duplicate removal logic could perhaps be separated out from the rest of the code, to be able to use `ErrorInfo` objects more freely. - `ErrorFormatter` doesn't really need to be an abstract class, but I think it would be better this way. If there's a different method that would be preferred, I'd be happy to know. - The `--output` CLI option is, most probably, not added in the correct place. Any help in how to do it properly would be appreciated, the mypy option parsing code seems very complex. - The ability to add custom output formats can be simply added by subclassing the `ErrorFormatter` class inside a mypy plugin, and adding a `name` field to the formatters. The mypy runtime can then check through the `__subclasses__` of the formatter and determine if such a formatter is present. The "checking for the `name` field" part of this code might be appropriate to add within this PR itself, instead of hard-coding `JSONFormatter`. Does that sound like a good idea? --------- Co-authored-by: Tushar Sadhwani <[email protected]> Co-authored-by: Tushar Sadhwani <[email protected]>
- Loading branch information
1 parent
fb31409
commit 35fbd2a
Showing
8 changed files
with
239 additions
and
16 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
"""Defines the different custom formats in which mypy can output.""" | ||
|
||
import json | ||
from abc import ABC, abstractmethod | ||
from typing import TYPE_CHECKING | ||
|
||
if TYPE_CHECKING: | ||
from mypy.errors import MypyError | ||
|
||
|
||
class ErrorFormatter(ABC): | ||
"""Base class to define how errors are formatted before being printed.""" | ||
|
||
@abstractmethod | ||
def report_error(self, error: "MypyError") -> str: | ||
raise NotImplementedError | ||
|
||
|
||
class JSONFormatter(ErrorFormatter): | ||
"""Formatter for basic JSON output format.""" | ||
|
||
def report_error(self, error: "MypyError") -> str: | ||
"""Prints out the errors as simple, static JSON lines.""" | ||
return json.dumps( | ||
{ | ||
"file": error.file_path, | ||
"line": error.line, | ||
"column": error.column, | ||
"message": error.message, | ||
"hint": None if len(error.hints) == 0 else "\n".join(error.hints), | ||
"code": None if error.errorcode is None else error.errorcode.code, | ||
"severity": error.severity, | ||
} | ||
) | ||
|
||
|
||
OUTPUT_CHOICES = {"json": JSONFormatter()} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
"""Test cases for `--output=json`. | ||
These cannot be run by the usual unit test runner because of the backslashes in | ||
the output, which get normalized to forward slashes by the test suite on Windows. | ||
""" | ||
|
||
from __future__ import annotations | ||
|
||
import os | ||
import os.path | ||
|
||
from mypy import api | ||
from mypy.defaults import PYTHON3_VERSION | ||
from mypy.test.config import test_temp_dir | ||
from mypy.test.data import DataDrivenTestCase, DataSuite | ||
|
||
|
||
class OutputJSONsuite(DataSuite): | ||
files = ["outputjson.test"] | ||
|
||
def run_case(self, testcase: DataDrivenTestCase) -> None: | ||
test_output_json(testcase) | ||
|
||
|
||
def test_output_json(testcase: DataDrivenTestCase) -> None: | ||
"""Runs Mypy in a subprocess, and ensures that `--output=json` works as intended.""" | ||
mypy_cmdline = ["--output=json"] | ||
mypy_cmdline.append(f"--python-version={'.'.join(map(str, PYTHON3_VERSION))}") | ||
|
||
# Write the program to a file. | ||
program_path = os.path.join(test_temp_dir, "main") | ||
mypy_cmdline.append(program_path) | ||
with open(program_path, "w", encoding="utf8") as file: | ||
for s in testcase.input: | ||
file.write(f"{s}\n") | ||
|
||
output = [] | ||
# Type check the program. | ||
out, err, returncode = api.run(mypy_cmdline) | ||
# split lines, remove newlines, and remove directory of test case | ||
for line in (out + err).rstrip("\n").splitlines(): | ||
if line.startswith(test_temp_dir + os.sep): | ||
output.append(line[len(test_temp_dir + os.sep) :].rstrip("\r\n")) | ||
else: | ||
output.append(line.rstrip("\r\n")) | ||
|
||
if returncode > 1: | ||
output.append("!!! Mypy crashed !!!") | ||
|
||
# Remove temp file. | ||
os.remove(program_path) | ||
|
||
# JSON encodes every `\` character into `\\`, so we need to remove `\\` from windows paths | ||
# and `/` from POSIX paths | ||
json_os_separator = os.sep.replace("\\", "\\\\") | ||
normalized_output = [line.replace(test_temp_dir + json_os_separator, "") for line in output] | ||
|
||
assert normalized_output == testcase.output |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
-- Test cases for `--output=json`. | ||
-- These cannot be run by the usual unit test runner because of the backslashes | ||
-- in the output, which get normalized to forward slashes by the test suite on | ||
-- Windows. | ||
|
||
[case testOutputJsonNoIssues] | ||
# flags: --output=json | ||
def foo() -> None: | ||
pass | ||
|
||
foo() | ||
[out] | ||
|
||
[case testOutputJsonSimple] | ||
# flags: --output=json | ||
def foo() -> None: | ||
pass | ||
|
||
foo(1) | ||
[out] | ||
{"file": "main", "line": 5, "column": 0, "message": "Too many arguments for \"foo\"", "hint": null, "code": "call-arg", "severity": "error"} | ||
|
||
[case testOutputJsonWithHint] | ||
# flags: --output=json | ||
from typing import Optional, overload | ||
|
||
@overload | ||
def foo() -> None: ... | ||
@overload | ||
def foo(x: int) -> None: ... | ||
|
||
def foo(x: Optional[int] = None) -> None: | ||
... | ||
|
||
reveal_type(foo) | ||
|
||
foo('42') | ||
|
||
def bar() -> None: ... | ||
bar('42') | ||
[out] | ||
{"file": "main", "line": 12, "column": 12, "message": "Revealed type is \"Overload(def (), def (x: builtins.int))\"", "hint": null, "code": "misc", "severity": "note"} | ||
{"file": "main", "line": 14, "column": 0, "message": "No overload variant of \"foo\" matches argument type \"str\"", "hint": "Possible overload variants:\n def foo() -> None\n def foo(x: int) -> None", "code": "call-overload", "severity": "error"} | ||
{"file": "main", "line": 17, "column": 0, "message": "Too many arguments for \"bar\"", "hint": null, "code": "call-arg", "severity": "error"} |