From ad28f071ffb205829b876fc3386449c350743820 Mon Sep 17 00:00:00 2001 From: Caleb White Date: Fri, 10 Jul 2026 16:50:17 -0500 Subject: [PATCH] fix: rank imported/same-namespace symbols above non-imported in completion Swap source_tier before origin_tier in the sort_text key for class, function, and constant completions. Previously origin_tier (project > core > vendor) came first, so a non-imported project class could outrank an already-imported vendor class. Now an imported symbol always ranks above a non-imported one regardless of provenance. Affects class_sort_text in class_completion.rs and flat_symbol_sort_text in symbol_ranking.rs. --- docs/CHANGELOG.md | 1 + src/completion/context/class_completion.rs | 9 +++++-- .../context/class_completion_tests.rs | 5 ++-- src/completion/context/symbol_ranking.rs | 4 +++- tests/integration/completion_class_names.rs | 24 ++++++++++++------- 5 files changed, 30 insertions(+), 13 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 44cc2980..08c8dda5 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Eloquent relation and column string completion.** Typing inside string arguments to `with()`, `load()`, `whereHas()`, and other Eloquent methods that accept relation names now offers relationship method names as completions, with dot-notation traversal for nested relations. Similarly, `where()`, `orderBy()`, `select()`, `pluck()`, and other column-accepting methods offer model column names (from `$casts`, `$fillable`, `@property` tags, timestamps, etc.). - **`model-property` pseudo-type recognition.** The Larastan `model-property` type no longer triggers "unknown class" diagnostics. It is treated as a string subtype. - **`compact()` strings are linked to local variables.** A string argument to `compact('user')` is now treated as a reference to the matching local variable. Renaming the variable updates the string (and renaming from the string updates the variable and its other uses), find-references includes the string, and go-to-definition on the string jumps to the variable's assignment. Contributed by @calebdw in https://github.com/PHPantom-dev/phpantom_lsp/pull/159. +- **Imported and same-namespace symbols rank first in completion.** Classes, functions, and constants that are already imported via a `use` statement or live in the same namespace now always appear above non-imported symbols in the completion list, regardless of dependency provenance. Previously a non-imported project class could outrank an already-imported vendor class, forcing users to scroll past irrelevant results. Contributed by @calebdw. - **Laravel route controller method navigation and completion.** Method-name strings inside `Route::controller(X::class)->group(fn(){…})` closures now resolve as references to the controller's methods. Go-to-definition, find-references, rename, hover, and diagnostics all work on the action string (e.g. `Route::patch('cancel', 'cancel')` resolves `'cancel'` to `WorkItemController::cancel()`). Autocompletion inside the action string offers the controller's methods. Handles `->controller()` anywhere in the fluent chain, chained route calls (`->name()`, etc.), and nested groups where an inner `->controller()` shadows the outer one. Contributed by @calebdw. ### Changed diff --git a/src/completion/context/class_completion.rs b/src/completion/context/class_completion.rs index 0fa2bc8d..d5d2918b 100644 --- a/src/completion/context/class_completion.rs +++ b/src/completion/context/class_completion.rs @@ -989,7 +989,7 @@ pub(in crate::completion) fn match_quality(short_name: &str, prefix: &str) -> ch /// Assemble a sort_text string for a class name completion item. /// -/// The format is `{match_quality}{origin_tier}{source_tier}{affinity}{demote}{gap}_{short_name_lower}` +/// The format is `{match_quality}{source_tier}{origin_tier}{affinity}{demote}{gap}_{short_name_lower}` /// where: /// - `match_quality`: `'a'` exact, `'b'` starts-with, `'c'` contains /// - `source_tier`: `'0'` use-imported, `'1'` same-namespace, `'2'` everything else @@ -1003,6 +1003,11 @@ pub(in crate::completion) fn match_quality(short_name: &str, prefix: &str) -> ch /// first. This smooths the visual transition as a prefix match /// narrows toward an exact match (e.g. typing "Pro" ranks `Product` /// above `ProductFilterTerm` when both share the same affinity). +/// +/// **`source_tier` sorts before `origin_tier`** so that an already-imported +/// class always ranks above a non-imported one, regardless of where it +/// comes from. A vendor class that's already in the file's `use` map +/// is more relevant than a project class the user hasn't imported yet. pub(in crate::completion) fn class_sort_text( short_name: &str, fqn: &str, @@ -1023,8 +1028,8 @@ pub(in crate::completion) fn class_sort_text( format!( "{}{}{}{}{}{}_{}", quality, - origin_tier, source_tier, + origin_tier, affinity, demote, gap, diff --git a/src/completion/context/class_completion_tests.rs b/src/completion/context/class_completion_tests.rs index dc252af1..fadd7392 100644 --- a/src/completion/context/class_completion_tests.rs +++ b/src/completion/context/class_completion_tests.rs @@ -825,8 +825,9 @@ fn test_class_sort_text_format() { false, &table, ); - // quality='a' (exact), origin='0', tier='2', affinity=9999-4=9995 → "9995", demote='0', gap=5-5=0 → "000" - assert_eq!(result, "a0299950000_order"); + // quality='a' (exact), source='2', origin='0', affinity=9999-4=9995 → "9995", demote='0', gap=5-5=0 → "000" + // source_tier sorts before origin_tier so imported items always rank first. + assert_eq!(result, "a2099950000_order"); } #[test] diff --git a/src/completion/context/symbol_ranking.rs b/src/completion/context/symbol_ranking.rs index fdd6ce36..4c71f341 100644 --- a/src/completion/context/symbol_ranking.rs +++ b/src/completion/context/symbol_ranking.rs @@ -11,11 +11,13 @@ pub(crate) fn flat_symbol_sort_text( source_tier: char, ) -> String { let quality = super::class_completion::match_quality(short_name, prefix); + // source_tier before origin_tier: imported symbols always rank above + // non-imported ones, regardless of provenance. format!( "{}{}{}_{}", quality, - origin_tier, source_tier, + origin_tier, short_name.to_lowercase() ) } diff --git a/tests/integration/completion_class_names.rs b/tests/integration/completion_class_names.rs index 61157e90..30ea5aa3 100644 --- a/tests/integration/completion_class_names.rs +++ b/tests/integration/completion_class_names.rs @@ -163,9 +163,14 @@ async fn test_class_name_completion_prioritizes_project_core_explicit_then_trans *backend.workspace_root().write() = Some(dir.path().to_path_buf()); backend.initialized(InitializedParams {}).await; + // Use a file in the App namespace so that App\RuntimeException is + // same-namespace (source '1') and all other RuntimeException variants + // are non-imported (source '2'). This isolates origin_tier ordering + // for non-imported classes. (Without a namespace the global core + // RuntimeException would be same-namespace and sort first.) let uri = Url::parse("file:///app.php").unwrap(); - let text = " = funcs.iter().filter_map(|i| i.sort_text.clone()).collect(); + // sort_text format: {quality}{source}{origin}_name + // source_tier sorts before origin_tier. assert!( sorts.iter().any(|s| s.starts_with("b00_")) - && sorts.iter().any(|s| s.starts_with("b10_")) - && sorts.iter().any(|s| s.starts_with("b21_")) - && sorts.iter().any(|s| s.starts_with("b31_")), + && sorts.iter().any(|s| s.starts_with("b01_")) + && sorts.iter().any(|s| s.starts_with("b12_")) + && sorts.iter().any(|s| s.starts_with("b13_")), "expected project/core/explicit/transitive sort tiers in function completions, got: {sorts:?}" ); } @@ -858,12 +865,13 @@ async fn test_class_name_completion_same_namespace() { class_fqns ); - // Same-namespace should have source tier '1' at position 2. + // Same-namespace should have source tier '1' at position 1 + // (source_tier sorts before origin_tier). let service_item = find_by_fqn(&classes, "App\\UserService").unwrap(); let sort = service_item.sort_text.as_deref().unwrap_or(""); assert!( - sort.len() > 2 && &sort[2..3] == "1", - "Same-namespace classes should have source tier '1' at position 2, got: {:?}", + sort.len() > 1 && &sort[1..2] == "1", + "Same-namespace classes should have source tier '1' at position 1, got: {:?}", sort ); }