Skip to content

Commit 4ca7459

Browse files
[ty] Consider type_check_only when ranking completions (#20910)
1 parent dab3d4e commit 4ca7459

File tree

13 files changed

+175
-7
lines changed

13 files changed

+175
-7
lines changed

crates/ty_completion_eval/completion-evaluation-tasks.csv

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@ higher-level-symbols-preferred,main.py,0,
44
higher-level-symbols-preferred,main.py,1,1
55
import-deprioritizes-dunder,main.py,0,1
66
import-deprioritizes-sunder,main.py,0,1
7+
import-deprioritizes-type_check_only,main.py,0,1
8+
import-deprioritizes-type_check_only,main.py,1,1
9+
import-deprioritizes-type_check_only,main.py,2,1
10+
import-deprioritizes-type_check_only,main.py,3,2
11+
import-deprioritizes-type_check_only,main.py,4,3
712
internal-typeshed-hidden,main.py,0,4
813
none-completion,main.py,0,11
914
numpy-array,main.py,0,
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[settings]
2+
auto-import = true
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
from module import UniquePrefixA<CURSOR:UniquePrefixAzurous>
2+
from module import unique_prefix_<CURSOR:unique_prefix_azurous>
3+
4+
from module import Class
5+
6+
Class.meth_<CURSOR:meth_azurous>
7+
8+
# TODO: bound methods don't preserve type-check-only-ness, this is a bug
9+
Class().meth_<CURSOR:meth_azurous>
10+
11+
# TODO: auto-imports don't take type-check-only-ness into account, this is a bug
12+
UniquePrefixA<CURSOR:module.UniquePrefixAzurous>
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
from typing import type_check_only
2+
3+
4+
@type_check_only
5+
class UniquePrefixApple: pass
6+
7+
class UniquePrefixAzurous: pass
8+
9+
10+
@type_check_only
11+
def unique_prefix_apple() -> None: pass
12+
13+
def unique_prefix_azurous() -> None: pass
14+
15+
16+
class Class:
17+
@type_check_only
18+
def meth_apple(self) -> None: pass
19+
20+
def meth_azurous(self) -> None: pass
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
[project]
2+
name = "test"
3+
version = "0.1.0"
4+
requires-python = ">=3.13"
5+
dependencies = []

crates/ty_completion_eval/truth/import-deprioritizes-type_check_only/uv.lock

Lines changed: 8 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/ty_ide/src/completion.rs

Lines changed: 76 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ pub struct Completion<'db> {
6565
/// use it mainly in tests so that we can write less
6666
/// noisy tests.
6767
pub builtin: bool,
68+
/// Whether this item only exists for type checking purposes and
69+
/// will be missing at runtime
70+
pub is_type_check_only: bool,
6871
/// The documentation associated with this item, if
6972
/// available.
7073
pub documentation: Option<Docstring>,
@@ -79,6 +82,7 @@ impl<'db> Completion<'db> {
7982
.ty
8083
.and_then(|ty| DefinitionsOrTargets::from_ty(db, ty));
8184
let documentation = definition.and_then(|def| def.docstring(db));
85+
let is_type_check_only = semantic.is_type_check_only(db);
8286
Completion {
8387
name: semantic.name,
8488
insert: None,
@@ -87,6 +91,7 @@ impl<'db> Completion<'db> {
8791
module_name: None,
8892
import: None,
8993
builtin: semantic.builtin,
94+
is_type_check_only,
9095
documentation,
9196
}
9297
}
@@ -294,6 +299,7 @@ fn add_keyword_value_completions<'db>(
294299
kind: None,
295300
module_name: None,
296301
import: None,
302+
is_type_check_only: false,
297303
builtin: true,
298304
documentation: None,
299305
});
@@ -339,6 +345,8 @@ fn add_unimported_completions<'db>(
339345
module_name: Some(symbol.module.name(db)),
340346
import: import_action.import().cloned(),
341347
builtin: false,
348+
// TODO: `is_type_check_only` requires inferring the type of the symbol
349+
is_type_check_only: false,
342350
documentation: None,
343351
});
344352
}
@@ -837,16 +845,21 @@ fn is_in_string(parsed: &ParsedModuleRef, offset: TextSize) -> bool {
837845
})
838846
}
839847

840-
/// Order completions lexicographically, with these exceptions:
848+
/// Order completions according to the following rules:
841849
///
842-
/// 1) A `_[^_]` prefix sorts last and
843-
/// 2) A `__` prefix sorts last except before (1)
850+
/// 1) Names with no underscore prefix
851+
/// 2) Names starting with `_` but not dunders
852+
/// 3) `__dunder__` names
853+
///
854+
/// Among each category, type-check-only items are sorted last,
855+
/// and otherwise completions are sorted lexicographically.
844856
///
845857
/// This has the effect of putting all dunder attributes after "normal"
846858
/// attributes, and all single-underscore attributes after dunder attributes.
847859
fn compare_suggestions(c1: &Completion, c2: &Completion) -> Ordering {
848860
let (kind1, kind2) = (NameKind::classify(&c1.name), NameKind::classify(&c2.name));
849-
kind1.cmp(&kind2).then_with(|| c1.name.cmp(&c2.name))
861+
862+
(kind1, c1.is_type_check_only, &c1.name).cmp(&(kind2, c2.is_type_check_only, &c2.name))
850863
}
851864

852865
#[cfg(test)]
@@ -3398,6 +3411,65 @@ from os.<CURSOR>
33983411
);
33993412
}
34003413

3414+
#[test]
3415+
fn import_type_check_only_lowers_ranking() {
3416+
let test = CursorTest::builder()
3417+
.source(
3418+
"main.py",
3419+
r#"
3420+
import foo
3421+
foo.A<CURSOR>
3422+
"#,
3423+
)
3424+
.source(
3425+
"foo/__init__.py",
3426+
r#"
3427+
from typing import type_check_only
3428+
3429+
@type_check_only
3430+
class Apple: pass
3431+
3432+
class Banana: pass
3433+
class Cat: pass
3434+
class Azorubine: pass
3435+
"#,
3436+
)
3437+
.build();
3438+
3439+
let settings = CompletionSettings::default();
3440+
let completions = completion(&test.db, &settings, test.cursor.file, test.cursor.offset);
3441+
3442+
let [apple_pos, banana_pos, cat_pos, azo_pos, ann_pos] =
3443+
["Apple", "Banana", "Cat", "Azorubine", "__annotations__"].map(|name| {
3444+
completions
3445+
.iter()
3446+
.position(|comp| comp.name == name)
3447+
.unwrap()
3448+
});
3449+
3450+
assert!(completions[apple_pos].is_type_check_only);
3451+
assert!(apple_pos > banana_pos.max(cat_pos).max(azo_pos));
3452+
assert!(ann_pos > apple_pos);
3453+
}
3454+
3455+
#[test]
3456+
fn type_check_only_is_type_check_only() {
3457+
// `@typing.type_check_only` is a function that's unavailable at runtime
3458+
// and so should be the last "non-underscore" completion in `typing`
3459+
let test = cursor_test("from typing import t<CURSOR>");
3460+
3461+
let settings = CompletionSettings::default();
3462+
let completions = completion(&test.db, &settings, test.cursor.file, test.cursor.offset);
3463+
let last_nonunderscore = completions
3464+
.into_iter()
3465+
.filter(|c| !c.name.starts_with('_'))
3466+
.next_back()
3467+
.unwrap();
3468+
3469+
assert_eq!(&last_nonunderscore.name, "type_check_only");
3470+
assert!(last_nonunderscore.is_type_check_only);
3471+
}
3472+
34013473
#[test]
34023474
fn regression_test_issue_642() {
34033475
// Regression test for https://github.com/astral-sh/ty/issues/642

crates/ty_python_semantic/src/semantic_model.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,12 @@ pub struct Completion<'db> {
342342
pub builtin: bool,
343343
}
344344

345+
impl<'db> Completion<'db> {
346+
pub fn is_type_check_only(&self, db: &'db dyn Db) -> bool {
347+
self.ty.is_some_and(|ty| ty.is_type_check_only(db))
348+
}
349+
}
350+
345351
pub trait HasType {
346352
/// Returns the inferred type of `self`.
347353
///

crates/ty_python_semantic/src/types.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ pub use crate::types::display::DisplaySettings;
5353
use crate::types::display::TupleSpecialization;
5454
use crate::types::enums::{enum_metadata, is_single_member_enum};
5555
use crate::types::function::{
56-
DataclassTransformerFlags, DataclassTransformerParams, FunctionSpans, FunctionType,
57-
KnownFunction,
56+
DataclassTransformerFlags, DataclassTransformerParams, FunctionDecorators, FunctionSpans,
57+
FunctionType, KnownFunction,
5858
};
5959
pub(crate) use crate::types::generics::GenericContext;
6060
use crate::types::generics::{
@@ -868,6 +868,17 @@ impl<'db> Type<'db> {
868868
matches!(self, Type::Dynamic(_))
869869
}
870870

871+
/// Is a value of this type only usable in typing contexts?
872+
pub(crate) fn is_type_check_only(&self, db: &'db dyn Db) -> bool {
873+
match self {
874+
Type::ClassLiteral(class_literal) => class_literal.type_check_only(db),
875+
Type::FunctionLiteral(f) => {
876+
f.has_known_decorator(db, FunctionDecorators::TYPE_CHECK_ONLY)
877+
}
878+
_ => false,
879+
}
880+
}
881+
871882
// If the type is a specialized instance of the given `KnownClass`, returns the specialization.
872883
pub(crate) fn known_specialization(
873884
&self,

crates/ty_python_semantic/src/types/call/bind.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,6 +1001,7 @@ impl<'db> Bindings<'db> {
10011001
class_literal.body_scope(db),
10021002
class_literal.known(db),
10031003
class_literal.deprecated(db),
1004+
class_literal.type_check_only(db),
10041005
Some(params),
10051006
class_literal.dataclass_transformer_params(db),
10061007
)));

0 commit comments

Comments
 (0)