python/mypy

Mypy could be clearer that an invalid type comment or annotation prevents it from scanning the rest of the project

marmistrz opened this issue · 7 comments

TL;DR: some errors prevent mypy from further checking the project and this should be clearly stated in the log.

I was trying to add mypy type-checks to a big Python project. The branch I reproduced this problem on is: https://github.com/marmistrz/golem/blob/mypy-quirks

There's a defective function here which is properly linted with

$ mypy --ignore-missing-imports --strict-optional golem/model.py
golem/model.py:143: error: Item "None" of "Optional[Node]" has no attribute "to_json"

But now any of these:

mypy golem --ignore-missing-imports --strict-optional
mypy -m golem --ignore-missing-imports --strict-optional
mypy -p golem --ignore-missing-imports --strict-optional

doesn't yield that result (the variants golem and -p golem yield other errors but not this, -m golem yields no results)

What's more interesting, if I run

mypy golem/network --ignore-missing-imports --strict-optional

the same error, in golem/model.py is detected! Even an xargs invocation failed...

What was the problem? The second error message!

golem/core/async.py:34: error: Name 'default_errback' already defined on line 17
apps/rendering/resources/imgrepr.py:181: error: invalid type comment or annotation

The annotation was indeed incorrect:

def load_as_PILImgRepr(file_) -> PILImgRepr():

The second error made mypy abort further checking. It should be stated more clearly that this error prevents further scanning (e.g. even with capitals). This is very important when introducing incremental linting (only banning new findings) and it really puzzled me for some time that my mistake was not detected.

With the --verbose option mypy shows LOG: Bailing due to parse errors. But yes, it would be useful to mark an error fatal (such as syntax error or invalid type comment). Also I think it would make sense to mention in the docs that there are two kinds of errors and list the fatal ones.

PS. Next time you have an update like this, please don't just rewrite the initial comment -- just add another.

Re:

apps/rendering/resources/imgrepr.py:181: error: invalid type comment or annotation

Did you not get that error with other invocations, or did you initially not think it worth mentioning? With the current state of your branch I see it when I run either of these:

mypy -p golem --ignore-missing-imports
mypy golem --ignore-missing-imports

With -m golem you won't see it because that only checks whatever is imported by golem/__init__.py.

I agree with @ilevkivskyi that we should mark "blocker" errors more clearly in the output -- perhaps "error:" should be replaced with "FATAL:"?

Yeah, I didn't find it worth mentioning.
FATAL seems nice, but a "Further scanning aborted" message would be nice to add too

I'm working on it

It looks like GitHub doesn't allow assigning issues to non-core members, but I hope no-one else will start working on this.

@ilevkivskyi if someone else starts, I will contact with him/her

I just added #9674 which will change the error summary when we encounter fatal / blocking errors. I'm going to go ahead and close, but let me know if you think this isn't enough.