microsoft / pyright

Static Type Checker for Python
Other
13.04k stars 1.39k forks source link

Inconsitstent typing of __doc__ in module and class #8388

Closed KPilnacek closed 1 month ago

KPilnacek commented 1 month ago

Describe the bug Revealed type of __doc__ on module level is different from __doc__ on class level even though both are writable (https://github.com/microsoft/pyright/issues/8006#issuecomment-2133804695) and both can become None when running with python -OO (https://github.com/python/mypy/issues/9170#issuecomment-661074382).

Code or Screenshots

"module doc"

reveal_type(__doc__)   # Type of "__doc__" is "str"

class Foo:
    """test_doc"""
    reveal_type(__doc__)  # Type of "__doc__" is "str | None"

Same is true even if docstring is missing on the class level.

"module doc"

reveal_type(__doc__)   # Type of "__doc__" is "str"

class Foo:
    reveal_type(__doc__)  # Type of "__doc__" is "str | None"

VS Code extension or command-line Example 1: https://pyright-play.net/?code=EQWw9gJgrgNgpgAgmAxsAsAKCwJzgNzgEMYB9AFwE8AHOAClNORUYEossUYiBnHhAGJgwALiwIJCYNPJwe5JqmkZMkhHkIkKNeo2ZsgA Example 2: https://pyright-play.net/?code=EQWw9gJgrgNgpgAgmAxsAsAKCwJzgNzgEMYB9AFwE8AHOAClNORUYEossUYiBnHhAGJgwALiwIJCPIRIUa9RszZA

Thanks for taking a look.

erictraut commented 1 month ago

Yeah, I agree this is inconsistent. Pyright manually adds symbols that are found on a module object, and it changes the type of __doc__ based on whether there's a module docstring present. This was done in response to this enhancement request from a couple of years ago. In retrospect, this was probably not the right thing to do because a module docstring, like its function counterpart, is writable, and None is a valid value. That means it should arguably have a declared type of str | None regardless of whether there's a docstring present.

I'll need to think about this more because making this consistent will probably be seen as some pyright users as a regression.

erictraut commented 1 month ago

@debonte, @rchiodo, the original enhancement request came in through pylance. Any thoughts on whether we should change this (effectively reverting the change I made back in Nov 2022) in the name of consistency?

rchiodo commented 1 month ago

The original enhancement was because the user didn't think they should have to mark their function as supporting None (if I'm reading it correctly?):

"""Hello"""

def echo(s: str):
    print(s)

echo(__doc__) # this used to show an error because echo doesn't support `str | None`

At first glance at least, it seems like the enhancement was wrong. Like the user could have done this dynamically later:

__doc__ = None
erictraut commented 1 month ago

OK, I've effectively reverted the previous change. The declared type of __doc__ is now str | None for both modules and functions. This change will be included in the next release.

erictraut commented 1 month ago

This is addressed in pyright 1.1.372.