Skip to content

[red-knot] Add Type.definition method #17153

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 4, 2025
Merged

[red-knot] Add Type.definition method #17153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/red_knot_ide/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ ruff_python_parser = { workspace = true }
ruff_text_size = { workspace = true }
red_knot_python_semantic = { workspace = true }

rustc-hash = { workspace = true }
salsa = { workspace = true }
smallvec = { workspace = true }
tracing = { workspace = true }
Expand Down
36 changes: 19 additions & 17 deletions crates/red_knot_ide/src/goto.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@ pub fn goto_type_definition(
ty.display(db.upcast())
);

let navigation_targets = ty.navigation_targets(db);

Some(RangedValue {
range: FileRange::new(file, goto_target.range()),
value: ty.navigation_targets(db),
value: navigation_targets,
})
}

Expand Down Expand Up @@ -391,12 +393,12 @@ mod tests {

test.write_file("lib.py", "a = 10").unwrap();

assert_snapshot!(test.goto_type_definition(), @r###"
assert_snapshot!(test.goto_type_definition(), @r"
info: lint:goto-type-definition: Type definition
--> /lib.py:1:1
|
1 | a = 10
| ^
| ^^^^^^
|
info: Source
--> /main.py:4:13
Expand All @@ -406,7 +408,7 @@ mod tests {
4 | lib
| ^^^
|
"###);
");
}

#[test]
Expand Down Expand Up @@ -756,14 +758,13 @@ f(**kwargs<CURSOR>)

assert_snapshot!(test.goto_type_definition(), @r"
info: lint:goto-type-definition: Type definition
--> stdlib/builtins.pyi:443:7
--> stdlib/types.pyi:677:11
|
441 | def __getitem__(self, key: int, /) -> str | int | None: ...
442 |
443 | class str(Sequence[str]):
| ^^^
444 | @overload
445 | def __new__(cls, object: object = ...) -> Self: ...
675 | if sys.version_info >= (3, 10):
676 | @final
677 | class NoneType:
| ^^^^^^^^
678 | def __bool__(self) -> Literal[False]: ...
|
info: Source
--> /main.py:3:17
Expand All @@ -774,13 +775,14 @@ f(**kwargs<CURSOR>)
|

info: lint:goto-type-definition: Type definition
--> stdlib/types.pyi:677:11
--> stdlib/builtins.pyi:443:7
|
675 | if sys.version_info >= (3, 10):
676 | @final
677 | class NoneType:
| ^^^^^^^^
678 | def __bool__(self) -> Literal[False]: ...
441 | def __getitem__(self, key: int, /) -> str | int | None: ...
442 |
443 | class str(Sequence[str]):
| ^^^
444 | @overload
445 | def __new__(cls, object: object = ...) -> Self: ...
|
info: Source
--> /main.py:3:17
Expand Down
176 changes: 46 additions & 130 deletions crates/red_knot_ide/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,17 @@ mod goto;
mod hover;
mod markup;

use std::ops::{Deref, DerefMut};

pub use db::Db;
pub use goto::goto_type_definition;
pub use hover::hover;
pub use markup::MarkupKind;
use red_knot_python_semantic::types::{
Class, ClassBase, ClassLiteralType, FunctionType, InstanceType, IntersectionType,
KnownInstanceType, ModuleLiteralType, Type,
};

use rustc_hash::FxHashSet;
use std::ops::{Deref, DerefMut};

use red_knot_python_semantic::types::{Type, TypeDefinition};
use ruff_db::files::{File, FileRange};
use ruff_db::source::source_text;
use ruff_text_size::{Ranged, TextLen, TextRange};
use ruff_text_size::{Ranged, TextRange};

/// Information associated with a text range.
#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
Expand Down Expand Up @@ -58,7 +56,7 @@ where
}

/// Target to which the editor can navigate to.
#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct NavigationTarget {
file: File,

Expand Down Expand Up @@ -99,6 +97,17 @@ impl NavigationTargets {
Self(smallvec::SmallVec::new())
}

fn unique(targets: impl IntoIterator<Item = NavigationTarget>) -> Self {
let unique: FxHashSet<_> = targets.into_iter().collect();
if unique.is_empty() {
Self::empty()
} else {
let mut targets = unique.into_iter().collect::<Vec<_>>();
targets.sort_by_key(|target| (target.file, target.focus_range.start()));
Self(targets.into())
}
}

fn iter(&self) -> std::slice::Iter<'_, NavigationTarget> {
self.0.iter()
}
Expand Down Expand Up @@ -129,7 +138,7 @@ impl<'a> IntoIterator for &'a NavigationTargets {

impl FromIterator<NavigationTarget> for NavigationTargets {
fn from_iter<T: IntoIterator<Item = NavigationTarget>>(iter: T) -> Self {
Self(iter.into_iter().collect())
Self::unique(iter)
}
}

Expand All @@ -140,143 +149,50 @@ pub trait HasNavigationTargets {
impl HasNavigationTargets for Type<'_> {
fn navigation_targets(&self, db: &dyn Db) -> NavigationTargets {
match self {
Type::BoundMethod(method) => method.function(db).navigation_targets(db),
Type::FunctionLiteral(function) => function.navigation_targets(db),
Type::ModuleLiteral(module) => module.navigation_targets(db),
Type::Union(union) => union
.iter(db.upcast())
.flat_map(|target| target.navigation_targets(db))
.collect(),
Type::ClassLiteral(class) => class.navigation_targets(db),
Type::Instance(instance) => instance.navigation_targets(db),
Type::KnownInstance(instance) => instance.navigation_targets(db),
Type::SubclassOf(subclass_of_type) => match subclass_of_type.subclass_of() {
ClassBase::Class(class) => class.navigation_targets(db),
ClassBase::Dynamic(_) => NavigationTargets::empty(),
},

Type::StringLiteral(_)
| Type::BooleanLiteral(_)
| Type::LiteralString
| Type::IntLiteral(_)
| Type::BytesLiteral(_)
| Type::SliceLiteral(_)
| Type::MethodWrapper(_)
| Type::WrapperDescriptor(_)
| Type::PropertyInstance(_)
| Type::Tuple(_) => self.to_meta_type(db.upcast()).navigation_targets(db),

Type::TypeVar(var) => {
let definition = var.definition(db);
let full_range = definition.full_range(db.upcast());

NavigationTargets::single(NavigationTarget {
file: full_range.file(),
focus_range: definition.focus_range(db.upcast()).range(),
full_range: full_range.range(),
})
Type::Intersection(intersection) => {
// Only consider the positive elements because the negative elements are mainly from narrowing constraints.
let mut targets = intersection
.iter_positive(db.upcast())
.filter(|ty| !ty.is_unknown());

let Some(first) = targets.next() else {
return NavigationTargets::empty();
};

match targets.next() {
Some(_) => {
// If there are multiple types in the intersection, we can't navigate to a single one
// because the type is the intersection of all those types.
NavigationTargets::empty()
}
None => first.navigation_targets(db),
}
}

Type::Intersection(intersection) => intersection.navigation_targets(db),

Type::Dynamic(_)
| Type::Never
| Type::Callable(_)
| Type::AlwaysTruthy
| Type::AlwaysFalsy => NavigationTargets::empty(),
ty => ty
.definition(db.upcast())
.map(|definition| definition.navigation_targets(db))
.unwrap_or_else(NavigationTargets::empty),
}
}
}

impl HasNavigationTargets for FunctionType<'_> {
fn navigation_targets(&self, db: &dyn Db) -> NavigationTargets {
let function_range = self.focus_range(db.upcast());
NavigationTargets::single(NavigationTarget {
file: function_range.file(),
focus_range: function_range.range(),
full_range: self.full_range(db.upcast()).range(),
})
}
}

impl HasNavigationTargets for Class<'_> {
impl HasNavigationTargets for TypeDefinition<'_> {
fn navigation_targets(&self, db: &dyn Db) -> NavigationTargets {
let class_range = self.focus_range(db.upcast());
let full_range = self.full_range(db.upcast());
NavigationTargets::single(NavigationTarget {
file: class_range.file(),
focus_range: class_range.range(),
full_range: self.full_range(db.upcast()).range(),
file: full_range.file(),
focus_range: self.focus_range(db.upcast()).unwrap_or(full_range).range(),
full_range: full_range.range(),
})
}
}

impl HasNavigationTargets for ClassLiteralType<'_> {
fn navigation_targets(&self, db: &dyn Db) -> NavigationTargets {
self.class().navigation_targets(db)
}
}

impl HasNavigationTargets for InstanceType<'_> {
fn navigation_targets(&self, db: &dyn Db) -> NavigationTargets {
self.class().navigation_targets(db)
}
}

impl HasNavigationTargets for ModuleLiteralType<'_> {
fn navigation_targets(&self, db: &dyn Db) -> NavigationTargets {
let file = self.module(db).file();
let source = source_text(db.upcast(), file);

NavigationTargets::single(NavigationTarget {
file,
focus_range: TextRange::default(),
full_range: TextRange::up_to(source.text_len()),
})
}
}

impl HasNavigationTargets for KnownInstanceType<'_> {
fn navigation_targets(&self, db: &dyn Db) -> NavigationTargets {
match self {
KnownInstanceType::TypeVar(var) => {
let definition = var.definition(db);
let full_range = definition.full_range(db.upcast());

NavigationTargets::single(NavigationTarget {
file: full_range.file(),
focus_range: definition.focus_range(db.upcast()).range(),
full_range: full_range.range(),
})
}

// TODO: Track the definition of `KnownInstance` and navigate to their definition.
_ => NavigationTargets::empty(),
}
}
}

impl HasNavigationTargets for IntersectionType<'_> {
fn navigation_targets(&self, db: &dyn Db) -> NavigationTargets {
// Only consider the positive elements because the negative elements are mainly from narrowing constraints.
let mut targets = self
.iter_positive(db.upcast())
.filter(|ty| !ty.is_unknown());

let Some(first) = targets.next() else {
return NavigationTargets::empty();
};

match targets.next() {
Some(_) => {
// If there are multiple types in the intersection, we can't navigate to a single one
// because the type is the intersection of all those types.
NavigationTargets::empty()
}
None => first.navigation_targets(db),
}
}
}

#[cfg(test)]
mod tests {
use crate::db::tests::TestDb;
Expand Down
16 changes: 6 additions & 10 deletions crates/red_knot_python_semantic/src/semantic_index/definition.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,13 @@ use crate::Db;

/// A definition of a symbol.
///
/// ## Module-local type
/// This type should not be used as part of any cross-module API because
/// it holds a reference to the AST node. Range-offset changes
/// then propagate through all usages, and deserialization requires
/// reparsing the entire module.
/// ## ID stability
/// The `Definition`'s ID is stable when the only field that change is its `kind` (AST node).
///
/// E.g. don't use this type in:
///
/// * a return type of a cross-module query
/// * a field of a type that is a return type of a cross-module query
/// * an argument of a cross-module query
/// The `Definition` changes when the `file`, `scope`, or `symbol` change. This can be
/// because a new scope gets inserted before the `Definition` or a new symbol is inserted
/// before this `Definition`. However, the ID can be considered stable and it is okay to use
/// `Definition` in cross-module` salsa queries or as a field on other salsa tracked structs.
#[salsa::tracked(debug)]
pub struct Definition<'db> {
/// The file in which the definition occurs.
Expand Down
Loading
Loading