From a9d5898ca439cdb03339196ae04b15b93ace7bac Mon Sep 17 00:00:00 2001 From: sidux Date: Thu, 27 Aug 2026 00:19:34 +0200 Subject: [PATCH 1/2] fix(navigation): Resolve inherited member implementations Share exact implementation locations across navigation and lenses, including concrete members supplied by traits or inherited from parent classes. --- src/definition/implementation.rs | 275 +++++++++++++++++----------- tests/integration/implementation.rs | 42 +++++ 2 files changed, 205 insertions(+), 112 deletions(-) diff --git a/src/definition/implementation.rs b/src/definition/implementation.rs index 855532689..1d45dec16 100644 --- a/src/definition/implementation.rs +++ b/src/definition/implementation.rs @@ -38,6 +38,7 @@ use tower_lsp::lsp_types::*; use super::member::MemberKind; use super::point_location; use crate::Backend; +use crate::atom::Atom; use crate::class_lookup::find_class_at_offset; use crate::config::IndexingStrategy; use crate::symbol_map::{SelfStaticParentKind, SymbolKind}; @@ -152,12 +153,6 @@ impl Backend { return None; } - // Whether the target is a concrete (non-abstract, non-interface) - // class. When it is, we include abstract subclasses in the - // results because the user is exploring the class hierarchy - // rather than looking for instantiable implementations. - let target_is_concrete = target.kind != ClassLikeKind::Interface && !target.is_abstract; - let target_short = target.name; // Compute target FQN from the class's own namespace (most // reliable), then fall back to fqn_uri_index, then to the FQN we @@ -177,31 +172,57 @@ impl Backend { } }; - let implementors = self.find_implementors( - &target_short, - &target_fqn, - &class_loader, - target_is_concrete, - false, - false, - ); - - if implementors.is_empty() { - return None; - } + let descendants = + self.implementation_descendants(&target, &target_fqn, &class_loader, false); + let locations = self.class_implementation_locations(uri, content, &target, &descendants); + (!locations.is_empty()).then_some(locations) + } - let mut locations = Vec::new(); - for imp in &implementors { - if let Some(loc) = self.locate_class_declaration(imp, uri, content) { - locations.push(loc); - } + /// Every class that extends, implements, or uses `target`, directly or + /// transitively, abstract ones included. Empty for a final target. + /// + /// Shared by go-to-implementation and the implementation lens so the + /// two agree on what counts as an implementation. + pub(crate) fn implementation_descendants( + &self, + target: &ClassInfo, + target_fqn: &str, + class_loader: &dyn Fn(&str) -> Option>, + project_only: bool, + ) -> Vec> { + if target.is_final { + return Vec::new(); } + self.find_implementors( + &target.name, + target_fqn, + class_loader, + true, + false, + project_only, + ) + } - if locations.is_empty() { - None - } else { - Some(locations) - } + /// The declaration locations of the `descendants` of `target`. + /// + /// Abstract descendants are only listed for a concrete target: there + /// the user is exploring the class hierarchy, while for an interface or + /// an abstract class they want the instantiable implementations. + pub(crate) fn class_implementation_locations( + &self, + uri: &str, + content: &str, + target: &ClassInfo, + descendants: &[Arc], + ) -> Vec { + let include_abstract = target.kind != ClassLikeKind::Interface && !target.is_abstract; + let mut locations: Vec = descendants + .iter() + .filter(|descendant| include_abstract || !descendant.is_abstract) + .filter_map(|descendant| self.locate_class_declaration(descendant, uri, content)) + .collect(); + sort_and_dedup_locations(&mut locations); + locations } /// Reverse jump: from a method definition in a concrete class to the @@ -313,13 +334,9 @@ impl Backend { member_name: &str, class_loader: &dyn Fn(&str) -> Option>, ) -> Option> { - let target_short = &interface_class.name; let target_fqn = self.implementor_target_fqn(interface_class); - - // Abstract classes are included: a class being abstract says - // nothing about whether the queried method has a body in it. - let implementors = - self.find_implementors(target_short, &target_fqn, class_loader, true, false, false); + let descendants = + self.implementation_descendants(interface_class, &target_fqn, class_loader, false); let member_kind = if interface_class .methods @@ -337,70 +354,52 @@ impl Backend { MemberKind::Constant }; - let mut locations = Vec::new(); - for imp in &implementors { - if let Some(loc) = self.locate_member_implementation( - imp, - member_name, - member_kind, - class_loader, - uri, - content, - ) && !locations.contains(&loc) - { - locations.push(loc); - } - } - - if locations.is_empty() { - None - } else { - Some(locations) - } + let locations = self.member_implementation_locations( + uri, + content, + &target_fqn, + member_name, + member_kind, + &descendants, + class_loader, + ); + (!locations.is_empty()).then_some(locations) } - /// The location of the implementation of `member_name` that `imp` - /// provides, or `None` when it provides none. - /// - /// The definition to jump to is the one `imp` declares itself or, when - /// `imp` only inherits the member, the one declared by the nearest - /// ancestor that has a body — a concrete class that inherits a method - /// unchanged still implements it, it just implements it elsewhere. A - /// method that is only ever re-declared `abstract` is another - /// declaration rather than an implementation, so it is skipped. - fn locate_member_implementation( + /// The locations of the declarations that implement `member_name` for + /// the `descendants` of the class `target_fqn`. + pub(crate) fn member_implementation_locations( &self, - imp: &ClassInfo, + uri: &str, + content: &str, + target_fqn: &str, member_name: &str, member_kind: MemberKind, + descendants: &[Arc], class_loader: &dyn Fn(&str) -> Option>, - current_uri: &str, - current_content: &str, - ) -> Option { - let declares = |cls: &ClassInfo| match member_kind { - MemberKind::Method => cls - .get_method_ci(member_name) - .is_some_and(|m| !m.is_abstract && !m.is_virtual), - MemberKind::Property => cls.properties.iter().any(|p| p.name == member_name), - MemberKind::Constant => cls.constants.iter().any(|c| c.name == member_name), - }; - - let locate = |cls: &ClassInfo| -> Option { - let cls_fqn = crate::util::build_fqn(&cls.name, cls.file_namespace.as_deref()); + ) -> Vec { + let mut locations: Vec = member_implementation_providers( + target_fqn, + member_name, + member_kind, + descendants, + class_loader, + ) + .into_iter() + .filter_map(|provider| { let (class_uri, class_content) = - self.find_class_file_content(&cls_fqn, current_uri, current_content)?; - let member_pos = - Self::find_member_position_in_class(&class_content, member_name, member_kind, cls)?; + self.find_class_file_content(&provider.fqn(), uri, content)?; + let member_pos = Self::find_member_position_in_class( + &class_content, + member_name, + member_kind, + &provider, + )?; Some(point_location(Url::parse(&class_uri).ok()?, member_pos)) - }; - - if declares(imp) { - return locate(imp); - } - - crate::inheritance::ancestors(imp, class_loader) - .find(|(_, parent_cls)| declares(parent_cls)) - .and_then(|(_, parent_cls)| locate(&parent_cls)) + }) + .collect(); + sort_and_dedup_locations(&mut locations); + locations } /// The FQN to search implementors of `cls` by: the namespace the class @@ -484,33 +483,21 @@ impl Backend { MemberKind::Property }; - let target_short = &candidate.name; let target_fqn = self.implementor_target_fqn(candidate); - - let implementors = self.find_implementors( - target_short, + let descendants = + self.implementation_descendants(candidate, &target_fqn, &class_loader, false); + all_locations.extend(self.member_implementation_locations( + uri, + content, &target_fqn, + member_name, + member_kind, + &descendants, &class_loader, - true, - false, - false, - ); - - for imp in &implementors { - if let Some(loc) = self.locate_member_implementation( - imp, - member_name, - member_kind, - &class_loader, - uri, - content, - ) && !all_locations.contains(&loc) - { - all_locations.push(loc); - } - } + )); } + sort_and_dedup_locations(&mut all_locations); if all_locations.is_empty() { return None; } @@ -1188,6 +1175,70 @@ impl Backend { } } +/// The classes whose declarations implement `member_name` for the +/// `descendants` of the class `target_fqn`, one entry per declaration. +/// +/// A descendant that declares the member itself provides it; one that +/// inherits it unchanged is provided for by the trait or ancestor it +/// inherits it from, in PHP's member precedence order. A method that is +/// only ever re-declared `abstract` is another declaration rather than an +/// implementation, and a member a descendant inherits from the target +/// itself is the declaration the search started from, so neither counts. +/// Neither does an interface's, since an interface cannot implement. +/// +/// Reads class metadata only, so the implementation lens can tell whether +/// a member has any implementation without opening their files. +pub(crate) fn member_implementation_providers( + target_fqn: &str, + member_name: &str, + member_kind: MemberKind, + descendants: &[Arc], + class_loader: &dyn Fn(&str) -> Option>, +) -> Vec> { + let declares = |cls: &ClassInfo| { + cls.kind != ClassLikeKind::Interface + && match member_kind { + MemberKind::Method => cls + .get_method_ci(member_name) + .is_some_and(|m| !m.is_abstract && !m.is_virtual), + MemberKind::Property => cls.properties.iter().any(|p| p.name == member_name), + MemberKind::Constant => cls.constants.iter().any(|c| c.name == member_name), + } + }; + + let mut seen: HashSet = HashSet::new(); + let mut providers = Vec::new(); + for descendant in descendants { + let provider = if declares(descendant) { + Arc::clone(descendant) + } else { + match crate::inheritance::find_declaring_ancestor(descendant, class_loader, &declares) { + Some((_, ancestor)) => ancestor, + None => continue, + } + }; + let provider_fqn = provider.fqn(); + if provider_fqn != target_fqn && seen.insert(provider_fqn) { + providers.push(provider); + } + } + providers +} + +/// Order `locations` by file and position and drop duplicates, so a +/// declaration several descendants inherit is listed once and the result +/// does not depend on the order the index was populated in. +fn sort_and_dedup_locations(locations: &mut Vec) { + locations.sort_by(|left, right| { + left.uri + .as_str() + .cmp(right.uri.as_str()) + .then(left.range.start.line.cmp(&right.range.start.line)) + .then(left.range.start.character.cmp(&right.range.start.character)) + }); + locations.dedup(); +} + #[cfg(test)] mod tests { use std::fs; diff --git a/tests/integration/implementation.rs b/tests/integration/implementation.rs index 869f919de..96e3ec1d3 100644 --- a/tests/integration/implementation.rs +++ b/tests/integration/implementation.rs @@ -769,6 +769,48 @@ async fn test_implementation_method_only_overriders() { ); } +#[tokio::test] +async fn test_implementation_method_follows_traits_and_inherited_members() { + let backend = create_test_backend(); + + let uri = Url::parse("file:///impl_inherited.php").unwrap(); + let text = concat!( + "render();\n", // 15 + "}\n", // 16 + ); + + open_php(&backend, &uri, text).await; + + let locations = implementation_at(&backend, &uri, 15, 12).await; + let lines = locations + .iter() + .map(|location| location.range.start.line) + .collect::>(); + assert!( + lines.contains(&5), + "trait-provided implementation should resolve to the trait method: {lines:?}" + ); + assert!( + lines.contains(&11), + "inherited implementation should resolve to the parent method: {lines:?}" + ); +} + // ─── Server capability test ───────────────────────────────────────────────── /// The server should advertise `implementationProvider` in its capabilities. From 814de084abe8e63a48c99d57e26cc433a5d96003 Mon Sep 17 00:00:00 2001 From: sidux Date: Thu, 27 Aug 2026 00:29:35 +0200 Subject: [PATCH 2/2] feat(php): Add implementation CodeLens Interfaces and abstract classes, and the methods they declare, show a clickable implementation count that lists every implementation, including methods inherited unchanged or supplied by a trait. The count is worked out from class metadata and the locations are resolved lazily, like the reference lens. It replaces the read-only implementation-count inlay hint. --- docs/ARCHITECTURE.md | 2 +- docs/CHANGELOG.md | 4 +- examples/php/code_lens.php | 25 ++++ src/code_lens.rs | 211 ++++++++++++++++++++++++++++--- src/definition/implementation.rs | 64 +++++----- src/definition/mod.rs | 2 +- src/inlay_hints.rs | 185 +-------------------------- tests/integration/code_lens.rs | 138 ++++++++++++++++++++ 8 files changed, 394 insertions(+), 237 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index e006d7fec..4b324a375 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -856,7 +856,7 @@ Phases 3–5 avoid expensive parsing by first reading the raw file content and c ### Member-Level Implementation -When the cursor is on a method call (e.g. `$repo->find()`), `resolve_member_implementations` first resolves the subject to candidate classes. If any candidate is an interface or abstract class, `find_implementors` is called and each implementor is checked for the specific method. The location returned for an implementor is the declaration that supplies the body: its own when it declares the method, otherwise the nearest ancestor that declares it, since a class that inherits a method unchanged still implements it. A method that is only re-declared `abstract` is another declaration rather than an implementation and is skipped, which is also why implementors are collected with abstract classes included — whether the *class* is abstract says nothing about the method that was asked for. +When the cursor is on a method call (e.g. `$repo->find()`), `resolve_member_implementations` first resolves the subject to candidate classes. If any candidate is an interface or abstract class, `find_implementors` is called and each implementor is checked for the specific method. The location returned for an implementor is the declaration that supplies the body: its own when it declares the method, otherwise the trait or ancestor it inherits the method from, searched in PHP's member precedence order, since a class that inherits a method unchanged still implements it. The implementation CodeLens counts through the same helpers, so its count and go-to-implementation's list agree. A method that is only re-declared `abstract` is another declaration rather than an implementation and is skipped, which is also why implementors are collected with abstract classes included — whether the *class* is abstract says nothing about the method that was asked for. ### Reverse Jump: Concrete Method → Prototype Declaration diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 5cd026972..86d5bb3b3 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -24,7 +24,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Folding ranges for Blade files.** `.blade.php` files now fold like any other file: PHP constructs inside `{{ }}`, `@php`/`@endphp`, and component tag attribute expressions land on the correct template lines instead of the virtual PHP's shifted ones, and Blade's own block directives (`@if`/`@endif` and friends, `@foreach`/`@endforeach`, `@section`/`@endsection`, `@push`/`@endpush`, …) and component tag bodies (``...``) fold too, matching the behaviour other Blade-aware editors already provide. - **Attribute completion for anonymous components that declare no `@props`.** A small partial usually reads `$title` or `$icon` straight out of the tag's attributes without a `@props()` line to declare them, and typing a space inside ` 'c']` is the full array shape. - **A by-reference parameter a callee sets before returning early reads back with every value it can hold.** With `if ($s === null) { $s = 5; return; } $s = 4;` in the callee, the caller's variable came out of the call as `4`, because only the end of the function body was read and a `return` never gets there. Each `return` now contributes what the parameter holds at that point, so the variable reads as `4|5`. - **A reference assignment used as a value reads as what it points to.** `$var = 0; ($a =& $var) ?? 'hello';` left `$a` looking like it held the coalesce's fallback, since `&$var` had no resolved type of its own to hand back to whatever used the assignment's value. `&$expr` now reads as `$expr`'s own type wherever it appears as a value, so `$a` comes out as `0`. diff --git a/examples/php/code_lens.php b/examples/php/code_lens.php index 8a1600449..872bea943 100644 --- a/examples/php/code_lens.php +++ b/examples/php/code_lens.php @@ -49,3 +49,28 @@ function codeLensFormatLabel(string $text): string // A function nothing calls shows "0 references", which is the quickest way // to spot dead code in a procedural file. function codeLensUnusedHelper(): void {} + + +// ── Code Lens: implementation counts ──────────────────────────────────────── +// Above an interface or abstract class, and above each method it declares, +// PHPantom shows how many classes implement it. Click the count to list them. + +// "2 implementations": CodeLensJsonExporter and CodeLensCsvExporter. +interface CodeLensExporter +{ + // "2 implementations": the method in CodeLensJsonExporter, and the one + // CodeLensCsvExporter inherits unchanged from CodeLensTextExporter. + public function export(array $rows): string; +} + +final class CodeLensJsonExporter implements CodeLensExporter +{ + public function export(array $rows): string { return '[' . implode(',', $rows) . ']'; } +} + +class CodeLensTextExporter +{ + public function export(array $rows): string { return implode("\n", $rows); } +} + +final class CodeLensCsvExporter extends CodeLensTextExporter implements CodeLensExporter {} diff --git a/src/code_lens.rs b/src/code_lens.rs index 62c6ed0f6..249d2bfac 100644 --- a/src/code_lens.rs +++ b/src/code_lens.rs @@ -1,11 +1,15 @@ //! Code Lens (`textDocument/codeLens`) support. //! -//! Shows reference counts plus override/implement annotations. +//! Shows reference and implementation counts plus override/implement +//! annotations. + +use std::sync::Arc; use tower_lsp::lsp_types::*; use crate::Backend; use crate::atom::Atom; +use crate::definition::implementation::member_implementation_providers; use crate::definition::member::MemberKind; use crate::inheritance::find_declaring_ancestor; use crate::reference_index::ReferenceIndexKey; @@ -61,8 +65,10 @@ struct Prototype { impl Backend { /// Handle a `textDocument/codeLens` request. /// - /// Returns reference lenses for PHP declarations and navigation lenses - /// for methods that override or implement an ancestor declaration. + /// Returns reference lenses for PHP declarations, implementation + /// lenses for interfaces and abstract classes and their methods, and + /// navigation lenses for methods that override or implement an + /// ancestor declaration. pub fn handle_code_lens(&self, uri: &str, content: &str) -> Option> { let classes = { let map = self.symbols.uri_classes_index.read(); @@ -76,19 +82,34 @@ impl Backend { let index = LineIndex::new(content); let mut lenses = Vec::new(); + let class_loader = |name: &str| self.find_or_load_class(name); for class in &classes { let class_fqn = class.fqn(); + let name_offset = class_declaration_name_offset(symbol_map.as_deref(), class); if let Some(lens) = self.build_declaration_reference_lens( uri, &index, - class_declaration_name_offset(symbol_map.as_deref(), class), + name_offset, &ReferenceIndexKey::class(&class_fqn), ) { lenses.push(lens); } + let descendants = self.implementation_lens_descendants(class, &class_loader); + if let Some(descendants) = &descendants { + let count = descendants.iter().filter(|d| !d.is_abstract).count(); + lenses.extend(Self::build_implementation_lens( + uri, + &index, + name_offset, + count, + class_fqn, + None, + )); + } + if let Some(lens) = self.build_covers_lens(class, uri, &index) { lenses.push(lens); } @@ -125,6 +146,26 @@ impl Backend { { lenses.push(lens); } + if let Some(descendants) = &descendants { + let count = member_implementation_providers( + &class_fqn, + &method.name, + MemberKind::Method, + descendants, + &class_loader, + ) + .len(); + if count > 0 { + lenses.extend(Self::build_implementation_lens( + uri, + &index, + method.name_offset, + count, + class_fqn, + Some(method.name), + )); + } + } if let Some(proto) = proto { let icon = if proto.is_interface { "◆" } else { "↑" }; let title = format!("{} {}::{}", icon, proto.ancestor_name, method.name); @@ -361,17 +402,42 @@ impl Backend { origin_uri: Url, origin_position: Position, locations: Vec, + ) -> Command { + Self::locations_lens_command( + origin_uri, + origin_position, + locations, + "reference", + "references", + ) + } + + fn implementation_lens_command( + origin_uri: Url, + origin_position: Position, + locations: Vec, + ) -> Command { + Self::locations_lens_command( + origin_uri, + origin_position, + locations, + "implementation", + "implementations", + ) + } + + /// A lens command titled with the number of `locations`, which opens + /// them as a list when clicked. + fn locations_lens_command( + origin_uri: Url, + origin_position: Position, + locations: Vec, + singular: &str, + plural: &str, ) -> Command { let count = locations.len(); Command { - title: format!( - "{count} {}", - if count == 1 { - "reference" - } else { - "references" - } - ), + title: format!("{count} {}", if count == 1 { singular } else { plural }), command: "editor.action.showReferences".to_string(), arguments: Some(vec![ serde_json::json!(origin_uri), @@ -381,6 +447,106 @@ impl Backend { } } + /// The classes the implementation lenses of `class` count, or `None` + /// when it gets none: only an interface or an abstract class does, and + /// only once the workspace index can answer. + fn implementation_lens_descendants( + &self, + class: &ClassInfo, + class_loader: &dyn Fn(&str) -> Option>, + ) -> Option>> { + if !(class.kind == ClassLikeKind::Interface || class.is_abstract) + || class.keyword_offset == 0 + || !self + .workspace_indexed + .load(std::sync::atomic::Ordering::Acquire) + { + return None; + } + Some(self.implementation_descendants(class, &class.fqn(), class_loader, true)) + } + + /// Build the implementation lens for a class, or for one of its + /// methods when `member` is given. + /// + /// `count` is worked out from class metadata alone. Locating each + /// implementation means reading the file that declares it, so a + /// non-zero lens leaves that to the resolve request, which only the + /// lenses the editor actually shows receive. + fn build_implementation_lens( + origin_uri: &str, + index: &LineIndex, + declaration_offset: u32, + count: usize, + class_fqn: Atom, + member: Option, + ) -> Option { + if declaration_offset == 0 { + return None; + } + let position = index.position(declaration_offset as usize); + let range = Range::new( + Position::new(position.line, 0), + Position::new(position.line, 0), + ); + if count == 0 { + return Some(CodeLens { + range, + command: Some(Self::implementation_lens_command( + Url::parse(origin_uri).ok()?, + position, + Vec::new(), + )), + data: None, + }); + } + + Some(CodeLens { + range, + command: None, + data: Some(serde_json::json!({ + "kind": "phpImplementations", + "uri": origin_uri, + "position": position, + "classFqn": class_fqn.as_str(), + "member": member.as_ref().map(Atom::as_str), + })), + }) + } + + /// The locations an implementation lens lists, recomputed from the + /// class and member it was built for. + fn implementation_lens_locations( + &self, + uri: &str, + content: &str, + class_fqn: &str, + member: Option<&str>, + ) -> Option> { + let class_loader = |name: &str| self.find_or_load_class(name); + let class = class_loader(class_fqn)?; + let descendants = self.implementation_descendants(&class, class_fqn, &class_loader, true); + Some(match member { + None => self.class_implementation_locations(uri, content, &class, &descendants), + Some(member) => { + let providers = member_implementation_providers( + class_fqn, + member, + MemberKind::Method, + &descendants, + &class_loader, + ); + self.member_implementation_locations( + uri, + content, + member, + MemberKind::Method, + &providers, + ) + } + }) + } + pub fn resolve_code_lens_item(&self, mut lens: CodeLens) -> CodeLens { if lens.command.is_some() { return lens; @@ -401,10 +567,10 @@ impl Backend { else { return lens; }; - // Both kinds resolve references, which needs the type engine, the - // chain cache and a parse of the file. Going through the shared - // request helper installs all of them (and the panic guard) once, - // and hands over the buffer without copying it. + // Resolving references needs the type engine, the chain cache and + // a parse of the file. Going through the shared request helper + // installs all of them (and the panic guard) once, and hands over + // the buffer without copying it. let locations = self.with_file_content("codeLens/resolve", uri, None, |content, _| match kind { "phpReferences" => self.find_references(uri, content, position, false), @@ -424,6 +590,11 @@ impl Backend { is_static, )) } + "phpImplementations" => { + let class_fqn = data.get("classFqn").and_then(serde_json::Value::as_str)?; + let member = data.get("member").and_then(serde_json::Value::as_str); + self.implementation_lens_locations(uri, content, class_fqn, member) + } _ => None, }); let Some(locations) = locations.flatten() else { @@ -433,9 +604,11 @@ impl Backend { return lens; }; - lens.command = Some(Self::reference_lens_command( - origin_uri, position, locations, - )); + lens.command = Some(if kind == "phpImplementations" { + Self::implementation_lens_command(origin_uri, position, locations) + } else { + Self::reference_lens_command(origin_uri, position, locations) + }); lens } diff --git a/src/definition/implementation.rs b/src/definition/implementation.rs index 1d45dec16..3004c0d5d 100644 --- a/src/definition/implementation.rs +++ b/src/definition/implementation.rs @@ -354,50 +354,47 @@ impl Backend { MemberKind::Constant }; - let locations = self.member_implementation_locations( - uri, - content, + let providers = member_implementation_providers( &target_fqn, member_name, member_kind, &descendants, class_loader, ); + let locations = self.member_implementation_locations( + uri, + content, + member_name, + member_kind, + &providers, + ); (!locations.is_empty()).then_some(locations) } - /// The locations of the declarations that implement `member_name` for - /// the `descendants` of the class `target_fqn`. + /// The locations of `member_name` in each of the `providers` that + /// [`member_implementation_providers`] found. pub(crate) fn member_implementation_locations( &self, uri: &str, content: &str, - target_fqn: &str, member_name: &str, member_kind: MemberKind, - descendants: &[Arc], - class_loader: &dyn Fn(&str) -> Option>, + providers: &[Arc], ) -> Vec { - let mut locations: Vec = member_implementation_providers( - target_fqn, - member_name, - member_kind, - descendants, - class_loader, - ) - .into_iter() - .filter_map(|provider| { - let (class_uri, class_content) = - self.find_class_file_content(&provider.fqn(), uri, content)?; - let member_pos = Self::find_member_position_in_class( - &class_content, - member_name, - member_kind, - &provider, - )?; - Some(point_location(Url::parse(&class_uri).ok()?, member_pos)) - }) - .collect(); + let mut locations: Vec = providers + .iter() + .filter_map(|provider| { + let (class_uri, class_content) = + self.find_class_file_content(&provider.fqn(), uri, content)?; + let member_pos = Self::find_member_position_in_class( + &class_content, + member_name, + member_kind, + provider, + )?; + Some(point_location(Url::parse(&class_uri).ok()?, member_pos)) + }) + .collect(); sort_and_dedup_locations(&mut locations); locations } @@ -486,14 +483,19 @@ impl Backend { let target_fqn = self.implementor_target_fqn(candidate); let descendants = self.implementation_descendants(candidate, &target_fqn, &class_loader, false); - all_locations.extend(self.member_implementation_locations( - uri, - content, + let providers = member_implementation_providers( &target_fqn, member_name, member_kind, &descendants, &class_loader, + ); + all_locations.extend(self.member_implementation_locations( + uri, + content, + member_name, + member_kind, + &providers, )); } diff --git a/src/definition/mod.rs b/src/definition/mod.rs index 9e584a8dd..0d41e296d 100644 --- a/src/definition/mod.rs +++ b/src/definition/mod.rs @@ -54,7 +54,7 @@ use tower_lsp::lsp_types::{Location, Position, Range, Url}; mod blade_component; -mod implementation; +pub(crate) mod implementation; pub(crate) mod member; mod resolve; mod type_definition; diff --git a/src/inlay_hints.rs b/src/inlay_hints.rs index ccd25afda..a641ab21f 100644 --- a/src/inlay_hints.rs +++ b/src/inlay_hints.rs @@ -7,23 +7,19 @@ //! parameters when the type can be inferred from the callable context. //! - **Closure return type hints** for closures/arrow functions without an //! explicit return type when the callable context specifies one. -//! - **Implementation counts** beside every interface and abstract class the -//! file declares. Reference counts are the CodeLens' job, which is -//! clickable; this is the one declaration annotation with no lens behind it. //! //! The handler walks precomputed [`CallSite`] entries from the //! [`SymbolMap`] within the requested viewport range, resolves each //! callable to obtain parameter metadata, and emits [`InlayHint`] //! entries for arguments that would benefit from a label. -use std::sync::atomic::Ordering; use tower_lsp::jsonrpc; use tower_lsp::lsp_types::*; use crate::Backend; use crate::symbol_map::{CallSite, UntypedClosureSite}; use crate::text_position::{LineIndex, position_to_offset}; -use crate::types::{ClassLikeKind, FileContext}; +use crate::types::FileContext; impl Backend { /// Entry point for the `textDocument/inlayHint` request. @@ -109,14 +105,6 @@ impl Backend { ); } - self.emit_implementation_count_hints( - uri, - &index, - &ctx, - (range_start, range_end), - &mut hints, - ); - // Translate hints back to Blade if needed. A hint anchored in the // injected prologue has no template text to attach to. if self.is_blade_file(uri) { @@ -134,53 +122,6 @@ impl Backend { Some(hints) } - /// Emit the number of implementations beside every interface and - /// abstract class the file declares. - /// - /// The reference count next to a declaration is a CodeLens, which can - /// be clicked to list what it counted; this is the one declaration - /// annotation with no lens behind it. - fn emit_implementation_count_hints( - &self, - uri: &str, - index: &LineIndex, - ctx: &FileContext, - range: (u32, u32), - hints: &mut Vec, - ) { - if !self.workspace_indexed.load(Ordering::Acquire) { - return; - } - - let Some(classes) = self.symbols.uri_classes_index.read().get(uri).cloned() else { - return; - }; - let class_loaders = self.class_loaders(ctx); - - for class in &classes { - if class.keyword_offset == 0 - || !offset_in_range(class.keyword_offset, range) - || !(class.kind == ClassLikeKind::Interface || class.is_abstract) - { - continue; - } - - let implementors = self.find_implementors( - &class.name, - &class.fqn(), - class_loaders.at(class.keyword_offset), - false, - false, - true, - ); - push_count_hint( - hints, - line_end_position(index, class.keyword_offset as usize), - implementation_label(implementors.len()), - ); - } - } - /// Emit parameter-name and by-reference hints for a single call site. /// /// `range` is the requested viewport as byte offsets, already @@ -458,44 +399,6 @@ impl Backend { } } -fn push_count_hint(hints: &mut Vec, position: Position, label: String) { - hints.push(InlayHint { - position, - label: InlayHintLabel::String(format!(" {label}")), - kind: None, - text_edits: None, - tooltip: None, - padding_left: None, - padding_right: None, - data: None, - }); -} - -fn implementation_label(count: usize) -> String { - if count == 1 { - "1 implementation".to_string() - } else { - format!("{count} implementations") - } -} - -fn offset_in_range(offset: u32, range: (u32, u32)) -> bool { - offset >= range.0 && offset <= range.1 -} - -fn line_end_position(index: &LineIndex, byte_offset: usize) -> Position { - let content = index.content(); - let line_end = content[byte_offset..] - .find('\n') - .map(|i| byte_offset + i) - .unwrap_or(content.len()); - - // Delegate to the canonical converter so the `character` column is - // counted in UTF-16 code units (per the LSP spec), consistent with - // every other position the server emits. - index.position(line_end) -} - /// Check whether the argument at `arg_offset` is a simple variable whose /// name (without `$`) matches the parameter name, making a hint redundant. /// @@ -753,92 +656,6 @@ fn is_obvious_single_param(call_expression: &str, _param_name: &str) -> bool { mod tests { use super::*; - /// Open a file and return the hints it carries. - fn declaration_hints(backend: &Backend, uri: &str, content: &str) -> Vec { - backend - .open_files - .write() - .insert(uri.to_string(), std::sync::Arc::new(content.to_string())); - backend.update_ast(uri, content); - backend.workspace_indexed.store(true, Ordering::Release); - - let range = Range { - start: Position { - line: 0, - character: 0, - }, - end: Position { - line: content.lines().count() as u32, - character: 0, - }, - }; - backend - .handle_inlay_hints(uri, content, range) - .unwrap_or_default() - } - - /// The label of the hint on `line`, if any. - fn hint_on_line(hints: &[InlayHint], line: u32) -> Option { - hints - .iter() - .find(|hint| hint.position.line == line) - .map(|hint| match &hint.label { - InlayHintLabel::String(label) => label.clone(), - InlayHintLabel::LabelParts(parts) => { - parts.iter().map(|part| part.value.as_str()).collect() - } - }) - } - - #[test] - fn an_interface_counts_the_classes_that_implement_it() { - let backend = Backend::new_test(); - backend.update_ast( - "file:///Pen.php", - " Option<(String, Vec)> { + let lens = lenses.iter().find(|lens| { + lens.range.start.line == line + && match &lens.command { + Some(command) => command.title.contains("implementation"), + None => { + lens.data.as_ref().and_then(|data| data.get("kind")) + == Some(&serde_json::json!("phpImplementations")) + } + } + })?; + let command = backend + .code_lens_resolve(lens.clone()) + .await + .unwrap() + .command?; + assert_eq!(command.command, "editor.action.showReferences"); + let locations: Vec = + serde_json::from_value(command.arguments?.get(2)?.clone()).ok()?; + Some(( + command.title, + locations + .iter() + .map(|location| location.range.start.line) + .collect(), + )) +} + +async fn indexed_lenses(content: &str) -> (phpantom_lsp::Backend, Vec) { + let (backend, dir) = create_psr4_workspace( + r#"{ "autoload": { "psr-4": { "App\\": "src/" } } }"#, + &[("src/Shapes.php", content)], + ); + let uri = Url::from_file_path(dir.path().join("src/Shapes.php")).unwrap(); + open_php(&backend, &uri, content).await; + warm_workspace_index(&backend, &uri, Position::new(2, 12)).await; + let lenses = backend + .code_lens(CodeLensParams { + text_document: TextDocumentIdentifier { uri }, + work_done_progress_params: WorkDoneProgressParams::default(), + partial_result_params: PartialResultParams::default(), + }) + .await + .unwrap() + .unwrap_or_default(); + (backend, lenses) +} + +#[tokio::test] +async fn implementation_lenses_list_every_implementation_of_an_interface() { + let content = r#"