Reject type[...] with NewType aliases at definition time
#1752
Conversation
|
Hi @terror! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
pyrefly/lib/test/new_type.rs
Outdated
| from typing import NewType | ||
| Thing = NewType("Thing", int) | ||
| ThingType = type[Thing] |
There was a problem hiding this comment.
I'm wondering if we should also spend effort flagging invalid type[NewType] usages at annotation/alias definition time? Would be willing to follow up in a separate PR.
There was a problem hiding this comment.
Yes, we should add that check here:
pyrefly/pyrefly/lib/alt/expr.rs
Line 1939 in f245fb3
There was a problem hiding this comment.
I think if we have a type[NewType] we may want to just emit the error at the definition site, and infer the type as Any.
Then we wouldn't need a custom error everywhere it's being used, so the error at the definition site would supersede this PR.
There was a problem hiding this comment.
Yeah this makes more sense to me, I've re-purposed this PR to go down this route instead.
|
@migeed-z can you take this one? |
terror
changed the title
type[NewType] with NewType aliasestype[...] with NewType aliases at definition time
terror
force-pushed
the
explicit-newtype-error
branch
from
c3561b3 to
662e0d4
Compare
terror
force-pushed
the
explicit-newtype-error
branch
from
f9b8928 to
739d67f
Compare
| ThingType = type[Thing] # E: NewType `Thing` is not a class and cannot be used with `type` or `Type` | ||
| OtherThingType = Type[Thing] # E: NewType `Thing` is not a class and cannot be used with `type` or `Type` | ||
| mapping: dict[int, ThingType] = {1: Thing} # E: `dict[int, type[Thing]]` is not assignable to `dict[int, type[Unknown]]` |
There was a problem hiding this comment.
note: I explicitly chose to keep cascading errors here. There may be a case for hiding them, but I think it makes sense to surface them 🤔
3 participants
Resolves #1709
This diff validates
type[...]/Type[...]arguments during annotation sotyping.NewTypealiases produce animmediate invalid-annotation error instead of flowing through subtyping.