From 3e335cc180e4a2feef44703b259f59906ed330e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9r=C3=A8?= Date: Mon, 11 May 2026 11:13:58 -0700 Subject: [PATCH 1/2] Prefer std lib over third parties for auto-import --- .../completion-evaluation-tasks.csv | 8 +- .../completion.toml | 0 .../stdlib-over-third-party-same-name/main.py | 3 + .../pyproject.toml | 0 .../uv.lock | 0 .../truth/third-party-over-stdlib/main.py | 12 -- crates/ty_ide/src/completion.rs | 166 +++++++++++------- 7 files changed, 107 insertions(+), 82 deletions(-) rename crates/ty_completion_eval/truth/{third-party-over-stdlib => stdlib-over-third-party-same-name}/completion.toml (100%) create mode 100644 crates/ty_completion_eval/truth/stdlib-over-third-party-same-name/main.py rename crates/ty_completion_eval/truth/{third-party-over-stdlib => stdlib-over-third-party-same-name}/pyproject.toml (100%) rename crates/ty_completion_eval/truth/{third-party-over-stdlib => stdlib-over-third-party-same-name}/uv.lock (100%) delete mode 100644 crates/ty_completion_eval/truth/third-party-over-stdlib/main.py diff --git a/crates/ty_completion_eval/completion-evaluation-tasks.csv b/crates/ty_completion_eval/completion-evaluation-tasks.csv index 0e6b29c96f74e..83b14bc4d3816 100644 --- a/crates/ty_completion_eval/completion-evaluation-tasks.csv +++ b/crates/ty_completion_eval/completion-evaluation-tasks.csv @@ -8,7 +8,7 @@ auto-import-skips-third-party-tests,main.py,1,1 class-arg-completion,main.py,0,1 class-arg-completion,main.py,1,1 deprecated-deprioritized,main.py,0,1 -deprecated-deprioritized,main.py,1,3 +deprecated-deprioritized,main.py,1,1 deprecated-deprioritized,main.py,2,1 exact-over-fuzzy,main.py,0,1 fstring-completions,main.py,0,1 @@ -22,11 +22,11 @@ import-deprioritizes-type_check_only,main.py,2,1 import-deprioritizes-type_check_only,main.py,3,2 import-deprioritizes-type_check_only,main.py,4,3 import-keyword-completion,main.py,0,1 -internal-typeshed-hidden,main.py,0,2 +internal-typeshed-hidden,main.py,0,1 local-over-auto-import,main.py,0,1 modules-over-other-symbols,main.py,0,1 none-completion,main.py,0,1 -numpy-array,main.py,0,14 +numpy-array,main.py,0,11 numpy-array,main.py,1,1 object-attr-instance-methods,main.py,0,1 object-attr-instance-methods,main.py,1,1 @@ -37,7 +37,7 @@ raise-uses-base-exception,main.py,2,1 scope-existing-over-new-import,main.py,0,1 scope-prioritize-closer,main.py,0,2 scope-simple-long-identifier,main.py,0,1 -third-party-over-stdlib,main.py,0,1 +stdlib-over-third-party-same-name,main.py,0,1 tighter-over-looser-scope,main.py,0,3 tstring-completions,main.py,0,1 ty-extensions-lower-stdlib,main.py,0,1 diff --git a/crates/ty_completion_eval/truth/third-party-over-stdlib/completion.toml b/crates/ty_completion_eval/truth/stdlib-over-third-party-same-name/completion.toml similarity index 100% rename from crates/ty_completion_eval/truth/third-party-over-stdlib/completion.toml rename to crates/ty_completion_eval/truth/stdlib-over-third-party-same-name/completion.toml diff --git a/crates/ty_completion_eval/truth/stdlib-over-third-party-same-name/main.py b/crates/ty_completion_eval/truth/stdlib-over-third-party-same-name/main.py new file mode 100644 index 0000000000000..560d587d4840f --- /dev/null +++ b/crates/ty_completion_eval/truth/stdlib-over-third-party-same-name/main.py @@ -0,0 +1,3 @@ +# Prefer stdlib symbols over same-name third-party symbols when ty can't tell +# whether the third-party package is a direct or transitive dependency. +fullmatch diff --git a/crates/ty_completion_eval/truth/third-party-over-stdlib/pyproject.toml b/crates/ty_completion_eval/truth/stdlib-over-third-party-same-name/pyproject.toml similarity index 100% rename from crates/ty_completion_eval/truth/third-party-over-stdlib/pyproject.toml rename to crates/ty_completion_eval/truth/stdlib-over-third-party-same-name/pyproject.toml diff --git a/crates/ty_completion_eval/truth/third-party-over-stdlib/uv.lock b/crates/ty_completion_eval/truth/stdlib-over-third-party-same-name/uv.lock similarity index 100% rename from crates/ty_completion_eval/truth/third-party-over-stdlib/uv.lock rename to crates/ty_completion_eval/truth/stdlib-over-third-party-same-name/uv.lock diff --git a/crates/ty_completion_eval/truth/third-party-over-stdlib/main.py b/crates/ty_completion_eval/truth/third-party-over-stdlib/main.py deleted file mode 100644 index a5d777bbb75b8..0000000000000 --- a/crates/ty_completion_eval/truth/third-party-over-stdlib/main.py +++ /dev/null @@ -1,12 +0,0 @@ -# This test was originally written to -# check that a third party dependency -# gets priority over stdlib. But it was -# not clearly the right choice[1]. -# -# 2026-01-09: We stuck with it for now, -# but it seems likely that we'll want -# to regress this task in favor of -# another. -# -# [1]: https://github.com/astral-sh/ruff/pull/22460#discussion_r2676343225 -fullma diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs index 6d5718390c050..7bb02a80d9418 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -288,6 +288,7 @@ pub struct Completion<'db> { /// This is generally only present when this is a completion /// suggestion for an unimported symbol. pub module_name: Option<&'db ModuleName>, + module_dependency_kind: Option, /// An import statement to insert (or ensure is already /// present) when this completion is selected. pub import: Option, @@ -450,6 +451,7 @@ impl<'db> CompletionBuilder<'db> { ty: self.ty, kind, module_name: self.module_name, + module_dependency_kind: self.module_dependency_kind, import: self.import, builtin: self.builtin, is_type_check_only: self.is_type_check_only, @@ -1114,6 +1116,26 @@ impl UserQuery { .map(|exact| &**exact == name) .unwrap_or(false) } + + fn match_quality(&self, name: &str) -> MatchQuality { + let Some(query) = self.exact.as_deref() else { + return MatchQuality::Fuzzy; + }; + + if query == name { + return MatchQuality::Exact; + } + + let query = query.to_lowercase(); + let name = name.to_lowercase(); + if name.starts_with(&query) { + MatchQuality::Prefix + } else if name.contains(&query) { + MatchQuality::Substring + } else { + MatchQuality::Fuzzy + } + } } /// Context used to help filter completions when collecting them. @@ -1246,25 +1268,16 @@ struct Relevance { /// type checking and not at runtime. type_check_only: Sort, /// Deprecated symbols appear lower in the completion result. - /// - /// This appears before `module_dependency_kind` so deprecation - /// downranking applies even when a symbol's module origin would - /// otherwise boost it, but after `type_check_only` so runtime- - /// unavailable symbols still sort last. deprecated: Sort, - /// The "dependency kind" of the module where this symbol - /// originates from. - /// - /// This lets us, e.g., prioritize first party project modules - /// over third party dependencies. This applies to both symbols - /// already in scope and unimported symbols, essentially forming a - /// preference ordering for symbols based on where they came from. + /// A broad origin ranking for completions. /// - /// Not all completions have this set. For example, keywords or - /// arguments. We assume that if it's not set, then there is some - /// other sorting criteria being applied or that it is generally - /// more specific than completions where this is set. - module_dependency_kind: Option, + /// Regular stdlib, namespace package and third-party completions + /// intentionally tie here so match quality and label ordering can + /// rank unrelated matches before full origin breaks same-label ties. + module_origin: Option, + /// A coarse measure of how tightly the completion label matches + /// the user's query. + match_quality: MatchQuality, } impl Relevance { @@ -1319,23 +1332,39 @@ impl Relevance { } else { Sort::Even }, - module_dependency_kind: c.module_dependency_kind, + module_origin: c + .module_dependency_kind + .map(ModuleDependencyKind::origin_rank), + match_quality: if c.is_type_check_only || c.deprecated { + MatchQuality::Fuzzy + } else { + query.match_quality(&c.name) + }, } } } -/// The dependency "kind" of a module. -/// -/// Everything above "current" is applied to unimported symbols. It -/// categorizes them by where the module is defined. We only support -/// three broad categories right now: stdlib, third party and project. -/// Ideally, we would distinguish between _direct_ third party code and -/// _indirect_ third party code, but ty doesn't yet understand how to -/// do this (as of 2026-01-08). +#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] +enum ModuleOriginRank { + Current, + Builtin, + Project, + StdlibSpecial, + Other, +} + +#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] +enum MatchQuality { + Exact, + Prefix, + Substring, + Fuzzy, +} + +/// Classifies a completion symbol by origin. /// -/// Note that these are defined in a particular order. That -/// is, modules in the project get higher priority than those -/// not in the project. +/// The variant order is the completion ranking order after broader relevance +/// criteria: earlier origins rank higher. #[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord)] enum ModuleDependencyKind { /// Symbols already in scope in the user's current module. @@ -1366,6 +1395,8 @@ enum ModuleDependencyKind { /// We also include `collections` /// for similar reasons. StdlibSpecial, + /// Symbols from the standard library. + Stdlib, /// A namespace package somewhat defies classification, since /// it can exist over multiple search paths. Since std doesn't /// use namespace packages, we just assume that they are roughly @@ -1374,23 +1405,26 @@ enum ModuleDependencyKind { /// This is an erroneous assumption when the namespace /// package is within the user's project. Probably we /// could do better once we know how to navigate namespace - /// packages better. Regardless, we put this between - /// strongly preferred and weakly preferred modules as a - /// bad compromise for now. + /// packages better. Namespace, /// Symbols defined somewhere in a dependency, direct or /// indirect. ThirdParty, - /// Symbols from the standard library get ranked last by - /// the logic that they are least specific to the end user's - /// context. - /// - /// This is somewhat specious since while they are least - /// specific, some stdlib modules are very commonly used. - Stdlib, } impl ModuleDependencyKind { + fn origin_rank(self) -> ModuleOriginRank { + match self { + ModuleDependencyKind::Current => ModuleOriginRank::Current, + ModuleDependencyKind::Builtin => ModuleOriginRank::Builtin, + ModuleDependencyKind::Project => ModuleOriginRank::Project, + ModuleDependencyKind::StdlibSpecial => ModuleOriginRank::StdlibSpecial, + ModuleDependencyKind::Stdlib + | ModuleDependencyKind::Namespace + | ModuleDependencyKind::ThirdParty => ModuleOriginRank::Other, + } + } + /// Determines the "kind" of a symbol based on the module it is /// defined in. /// @@ -2646,17 +2680,23 @@ impl PartialEq for CompletionRanker<'_> { fn eq(&self, rhs: &CompletionRanker<'_>) -> bool { self.0.relevance == rhs.0.relevance && self.0.name == rhs.0.name + && self.0.module_dependency_kind == rhs.0.module_dependency_kind && self.0.module_name == rhs.0.module_name } } impl Ord for CompletionRanker<'_> { fn cmp(&self, rhs: &CompletionRanker<'_>) -> Ordering { - (&self.0.relevance, &self.0.name, &self.0.module_name).cmp(&( - &rhs.0.relevance, - &rhs.0.name, - &rhs.0.module_name, - )) + (&self.0.relevance, &self.0.name) + .cmp(&(&rhs.0.relevance, &rhs.0.name)) + // Prefer stdlib over third-party modules only for equally named + // completions; unrelated labels should keep their name ordering. + .then_with(|| { + self.0 + .module_dependency_kind + .cmp(&rhs.0.module_dependency_kind) + }) + .then_with(|| self.0.module_name.cmp(&rhs.0.module_name)) } } @@ -8728,54 +8768,48 @@ def no_type_check_decorator(): } #[test] - fn auto_import_keeps_sys_below_third_party() { + fn auto_import_prefers_std_lib_symbol_over_third_party_symbol_with_same_name() { let builder = CursorTest::builder() .with_site_packages() - .source("main.py", "argv") + .source("main.py", "getpid") .site_packages( "thirdparty/__init__.py", r#" -from sys import argv as argv +from os import getpid as getpid "#, ) .completion_test_builder() .module_names() .filter(|c| { - c.name == "argv" + c.name == "getpid" && matches!( c.module_name.map(ModuleName::as_str), - Some("sys" | "thirdparty") + Some("os" | "thirdparty") ) }); assert_snapshot!(builder.build().snapshot(), @" - argv :: thirdparty - argv :: sys + getpid :: os + getpid :: thirdparty "); } #[test] - fn auto_import_keeps_os_below_third_party() { + fn auto_import_preserves_name_order_for_unrelated_third_party_symbol() { let builder = CursorTest::builder() .with_site_packages() - .source("main.py", "getpid") - .site_packages( - "thirdparty/__init__.py", - r#" -from os import getpid as getpid -"#, - ) + .source("main.py", "av") + .site_packages("thirdparty/__init__.py", "aardvark = 1") .completion_test_builder() .module_names() .filter(|c| { - c.name == "getpid" - && matches!( - c.module_name.map(ModuleName::as_str), - Some("os" | "thirdparty") - ) + matches!( + (c.name.as_str(), c.module_name.map(ModuleName::as_str)), + ("argv", Some("sys")) | ("aardvark", Some("thirdparty")) + ) }); assert_snapshot!(builder.build().snapshot(), @" - getpid :: thirdparty - getpid :: os + aardvark :: thirdparty + argv :: sys "); } From 6c9de06dd196e4dda1909c72746b00069543ea1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9r=C3=A8?= Date: Fri, 15 May 2026 16:42:32 -0700 Subject: [PATCH 2/2] Prefer std lib over third parties for regular imports --- crates/ty_ide/src/completion.rs | 101 ++++++++++++++++++++++++++++++-- 1 file changed, 95 insertions(+), 6 deletions(-) diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs index 7bb02a80d9418..2adc63dc3b70d 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -2476,30 +2476,50 @@ impl<'a> ImportStatement<'a> { match *self { ImportStatement::Import(Import { ref kind, .. }) => match *kind { ImportKind::Module => { - completions.extend(model.import_completions()); + add_import_semantic_completions(db, completions, model.import_completions()); } ImportKind::Submodule { ref parent } => { - completions.extend(model.import_submodule_completions_for_name(parent)); + add_import_semantic_completions( + db, + completions, + model.import_submodule_completions_for_name(parent), + ); } }, ImportStatement::FromImport(FromImport { ast, ref kind }) => match *kind { FromImportKind::Module => { - completions.extend(model.import_completions()); + add_import_semantic_completions(db, completions, model.import_completions()); } FromImportKind::Submodule { ref parent } => { - completions.extend(model.import_submodule_completions_for_name(parent)); + add_import_semantic_completions( + db, + completions, + model.import_submodule_completions_for_name(parent), + ); } FromImportKind::Relative { ref parent, import_keyword_allowed, } => { - completions.extend(model.import_submodule_completions_for_name(parent)); + add_import_semantic_completions( + db, + completions, + model.import_submodule_completions_for_name(parent), + ); if import_keyword_allowed { completions.add(CompletionBuilder::keyword("import")); } } FromImportKind::Attribute => { - completions.extend(model.from_import_completions(ast)); + let module_dependency_kind = model + .resolve_module(ast.module.as_ref().map(ast::Identifier::as_str), ast.level) + .map(|module| ModuleDependencyKind::from_module(db, module)); + add_import_semantic_completions_with_module_dependency_kind( + db, + completions, + model.from_import_completions(ast), + module_dependency_kind, + ); } }, ImportStatement::Incomplete(IncompleteImport::As) => { @@ -2512,6 +2532,45 @@ impl<'a> ImportStatement<'a> { } } +fn add_import_semantic_completions_with_module_dependency_kind<'db>( + db: &'db dyn Db, + completions: &mut Completions<'db>, + semantic_completions: impl IntoIterator>, + module_dependency_kind: Option, +) { + for semantic in semantic_completions { + let mut builder = CompletionBuilder::from_semantic_completion(db, semantic); + if let Some(module_dependency_kind) = module_dependency_kind { + builder = builder.module_dependency_kind(module_dependency_kind); + } + completions.add(builder); + } +} + +fn add_import_semantic_completions<'db>( + db: &'db dyn Db, + completions: &mut Completions<'db>, + semantic_completions: impl IntoIterator>, +) { + for semantic in semantic_completions { + let module_dependency_kind = if let Some(Type::ModuleLiteral(module_literal)) = semantic.ty + { + Some(ModuleDependencyKind::from_module( + db, + module_literal.module(db), + )) + } else { + None + }; + + let mut builder = CompletionBuilder::from_semantic_completion(db, semantic); + if let Some(module_dependency_kind) = module_dependency_kind { + builder = builder.module_dependency_kind(module_dependency_kind); + } + completions.add(builder); + } +} + /// Finds the AST node, if available, corresponding to the given `from` /// token. /// @@ -6096,6 +6155,36 @@ from builder.build().contains("collections"); } + #[test] + fn import_statement_preserves_name_order_for_unrelated_third_party_module() { + let builder = CursorTest::builder() + .with_site_packages() + .source("main.py", "import path") + .site_packages("patha.py", "") + .completion_test_builder() + .filter(|c| matches!(c.name.as_str(), "pathlib" | "patha")); + + assert_snapshot!(builder.build().snapshot(), @" + patha + pathlib + "); + } + + #[test] + fn from_import_preserves_name_order_for_unrelated_namespace_package() { + let builder = CursorTest::builder() + .with_site_packages() + .source("main.py", "from path") + .site_packages("patha/foo.py", "") + .completion_test_builder() + .filter(|c| matches!(c.name.as_str(), "pathlib" | "patha")); + + assert_snapshot!(builder.build().snapshot(), @" + patha + pathlib + "); + } + #[test] fn import_statement_with_submodule_with_leading_character() { let builder = completion_test_builder(