Update ModuleType.__file__ to be Optional

[ozanani]

Per the Python documentation, ModuleType.__file__ is Optional:

file is optional. If set, this attribute’s value must be a string. The import system may opt to leave file unset if it has no semantic meaning (e.g. a module loaded from a database).

Source: https://docs.python.org/3/reference/import.html#file__

[JelleZijlstra]

It sounds like they mean it is either a str or unset, not that it's a str or None. It's optional, not typing.Optional.

Unfortunately the type system has no way to express that an attribute may not exist.

[github-actions]

Diff from mypy_primer, showing the effect of this PR on open source code:

pip (https://github.com/pypa/pip.git)
+ src/pip/_internal/commands/debug.py:70: error: Value of type variable "AnyStr" of "dirname" cannot be "Optional[str]"
+ src/pip/_internal/commands/debug.py:70: error: List item 0 has incompatible type "Optional[str]"; expected "str"

ibis (https://github.com/ibis-project/ibis.git)
+ ibis/backends/tests/base.py:82: error: Argument 1 to "Path" has incompatible type "Union[str, None, Any]"; expected "Union[str, PathLike[str]]"

pytest (https://github.com/pytest-dev/pytest.git)
+ src/_pytest/pathlib.py:542: error: Item "None" of "Optional[str]" has no attribute "endswith"  [union-attr]
+ src/_pytest/pathlib.py:543: error: Value of type "Optional[str]" is not indexable  [index]
+ src/_pytest/pathlib.py:544: error: Item "None" of "Optional[str]" has no attribute "endswith"  [union-attr]
+ src/_pytest/pathlib.py:545: error: Value of type "Optional[str]" is not indexable  [index]
+ src/_pytest/pathlib.py:548: error: Argument 2 to "_is_same" has incompatible type "Optional[str]"; expected "str"  [arg-type]
+ src/_pytest/config/__init__.py:1469: error: Argument 1 to "Path" has incompatible type "Optional[str]"; expected "Union[str, PathLike[str]]"  [arg-type]
+ src/_pytest/python.py:330: error: Item "None" of "Optional[str]" has no attribute "endswith"  [union-attr]
+ src/_pytest/python.py:331: error: Value of type "Optional[str]" is not indexable  [index]
+ src/_pytest/python.py:332: error: Incompatible types in assignment (expression has type "Optional[str]", variable has type "Union[PathLike[str], str]")  [assignment]
- src/_pytest/python.py:946: error: Unexpected keyword argument "arg2scope" for "CallSpec2"  [call-arg]
- src/_pytest/python.py:946: error: Unexpected keyword argument "indices" for "CallSpec2"  [call-arg]
- src/_pytest/python.py:946: error: Unexpected keyword argument "idlist" for "CallSpec2"  [call-arg]
- src/_pytest/python.py:946: error: Unexpected keyword argument "marks" for "CallSpec2"  [call-arg]
- src/_pytest/logging.py:634: note: (Skipping most remaining errors due to unresolved imports or missing stubs; fix these first)

rich (https://github.com/willmcgugan/rich.git)
+ rich/traceback.py:249: error: Value of type variable "AnyStr" of "dirname" cannot be "Optional[str]"
+ rich/traceback.py:252: error: Value of type variable "AnyStr" of "normpath" cannot be "Optional[str]"
+ rich/traceback.py:252: error: Value of type variable "AnyStr" of "abspath" cannot be "Optional[str]"
+ rich/traceback.py:253: error: Argument 1 to "append" of "list" has incompatible type "Optional[str]"; expected "str"

[ozanani]

@JelleZijlstra I agree with your interpretation of the docs. However, I encountered a case in which __file__ was accessible and set to None which I'm trying to repro.
Also, the Python2 version of this file marks __file__ as Optional:
https://github.com/python/typeshed/blob/master/stdlib/@python2/types.pyi#L126

Which looks like an undesirable inconsistency between the two.

[Akuli]

However, I encountered a case in which _file_ was accessible and set to None which I'm trying to repro.

I would appreciate this. For imports not coming from files, I tried sys (built-in module, __file__ attribute doesn't exist) and gi.repository.Gtk (installed on most Linux systems, __file__ set to a dummy string '<gi.repository.Gtk>' as the importer follows PEP 302).

Also, the Python2 version of this file marks __file__ as Optional

This was introduced in c0aea8e which was before typeshed used pull requests, and the commit message doesn't say anything about this, so we can't really know whether this was intentional.

[ozanani]

Ok, apparently the repro is easy (when not using IPython, behaviour is different there).
When querying package.__file__, None is returned.

Repro by creating:

    package (directory)
    runner.py

Code for runner.py:

import package

print("package.__file__:", package.__file__)

And run:

$ python3 runner.py
package.__file__: None

[Akuli]

The repro applies only to namespace packages:

akuli@akuli-desktop:/tmp$ mkdir package
akuli@akuli-desktop:/tmp$ python3 -c 'import package; print(package.__file__)'
None
akuli@akuli-desktop:/tmp$ touch package/__init__.py
akuli@akuli-desktop:/tmp$ python3 -c 'import package; print(package.__file__)'
/tmp/package/__init__.py
akuli@akuli-desktop:/tmp$ 

On the one hand, namespace packages are quite rare in practice, so not worrying about them would be in the spirit of avoiding false positives. On the other hand, this seems like a corner case you would want to handle correctly, even if it's rare.

[hauntsaninja]

Trying to trace the mypy primer examples, I think this results in false positives (e.g. for rich, don't think you can get a traceback into a namespace, for pytest, it seems to check existence of __init__.py)
If you also consider the aspect that __file__ might not even exist, so we're not even warning about all the rare corner cases, I think I'd stick with the happy __file__: str

[ozanani]

So __file__ could be one of three things:

1. str
2. None (namespace packages)
3. Not set

I think that keeping it as __file__: str is fragile (and have already seen code breaks because of it). Solid str will make the reader rely on this without worrying too much. Setting it as Optional might make the reader google it and learn about the different cases, which could be beneficial.

[ozanani]

@Akuli , wdyt?

[Akuli]

I think this is a good idea. Other maintainers can merge if they agree.

[ozanani]

cc @srittau, @JelleZijlstra

[srittau]

I'm fine with this change.