Repository navigation
Idea: allow assert with platform or version check at top of file #5308
Description
Activity
I like it.
Another (minor) problem with the approach of enclosing the entire file in
if ...:is that mypy won't flag imports of the file, only attempts to use names from it. In other words, if we're using theif ...:approach and I'm not on Windows, I would not get an error forimport only_on_windows, but only foronly_on_windows.some_function(). This could be confusing for users.I think this is a good idea.
We also probably want to support this related idiom:
if sys.platform == 'linux': # stuff elif sys.platform == 'darwin': # other stuff else: assert False, "unsupported"@msullivan We already support platform checks. So if I run this:
import sys if sys.platform == 'darwin': def f() -> str: return '' else: assert False, "unsupported" reveal_type(f)on Linux, I get
fisAny, as the name is not defined on Linux/Windows. Are you suggesting that there be an error on the line withassert False?I think giving an error when mypy finds a reachable
assert Falseis going to cause a lot of false positives. And detecting sequences of if-elif-elif-else where all tests are sys.platform checks seems complicated and not worth it.Anyway, I just rechecked our original use case, and there we would actually want mypy to be silent if it finds a file containing
assert sys.platform == 'win32'and the platform isn't'win32'. The reason is that we statically compute a list of files to pass to mypy, and this file is always in it (the static computation is way too stupid to understand sys.platform checks). And on a non-win32 platform the contents of the file won't type-check at all (it imports other win32-specific modules, but since it is itself a win32-specific module, it doesn't wrap those in sys.platform checks). And the code that imports the module is inside a sys.platform check. And finally we don't want to edit the file to indent everything by an extra level and inserting a sys.platform check at the top, but we're fine adding a single assert at the top.So really all I want is for mypy to silently ignore the rest of a file when it encounters a top-level assert with a sys.platform check in it.
@ethanhs When I check that, I get:
lol.py:8: error: Revealed type is 'Any' lol.py:8: error: Name 'f' is not definedI don't mean giving an error on
assert False, I mean suppressing thefnot defined error (by not checking it).This is prompted by a request from an internal user trying to run mypy on multiple platforms with some code that looks like what I had above.
But if supporting it directly is a pain we can easily require adding a toplevel assert, if we implement that.I like the top-level assert idea. It looks like this is something we should prioritize?
- If it’s simple to do, yes.
This keeps coming, so I am raising priority to high.
To be clear, the implementation we're looking for is
for mypy to silently ignore the rest of a file when it encounters a top-level assert with a [non-matching]
sys.platformcheck in itWould it be really lame if I implemented this only if the
assertoccurs at the top level in a file, without any indentation (i.e. not inside e.g.if, and certainly not inside a function)?Would it be really lame if I implemented this only if the assert occurs at the top level in a file, without any indentation
If that would be much easier implement than supporting this everywhere, I don' think that it would very lame. We'd just need to document it as a very specific feature -- ignoring the whole file (or anything after the assert) on some platform(s).
I think it would be much easier. In semanal_pass1.py we can just truncate the list of defs when we encounter such an assert.
- added a commit that references this issue
on Nov 14, 2018 - added a commit that references this issue
on Nov 15, 2018
Some files only work for certain platforms, but sometimes it's inconvenient to move the file to a location where it'll only be picked up for that platform. And yet the file doesn't type check cleanly on another platform (or ditto for Python versions).
We could enclose the entire file in
if sys.platform == 'win32':but that seems overkill (and adding the indentation makes for an awkward diff). I propose an alternative that would have the same effect:assert sys.platform == 'win32'.