-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ty] Forbid use of super() in NamedTuple subclasses
#21700
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
Conversation
42b3fb3 to
05bcf6b
Compare
super() in NamedTuple subclassessuper() in NamedTuple subclasses
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
ca3d7ae to
25b9574
Compare
AlexWaygood
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, this is excellent! Just a few nitpicks really
|
|
||
| ## `super()` is not supported in NamedTuple methods | ||
|
|
||
| Using `super()` in a method of a `NamedTuple` subclass will raise an exception at runtime. In Python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extremely nitpicky comment: my preferred term is "NamedTuple class" rather than "NamedTuple subclass". The generated class doesn't actually have NamedTuple in its MRO (because NamedTuple is actually a function rather than a class!) -- the generated class actually inherits directly from tuple. The runtime does some magic to swap out NamedTuple for tuple in the class's bases, so it feels slightly incorrect to refer to it as a "subclass of" NamedTuple, even if NamedTuple is included in the class's bases.
| Using `super()` in a method of a `NamedTuple` subclass will raise an exception at runtime. In Python | |
| Using `super()` in a method of a `NamedTuple` class will raise an exception at runtime. In Python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could also add a test to verify that using super() on a NamedTuple class works fine if it occurs outside the class, e.g.
class F(NamedTuple):
x: int
super(F, F(42)) # fine| .report_lint(&SUPER_CALL_IN_NAMED_TUPLE_METHOD, call_expression) | ||
| { | ||
| builder.into_diagnostic(format_args!( | ||
| "Cannot use `super()` in a method of NamedTuple subclass `{}`", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "Cannot use `super()` in a method of NamedTuple subclass `{}`", | |
| "Cannot use `super()` in a method of NamedTuple class `{}`", |
| .report_lint(&SUPER_CALL_IN_NAMED_TUPLE_METHOD, call_expression) | ||
| { | ||
| builder.into_diagnostic(format_args!( | ||
| "Cannot use `super()` in a method of NamedTuple subclass `{}`", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "Cannot use `super()` in a method of NamedTuple subclass `{}`", | |
| "Cannot use `super()` in a method of NamedTuple class `{}`", |
|
|
||
| declare_lint! { | ||
| /// ## What it does | ||
| /// Checks for calls to `super()` inside methods of `NamedTuple` subclasses. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| /// Checks for calls to `super()` inside methods of `NamedTuple` subclasses. | |
| /// Checks for calls to `super()` inside methods of `NamedTuple` classes. |
| /// Checks for calls to `super()` inside methods of `NamedTuple` subclasses. | ||
| /// | ||
| /// ## Why is this bad? | ||
| /// Using `super()` in a method of a `NamedTuple` subclass will raise an exception at runtime. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| /// Using `super()` in a method of a `NamedTuple` subclass will raise an exception at runtime. | |
| /// Using `super()` in a method of a `NamedTuple` class will raise an exception at runtime. |
| summary: "detects `super()` calls in methods of `NamedTuple` subclasses", | ||
| status: LintStatus::preview("1.0.0"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| summary: "detects `super()` calls in methods of `NamedTuple` subclasses", | |
| status: LintStatus::preview("1.0.0"), | |
| summary: "detects `super()` calls in methods of `NamedTuple` classes", | |
| status: LintStatus::preview("0.0.1-alpha.30"), |
yeah, I think that comment was just incorrect |
aa3249c to
664df36
Compare
CodSpeed Performance ReportMerging #21700 will improve performances by 4.45%Comparing Summary
Benchmarks breakdown
Footnotes
|
Summary
The exact behavior around what's allowed vs. disallowed was partly detected through trial and error in the runtime.
I was a little confused by this comment that says "
NamedTuplesubclasses cannot be inherited from" because in practice that doesn't appear to error at runtime.Closes #1683.