stubtest: error if a class should be decorated with @final

[Akuli]

Description

Stubtest now errors if class SubClass(some_class): pass errors at runtime, but some_class isn't marked with @final in the stub.

Test Plan

New errors from --check-typeshed, checked against typeshed commit 92d135a3:

_curses.window cannot be subclassed at runtime, but isn't marked with @final in the stub
_json.make_encoder cannot be subclassed at runtime, but isn't marked with @final in the stub
_json.make_scanner cannot be subclassed at runtime, but isn't marked with @final in the stub
_tkinter.TkttType cannot be subclassed at runtime, but isn't marked with @final in the stub
asyncio.constants._SendfileMode cannot be subclassed at runtime, but isn't marked with @final in the stub
ctypes.Array cannot be subclassed at runtime, but isn't marked with @final in the stub
ctypes._SimpleCData cannot be subclassed at runtime, but isn't marked with @final in the stub
curses.window cannot be subclassed at runtime, but isn't marked with @final in the stub
hashlib.blake2b cannot be subclassed at runtime, but isn't marked with @final in the stub
hashlib.blake2s cannot be subclassed at runtime, but isn't marked with @final in the stub
http.HTTPStatus cannot be subclassed at runtime, but isn't marked with @final in the stub
inspect._ParameterKind cannot be subclassed at runtime, but isn't marked with @final in the stub
parser.STType cannot be subclassed at runtime, but isn't marked with @final in the stub
plistlib.PlistFormat cannot be subclassed at runtime, but isn't marked with @final in the stub
pstats.SortKey cannot be subclassed at runtime, but isn't marked with @final in the stub
py_compile.PycInvalidationMode cannot be subclassed at runtime, but isn't marked with @final in the stub
re.RegexFlag cannot be subclassed at runtime, but isn't marked with @final in the stub
select.epoll cannot be subclassed at runtime, but isn't marked with @final in the stub
signal.Handlers cannot be subclassed at runtime, but isn't marked with @final in the stub
signal.Sigmasks cannot be subclassed at runtime, but isn't marked with @final in the stub
signal.Signals cannot be subclassed at runtime, but isn't marked with @final in the stub
socket.AddressFamily cannot be subclassed at runtime, but isn't marked with @final in the stub
socket.AddressInfo cannot be subclassed at runtime, but isn't marked with @final in the stub
socket.MsgFlag cannot be subclassed at runtime, but isn't marked with @final in the stub
socket.SocketKind cannot be subclassed at runtime, but isn't marked with @final in the stub
sqlite3.PrepareProtocol cannot be subclassed at runtime, but isn't marked with @final in the stub
sqlite3.dbapi2.PrepareProtocol cannot be subclassed at runtime, but isn't marked with @final in the stub
ssl.AlertDescription cannot be subclassed at runtime, but isn't marked with @final in the stub
ssl.MemoryBIO cannot be subclassed at runtime, but isn't marked with @final in the stub
ssl.Options cannot be subclassed at runtime, but isn't marked with @final in the stub
ssl.Purpose cannot be subclassed at runtime, but isn't marked with @final in the stub
ssl.SSLErrorNumber cannot be subclassed at runtime, but isn't marked with @final in the stub
ssl.SSLSession cannot be subclassed at runtime, but isn't marked with @final in the stub
ssl.TLSVersion cannot be subclassed at runtime, but isn't marked with @final in the stub
ssl.VerifyFlags cannot be subclassed at runtime, but isn't marked with @final in the stub
ssl.VerifyMode cannot be subclassed at runtime, but isn't marked with @final in the stub
ssl._SSLMethod cannot be subclassed at runtime, but isn't marked with @final in the stub
tkinter.EventType cannot be subclassed at runtime, but isn't marked with @final in the stub
typing.ForwardRef cannot be subclassed at runtime, but isn't marked with @final in the stub
typing.TypeVar cannot be subclassed at runtime, but isn't marked with @final in the stub
typing._SpecialForm cannot be subclassed at runtime, but isn't marked with @final in the stub
typing_extensions._SpecialForm cannot be subclassed at runtime, but isn't marked with @final in the stub
unicodedata.UCD cannot be subclassed at runtime, but isn't marked with @final in the stub
uuid.SafeUUID cannot be subclassed at runtime, but isn't marked with @final in the stub
xxlimited.Xxo cannot be subclassed at runtime, but isn't marked with @final in the stub

[AlexWaygood]

There are several false positives in that list due to the fact that Enum classes with members are implicitly unsubclassable, even if they are not decorated with @final.

[AlexWaygood]

There are several false positives in that list due to the fact that Enum classes with members are implicitly unsubclassable, even if they are not decorated with @final.

In python/typeshed#6299, I avoided these by just doing a simple if not isinstance(cls, EnumMeta) check.

(I think you've picked up on several true positives that I missed due to the fact that I'm on Windows, while you're on Linux.)

[Akuli]

It now ignores enum classes. Here are the new results:

_curses.window cannot be subclassed at runtime, but isn't marked with @final in the stub
_json.make_encoder cannot be subclassed at runtime, but isn't marked with @final in the stub
_json.make_scanner cannot be subclassed at runtime, but isn't marked with @final in the stub
_tkinter.TkttType cannot be subclassed at runtime, but isn't marked with @final in the stub
ctypes.Array cannot be subclassed at runtime, but isn't marked with @final in the stub
ctypes._SimpleCData cannot be subclassed at runtime, but isn't marked with @final in the stub
curses.window cannot be subclassed at runtime, but isn't marked with @final in the stub
hashlib.blake2b cannot be subclassed at runtime, but isn't marked with @final in the stub
hashlib.blake2s cannot be subclassed at runtime, but isn't marked with @final in the stub
parser.STType cannot be subclassed at runtime, but isn't marked with @final in the stub
select.epoll cannot be subclassed at runtime, but isn't marked with @final in the stub
sqlite3.PrepareProtocol cannot be subclassed at runtime, but isn't marked with @final in the stub
sqlite3.dbapi2.PrepareProtocol cannot be subclassed at runtime, but isn't marked with @final in the stub
ssl.MemoryBIO cannot be subclassed at runtime, but isn't marked with @final in the stub
ssl.SSLSession cannot be subclassed at runtime, but isn't marked with @final in the stub
typing.ForwardRef cannot be subclassed at runtime, but isn't marked with @final in the stub
typing.TypeVar cannot be subclassed at runtime, but isn't marked with @final in the stub
typing._SpecialForm cannot be subclassed at runtime, but isn't marked with @final in the stub
typing_extensions._SpecialForm cannot be subclassed at runtime, but isn't marked with @final in the stub
unicodedata.UCD cannot be subclassed at runtime, but isn't marked with @final in the stub
xxlimited.Xxo cannot be subclassed at runtime, but isn't marked with @final in the stub

[AlexWaygood]

It now ignores enum classes. Here are the new results:

Haven't manually tested them all, but looks good to me!

[Akuli]

I made it not complain about classes that define __init_subclass__. This does not affect --check-typeshed output.

You can do this, for example, taken from the tests:

>>> class Foo:
...     def __init_subclass__(cls, e): print(e)
... 

Because __init_subclass__ exists, this obviously shouldn't be @final. But class Bar(Foo): pass still errors, because you need to pass an argument e when subclassing.

>>> class Bar(Foo): pass
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: __init_subclass__() missing 1 required positional argument: 'e'
>>> class Bar(Foo, e=123): pass
... 
123
>>> 

[Akuli]

I no longer think that looking at __init_subclass__ is a good idea, because defining __init_subclass__ and making it always raise an exception is an easy way to make a class not subclassable.

[AlexWaygood]

I no longer think that looking at __init_subclass__ is a good idea, because defining __init_subclass__ and making it always raise an exception is an easy way to make a class not subclassable.

If I were defining __init_subclass__ to make a class unsubclassable, I would do it like this:

class Uncooperative:
    def __init_subclass__(cls, **kwargs):
        raise TypeError("No.")

But if I were defining __init_subclass__ for some other, fancy reason, I would do it like this:

class Fancy:
    def __init_subclass__(cls, *, extra_param, **kwargs):
        # do something with extra_param
        super().__init_subclass__(**kwargs)

So perhaps you could distinguish between classes that have __init_subclass__ with extra named parameters, and those that don't.

[Akuli]

I could look at __init_subclass__ signature, but I want to keep this simple, at least for now. I can tweak this in future PRs if needed.

[AlexWaygood]

I could look at __init_subclass__ signature, but I want to keep this simple, at least for now. I can tweak this in future PRs if needed.

Makes sense!

[hauntsaninja]

Thanks!

Agreed that the false positive for __init_subclass__ doesn't matter too much. But I think I would prefer if we only reported the message for TypeError, e.g. the ctypes one is a false positive.

I'm curious if you think there's any value reporting for the reverse: stub marked final, but runtime not.

This is a fun step for stubtest. So far it's been pretty passive in how it interacts with runtime, just imports, getattrs and instancechecks. I'm totally fine with this, but figured I'd mention in case anyone knows some common evil patterns with side-effecting metaclasses or something. Note that there is a more passive version of this check that would work for C classes that aren't subclassable: bool(getattr(runtime, "__flags__") ^ (1 << 10))

[Akuli]

But I think I would prefer if we only reported the message for TypeError, e.g. the ctypes one is a false positive.

Done. Here's the diff of results:

-ctypes.Array cannot be subclassed at runtime, but isn't marked with @final in the stub
-ctypes._SimpleCData cannot be subclassed at runtime, but isn't marked with @final in the stub

I'm curious if you think there's any value reporting for the reverse: stub marked final, but runtime not.

I added a check like this, but didn't push it yet. Here are the errors it finds in stdlib:

+collections._OrderedDictItemsView can be subclassed at runtime, but is marked with @final in the stub
+collections._OrderedDictKeysView can be subclassed at runtime, but is marked with @final in the stub
+collections._OrderedDictValuesView can be subclassed at runtime, but is marked with @final in the stub
+functools._lru_cache_wrapper can be subclassed at runtime, but is marked with @final in the stub

All of these were added in python/typeshed#6299. @AlexWaygood Are these intentionally @final regardless of being subclassable, e.g. because subclassing them is a bad idea?

common evil patterns with side-effecting metaclasses or something

I don't recall coming across something like this, but I have seen plenty of magic done in __getattr__ :) But except Exception should be prepared for this anyway.

[AlexWaygood]

Check it out:

>>> import collections
>>> class Foo(type(collections.OrderedDict().keys())): ...
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: type 'odict_keys' is not valid as a base type
>>> class Foo(collections._OrderedDictKeysView): ...
>>> collections._OrderedDictKeysView is type(collections.OrderedDict().keys())
False

The C implementations of these classes differ from the pure-Python implementations of these classes.

[AlexWaygood]

figured I'd mention in case anyone knows some common evil patterns with side-effecting metaclasses or something.

I guess there's a theoretical risk of someone doing

class Foo:
    def __init_subclass__(cls, **kwargs):
        download_my_evil_virus()

Such that download_my_evil_virus() would be called when stubtest tried to subclass Foo. But I've never seen anything like that done in real code.

[Akuli]

Or you know, just call download_my_evil_virus() unconditionally when the module is imported :)

Stubtesting third-party stubs should be done with restricted permissions anyway, because it imports a lot of unreviewed code from pypi by design. This is why typeshed lets stubtest CI jobs only read the repository, for example.

[Akuli]

The C implementations of these classes differ from the pure-Python implementations of these classes.

In other words, the "stub final but runtime subclassable" error found 4 false positives and 0 true positives in stdlib. Not worth pushing, at least for now.

[AlexWaygood]

In other words, the "stub final but runtime subclassable" error found 4 false positives and 0 true positives in stdlib. Not worth pushing, at least for now.

I agree.

[hauntsaninja]

Thank you!

[sobolevn]

We still can have a reverse problem: @final stub annotation on regular type. See python/typeshed#8544

I will send a PR for this.

[AlexWaygood]

We still can have a reverse problem: @final stub annotation on regular type. See python/typeshed#8544

I will send a PR for this.

Yes, but see my comment here: #12091 (comment), and our ensuing discussion.