[red-knot] Goto type definition (#16901)

## Summary

Implement basic *Goto type definition* support for Red Knot's LSP.

This PR also builds the foundation for other LSP operations. E.g., Goto
definition, hover, etc., should be able to reuse some, if not most,
logic introduced in this PR.

The basic steps of resolving the type definitions are:

1. Find the closest token for the cursor offset. This is a bit more
subtle than I first anticipated because the cursor could be positioned
right between the callee and the `(` in `call(test)`, in which case we
want to resolve the type for `call`.
2. Find the node with the minimal range that fully encloses the token
found in 1. I somewhat suspect that 1 and 2 could be done at the same
time but it complicated things because we also need to compute the spine
(ancestor chain) for the node and there's no guarantee that the found
nodes have the same ancestors
3. Reduce the node found in 2. to a node that is a valid goto target.
This may require traversing upwards to e.g. find the closest expression.
4. Resolve the type for the goto target
5. Resolve the location for the type, return it to the LSP

## Design decisions

The current implementation navigates to the inferred type. I think this
is what we want because it means that it correctly accounts for
narrowing (in which case we want to go to the narrowed type because
that's the value's type at the given position). However, it does have
the downside that Goto type definition doesn't work whenever we infer `T
& Unknown` because intersection types aren't supported. I'm not sure
what to do about this specific case, other than maybe ignoring `Unkown`
in Goto type definition if the type is an intersection?

## Known limitations

* Types defined in the vendored typeshed aren't supported because the
client can't open files from the red knot binary (we can either
implement our own file protocol and handler OR extract the typeshed
files and point there). See
https://github.com/astral-sh/ruff/issues/17041
* Red Knot only exposes an API to get types for expressions and
definitions. However, there are many other nodes with identifiers that
can have a type (e.g. go to type of a globals statement, match patterns,
...). We can add support for those in separate PRs (after we figure out
how to query the types from the semantic model). See
https://github.com/astral-sh/ruff/issues/17113
* We should have a higher-level API for the LSP that doesn't directly
call semantic queries. I intentionally decided not to design that API
just yet.


## Test plan


https://github.com/user-attachments/assets/fa077297-a42d-4ec8-b71f-90c0802b4edb

Goto type definition on a union

<img width="1215" alt="Screenshot 2025-04-01 at 13 02 55"
src="https://github.com/user-attachments/assets/689cabcc-4a86-4a18-b14a-c56f56868085"
/>



Note: I recorded this using a custom typeshed path so that navigating to
builtins works.
This commit is contained in:
Micha Reiser 2025-04-02 14:12:48 +02:00 committed by GitHub
parent 7e97910704
commit 2ae39edccf
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
32 changed files with 1765 additions and 84 deletions

View file

@ -1,6 +1,6 @@
use std::ops::Deref;
use ruff_db::files::File;
use ruff_db::files::{File, FileRange};
use ruff_db::parsed::ParsedModule;
use ruff_python_ast as ast;
use ruff_text_size::{Ranged, TextRange};
@ -52,6 +52,14 @@ impl<'db> Definition<'db> {
pub(crate) fn scope(self, db: &'db dyn Db) -> ScopeId<'db> {
self.file_scope(db).to_scope_id(db, self.file(db))
}
pub fn full_range(self, db: &'db dyn Db) -> FileRange {
FileRange::new(self.file(db), self.kind(db).full_range())
}
pub fn focus_range(self, db: &'db dyn Db) -> FileRange {
FileRange::new(self.file(db), self.kind(db).target_range())
}
}
/// One or more [`Definition`]s.
@ -559,8 +567,6 @@ impl DefinitionKind<'_> {
///
/// A definition target would mainly be the node representing the symbol being defined i.e.,
/// [`ast::ExprName`] or [`ast::Identifier`] but could also be other nodes.
///
/// This is mainly used for logging and debugging purposes.
pub(crate) fn target_range(&self) -> TextRange {
match self {
DefinitionKind::Import(import) => import.alias().range(),
@ -587,6 +593,33 @@ impl DefinitionKind<'_> {
}
}
/// Returns the [`TextRange`] of the entire definition.
pub(crate) fn full_range(&self) -> TextRange {
match self {
DefinitionKind::Import(import) => import.alias().range(),
DefinitionKind::ImportFrom(import) => import.alias().range(),
DefinitionKind::StarImport(import) => import.import().range(),
DefinitionKind::Function(function) => function.range(),
DefinitionKind::Class(class) => class.range(),
DefinitionKind::TypeAlias(type_alias) => type_alias.range(),
DefinitionKind::NamedExpression(named) => named.range(),
DefinitionKind::Assignment(assignment) => assignment.name().range(),
DefinitionKind::AnnotatedAssignment(assign) => assign.range(),
DefinitionKind::AugmentedAssignment(aug_assign) => aug_assign.range(),
DefinitionKind::For(for_stmt) => for_stmt.name().range(),
DefinitionKind::Comprehension(comp) => comp.target().range(),
DefinitionKind::VariadicPositionalParameter(parameter) => parameter.range(),
DefinitionKind::VariadicKeywordParameter(parameter) => parameter.range(),
DefinitionKind::Parameter(parameter) => parameter.parameter.range(),
DefinitionKind::WithItem(with_item) => with_item.name().range(),
DefinitionKind::MatchPattern(match_pattern) => match_pattern.identifier.range(),
DefinitionKind::ExceptHandler(handler) => handler.node().range(),
DefinitionKind::TypeVar(type_var) => type_var.range(),
DefinitionKind::ParamSpec(param_spec) => param_spec.range(),
DefinitionKind::TypeVarTuple(type_var_tuple) => type_var_tuple.range(),
}
}
pub(crate) fn category(&self, in_stub: bool) -> DefinitionCategory {
match self {
// functions, classes, and imports always bind, and we consider them declarations

View file

@ -160,6 +160,7 @@ impl_binding_has_ty!(ast::StmtFunctionDef);
impl_binding_has_ty!(ast::StmtClassDef);
impl_binding_has_ty!(ast::Parameter);
impl_binding_has_ty!(ast::ParameterWithDefault);
impl_binding_has_ty!(ast::ExceptHandlerExceptHandler);
impl HasType for ast::Alias {
fn inferred_type<'db>(&self, model: &SemanticModel<'db>) -> Type<'db> {

View file

@ -7,7 +7,7 @@ use call::{CallDunderError, CallError, CallErrorKind};
use context::InferContext;
use diagnostic::{INVALID_CONTEXT_MANAGER, NOT_ITERABLE};
use itertools::EitherOrBoth;
use ruff_db::files::File;
use ruff_db::files::{File, FileRange};
use ruff_python_ast as ast;
use ruff_python_ast::name::Name;
use ruff_text_size::{Ranged, TextRange};
@ -33,14 +33,16 @@ use crate::semantic_index::{imported_modules, semantic_index};
use crate::suppression::check_suppressions;
use crate::symbol::{imported_symbol, Boundness, Symbol, SymbolAndQualifiers};
use crate::types::call::{Bindings, CallArgumentTypes};
use crate::types::class_base::ClassBase;
pub use crate::types::class_base::ClassBase;
use crate::types::diagnostic::{INVALID_TYPE_FORM, UNSUPPORTED_BOOL_CONVERSION};
use crate::types::infer::infer_unpack_types;
use crate::types::mro::{Mro, MroError, MroIterator};
pub(crate) use crate::types::narrow::infer_narrowing_constraint;
use crate::types::signatures::{Parameter, ParameterForm, ParameterKind, Parameters};
use crate::{Db, FxOrderSet, Module, Program};
pub(crate) use class::{Class, ClassLiteralType, InstanceType, KnownClass, KnownInstanceType};
pub use class::Class;
pub(crate) use class::KnownClass;
pub use class::{ClassLiteralType, InstanceType, KnownInstanceType};
mod builder;
mod call;
@ -3785,6 +3787,9 @@ pub struct TypeVarInstance<'db> {
#[return_ref]
name: ast::name::Name,
/// The type var's definition
pub definition: Definition<'db>,
/// The upper bound or constraint on the type of this TypeVar
bound_or_constraints: Option<TypeVarBoundOrConstraints<'db>>,
@ -4461,6 +4466,21 @@ impl<'db> FunctionType<'db> {
Type::Callable(CallableType::new(db, self.signature(db).clone()))
}
/// Returns the [`FileRange`] of the function's name.
pub fn focus_range(self, db: &dyn Db) -> FileRange {
FileRange::new(
self.body_scope(db).file(db),
self.body_scope(db).node(db).expect_function().name.range,
)
}
pub fn full_range(self, db: &dyn Db) -> FileRange {
FileRange::new(
self.body_scope(db).file(db),
self.body_scope(db).node(db).expect_function().range,
)
}
/// Typed externally-visible signature for this function.
///
/// This is the signature as seen by external callers, possibly modified by decorators and/or
@ -4622,7 +4642,7 @@ impl KnownFunction {
pub struct BoundMethodType<'db> {
/// The function that is being bound. Corresponds to the `__func__` attribute on a
/// bound method object
pub(crate) function: FunctionType<'db>,
pub function: FunctionType<'db>,
/// The instance on which this method has been called. Corresponds to the `__self__`
/// attribute on a bound method object
self_instance: Type<'db>,
@ -5332,6 +5352,10 @@ impl<'db> UnionType<'db> {
Self::from_elements(db, self.elements(db).iter().filter(filter_fn))
}
pub fn iter(&self, db: &'db dyn Db) -> Iter<Type<'db>> {
self.elements(db).iter()
}
pub(crate) fn map_with_boundness(
self,
db: &'db dyn Db,
@ -5735,6 +5759,10 @@ impl<'db> IntersectionType<'db> {
qualifiers,
}
}
pub fn iter_positive(&self, db: &'db dyn Db) -> impl Iterator<Item = Type<'db>> {
self.positive(db).iter().copied()
}
}
#[salsa::interned(debug)]

View file

@ -18,7 +18,7 @@ use crate::{
};
use indexmap::IndexSet;
use itertools::Itertools as _;
use ruff_db::files::File;
use ruff_db::files::{File, FileRange};
use ruff_python_ast::{self as ast, PythonVersion};
use rustc_hash::FxHashSet;
@ -153,6 +153,15 @@ impl<'db> Class<'db> {
self.body_scope(db).node(db).expect_class()
}
/// Returns the file range of the class's name.
pub fn focus_range(self, db: &dyn Db) -> FileRange {
FileRange::new(self.file(db), self.node(db).name.range)
}
pub fn full_range(self, db: &dyn Db) -> FileRange {
FileRange::new(self.file(db), self.node(db).range)
}
/// Return the types of the decorators on this class
#[salsa::tracked(return_ref)]
fn decorators(self, db: &'db dyn Db) -> Box<[Type<'db>]> {
@ -754,7 +763,7 @@ pub struct ClassLiteralType<'db> {
}
impl<'db> ClassLiteralType<'db> {
pub(crate) fn class(self) -> Class<'db> {
pub fn class(self) -> Class<'db> {
self.class
}
@ -780,7 +789,7 @@ pub struct InstanceType<'db> {
}
impl<'db> InstanceType<'db> {
pub(super) fn class(self) -> Class<'db> {
pub fn class(self) -> Class<'db> {
self.class
}

View file

@ -8,7 +8,7 @@ use itertools::Either;
/// all types that would be invalid to have as a class base are
/// transformed into [`ClassBase::unknown`]
#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, salsa::Update)]
pub(crate) enum ClassBase<'db> {
pub enum ClassBase<'db> {
Dynamic(DynamicType),
Class(Class<'db>),
}
@ -18,7 +18,7 @@ impl<'db> ClassBase<'db> {
Self::Dynamic(DynamicType::Any)
}
pub(crate) const fn unknown() -> Self {
pub const fn unknown() -> Self {
Self::Dynamic(DynamicType::Unknown)
}

View file

@ -2016,6 +2016,7 @@ impl<'db> TypeInferenceBuilder<'db> {
let ty = Type::KnownInstance(KnownInstanceType::TypeVar(TypeVarInstance::new(
self.db(),
name.id.clone(),
definition,
bound_or_constraint,
default_ty,
)));

View file

@ -52,7 +52,7 @@ impl<'db> SubclassOfType<'db> {
}
/// Return the inner [`ClassBase`] value wrapped by this `SubclassOfType`.
pub(crate) const fn subclass_of(self) -> ClassBase<'db> {
pub const fn subclass_of(self) -> ClassBase<'db> {
self.subclass_of
}