-
Notifications
You must be signed in to change notification settings - Fork 244
Fix implicit-any error for bare tuple and Callable (#1970) #1973
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
base: main
Are you sure you want to change the base?
Conversation
When the implicit-any error is enabled, bare `tuple` and `Callable` annotations now generate errors like `list` already does. - Add error for `SpecialForm::Tuple`, `SpecialForm::Callable`, and `SpecialForm::Type` in `canonicalize_all_class_types` - Add error for `ClassDef(tuple)` to handle builtin tuple - Add test case covering the fix Note: bare builtin `type` still doesn't error because it's not defined as generic in typeshed. `typing.Type` does error correctly.
This comment has been minimized.
This comment has been minimized.
rchen152
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.
Thanks! The logic looks good, but it would be nice if we could reduce code duplication a bit.
pyrefly/lib/alt/expr.rs
Outdated
| errors.add( | ||
| range, | ||
| ErrorInfo::Kind(ErrorKind::ImplicitAny), | ||
| vec1![ |
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.
Would it be possible to pull out the error message creation from
pyrefly/pyrefly/lib/alt/class/targs.rs
Lines 176 to 183 in 713e5b7
| vec1![ | |
| format!( | |
| "Cannot determine the type parameter `{}` for generic class `{}`", | |
| tparam.name(), | |
| cls.name(), | |
| ), | |
| "Either specify the type argument explicitly, or specify a default for the type variable.".to_owned(), | |
| ], |
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
|
@grievejia has imported this pull request. If you are a Meta employee, you can view this in D90608813. |
rchen152
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.
Review automatically exported from Phabricator review in Meta.
Summary
Fixes #1970
When the
implicit-anyerror is enabled, baretupleandCallableannotations now generate errors, matching the existing behavior forlist.Before
After
Changes
ImplicitAnyerror forSpecialForm::Tuple,SpecialForm::Callable, andSpecialForm::Typeincanonicalize_all_class_typesImplicitAnyerror forClassDef(tuple)to handle the builtintupleNotes
typestill doesn't error because it's not defined as generic in typeshed (thetypeclass has no type parameters). However,typing.Typedoes error correctly.