Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Let's get serious about handling private constructors and preventing subclassing #1021

Closed
njsmith opened this issue Apr 24, 2019 · 4 comments

Comments

@njsmith
Copy link
Member

commented Apr 24, 2019

In Trio we're pretty careful about our public API, because if it takes off then we want to defend ourselves against Hyrum's law. And because some of its internal state really is pretty delicate.

One consequence: we're kind of protective of some internal classes. For example, nurseries and cancel scopes both originally had no public class at all, because we didn't want to have a public constructor, or allow subclassing. (Even if not everyone shares my opinion that subclassing is a bad idea in general, I think it is at least generally agreed that you shouldn't subclass a type unless that type was intentionally written to enable subclassing, and these types definitely were not.)

Now, cancel scopes have gained a public constructor, and become a public class. And we want to make nurseries a public class too, to enable type annotations (#778). And I forget where now, but I saw someone post some sample code that subclassed CancelScope, and it was scary :-). So, we need to rethink this.

So, proposal: Come up with one metaclass/decorator/something that prevents subclassing, and one that prevents subclassing + hides the constructor. Use these for CancelScope, Nursery, etc. (And maybe everything else too!)

Preventing subclassing

In 3.6+, this is easy using PEP 487:

class Blah:
    def __init_subclass__(cls):
        raise TypeError("subclassing not supported")

You could also easily define a @final decorator to inject this into a class when it's defined. PEP 591 proposes to add a similar @final decorator, which works statically, rather than dynamically. You could combine the two using something like:

if TYPE_CHECKING:
    # static version
    from typing import final
else:
    # runtime version
    from ._util import final

However, for now we still need to support Python 3.5, which is a bit trickier. On 3.5 it requires defining a custom metaclass. Something like:

class Final(type):
    def __new__(cls, name, bases, cls_namespace):
        for base in bases:
            if isinstance(base, Final):
                raise TypeError(f"{base.__name__} doesn't support subclassing")
        return type(name, bases, cls_namespace)

It's not as easy to inject a metaclass using a decorator (it requires reconstructing the class object), but it's easy to use like:

@typing.final
class Nursery(metaclass=Final):
    ...

No public constructor

Say we have a normal class:

class NormalClass:
    def __init__(self, ...):
        ...

When we call NormalClass(...), that invokes type(NormalClass).__call__(...) (this is the same as calling any object in python!). Since type(NormalClass is type, this does type.__call__. And then type.__call__ implements all the logic we expect: calling NormalClass.__new__ and NormalClass.__init__, etc.

Now, we want to define a special class, where SpecialClass(...) raises an exception, but we can still construct new instances of SpecialClass – maybe via SpecialClass._create(...).

class NoPublicConstructor(type):
    def __call__(self, *args, **kwargs):
        raise TypeError("no public constructor")

    def _create(self, *args, **kwargs):
        return super().__call__(*args, **kwargs)

class SpecialClass(metaclass=NoPublicConstructor):
    def __init__(self, ...):
        # the class body looks totally normal
        # you just have to construct it using SpecialClass._create

Combining them

On 3.6+, then it's pretty easy to treat these as orthogonal: you can use a decorator to inject __init_subclass__, and then separately use metaclass=NoPublicConstructor. And if PEP 591 is accepted then this is probably ideal. So defining a class with both features would look like:

@final
class ExtraSpecialClass(metaclass=NoPublicConstructor):
    ...

To support 3.5, they both have to use a metaclass, and a class can only have one metaclass. In practice, everywhere we want to use NoPublicConstructor, we also want to forbid subclassing. So the simplest thing would be to make NoPublicConstructor a subclass of Final (ironic!). And then you'd use them like:

# Final class with public constructor
@typing.final  # purely for mypy's benefit
class FinalClass(metaclass=Final):
    ...

# Final class with no public constructor
@typing.final  # purely for mypy's benefit
class ExtraSpecialClass(metaclass=NoPublicConstructor):
    ...
@njsmith

This comment has been minimized.

Copy link
Member Author

commented Apr 25, 2019

Adding the good first issue tag... it's a bit more complicated than we usually like for good first issues, but with pycon sprints coming up I want to err on the side of having more issues tagged rather than less :-)

@jab

This comment has been minimized.

Copy link
Contributor

commented May 8, 2019

Could support for 3.5 be dropped anytime soon, given how much simpler it’d make things like this, and (I presume) 3.5’s comparatively low usage with Trio?

Looking at #75, looks like support for 3.5 could actually be dropped soon?

@smurfix

This comment has been minimized.

Copy link
Contributor

commented May 8, 2019

If by "soon" you mean "sometime after Debian releases Buster", then yes.

@njsmith

This comment has been minimized.

Copy link
Member Author

commented Jun 9, 2019

We have these metaclasses now (in #1043). There are still a ton of places where we should be using them where we aren't yet, but that's an ongoing project and we have #1044 for that, so I guess I'll close this in favor of #1044.

@njsmith njsmith closed this Jun 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.