Replace assert with warning #7567
Conversation
| @@ -310,7 +310,11 @@ def report(self, | |||
| self.add_error_info(info) | |||
|
|
|||
| def _add_error_info(self, file: str, info: ErrorInfo) -> None: | |||
| assert file not in self.flushed_files | |||
| if file in self.flushed_files: | |||
| self.report(info.line, None, | |||
There was a problem hiding this comment.
I copied the basic format for the warning from
Line 122 in 00530db
Note that only_once=True is necessary to prevent recursing infinitely.
| self.report(info.line, None, | ||
| "Trying to report an error in a file that has already been flushed." | ||
| " Skipping...", severity='warning', file=file, only_once=True) | ||
| return |
There was a problem hiding this comment.
Hmm... I think I want to return here so that the errors don't get logged multiple times. But that means this warning gets skipped too :/
There was a problem hiding this comment.
Thanks for PR!
One general comment: I think we should give this warning, even if there were no errors in the file, so maybe this is not the best place to show it. I would even make this a blocker error somewhere in mypy/build.py, similarly to how we ban duplicate modules.
Also maybe try adding some diagnostics to the error message, something like:
some/bad.py: error: This file name can be interpreted as more than one module
some/bad.py: error: When read from MYPYPATH: src.some.bad
some/bad.py: error: When read from command line: some.bad
The latter is optional.
|
I'm hesitant to make this a blocker error, since (as an end user) it's not clear to me how to resolve it. It would also be a potentially breaking change for anybody that's currently in this situation but not hitting any errors and thus not hitting the assert (although that only affects people with perfect code 😄). Also, I should clarify that I'm encountering this without setting |
Even if you will not make it a blocker, it effectively will be one, since there is no line number it can be attributed to, one can't put a Also conceptually if there is this error, it means that something went really wrong, and likely type-checking results can't be trusted. |
|
Agreed that it should be a blocker |
|
I worked around the original problem by not using namespace packages. |
Addresses issue #7510 (along with a few others that have a different root cause. The conclusion from the discussion was that if we check the same file twice, something is probably weird, but mypy shouldn't crash.
I still need to add a test for this, but I didn't see a test file corresponding to this one. Suggestions on where to add it?