mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-09-30 13:51:16 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

[ty] Add builtins to completions derived from scope (#18982)

Browse source 
Most of the work here was doing some light refactoring to facilitate
sensible testing. That is, we don't want to list every builtin included
in most tests, so we add some structure to the completion type returned.
Tests can now filter based on whether a completion is a builtin or not.

Otherwise, builtins are found using the existing infrastructure for
`object.attr` completions (where we hard-code the module name
`builtins`).

I did consider changing the sort order based on whether a completion
suggestion was a builtin or not. In particular, it seemed like it might
be a good idea to sort builtins after other scope based completions,
but before the dunder and sunder attributes. Namely, it seems likely
that there is an inverse correlation between the size of a scope and
the likelihood of an item in that scope being used at any given point.
So it *might* be a good idea to prioritize the likelier candidates in
the completions returned.

Additionally, the number of items introduced by adding builtins is quite
large. So I wondered whether mixing them in with everything else would
become too noisy.

However, it's not totally clear to me that this is the right thing to
do. Right now, I feel like there is a very obvious lexicographic
ordering that makes "finding" the right suggestion to activate
potentially easier than if the ranking mechanism is less clear.
(Technically, the dunder and sunder attributes are not sorted
lexicographically, but I'd put forward that most folks don't have an
intuitive understanding of where `_` ranks lexicographically with
respect to "regular" letters. Moreover, since dunder and sunder
attributes are all grouped together, I think the ordering here ends up
being very obvious after even a quick glance.)
This commit is contained in:
Andrew Gallant 2025-06-27 10:20:01 -04:00 • committed by GitHub
parent a3c79d8170
commit 5f6b0ded21
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 212 additions and 113 deletions
Download patch file Download diff file Expand all files Collapse all files

253
crates/ty_ide/src/completion.rs
View file

@ -5,14 +5,11 @@ use ruff_db::parsed::{ParsedModuleRef, parsed_module};
use ruff_python_ast as ast; use ruff_python_ast as ast;
use ruff_python_parser::{Token, TokenAt, TokenKind}; use ruff_python_parser::{Token, TokenAt, TokenKind};
use ruff_text_size::{Ranged, TextRange, TextSize}; use ruff_text_size::{Ranged, TextRange, TextSize};
use ty_python_semantic::{Completion, SemanticModel};
use crate::Db; use crate::Db;
use crate::find_node::covering_node; use crate::find_node::covering_node;
pub struct Completion {
pub label: String,
}
pub fn completion(db: &dyn Db, file: File, offset: TextSize) -> Vec<Completion> { pub fn completion(db: &dyn Db, file: File, offset: TextSize) -> Vec<Completion> {
let parsed = parsed_module(db.upcast(), file).load(db.upcast()); let parsed = parsed_module(db.upcast(), file).load(db.upcast());
@ -23,18 +20,15 @@ pub fn completion(db: &dyn Db, file: File, offset: TextSize) -> Vec<Completion>
return vec![]; return vec![];
}; };
let model = ty_python_semantic::SemanticModel::new(db.upcast(), file); let model = SemanticModel::new(db.upcast(), file);
let mut completions = match target { let mut completions = match target {
CompletionTargetAst::ObjectDot { expr } => model.attribute_completions(expr), CompletionTargetAst::ObjectDot { expr } => model.attribute_completions(expr),
CompletionTargetAst::ImportFrom { import, name } => model.import_completions(import, name), CompletionTargetAst::ImportFrom { import, name } => model.import_completions(import, name),
CompletionTargetAst::Scoped { node } => model.scoped_completions(node), CompletionTargetAst::Scoped { node } => model.scoped_completions(node),
}; };
completions.sort_by(|name1, name2| compare_suggestions(name1, name2)); completions.sort_by(compare_suggestions);
completions.dedup(); completions.dedup_by(|c1, c2| c1.name == c2.name);
completions completions
.into_iter()
.map(|name| Completion { label: name.into() })
.collect()
} }
/// The kind of tokens identified under the cursor. /// The kind of tokens identified under the cursor.
@ -330,7 +324,7 @@ fn import_from_tokens(tokens: &[Token]) -> Option<&Token> {
/// ///
/// This has the effect of putting all dunder attributes after "normal" /// This has the effect of putting all dunder attributes after "normal"
/// attributes, and all single-underscore attributes after dunder attributes. /// attributes, and all single-underscore attributes after dunder attributes.
fn compare_suggestions(name1: &str, name2: &str) -> Ordering { fn compare_suggestions(c1: &Completion, c2: &Completion) -> Ordering {
/// A helper type for sorting completions based only on name. /// A helper type for sorting completions based only on name.
/// ///
/// This sorts "normal" names first, then dunder names and finally /// This sorts "normal" names first, then dunder names and finally
@ -345,16 +339,16 @@ fn compare_suggestions(name1: &str, name2: &str) -> Ordering {
} }
impl Kind { impl Kind {
fn classify(name: &str) -> Kind { fn classify(c: &Completion) -> Kind {
// Dunder needs a prefix and suffix double underscore. // Dunder needs a prefix and suffix double underscore.
// When there's only a prefix double underscore, this // When there's only a prefix double underscore, this
// results in explicit name mangling. We let that be // results in explicit name mangling. We let that be
// classified as-if they were single underscore names. // classified as-if they were single underscore names.
// //
// Ref: <https://docs.python.org/3/reference/lexical_analysis.html#reserved-classes-of-identifiers> // Ref: <https://docs.python.org/3/reference/lexical_analysis.html#reserved-classes-of-identifiers>
if name.starts_with("__") && name.ends_with("__") { if c.name.starts_with("__") && c.name.ends_with("__") {
Kind::Dunder Kind::Dunder
} else if name.starts_with('_') { } else if c.name.starts_with('_') {
Kind::Sunder Kind::Sunder
} else { } else {
Kind::Normal Kind::Normal
@ -362,14 +356,15 @@ fn compare_suggestions(name1: &str, name2: &str) -> Ordering {
} }
} }
let (kind1, kind2) = (Kind::classify(name1), Kind::classify(name2)); let (kind1, kind2) = (Kind::classify(c1), Kind::classify(c2));
kind1.cmp(&kind2).then_with(|| name1.cmp(name2)) kind1.cmp(&kind2).then_with(|| c1.name.cmp(&c2.name))
} }
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use insta::assert_snapshot; use insta::assert_snapshot;
use ruff_python_parser::{Mode, ParseOptions, TokenKind, Tokens}; use ruff_python_parser::{Mode, ParseOptions, TokenKind, Tokens};
use ty_python_semantic::Completion;
use crate::completion; use crate::completion;
use crate::tests::{CursorTest, cursor_test}; use crate::tests::{CursorTest, cursor_test};
@ -463,7 +458,42 @@ mod tests {
", ",
); );
assert_snapshot!(test.completions(), @"<No completions found>"); assert_snapshot!(
test.completions_without_builtins(),
@"<No completions found after filtering out completions>",
);
}
#[test]
fn builtins() {
let test = cursor_test(
"\
<CURSOR>
",
);
test.assert_completions_include("filter");
}
#[test]
fn builtins_not_included_object_attr() {
let test = cursor_test(
"\
import re
re.<CURSOR>
",
);
test.assert_completions_do_not_include("filter");
}
#[test]
fn builtins_not_included_import() {
let test = cursor_test(
"\
from re import <CURSOR>
",
);
test.assert_completions_do_not_include("filter");
} }
#[test] #[test]
@ -476,7 +506,7 @@ import re
", ",
); );
assert_snapshot!(test.completions(), @"re"); assert_snapshot!(test.completions_without_builtins(), @"re");
} }
#[test] #[test]
@ -489,7 +519,7 @@ from os import path
", ",
); );
assert_snapshot!(test.completions(), @"path"); assert_snapshot!(test.completions_without_builtins(), @"path");
} }
// N.B. We don't currently explore module APIs. This // N.B. We don't currently explore module APIs. This
@ -516,7 +546,7 @@ f<CURSOR>
", ",
); );
assert_snapshot!(test.completions(), @"foo"); assert_snapshot!(test.completions_without_builtins(), @"foo");
} }
#[test] #[test]
@ -529,7 +559,7 @@ g<CURSOR>
", ",
); );
assert_snapshot!(test.completions(), @"foo"); assert_snapshot!(test.completions_without_builtins(), @"foo");
} }
#[test] #[test]
@ -542,7 +572,7 @@ def foo(): ...
", ",
); );
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
foo foo
"); ");
} }
@ -558,7 +588,7 @@ f<CURSOR>
", ",
); );
assert_snapshot!(test.completions(), @"foo"); assert_snapshot!(test.completions_without_builtins(), @"foo");
} }
#[test] #[test]
@ -572,7 +602,7 @@ def foo():
", ",
); );
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
foo foo
"); ");
} }
@ -587,7 +617,7 @@ def foo():
", ",
); );
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
foo foo
foofoo foofoo
"); ");
@ -621,7 +651,7 @@ def foo():
// matches the current cursor's indentation. This seems fraught // matches the current cursor's indentation. This seems fraught
// however. It's not clear to me that we can always assume a // however. It's not clear to me that we can always assume a
// correspondence between scopes and indentation level. // correspondence between scopes and indentation level.
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
foo foo
"); ");
} }
@ -637,7 +667,7 @@ def foo():
", ",
); );
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
foo foo
foofoo foofoo
"); ");
@ -653,7 +683,7 @@ def foo():
f<CURSOR>", f<CURSOR>",
); );
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
foo foo
foofoo foofoo
"); ");
@ -671,7 +701,7 @@ def frob(): ...
", ",
); );
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
foo foo
foofoo foofoo
frob frob
@ -690,7 +720,7 @@ def frob(): ...
", ",
); );
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
foo foo
frob frob
"); ");
@ -708,7 +738,7 @@ def frob(): ...
", ",
); );
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
foo foo
foofoo foofoo
foofoofoo foofoofoo
@ -736,7 +766,7 @@ def foo():
// account for the indented whitespace, or some other technique // account for the indented whitespace, or some other technique
// needs to be used to get the scope containing `foofoo` but not // needs to be used to get the scope containing `foofoo` but not
// `foofoofoo`. // `foofoofoo`.
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
foo foo
"); ");
} }
@ -752,7 +782,7 @@ def foo():
); );
// FIXME: Should include `foofoo` (but not `foofoofoo`). // FIXME: Should include `foofoo` (but not `foofoofoo`).
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
foo foo
"); ");
} }
@ -770,7 +800,7 @@ def frob(): ...
); );
// FIXME: Should include `foofoo` (but not `foofoofoo`). // FIXME: Should include `foofoo` (but not `foofoofoo`).
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
foo foo
frob frob
"); ");
@ -790,7 +820,7 @@ def frob(): ...
); );
// FIXME: Should include `foofoo` (but not `foofoofoo`). // FIXME: Should include `foofoo` (but not `foofoofoo`).
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
foo foo
frob frob
"); ");
@ -811,7 +841,7 @@ def frob(): ...
); );
// FIXME: Should include `foofoo` (but not `foofoofoo`). // FIXME: Should include `foofoo` (but not `foofoofoo`).
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
foo foo
frob frob
"); ");
@ -828,7 +858,10 @@ def frob(): ...
// TODO: it would be good if `bar` was included here, but // TODO: it would be good if `bar` was included here, but
// the list comprehension is not yet valid and so we do not // the list comprehension is not yet valid and so we do not
// detect this as a definition of `bar`. // detect this as a definition of `bar`.
assert_snapshot!(test.completions(), @"<No completions found>"); assert_snapshot!(
test.completions_without_builtins(),
@"<No completions found after filtering out completions>",
);
} }
#[test] #[test]
@ -839,7 +872,7 @@ def frob(): ...
", ",
); );
assert_snapshot!(test.completions(), @"foo"); assert_snapshot!(test.completions_without_builtins(), @"foo");
} }
#[test] #[test]
@ -850,7 +883,7 @@ def frob(): ...
", ",
); );
assert_snapshot!(test.completions(), @"foo"); assert_snapshot!(test.completions_without_builtins(), @"foo");
} }
#[test] #[test]
@ -861,7 +894,7 @@ def frob(): ...
", ",
); );
assert_snapshot!(test.completions(), @"foo"); assert_snapshot!(test.completions_without_builtins(), @"foo");
} }
#[test] #[test]
@ -872,7 +905,7 @@ def frob(): ...
", ",
); );
assert_snapshot!(test.completions(), @"foo"); assert_snapshot!(test.completions_without_builtins(), @"foo");
} }
#[test] #[test]
@ -883,7 +916,7 @@ def frob(): ...
", ",
); );
assert_snapshot!(test.completions(), @"foo"); assert_snapshot!(test.completions_without_builtins(), @"foo");
} }
#[test] #[test]
@ -894,7 +927,7 @@ def frob(): ...
", ",
); );
assert_snapshot!(test.completions(), @"foo"); assert_snapshot!(test.completions_without_builtins(), @"foo");
} }
#[test] #[test]
@ -916,7 +949,10 @@ def frob(): ...
// //
// The `lambda_blank1` test works because there are expressions // The `lambda_blank1` test works because there are expressions
// on either side of <CURSOR>. // on either side of <CURSOR>.
assert_snapshot!(test.completions(), @"<No completions found>"); assert_snapshot!(
test.completions_without_builtins(),
@"<No completions found after filtering out completions>",
);
} }
#[test] #[test]
@ -928,7 +964,10 @@ def frob(): ...
); );
// FIXME: Should include `foo`. // FIXME: Should include `foo`.
assert_snapshot!(test.completions(), @"<No completions found>"); assert_snapshot!(
test.completions_without_builtins(),
@"<No completions found after filtering out completions>",
);
} }
#[test] #[test]
@ -940,7 +979,10 @@ def frob(): ...
); );
// FIXME: Should include `foo`. // FIXME: Should include `foo`.
assert_snapshot!(test.completions(), @"<No completions found>"); assert_snapshot!(
test.completions_without_builtins(),
@"<No completions found after filtering out completions>",
);
} }
#[test] #[test]
@ -954,7 +996,7 @@ class Foo:
", ",
); );
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
Foo Foo
bar bar
frob frob
@ -972,7 +1014,7 @@ class Foo:
", ",
); );
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
Foo Foo
bar bar
quux quux
@ -996,7 +1038,7 @@ class Foo:
// //
// These don't work for similar reasons as other // These don't work for similar reasons as other
// tests above with the <CURSOR> inside of whitespace. // tests above with the <CURSOR> inside of whitespace.
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
Foo Foo
"); ");
} }
@ -1015,7 +1057,7 @@ class Foo:
// FIXME: Should include `bar`, `quux` and `frob`. // FIXME: Should include `bar`, `quux` and `frob`.
// (Unclear if `Foo` should be included, but a false // (Unclear if `Foo` should be included, but a false
// positive isn't the end of the world.) // positive isn't the end of the world.)
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
Foo Foo
"); ");
} }
@ -1031,7 +1073,7 @@ class Foo(<CURSOR>):
", ",
); );
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
Bar Bar
Foo Foo
"); ");
@ -1048,7 +1090,7 @@ class Bar: ...
", ",
); );
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
Bar Bar
Foo Foo
"); ");
@ -1065,7 +1107,7 @@ class Bar: ...
", ",
); );
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
Bar Bar
Foo Foo
"); ");
@ -1080,7 +1122,7 @@ class Bar: ...
class Foo(<CURSOR>", class Foo(<CURSOR>",
); );
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
Bar Bar
Foo Foo
"); ");
@ -1101,7 +1143,7 @@ quux.<CURSOR>
", ",
); );
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
bar bar
baz baz
foo foo
@ -1146,7 +1188,7 @@ quux.b<CURSOR>
", ",
); );
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
bar bar
baz baz
foo foo
@ -1196,7 +1238,7 @@ class Quux:
// of available attributes. // of available attributes.
// //
// See: https://github.com/astral-sh/ty/issues/159 // See: https://github.com/astral-sh/ty/issues/159
assert_snapshot!(test.completions(), @"<No completions found>"); assert_snapshot!(test.completions_without_builtins(), @"<No completions found>");
} }
// We don't yet take function parameters into account. // We don't yet take function parameters into account.
@ -1212,7 +1254,7 @@ bar(o<CURSOR>
", ",
); );
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
bar bar
foo foo
"); ");
@ -1230,7 +1272,7 @@ bar(<CURSOR>
", ",
); );
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
bar bar
foo foo
"); ");
@ -1249,7 +1291,7 @@ class C:
", ",
); );
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
C C
bar bar
foo foo
@ -1268,7 +1310,7 @@ class C:
", ",
); );
assert_snapshot!(test.completions(), @"C"); assert_snapshot!(test.completions_without_builtins(), @"C");
} }
#[test] #[test]
@ -1285,7 +1327,7 @@ class C:
// FIXME: Should NOT include `foo` here, since // FIXME: Should NOT include `foo` here, since
// that is only a method that can be called on // that is only a method that can be called on
// `self`. // `self`.
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
C C
bar bar
foo foo
@ -1303,7 +1345,7 @@ class<CURSOR>
", ",
); );
assert_snapshot!(test.completions(), @"classy_variable_name"); assert_snapshot!(test.completions_without_builtins(), @"classy_variable_name");
} }
#[test] #[test]
@ -1316,7 +1358,7 @@ print(f\"{some<CURSOR>
", ",
); );
assert_snapshot!(test.completions(), @"some_symbol"); assert_snapshot!(test.completions_without_builtins(), @"some_symbol");
} }
#[test] #[test]
@ -1330,7 +1372,10 @@ hidden_<CURSOR>
", ",
); );
assert_snapshot!(test.completions(), @"<No completions found>"); assert_snapshot!(
test.completions_without_builtins(),
@"<No completions found after filtering out completions>",
);
} }
#[test] #[test]
@ -1349,7 +1394,7 @@ if sys.platform == \"not-my-current-platform\":
// TODO: ideally, `only_available_in_this_branch` should be available here, but we // TODO: ideally, `only_available_in_this_branch` should be available here, but we
// currently make no effort to provide a good IDE experience within sections that // currently make no effort to provide a good IDE experience within sections that
// are unreachable // are unreachable
assert_snapshot!(test.completions(), @"sys"); assert_snapshot!(test.completions_without_builtins(), @"sys");
} }
#[test] #[test]
@ -1454,7 +1499,7 @@ A().<CURSOR>
", ",
); );
assert_snapshot!(test.completions(), @"<No completions found>"); assert_snapshot!(test.completions_without_builtins(), @"<No completions found>");
} }
#[test] #[test]
@ -1670,7 +1715,7 @@ q<CURSOR>.foo.xyz
", ",
); );
assert_snapshot!(test.completions(), @"<No completions found>"); assert_snapshot!(test.completions_without_builtins(), @"<No completions found>");
} }
#[test] #[test]
@ -1681,7 +1726,7 @@ q<CURSOR>.foo.xyz
", ",
); );
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
__annotations__ __annotations__
__class__ __class__
__delattr__ __delattr__
@ -1716,7 +1761,7 @@ class Foo: ...<CURSOR>
", ",
); );
assert_snapshot!(test.completions(), @"<No completions found>"); assert_snapshot!(test.completions_without_builtins(), @"<No completions found>");
} }
#[test] #[test]
@ -1738,7 +1783,7 @@ A.<CURSOR>
); );
assert_snapshot!( assert_snapshot!(
test.completions_if(|name| name.contains("FOO") || name.contains("foo")), test.completions_if(|c| c.name.contains("FOO") || c.name.contains("foo")),
@r" @r"
FOO FOO
foo foo
@ -1761,7 +1806,7 @@ def m<CURSOR>
", ",
); );
assert_snapshot!(test.completions(), @"<No completions found>"); assert_snapshot!(test.completions_without_builtins(), @"<No completions found>");
} }
// Ref: https://github.com/astral-sh/ty/issues/572 // Ref: https://github.com/astral-sh/ty/issues/572
@ -1773,7 +1818,7 @@ def m<CURSOR>(): pass
", ",
); );
assert_snapshot!(test.completions(), @"<No completions found>"); assert_snapshot!(test.completions_without_builtins(), @"<No completions found>");
} }
// Ref: https://github.com/astral-sh/ty/issues/572 // Ref: https://github.com/astral-sh/ty/issues/572
@ -1786,7 +1831,7 @@ def m(): pass
", ",
); );
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
m m
"); ");
} }
@ -1800,7 +1845,7 @@ class M<CURSOR>
", ",
); );
assert_snapshot!(test.completions(), @"<No completions found>"); assert_snapshot!(test.completions_without_builtins(), @"<No completions found>");
} }
// Ref: https://github.com/astral-sh/ty/issues/572 // Ref: https://github.com/astral-sh/ty/issues/572
@ -1812,7 +1857,7 @@ Fo<CURSOR> = float
", ",
); );
assert_snapshot!(test.completions(), @"Fo"); assert_snapshot!(test.completions_without_builtins(), @"Fo");
} }
// Ref: https://github.com/astral-sh/ty/issues/572 // Ref: https://github.com/astral-sh/ty/issues/572
@ -1824,7 +1869,7 @@ import fo<CURSOR>
", ",
); );
assert_snapshot!(test.completions(), @"<No completions found>"); assert_snapshot!(test.completions_without_builtins(), @"<No completions found>");
} }
// Ref: https://github.com/astral-sh/ty/issues/572 // Ref: https://github.com/astral-sh/ty/issues/572
@ -1836,7 +1881,7 @@ import foo as ba<CURSOR>
", ",
); );
assert_snapshot!(test.completions(), @"<No completions found>"); assert_snapshot!(test.completions_without_builtins(), @"<No completions found>");
} }
// Ref: https://github.com/astral-sh/ty/issues/572 // Ref: https://github.com/astral-sh/ty/issues/572
@ -1848,7 +1893,7 @@ from fo<CURSOR> import wat
", ",
); );
assert_snapshot!(test.completions(), @"<No completions found>"); assert_snapshot!(test.completions_without_builtins(), @"<No completions found>");
} }
// Ref: https://github.com/astral-sh/ty/issues/572 // Ref: https://github.com/astral-sh/ty/issues/572
@ -1860,7 +1905,7 @@ from foo import wa<CURSOR>
", ",
); );
assert_snapshot!(test.completions(), @"<No completions found>"); assert_snapshot!(test.completions_without_builtins(), @"<No completions found>");
} }
// Ref: https://github.com/astral-sh/ty/issues/572 // Ref: https://github.com/astral-sh/ty/issues/572
@ -1872,7 +1917,7 @@ from foo import wat as ba<CURSOR>
", ",
); );
assert_snapshot!(test.completions(), @"<No completions found>"); assert_snapshot!(test.completions_without_builtins(), @"<No completions found>");
} }
// Ref: https://github.com/astral-sh/ty/issues/572 // Ref: https://github.com/astral-sh/ty/issues/572
@ -1887,7 +1932,10 @@ except Type<CURSOR>:
", ",
); );
assert_snapshot!(test.completions(), @"<No completions found>"); assert_snapshot!(
test.completions_without_builtins(),
@"<No completions found after filtering out completions>",
);
} }
// Ref: https://github.com/astral-sh/ty/issues/572 // Ref: https://github.com/astral-sh/ty/issues/572
@ -1900,7 +1948,7 @@ def _():
", ",
); );
assert_snapshot!(test.completions(), @"<No completions found>"); assert_snapshot!(test.completions_without_builtins(), @"<No completions found>");
} }
#[test] #[test]
@ -1925,7 +1973,7 @@ f = Foo()
// but we instead fall back to scope based completions. Since // but we instead fall back to scope based completions. Since
// we're inside a string, we should avoid giving completions at // we're inside a string, we should avoid giving completions at
// all. // all.
assert_snapshot!(test.completions(), @r" assert_snapshot!(test.completions_without_builtins(), @r"
Foo Foo
bar bar
f f
@ -2096,7 +2144,7 @@ foo = 1
from ? import <CURSOR> from ? import <CURSOR>
", ",
); );
assert_snapshot!(test.completions(), @r"<No completions found>"); assert_snapshot!(test.completions_without_builtins(), @r"<No completions found>");
} }
#[test] #[test]
@ -2226,25 +2274,38 @@ importlib.<CURSOR>
"#, "#,
); );
assert_snapshot!(test.completions(), @r"<No completions found>"); assert_snapshot!(
test.completions_without_builtins(),
@"<No completions found after filtering out completions>",
);
} }
impl CursorTest { impl CursorTest {
fn completions(&self) -> String { /// Returns all completions except for builtins.
self.completions_if(|_| true) fn completions_without_builtins(&self) -> String {
self.completions_if(|c| !c.builtin)
} }
fn completions_if(&self, predicate: impl Fn(&str) -> bool) -> String { fn completions_if(&self, predicate: impl Fn(&Completion) -> bool) -> String {
let completions = completion(&self.db, self.cursor.file, self.cursor.offset); let completions = completion(&self.db, self.cursor.file, self.cursor.offset);
if completions.is_empty() { if completions.is_empty() {
return "<No completions found>".to_string(); return "<No completions found>".to_string();
} }
completions let included = completions
.into_iter() .iter()
.map(|completion| completion.label)
.filter(|label| predicate(label)) .filter(|label| predicate(label))
.collect::<Vec<String>>() .map(|completion| completion.name.as_str().to_string())
.join("\n") .collect::<Vec<String>>();
if included.is_empty() {
// It'd be nice to include the actual number of
// completions filtered out, but in practice, the
// number is environment dependent. For example, on
// Windows, there are 231 builtins, but on Unix, there
// are 230. So we just leave out the number I guess.
// ---AG
return "<No completions found after filtering out completions>".to_string();
}
included.join("\n")
} }
#[track_caller] #[track_caller]
@ -2254,7 +2315,7 @@ importlib.<CURSOR>
assert!( assert!(
completions completions
.iter() .iter()
.any(|completion| completion.label == expected), .any(|completion| completion.name == expected),
"Expected completions to include `{expected}`" "Expected completions to include `{expected}`"
); );
} }
@ -2266,7 +2327,7 @@ importlib.<CURSOR>
assert!( assert!(
completions completions
.iter() .iter()
.all(|completion| completion.label != unexpected), .all(|completion| completion.name != unexpected),
"Expected completions to not include `{unexpected}`", "Expected completions to not include `{unexpected}`",
); );
} }

2
crates/ty_python_semantic/src/lib.rs
View file

@ -15,7 +15,7 @@ pub use program::{
PythonVersionWithSource, SearchPathSettings, PythonVersionWithSource, SearchPathSettings,
}; };
pub use python_platform::PythonPlatform; pub use python_platform::PythonPlatform;
pub use semantic_model::{HasType, SemanticModel}; pub use semantic_model::{Completion, HasType, SemanticModel};
pub use site_packages::{PythonEnvironment, SitePackagesPaths, SysPrefixPathOrigin}; pub use site_packages::{PythonEnvironment, SitePackagesPaths, SysPrefixPathOrigin};
pub use util::diagnostics::add_inferred_python_version_hint_to_diagnostic; pub use util::diagnostics::add_inferred_python_version_hint_to_diagnostic;

64
crates/ty_python_semantic/src/semantic_model.rs
View file

@ -6,7 +6,7 @@ use ruff_source_file::LineIndex;
use crate::Db; use crate::Db;
use crate::module_name::ModuleName; use crate::module_name::ModuleName;
use crate::module_resolver::{Module, resolve_module}; use crate::module_resolver::{KnownModule, Module, resolve_module};
use crate::semantic_index::ast_ids::HasScopedExpressionId; use crate::semantic_index::ast_ids::HasScopedExpressionId;
use crate::semantic_index::place::FileScopeId; use crate::semantic_index::place::FileScopeId;
use crate::semantic_index::semantic_index; use crate::semantic_index::semantic_index;
@ -46,7 +46,7 @@ impl<'db> SemanticModel<'db> {
&self, &self,
import: &ast::StmtImportFrom, import: &ast::StmtImportFrom,
_name: Option<usize>, _name: Option<usize>,
) -> Vec<Name> { ) -> Vec<Completion> {
let module_name = match ModuleName::from_import_statement(self.db, self.file, import) { let module_name = match ModuleName::from_import_statement(self.db, self.file, import) {
Ok(module_name) => module_name, Ok(module_name) => module_name,
Err(err) => { Err(err) => {
@ -58,18 +58,36 @@ impl<'db> SemanticModel<'db> {
return vec![]; return vec![];
} }
}; };
let Some(module) = resolve_module(self.db, &module_name) else { self.module_completions(&module_name)
}
/// Returns completions for symbols available in the given module as if
/// it were imported by this model's `File`.
fn module_completions(&self, module_name: &ModuleName) -> Vec<Completion> {
let Some(module) = resolve_module(self.db, module_name) else {
tracing::debug!("Could not resolve module from `{module_name:?}`"); tracing::debug!("Could not resolve module from `{module_name:?}`");
return vec![]; return vec![];
}; };
let ty = Type::module_literal(self.db, self.file, &module); let ty = Type::module_literal(self.db, self.file, &module);
crate::types::all_members(self.db, ty).into_iter().collect() crate::types::all_members(self.db, ty)
.into_iter()
.map(|name| Completion {
name,
builtin: module.is_known(KnownModule::Builtins),
})
.collect()
} }
/// Returns completions for symbols available in a `object.<CURSOR>` context. /// Returns completions for symbols available in a `object.<CURSOR>` context.
pub fn attribute_completions(&self, node: &ast::ExprAttribute) -> Vec<Name> { pub fn attribute_completions(&self, node: &ast::ExprAttribute) -> Vec<Completion> {
let ty = node.value.inferred_type(self); let ty = node.value.inferred_type(self);
crate::types::all_members(self.db, ty).into_iter().collect() crate::types::all_members(self.db, ty)
.into_iter()
.map(|name| Completion {
name,
builtin: false,
})
.collect()
} }
/// Returns completions for symbols available in the scope containing the /// Returns completions for symbols available in the scope containing the
@ -77,7 +95,7 @@ impl<'db> SemanticModel<'db> {
/// ///
/// If a scope could not be determined, then completions for the global /// If a scope could not be determined, then completions for the global
/// scope of this model's `File` are returned. /// scope of this model's `File` are returned.
pub fn scoped_completions(&self, node: ast::AnyNodeRef<'_>) -> Vec<Name> { pub fn scoped_completions(&self, node: ast::AnyNodeRef<'_>) -> Vec<Completion> {
let index = semantic_index(self.db, self.file); let index = semantic_index(self.db, self.file);
// TODO: We currently use `try_expression_scope_id` here as a hotfix for [1]. // TODO: We currently use `try_expression_scope_id` here as a hotfix for [1].
@ -96,17 +114,37 @@ impl<'db> SemanticModel<'db> {
}) else { }) else {
return vec![]; return vec![];
}; };
let mut symbols = vec![]; let mut completions = vec![];
for (file_scope, _) in index.ancestor_scopes(file_scope) { for (file_scope, _) in index.ancestor_scopes(file_scope) {
symbols.extend(all_declarations_and_bindings( completions.extend(
self.db, all_declarations_and_bindings(self.db, file_scope.to_scope_id(self.db, self.file))
file_scope.to_scope_id(self.db, self.file), .map(|name| Completion {
)); name,
builtin: false,
}),
);
} }
symbols // Builtins are available in all scopes.
let builtins = ModuleName::new("builtins").expect("valid module name");
completions.extend(self.module_completions(&builtins));
completions
} }
} }
/// A suggestion for code completion.
#[derive(Clone, Debug)]
pub struct Completion {
/// The label shown to the user for this suggestion.
pub name: Name,
/// Whether this suggestion came from builtins or not.
///
/// At time of writing (2025-06-26), this information
/// doesn't make it into the LSP response. Instead, we
/// use it mainly in tests so that we can write less
/// noisy tests.
pub builtin: bool,
}
pub trait HasType { pub trait HasType {
/// Returns the inferred type of `self`. /// Returns the inferred type of `self`.
/// ///

2
crates/ty_server/src/server/api/requests/completion.rs
View file

@ -56,7 +56,7 @@ impl BackgroundDocumentRequestHandler for CompletionRequestHandler {
.into_iter() .into_iter()
.enumerate() .enumerate()
.map(|(i, comp)| CompletionItem { .map(|(i, comp)| CompletionItem {
label: comp.label, label: comp.name.into(),
sort_text: Some(format!("{i:-max_index_len$}")), sort_text: Some(format!("{i:-max_index_len$}")),
..Default::default() ..Default::default()
}) })

4
crates/ty_wasm/src/lib.rs
View file

@ -309,7 +309,7 @@ impl Workspace {
Ok(completions Ok(completions
.into_iter() .into_iter()
.map(|completion| Completion { .map(|completion| Completion {
label: completion.label, name: completion.name.into(),
}) })
.collect()) .collect())
} }
@ -619,7 +619,7 @@ pub struct Hover {
#[derive(Debug, Clone, PartialEq, Eq)] #[derive(Debug, Clone, PartialEq, Eq)]
pub struct Completion { pub struct Completion {
#[wasm_bindgen(getter_with_clone)] #[wasm_bindgen(getter_with_clone)]
pub label: String, pub name: String,
} }
#[wasm_bindgen] #[wasm_bindgen]