From fc016d23836a14f69428381f8d5ef5799711e766 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 4 Mar 2026 09:46:46 -0500 Subject: [PATCH 1/3] [ty] Fix panic on incomplete except handlers --- crates/ty_ide/src/completion.rs | 17 +++++++ crates/ty_ide/src/goto.rs | 20 ++++---- crates/ty_ide/src/hover.rs | 14 ++++++ .../ty_python_semantic/src/semantic_model.rs | 47 ++++++++++++++++--- 4 files changed, 83 insertions(+), 15 deletions(-) diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs index eea8dcdd5a6825..a868b317c28026 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -5334,6 +5334,23 @@ except Type: ); } + // Ref: https://github.com/astral-sh/ty/issues/2401 + #[test] + fn no_panic_incomplete_except_handler() { + let builder = completion_test_builder( + "\ +try: + print() +except # Trigger completion/hover here +", + ); + + assert_snapshot!( + builder.skip_keywords().skip_builtins().skip_auto_import().build().snapshot(), + @"", + ); + } + // Ref: https://github.com/astral-sh/ty/issues/572 #[test] fn scope_id_missing_global1() { diff --git a/crates/ty_ide/src/goto.rs b/crates/ty_ide/src/goto.rs index d00e2b93e7c80b..869f1c70500446 100644 --- a/crates/ty_ide/src/goto.rs +++ b/crates/ty_ide/src/goto.rs @@ -515,11 +515,11 @@ impl GotoTarget<'_> { )), // For exception variables, they are their own definitions (like parameters) - GotoTarget::ExceptVariable(except_handler) => { - Some(vec![ResolvedDefinition::Definition( + GotoTarget::ExceptVariable(except_handler) => except_handler.name.as_ref().map(|_| { + vec![ResolvedDefinition::Definition( except_handler.definition(model), - )]) - } + )] + }), // Patterns are glorified assignments but we have to look them up by ident // because they're not expressions @@ -949,9 +949,10 @@ impl GotoTarget<'_> { None } - Some(AnyNodeRef::ExceptHandlerExceptHandler(handler)) => { - Some(GotoTarget::ExceptVariable(handler)) - } + Some(AnyNodeRef::ExceptHandlerExceptHandler(handler)) => handler + .name + .is_some() + .then_some(GotoTarget::ExceptVariable(handler)), Some(AnyNodeRef::Keyword(keyword)) => { // Find the containing call expression from the ancestor chain let call_expression = covering_node @@ -1139,7 +1140,10 @@ impl Ranged for GotoTarget<'_> { } => *component_range, GotoTarget::StringAnnotationSubexpr { subrange, .. } => *subrange, GotoTarget::ImportModuleAlias { asname, .. } => asname.range, - GotoTarget::ExceptVariable(except) => except.name.as_ref().unwrap().range, + GotoTarget::ExceptVariable(except) => except + .name + .as_ref() + .map_or(except.range(), |name| name.range), GotoTarget::KeywordArgument { keyword, .. } => keyword.arg.as_ref().unwrap().range, GotoTarget::PatternMatchRest(rest) => rest.rest.as_ref().unwrap().range, GotoTarget::PatternKeywordArgument(keyword) => keyword.attr.range, diff --git a/crates/ty_ide/src/hover.rs b/crates/ty_ide/src/hover.rs index a448a7b2fc9c37..c36fc71f0fbbbb 100644 --- a/crates/ty_ide/src/hover.rs +++ b/crates/ty_ide/src/hover.rs @@ -4952,6 +4952,20 @@ def function(): "); } + // Ref: https://github.com/astral-sh/ty/issues/2401 + #[test] + fn hover_incomplete_except_handler() { + let test = cursor_test( + "\ +try: + print() +except # Trigger completion/hover here +", + ); + + assert_snapshot!(test.hover(), @"Hover provided no content"); + } + impl CursorTest { fn hover(&self) -> String { use std::fmt::Write; diff --git a/crates/ty_python_semantic/src/semantic_model.rs b/crates/ty_python_semantic/src/semantic_model.rs index 999750e27682b2..7f3859b4f5fac1 100644 --- a/crates/ty_python_semantic/src/semantic_model.rs +++ b/crates/ty_python_semantic/src/semantic_model.rs @@ -304,12 +304,24 @@ 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) => { + if handler.name.is_some() { + Some( + handler + .definition(self) + .scope(self.db) + .file_scope_id(self.db), + ) + } 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)) } @@ -641,9 +653,30 @@ 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); impl_binding_has_ty_def!(ast::TypeParamTypeVar); +impl HasDefinition for ast::ExceptHandlerExceptHandler { + #[inline] + fn definition<'db>(&self, model: &SemanticModel<'db>) -> Definition<'db> { + debug_assert!( + self.name.is_some(), + "except handlers only have a definition when they bind a name" + ); + + let index = semantic_index(model.db, model.file); + index.expect_single_definition(self) + } +} + +impl HasType for ast::ExceptHandlerExceptHandler { + #[inline] + fn inferred_type<'db>(&self, model: &SemanticModel<'db>) -> Option> { + self.name.as_ref()?; + let binding = HasDefinition::definition(self, model); + Some(binding_type(model.db, binding)) + } +} + impl HasType for ast::Alias { fn inferred_type<'db>(&self, model: &SemanticModel<'db>) -> Option> { if &self.name == "*" { From 4c68f9773a7d71f900258fdd66301a596e8d1376 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 4 Mar 2026 09:54:44 -0500 Subject: [PATCH 2/3] Make optional --- crates/ty_ide/src/goto.rs | 18 +++++---- .../ty_python_semantic/src/semantic_model.rs | 40 +++++++++---------- .../src/types/ide_support.rs | 6 ++- 3 files changed, 35 insertions(+), 29 deletions(-) diff --git a/crates/ty_ide/src/goto.rs b/crates/ty_ide/src/goto.rs index 869f1c70500446..cd2e6bd2038e71 100644 --- a/crates/ty_ide/src/goto.rs +++ b/crates/ty_ide/src/goto.rs @@ -459,13 +459,15 @@ impl GotoTarget<'_> { Some(defs) } - GotoTarget::ClassDef(class) => Some(vec![ResolvedDefinition::Definition( - class.definition(model), - )]), + GotoTarget::ClassDef(class) => class + .definition(model) + .map(ResolvedDefinition::Definition) + .map(|definition| vec![definition]), - GotoTarget::Parameter(parameter) => Some(vec![ResolvedDefinition::Definition( - parameter.definition(model), - )]), + GotoTarget::Parameter(parameter) => parameter + .definition(model) + .map(ResolvedDefinition::Definition) + .map(|definition| vec![definition]), // For import aliases (offset within 'y' or 'z' in "from x import y as z") GotoTarget::ImportSymbolAlias { asname, .. } => Some(definitions_for_name( @@ -517,7 +519,9 @@ impl GotoTarget<'_> { // For exception variables, they are their own definitions (like parameters) GotoTarget::ExceptVariable(except_handler) => except_handler.name.as_ref().map(|_| { vec![ResolvedDefinition::Definition( - except_handler.definition(model), + except_handler + .definition(model) + .expect("named except handlers should have a definition"), )] }), diff --git a/crates/ty_python_semantic/src/semantic_model.rs b/crates/ty_python_semantic/src/semantic_model.rs index 7f3859b4f5fac1..cbf87bc50f3723 100644 --- a/crates/ty_python_semantic/src/semantic_model.rs +++ b/crates/ty_python_semantic/src/semantic_model.rs @@ -285,22 +285,25 @@ impl<'db> SemanticModel<'db> { // Nodes implementing `HasDefinition` ast::AnyNodeRef::StmtFunctionDef(function) => Some( function - .definition(self) + .definition(self)? + .scope(self.db) + .file_scope_id(self.db), + ), + ast::AnyNodeRef::StmtClassDef(class) => Some( + class + .definition(self)? .scope(self.db) .file_scope_id(self.db), ), - ast::AnyNodeRef::StmtClassDef(class) => { - Some(class.definition(self).scope(self.db).file_scope_id(self.db)) - } ast::AnyNodeRef::Parameter(parameter) => Some( parameter - .definition(self) + .definition(self)? .scope(self.db) .file_scope_id(self.db), ), ast::AnyNodeRef::ParameterWithDefault(parameter) => Some( parameter - .definition(self) + .definition(self)? .scope(self.db) .file_scope_id(self.db), ), @@ -308,7 +311,7 @@ impl<'db> SemanticModel<'db> { if handler.name.is_some() { Some( handler - .definition(self) + .definition(self)? .scope(self.db) .file_scope_id(self.db), ) @@ -323,7 +326,7 @@ impl<'db> SemanticModel<'db> { } } ast::AnyNodeRef::TypeParamTypeVar(var) => { - Some(var.definition(self).scope(self.db).file_scope_id(self.db)) + Some(var.definition(self)?.scope(self.db).file_scope_id(self.db)) } // Fallback @@ -526,7 +529,7 @@ pub trait HasDefinition { /// /// ## Panics /// May panic if `self` is from another file than `model`. - fn definition<'db>(&self, model: &SemanticModel<'db>) -> Definition<'db>; + fn definition<'db>(&self, model: &SemanticModel<'db>) -> Option>; } impl HasType for ast::ExprRef<'_> { @@ -633,16 +636,16 @@ macro_rules! impl_binding_has_ty_def { ($ty: ty) => { impl HasDefinition for $ty { #[inline] - fn definition<'db>(&self, model: &SemanticModel<'db>) -> Definition<'db> { + fn definition<'db>(&self, model: &SemanticModel<'db>) -> Option> { let index = semantic_index(model.db, model.file); - index.expect_single_definition(self) + Some(index.expect_single_definition(self)) } } impl HasType for $ty { #[inline] fn inferred_type<'db>(&self, model: &SemanticModel<'db>) -> Option> { - let binding = HasDefinition::definition(self, model); + let binding = HasDefinition::definition(self, model)?; Some(binding_type(model.db, binding)) } } @@ -657,22 +660,17 @@ impl_binding_has_ty_def!(ast::TypeParamTypeVar); impl HasDefinition for ast::ExceptHandlerExceptHandler { #[inline] - fn definition<'db>(&self, model: &SemanticModel<'db>) -> Definition<'db> { - debug_assert!( - self.name.is_some(), - "except handlers only have a definition when they bind a name" - ); - + fn definition<'db>(&self, model: &SemanticModel<'db>) -> Option> { + self.name.as_ref()?; let index = semantic_index(model.db, model.file); - index.expect_single_definition(self) + Some(index.expect_single_definition(self)) } } impl HasType for ast::ExceptHandlerExceptHandler { #[inline] fn inferred_type<'db>(&self, model: &SemanticModel<'db>) -> Option> { - self.name.as_ref()?; - let binding = HasDefinition::definition(self, model); + let binding = HasDefinition::definition(self, model)?; Some(binding_type(model.db, binding)) } } diff --git a/crates/ty_python_semantic/src/types/ide_support.rs b/crates/ty_python_semantic/src/types/ide_support.rs index 5a545310b38937..4e101fec125748 100644 --- a/crates/ty_python_semantic/src/types/ide_support.rs +++ b/crates/ty_python_semantic/src/types/ide_support.rs @@ -554,7 +554,11 @@ pub fn definitions_and_overloads_for_function<'db>( .map(ResolvedDefinition::Definition) .collect() } else { - vec![ResolvedDefinition::Definition(function.definition(model))] + function + .definition(model) + .map(ResolvedDefinition::Definition) + .into_iter() + .collect() } } From 223ae1f63a66689694109e15732074b2a970af4e Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 4 Mar 2026 19:07:33 -0500 Subject: [PATCH 3/3] Remove implementation --- crates/ty_ide/src/completion.rs | 20 ++++ crates/ty_ide/src/goto.rs | 26 ++---- .../ty_python_semantic/src/semantic_model.rs | 92 +++++++++---------- .../src/types/ide_support.rs | 6 +- 4 files changed, 74 insertions(+), 70 deletions(-) diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs index a868b317c28026..40ef8c47da5efe 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -5351,6 +5351,26 @@ except # Trigger completion/hover here ); } + // Ref: https://github.com/astral-sh/ty/issues/2401 + #[test] + fn incomplete_except_handler_uses_enclosing_scope() { + completion_test_builder( + "\ +def f(): + sentinel = 1 + try: + print() + except as err: + pass +", + ) + .skip_keywords() + .skip_builtins() + .skip_auto_import() + .build() + .contains("sentinel"); + } + // Ref: https://github.com/astral-sh/ty/issues/572 #[test] fn scope_id_missing_global1() { diff --git a/crates/ty_ide/src/goto.rs b/crates/ty_ide/src/goto.rs index cd2e6bd2038e71..dae5ec67cdd608 100644 --- a/crates/ty_ide/src/goto.rs +++ b/crates/ty_ide/src/goto.rs @@ -331,7 +331,7 @@ impl GotoTarget<'_> { GotoTarget::ImportSymbolAlias { alias, .. } | GotoTarget::ImportModuleAlias { alias, .. } | GotoTarget::ImportExportedName { alias, .. } => alias.inferred_type(model), - GotoTarget::ExceptVariable(except) => except.inferred_type(model), + GotoTarget::ExceptVariable(except) => model.except_handler_type(except), GotoTarget::KeywordArgument { keyword, .. } => keyword.value.inferred_type(model), // When asking the type of a callable, usually you want the callable itself? // (i.e. the type of `MyClass` in `MyClass()` is `` and not `() -> MyClass`) @@ -459,15 +459,13 @@ impl GotoTarget<'_> { Some(defs) } - GotoTarget::ClassDef(class) => class - .definition(model) - .map(ResolvedDefinition::Definition) - .map(|definition| vec![definition]), + GotoTarget::ClassDef(class) => Some(vec![ResolvedDefinition::Definition( + class.definition(model), + )]), - GotoTarget::Parameter(parameter) => parameter - .definition(model) - .map(ResolvedDefinition::Definition) - .map(|definition| vec![definition]), + GotoTarget::Parameter(parameter) => Some(vec![ResolvedDefinition::Definition( + parameter.definition(model), + )]), // For import aliases (offset within 'y' or 'z' in "from x import y as z") GotoTarget::ImportSymbolAlias { asname, .. } => Some(definitions_for_name( @@ -517,13 +515,9 @@ impl GotoTarget<'_> { )), // For exception variables, they are their own definitions (like parameters) - GotoTarget::ExceptVariable(except_handler) => except_handler.name.as_ref().map(|_| { - vec![ResolvedDefinition::Definition( - except_handler - .definition(model) - .expect("named except handlers should have a definition"), - )] - }), + GotoTarget::ExceptVariable(except_handler) => model + .except_handler_definition(except_handler) + .map(|definition| vec![ResolvedDefinition::Definition(definition)]), // Patterns are glorified assignments but we have to look them up by ident // because they're not expressions diff --git a/crates/ty_python_semantic/src/semantic_model.rs b/crates/ty_python_semantic/src/semantic_model.rs index cbf87bc50f3723..2aaadea41a811e 100644 --- a/crates/ty_python_semantic/src/semantic_model.rs +++ b/crates/ty_python_semantic/src/semantic_model.rs @@ -285,48 +285,36 @@ impl<'db> SemanticModel<'db> { // Nodes implementing `HasDefinition` ast::AnyNodeRef::StmtFunctionDef(function) => Some( function - .definition(self)? - .scope(self.db) - .file_scope_id(self.db), - ), - ast::AnyNodeRef::StmtClassDef(class) => Some( - class - .definition(self)? + .definition(self) .scope(self.db) .file_scope_id(self.db), ), + ast::AnyNodeRef::StmtClassDef(class) => { + Some(class.definition(self).scope(self.db).file_scope_id(self.db)) + } ast::AnyNodeRef::Parameter(parameter) => Some( parameter - .definition(self)? + .definition(self) .scope(self.db) .file_scope_id(self.db), ), ast::AnyNodeRef::ParameterWithDefault(parameter) => Some( parameter - .definition(self)? + .definition(self) .scope(self.db) .file_scope_id(self.db), ), - ast::AnyNodeRef::ExceptHandlerExceptHandler(handler) => { - if handler.name.is_some() { - Some( - handler - .definition(self)? - .scope(self.db) - .file_scope_id(self.db), - ) - } else { - handler - .type_ - .as_deref() - .and_then(|handled_exceptions| { - index.try_expression_scope_id(handled_exceptions) - }) - .or(Some(FileScopeId::global())) - } - } + 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)) + Some(var.definition(self).scope(self.db).file_scope_id(self.db)) } // Fallback @@ -340,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> { + handler.name.as_ref()?; + let index = semantic_index(self.db, self.file); + Some(index.expect_single_definition(handler)) + } + + /// 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> { + 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. @@ -529,7 +540,7 @@ pub trait HasDefinition { /// /// ## Panics /// May panic if `self` is from another file than `model`. - fn definition<'db>(&self, model: &SemanticModel<'db>) -> Option>; + fn definition<'db>(&self, model: &SemanticModel<'db>) -> Definition<'db>; } impl HasType for ast::ExprRef<'_> { @@ -636,16 +647,16 @@ macro_rules! impl_binding_has_ty_def { ($ty: ty) => { impl HasDefinition for $ty { #[inline] - fn definition<'db>(&self, model: &SemanticModel<'db>) -> Option> { + fn definition<'db>(&self, model: &SemanticModel<'db>) -> Definition<'db> { let index = semantic_index(model.db, model.file); - Some(index.expect_single_definition(self)) + index.expect_single_definition(self) } } impl HasType for $ty { #[inline] fn inferred_type<'db>(&self, model: &SemanticModel<'db>) -> Option> { - let binding = HasDefinition::definition(self, model)?; + let binding = HasDefinition::definition(self, model); Some(binding_type(model.db, binding)) } } @@ -658,23 +669,6 @@ impl_binding_has_ty_def!(ast::Parameter); impl_binding_has_ty_def!(ast::ParameterWithDefault); impl_binding_has_ty_def!(ast::TypeParamTypeVar); -impl HasDefinition for ast::ExceptHandlerExceptHandler { - #[inline] - fn definition<'db>(&self, model: &SemanticModel<'db>) -> Option> { - self.name.as_ref()?; - let index = semantic_index(model.db, model.file); - Some(index.expect_single_definition(self)) - } -} - -impl HasType for ast::ExceptHandlerExceptHandler { - #[inline] - fn inferred_type<'db>(&self, model: &SemanticModel<'db>) -> Option> { - let binding = HasDefinition::definition(self, model)?; - Some(binding_type(model.db, binding)) - } -} - impl HasType for ast::Alias { fn inferred_type<'db>(&self, model: &SemanticModel<'db>) -> Option> { if &self.name == "*" { diff --git a/crates/ty_python_semantic/src/types/ide_support.rs b/crates/ty_python_semantic/src/types/ide_support.rs index 4e101fec125748..5a545310b38937 100644 --- a/crates/ty_python_semantic/src/types/ide_support.rs +++ b/crates/ty_python_semantic/src/types/ide_support.rs @@ -554,11 +554,7 @@ pub fn definitions_and_overloads_for_function<'db>( .map(ResolvedDefinition::Definition) .collect() } else { - function - .definition(model) - .map(ResolvedDefinition::Definition) - .into_iter() - .collect() + vec![ResolvedDefinition::Definition(function.definition(model))] } }