python
Add Error format support, and JSON output option
Description
Resolves #10816
The changes this PR makes are relatively small. It currently:
- Adds an
--outputoption to mypy CLI - Adds a
ErrorFormatterabstract base class, which can be subclassed to create new output formats - Adds a
MypyErrorclass that represents the external format of a mypy error. - Adds a check for
--outputbeing'json', in which case theJSONFormatteris used to produce the reported output.
Demo:
$ 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
ErrorTuples created during error reporting, instead of using the more generalErrorInfoclass, because a lot of machinery already exists in mypy for sorting and removing duplicate error reports, which producesErrorTuples 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 useErrorInfoobjects more freely. -
ErrorFormatterdoesn'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
--outputCLI 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
ErrorFormatterclass inside a mypy plugin, and adding anamefield 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 thenamefield" part of this code might be appropriate to add within this PR itself, instead of hard-codingJSONFormatter. Does that sound like a good idea?
I'd like to make the case again for the existing JSON-based standard: Static Analysis Results Interchange Format (SARIF)
While that does not preclude support for a custom JSON format as well, perhaps if mypy were to support SARIF, there would be no need for a custom format?
Pros of SARIF:
-
There is no need to reinvent the wheel.
-
Can already integrate with existing tools like GitHub code scanning, Visual Studio, VS Code
-
Every IDE shouldn't have to implement its own mypy-specific integration. This is exemplified by the situation in IntelliJ IDEA/PyCharm: there are two 3rd party mypy-specific plugins, both of which have major problems that aren't getting fixed. [1]
Clearly the momentum isn't there to provide good mypy-specific IDE integrations. Adding mypy support for SARIF, and letting IDEs take care of parsing SARIF and implementing a good UI on top, seems like a far more sustainable option.
Cons:
- SARIF is clearly infected with "design by committee", the data structures are verbose and deeply nested. However, advanced features are optional, a minimal implementation is straightforward, see example below.
- No streaming output; all results would have to be collected before the JSON output can be serialized.
[1] The plugin ratings are 3.4 and 2.9 out of 5 😖 https://plugins.jetbrains.com/search?search=mypy
Example minimal mypy SARIF output
{
"version": "2.1.0",
"$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0.json",
"runs": [
{
"tool": {
"driver": {
"name": "mypy",
"version": "0.910"
}
},
"results": [
{
"ruleId": "assignment",
"level": "error",
"message": {
"text": "Incompatible types in assignment (expression has type \"str\", variable has type \"int\")"
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "mytest.py"
},
"region": {
"startLine": 2,
"startColumn": 4
}
}
}
]
},
{
"ruleId": "name-defined",
"level": "error",
"message": {
"text": "Name \"z\" is not defined"
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "mytest.py"
},
"region": {
"startLine": 3,
"startColumn": 4
}
}
}
]
}
]
}
]
}
+1 for SARIF format, let's use a common format instead of a mypy custom one ! :)
I'm willing to turn it into SARIF (basing it on the json snippet provided by @intgr), if that's what I need to get this into mypy 😄
@ethanhs @TH3CHARLie what do you think?
After sitting on it a little, I feel that there's room for more than one machine-readable format. The simpler json-line-based format is probably better for simple tools that only care about mypy. I'm willing to implement the SARIF part myself.
But I shouldn't be the one to decide that, I'm just an occasional lurker here. No mypy developers have chipped in yet.
I'm just a tourist here, but i'm currently activating SARIF output for all linters of MegaLinter, and having native SARIF output is a great benefit for linters ^^ ( + the Github CodeQL that natively understands SARIF format ^^ )
Some other python linters already have SARIF output, like bandit , maybe there is some code to reuse to manage the format ?
@nvuillam this PR introduces an ErrorFormatter class. By the time this PR is finalized, even if it doesn't use SARIF you will be able to define your own ErrorFormatter class in a plugin probably, and tell it to output the SARIF format, it'll be really easy.
Pylint has the same setup.
@tusharsadhwani thank but... I don't have the bandwidth to implement SARIF output on all linters that do not manage it yet 😅
I think one property that machine-readable formats should have is: if one error causes multiple lines of output, then that should appear as one result item rather than multiple.
So for example with mypy --show-error-context
_local/multi_error.py: note: In function "foo":
_local/multi_error.py:5:9: error: No overload variant of "get" of "Mapping" matches argument type "str" [call-overload]
_local/multi_error.py:5:9: note: Possible overload variants:
_local/multi_error.py:5:9: note: def get(self, key: Any) -> Any
_local/multi_error.py:5:9: note: def [_T] get(self, key: Any, default: Union[Any, _T]) -> Union[Any, _T]
Found 1 error in 1 file (checked 1 source file)
should maybe be output as
{
"file": "_local/multi_error.py",
"line": 5,
"column": 8,
"severity": "error",
"context": "In function \"foo\":",
"message": "No overload variant of \"get\" of \"Mapping\" matches argument type \"str\"",
"hint": "Possible overload variants:\n def get(self, key: Any) -> Any\n def [_T] get(self, key: Any, default: Union[Any, _T]) -> Union[Any, _T]",
"code": "call-overload"
}
But again, maybe that shouldn't be a blocker for this PR, getting machine-readable output can be useful even when findings aren't accurately grouped.
The negative line/column numbers right now seem awkward though:
{"file": "_local/multi_error.py", "line": -1, "column": -1, "severity": "note", "message": "In function \"foo\":", "code": null}
That's definitely a bug.
The mypy code that generates this output was ridiculously coupled. I'll take a look at if this can be fixed.
It has been many months as the project has hoped for a best solution over an existing solution. Can we merge this and accept future improvement using --output serif if someone comes along and implements it?
--output json option gets ignored after some usage with mypy.api.run_dmypy . Pasing json option to dmypy CLI worked fine, but invoking run_dmypy function in a Python script returns plain string output after some usage. Can someone help me troubleshoot this?
@sehyun-hwang sure thing. If you could provide a reproducible example I can look into it right away.
@tusharsadhwani Thank you! I tried linting jobs in a container, and was unable to reproduce it. The problem occurs only when my IDE invokes dmypy, so I'm suspecting this has to do with concurrent execution. Do you see a chance where --output json option gets ignored when multiple clients are connected, or clients are threaded or running in a child process?
I found that the first file linted output a json format, but from the second one it goes back to the string format. Can you try to reproduce this?
centos@www /m/tax-automation (99-basic-types) [1]> dmypy check -- batch_api.py
{"file": "batch_api.py", "line": 5, "column": 0, "message": "Cannot find implementation or library stub for module named \"LAMBDA_CONFIG\"", "hint": "See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports", "code": "import"}
{"file": "batch_api.py", "line": 6, "column": 0, "message": "Cannot find implementation or library stub for module named \"common.utils\"", "hint": "", "code": "import"}
centos@www /m/tax-automation (99-basic-types) [1]> dmypy check -- foo.py
foo.py:2: error: Name "bar" is not defined
foo.py:5: error: Argument 1 to "foo" has incompatible type "str"; expected "int"```
@sehyun-hwang This seems like a dmypy bug. It seems that dmypy ignores cli flags or config file flags in certain cases.
Here's a file structure:
$ tree .
.
├── a.py
├── b.py
└── mypy.ini
And mypy.ini contains the following:
[mypy]
strict = True
a.py and b.py both contain:
def foo(): pass
This way mypy a.py won't show errors but mypy --strict a.py will.
When alternating between dmypy check a.py and dmypy check b.py, at some point dmypy stops showing strict output. For the same reason it might be forgetting the --output flag being set as well.
https://github.com/python/mypy/pull/11396#issuecomment-1048483794
@intgr Could you take a look at the comment by @tusharsadhwani ?
I'm afraid I can't help much with this. Just to clarify, I think my name shows up in the "reviewers" list only because I left a code comment about this PR earlier. I'm not a maintainer or reviewer in an official capacity and I don't have much knowledge of mypy's internals.
I'm just a user interested in consuming structured output from mypy (and potentially adding SARIF support later on). That's why I shared my opinions in this comment thread.
- the Github CodeQL that natively understands SARIF format
Github might, but Gitlab sadly doesn't :disappointed: https://docs.gitlab.com/ee/ci/testing/code_quality.html
I'd prefer if this wasn't a "SARIF MR" but rather as an "extensible output MR"
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
@JukkaL @ilevkivskyi hi!
Looks like this issue can unblock several interesting use-cases, like https://github.com/typeddjango/pytest-mypy-plugins/issues/96
What do you think?
@tusharsadhwani can you please solve the merge conflict?
@sobolevn Please feel free to finish the review yourself. I may not have time to look at this again this week.
