-
Notifications
You must be signed in to change notification settings - Fork 2.2k
[ty] Fix panic on incomplete except handlers #23708
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -304,12 +304,15 @@ impl<'db> SemanticModel<'db> { | |
| .scope(self.db) | ||
| .file_scope_id(self.db), | ||
| ), | ||
| ast::AnyNodeRef::ExceptHandlerExceptHandler(handler) => Some( | ||
| handler | ||
| .definition(self) | ||
| .scope(self.db) | ||
| .file_scope_id(self.db), | ||
| ), | ||
| ast::AnyNodeRef::ExceptHandlerExceptHandler(handler) => self | ||
| .except_handler_definition(handler) | ||
| .map(|definition| definition.scope(self.db).file_scope_id(self.db)) | ||
| .or_else(|| { | ||
| handler.type_.as_deref().and_then(|handled_exceptions| { | ||
| index.try_expression_scope_id(handled_exceptions) | ||
| }) | ||
| }) | ||
| .or(Some(FileScopeId::global())), | ||
| ast::AnyNodeRef::TypeParamTypeVar(var) => { | ||
| Some(var.definition(self).scope(self.db).file_scope_id(self.db)) | ||
| } | ||
|
|
@@ -325,6 +328,29 @@ impl<'db> SemanticModel<'db> { | |
| } | ||
| } | ||
|
|
||
| /// Returns the definition for an exception-handler variable. | ||
| /// | ||
| /// Exception handlers only have a definition when they bind a name (`except E as name:`). | ||
| pub fn except_handler_definition( | ||
| &self, | ||
| handler: &ast::ExceptHandlerExceptHandler, | ||
| ) -> Option<Definition<'db>> { | ||
| handler.name.as_ref()?; | ||
| let index = semantic_index(self.db, self.file); | ||
| Some(index.expect_single_definition(handler)) | ||
| } | ||
|
Comment on lines
+331
to
+341
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The API doesn't feel well aligned with
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| /// Returns the inferred type of an exception-handler variable. | ||
| /// | ||
| /// Exception handlers only bind a variable when they have a name (`except E as name:`). | ||
| pub fn except_handler_type( | ||
| &self, | ||
| handler: &ast::ExceptHandlerExceptHandler, | ||
| ) -> Option<Type<'db>> { | ||
| let definition = self.except_handler_definition(handler)?; | ||
| Some(binding_type(self.db, definition)) | ||
| } | ||
|
|
||
| /// Get a "safe" [`ast::AnyNodeRef`] to use for referring to the given (sub-)AST node. | ||
| /// | ||
| /// If we're analyzing a string annotation, it will return the string literal's node. | ||
|
|
@@ -641,7 +667,6 @@ impl_binding_has_ty_def!(ast::StmtFunctionDef); | |
| impl_binding_has_ty_def!(ast::StmtClassDef); | ||
| impl_binding_has_ty_def!(ast::Parameter); | ||
| impl_binding_has_ty_def!(ast::ParameterWithDefault); | ||
| impl_binding_has_ty_def!(ast::ExceptHandlerExceptHandler); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it was necessary to remove both the |
||
| impl_binding_has_ty_def!(ast::TypeParamTypeVar); | ||
|
|
||
| impl HasType for ast::Alias { | ||
|
|
||
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.