[ty] guard against recursion in determining completion kind (#20354)

## Summary

Fixes https://github.com/astral-sh/ty/issues/1158

## Test Plan

Added test.
This commit is contained in:
Carl Meyer 2025-09-11 11:25:06 -07:00 committed by GitHub
parent 5bf6977ded
commit c4cd5c00fd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 46 additions and 9 deletions

View file

@ -551,6 +551,8 @@ mod tests {
use super::{CompletionSettings, token_suffix_by_kinds}; use super::{CompletionSettings, token_suffix_by_kinds};
use ty_python_semantic::CompletionKind;
#[test] #[test]
fn token_suffixes_match() { fn token_suffixes_match() {
insta::assert_debug_snapshot!( insta::assert_debug_snapshot!(
@ -3093,6 +3095,27 @@ from os.<CURSOR>
); );
} }
#[test]
fn completion_kind_recursive_type_alias() {
let test = cursor_test(
r#"
type T = T | None
def f(rec: T):
re<CURSOR>
"#,
);
let completions = completion(
&test.db,
&CompletionSettings::default(),
test.cursor.file,
test.cursor.offset,
);
let completion = completions.iter().find(|c| c.name == "rec").unwrap();
assert_eq!(completion.kind(&test.db), Some(CompletionKind::Struct));
}
// NOTE: The methods below are getting somewhat ridiculous. // NOTE: The methods below are getting somewhat ridiculous.
// We should refactor this by converting to using a builder // We should refactor this by converting to using a builder
// to set different modes. ---AG // to set different modes. ---AG

View file

@ -11,7 +11,7 @@ use crate::semantic_index::definition::Definition;
use crate::semantic_index::scope::FileScopeId; use crate::semantic_index::scope::FileScopeId;
use crate::semantic_index::semantic_index; use crate::semantic_index::semantic_index;
use crate::types::ide_support::all_declarations_and_bindings; use crate::types::ide_support::all_declarations_and_bindings;
use crate::types::{Type, binding_type, infer_scope_types}; use crate::types::{CycleDetector, Type, binding_type, infer_scope_types};
pub struct SemanticModel<'db> { pub struct SemanticModel<'db> {
db: &'db dyn Db, db: &'db dyn Db,
@ -319,7 +319,11 @@ impl<'db> Completion<'db> {
/// the client uses this information to help improve the UX (perhaps by /// the client uses this information to help improve the UX (perhaps by
/// assigning an icon of some kind to the completion). /// assigning an icon of some kind to the completion).
pub fn kind(&self, db: &'db dyn Db) -> Option<CompletionKind> { pub fn kind(&self, db: &'db dyn Db) -> Option<CompletionKind> {
fn imp<'db>(db: &'db dyn Db, ty: Type<'db>) -> Option<CompletionKind> { fn imp<'db>(
db: &'db dyn Db,
ty: Type<'db>,
visitor: &CompletionKindVisitor<'db>,
) -> Option<CompletionKind> {
Some(match ty { Some(match ty {
Type::FunctionLiteral(_) Type::FunctionLiteral(_)
| Type::DataclassDecorator(_) | Type::DataclassDecorator(_)
@ -346,23 +350,33 @@ impl<'db> Completion<'db> {
Type::EnumLiteral(_) => CompletionKind::Enum, Type::EnumLiteral(_) => CompletionKind::Enum,
Type::ProtocolInstance(_) => CompletionKind::Interface, Type::ProtocolInstance(_) => CompletionKind::Interface,
Type::NonInferableTypeVar(_) | Type::TypeVar(_) => CompletionKind::TypeParameter, Type::NonInferableTypeVar(_) | Type::TypeVar(_) => CompletionKind::TypeParameter,
Type::Union(union) => union.elements(db).iter().find_map(|&ty| imp(db, ty))?, Type::Union(union) => union
Type::Intersection(intersection) => { .elements(db)
intersection.iter_positive(db).find_map(|ty| imp(db, ty))? .iter()
} .find_map(|&ty| imp(db, ty, visitor))?,
Type::Intersection(intersection) => intersection
.iter_positive(db)
.find_map(|ty| imp(db, ty, visitor))?,
Type::Dynamic(_) Type::Dynamic(_)
| Type::Never | Type::Never
| Type::SpecialForm(_) | Type::SpecialForm(_)
| Type::KnownInstance(_) | Type::KnownInstance(_)
| Type::AlwaysTruthy | Type::AlwaysTruthy
| Type::AlwaysFalsy => return None, | Type::AlwaysFalsy => return None,
Type::TypeAlias(alias) => imp(db, alias.value_type(db))?, Type::TypeAlias(alias) => {
visitor.visit(ty, || imp(db, alias.value_type(db), visitor))?
}
}) })
} }
self.kind.or_else(|| self.ty.and_then(|ty| imp(db, ty))) self.kind.or_else(|| {
self.ty
.and_then(|ty| imp(db, ty, &CompletionKindVisitor::default()))
})
} }
} }
type CompletionKindVisitor<'db> = CycleDetector<CompletionKind, Type<'db>, Option<CompletionKind>>;
/// The "kind" of a completion. /// The "kind" of a completion.
/// ///
/// This is taken directly from the LSP completion specification: /// This is taken directly from the LSP completion specification:
@ -372,7 +386,7 @@ impl<'db> Completion<'db> {
/// `Type` (and possibly other information), which might be interesting and /// `Type` (and possibly other information), which might be interesting and
/// contentious. Then the outer edges map this to the LSP types, which is /// contentious. Then the outer edges map this to the LSP types, which is
/// expected to be mundane and boring. /// expected to be mundane and boring.
#[derive(Clone, Copy, Debug)] #[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum CompletionKind { pub enum CompletionKind {
Text, Text,
Method, Method,