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

Add static analysis support to _util.Final #2683

Closed
jakkdl opened this issue Jul 4, 2023 · 3 comments · Fixed by #2793
Closed

Add static analysis support to _util.Final #2683

jakkdl opened this issue Jul 4, 2023 · 3 comments · Fixed by #2793
Labels
typing Adding static types to trio's interface

Comments

@jakkdl
Copy link
Member

jakkdl commented Jul 4, 2023

See discussion in #2668 (comment)

@final was added in 3.8, so it would be good to make classes decorated with _util.Final also be treated as @final by type checkers. I didn't find a good way of applying both with a single decorator or metaclass, so unless I missed a way we might have to have both on all classes. Or possibly skip one of them.

@A5rocks
Copy link
Contributor

A5rocks commented Jul 6, 2023

Might be a nice case for just a lint to ensure we have both? I can't think of a solution here unfortunately.

@A5rocks A5rocks added the typing Adding static types to trio's interface label Jul 21, 2023
@TeamSpen210
Copy link
Contributor

There is actually a better way, but it'd require some churn. By using __init_subclass__, we can implement _util.Final as a decorator. Then we can just make it an alias to the typing version if TYPE_CHECKING. A decorator's better anyway since we don't have to worry about metaclass conflicts.

The biggest downside is that we have to remember to use @final alongside NoPublicConstructor, since I can't think of a way to make the latter a decorator. We could fairly easily add a check to the test_exports logic to look for these cases though.

I don't think we need to care about backwards compatibility, since these are both private.

@jakkdl
Copy link
Member Author

jakkdl commented Sep 5, 2023

Sounds like a decent enough solution to me 👍

TeamSpen210 added a commit that referenced this issue Sep 18, 2023
…decorator. (#2793)

* Reimplement _util.Final as a decorator, replace usages.
* Use util.final along NoPublicConstructor
* Add test to verify that all NoPublicConstructor classes are also final.

---------


Co-authored-by: EXPLOSION <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typing Adding static types to trio's interface
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants