Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork3.1k
Add Error format support, and JSON output option#11396
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
mypy/error_formatter.py Outdated
| returnjson.dumps({ | ||
| 'file':file, | ||
| 'line':line, | ||
| 'column':column, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
line,column seems short-sighed.
MaybestartLine,startColumn, which would leave room to later addendLine,endColumn. This information is useful for IDEs to know how what span to highlight.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yeah, good point.
But mypy currently only does line and column, and it might be very long before spans are added in. It could be argued that the change tostartLine andstartColumn can be made when the feature exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Changing the field names later will break tools though. AndstartLine,startColumn would already be accurate right now, because mypy currently points out the start of the span.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'll leave this convention for the separate--output=sarif format.
intgr commentedOct 28, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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:
Cons:
[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 } } } ] } ] } ]} |
nvuillam commentedDec 8, 2021
+1 for SARIF format, let's use a common format instead of a mypy custom one ! :) |
tusharsadhwani commentedDec 8, 2021
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? |
intgr commentedDec 8, 2021
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. |
nvuillam commentedDec 8, 2021
I'm just a tourist here, but i'm currently activating SARIF output for all linters ofMegaLinter, 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, likebandit , maybe there is some code to reuse to manage the format ? |
tusharsadhwani commentedDec 8, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@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. |
nvuillam commentedDec 8, 2021
@tusharsadhwani thank but... I don't have the bandwidth to implement SARIF output on all linters that do not manage it yet 😅 |
intgr commentedDec 8, 2021
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 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} |
tusharsadhwani commentedDec 8, 2021
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. |
TomMD commentedJan 14, 2022
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 |
tusharsadhwani commentedJan 19, 2022
@intgr I did it properly this time. Hints should be fixed now. |
sehyun-hwang commentedFeb 22, 2022
|
tusharsadhwani commentedFeb 22, 2022
@sehyun-hwang sure thing. If you could provide a reproducible example I can look into it right away. |
sehyun-hwang commentedFeb 23, 2022
@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 |
sehyun-hwang commentedFeb 23, 2022
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? |
tusharsadhwani commentedFeb 23, 2022
@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]strict = True
deffoo():pass This way |
sehyun-hwang commentedFeb 23, 2022
@intgr Could you take a look at the comment by@tusharsadhwani ? |
intgr commentedFeb 23, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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. |
2bndy5 commentedMay 9, 2024
Yeah, the scope of this project has nothing to do with integrating various git servers' CI runners. That would be a rabbit hole of tech debt. |
mypy/errors.py Outdated
| # No error tuple found for this hint. Ignoring it | ||
| continue | ||
| iferror.hint=="": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Could we make hint into a list of strings instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It's often that a single hint is wrapped into multiple lines. For internal representaiton we can keep it as a list of strings but for the user I think it makes most sense to display it as a single string.
mypy/errors.py Outdated
| error_location= (file_path,line,column) | ||
| error=latest_error_at_location.get(error_location) | ||
| iferrorisNone: | ||
| # No error tuple found for this hint. Ignoring it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Why not instead generate a MypyError with some field that lets us indicate that this is a note?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Addressed.
mypy/options.py Outdated
| self.force_union_syntax=False | ||
| # Sets custom output format | ||
| self.output="" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Would None be a better default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yup.
| foo('42') | ||
| def bar() -> None: ... | ||
| bar('42') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Add a test case withreveal_type(); let's make sure that shows up in the output.
intgr commentedMay 10, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
If mypy will only support 1 machine-parsable output format, then please choose a standard or de facto recognized format. Don't create yet another tool-specific mypy output format that everybody then has to write another set of adapters for. For example, tools already exist to convert SARIF into GitLab "CodeQuality" reportshttps://gitlab.com/ahogen/sarif-to-codequality. |
According tomypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
tusharsadhwani commentedMay 10, 2024
FWIW I prefer current output as it is leaner and it's trivial for any tool to translate it to SARIF (pyright rejected the idea of adding SARIF forthe same reason). But I will add a SARIF formatter in the same PR if maintainers agree to it. |
test-data/unit/outputjson.test Outdated
| [out] | ||
| {"file": "main", "line": 12, "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"} | ||
| {"file": "main", "line": 15, "column": 0, "message": "Too many arguments for \"bar\"", "hint": null, "code": "call-arg"} | ||
| {"file": "main", "line": 12, "column": 12, "message": "Revealed type is \"Overload(def (), def (x: builtins.int))\"", "hint": null, "code": "misc", "is_note": true} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Could we change this to"severity": "note", mirroring mypy's Python code (and severity error for the errors)? That will also make it easier to extend to other kinds of severity in the future.
dgutson commentedMay 10, 2024
which I just commented.
Otherwise it will require yet another converter-to-sarif tool. There is plenty of support already in IDEs, linters aggregators, and other analysis tools. If you think that SARIF could be leaner, then I do propose you to participate in its committee (I'm not being cinic, I mean it, you may have a good idea), make your proposal, and discuss it. That would be the right approach so the whole static analysis community gets benefited. |
JelleZijlstra commentedMay 10, 2024
If I merge this PR, I take on the responsibility (at least in my mind) for triaging and reviewing future fixes to this area. I'm OK with doing that for this simple JSON format; I'm not OK with having to maintain compliance with the external SARIF spec. If another maintainer wants to add SARIF support, I'd welcome it, since clearly it will help some users, but I'm not interested in taking ownership of it. |
According tomypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
JelleZijlstra commentedMay 10, 2024
I think this is close to ready. We'll also want to document the JSON format in the mypy documentation, but that can wait for a separate PR. |
tusharsadhwani commentedMay 10, 2024
@dgutson Thanks for bringing this PR to my attention again :)
I just made and verified a scipt to run mypy with SARIF output, dropping it here incase it is useful to someone:https://gist.github.com/tusharsadhwani/5fd7a3c32d73f10fcb86e8deac4b60cf |
dgutson commentedMay 10, 2024
could you please add it to the source tree? Otherwise it will be lost in a PR comment :) |
tusharsadhwani commentedMay 10, 2024
I think Jelle has made his stance clear. I'd rather not have this feature delayed potentially a few more months over that. Although I don't mind SARIF support, I'm not particularly looking for it, and neither are the maintainers super stoked about it yet, it seems. At this moment I think it'd be best to make a different issue for SARIF support. Feel free to use the gist as a basis for the PR against that issue. |
patrick91 commentedMay 27, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@JelleZijlstra sorry for always being that person, but is this released? Was just working on some automated tests for Strawberry and having JSON output would be make it much easier (we use that for pyright as well) |
AlexWaygood commentedMay 27, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
not yet released, but you can track the progress of the next release in#17285. It should include this PR. |
JelleZijlstra commentedMay 27, 2024
Documents the new option added by#11396---------Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
claudeviool commentedFeb 5, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Nice, would it be possible to have mypy write this to a file (and otherwise retain human-readable output)? Or, have this JSON output format work like the other 'reports' that MyPy provides? |
tusharsadhwani commentedFeb 5, 2025
@x0f0 what's the use case? |

Uh oh!
There was an error while loading.Please reload this page.
Description
Resolves#10816
The changes this PR makes are relatively small.
It currently:
--outputoption to mypy CLIErrorFormatterabstract base class, which can be subclassed to create new output formatsMypyErrorclass that represents the external format of a mypy error.--outputbeing'json', in which case theJSONFormatteris used to produce the reported output.Demo:
A few notes regarding the changes:
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.--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.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 the
namefield" part of this code might be appropriate to add within this PR itself, instead of hard-codingJSONFormatter. Does that sound like a good idea?