From 40936b567821e4a8ca75c8456cc6d8a7dc3055e8 Mon Sep 17 00:00:00 2001 From: EranNL Date: Tue, 1 Sep 2026 22:26:51 +0200 Subject: [PATCH 1/4] fix: go-to-usages on implemented methods do not go to their parent anymore --- docs/CHANGELOG.md | 1 + src/definition/resolve.rs | 261 ++---------------------- tests/integration/definition_members.rs | 85 ++++++++ 3 files changed, 100 insertions(+), 247 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index fb39f163b..66efc16cd 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -38,6 +38,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Go-to-definition on a method name at its own declaration no longer jumps to the implemented interface.** Invoking go-to-definition on a method declaration in a class that implements an interface navigated to the interface method instead of returning the concrete method's own location, which made "Declaration or Usages" (PHPStorm's CMD+B) unable to show where the concrete method is used. The declaration now answers with its own location so editors offer Find Usages; the `implements` clause and the `Go to Implementation` command remain the routes to the interface. Closes #412. - **`phpantom_lsp fix` runs its workers on the same stack as `analyze`.** The parse and fix workers were spawned with the 2 MB default a thread gets, where `analyze` gives its workers the 8 MB the recursive parser and type walker need, so a project holding a deeply nested file could crash `fix` outright where `analyze` completed. Both commands now share one parse phase, and `fix` also runs the Laravel discovery `analyze` does before fixing, so the two see the same project. - **A formatting edit measures the last line in UTF-16 units.** The whole-document replacement a formatter produces ended at a column counted in bytes, so a file whose unterminated last line held multibyte text was sent an end position past that line. - **Pint reads the project's `pint.json`.** Pint looks for its configuration in the directory it is started from, and it was started in the language server's own directory, so an editor that launches the server from a subdirectory or a multi-root workspace had Blade and PHP files formatted with Pint's default `laravel` preset rather than the project's. Pint, php-cs-fixer, and phpcbf now run with the workspace root as their working directory. diff --git a/src/definition/resolve.rs b/src/definition/resolve.rs index 2ade50c39..d3e8284ac 100644 --- a/src/definition/resolve.rs +++ b/src/definition/resolve.rs @@ -26,18 +26,10 @@ use crate::class_lookup::find_class_at_offset; use crate::composer; use crate::symbol_map::{SelfStaticParentKind, SymbolKind}; use crate::text_position::position_to_offset; -use crate::types::{AccessKind, ClassInfo, MAX_INHERITANCE_DEPTH}; +use crate::types::{AccessKind, ClassInfo}; use crate::util::short_name; use crate::virtual_members::laravel; -struct MemberPrototypeSearch<'a> { - member_name: &'a str, - kind: MemberKind, - uri: &'a str, - content: &'a str, - class_loader: &'a dyn Fn(&str) -> Option>, -} - impl Backend { /// Handle a "go to definition" request. /// @@ -55,7 +47,7 @@ impl Backend { let symbol = self.lookup_symbol_at_position(uri, content, position); if let Some(ref s) = symbol && let Some(resolved) = - self.resolve_from_symbol(&s.kind, uri, content, position, s.start) + self.resolve_from_symbol(&s.kind, uri, content, position, s.start) { return resolved; } @@ -63,7 +55,7 @@ impl Backend { // Laravel config fallback: declaration sites in config/*.php if self.resolved_class_cache.read().is_laravel() && let Some(loc) = - laravel::resolve_config_key_definition_fallback(self, uri, content, position) + laravel::resolve_config_key_definition_fallback(self, uri, content, position) { return vec![loc]; } @@ -316,35 +308,14 @@ impl Backend { .resolve_class_reference(uri, content, name, *is_fqn, cursor_offset) .map(|loc| vec![loc]), - SymbolKind::MemberDeclaration { name, is_static } => { - // If this method/property overrides a parent or implements - // an interface member, jump to the prototype declaration. - let ctx = self.file_context(uri); - let class_loader = self.class_loader(&ctx); - let current_class = - crate::class_lookup::find_class_at_offset(&ctx.classes, cursor_offset); - if let Some(cls) = current_class - && let Some(kind) = self.infer_member_declaration_kind(cls, name, *is_static) - && let Some(loc) = self.resolve_member_declaration_prototype( - uri, - content, - cls, - name, - kind, - &class_loader, - ) - { - return Some(vec![loc]); - } - - if let Some(cls) = current_class - && let Some(locs) = - self.resolve_reverse_implementation(uri, content, cls, name, &class_loader) - && !locs.is_empty() - { - return Some(locs); - } - + SymbolKind::MemberDeclaration { name, .. } => { + // Return self-location so editors detect "definition == + // cursor" and offer Find Usages instead of navigating. + // Navigating to the interface or abstract prototype from a + // declaration site makes the concrete method's usages + // unreachable; the `implements`/`extends` clause and the + // `textDocument/implementation` command handle prototype + // navigation. self.declaration_or_usages(uri, content, cursor_offset, name) } @@ -453,210 +424,6 @@ impl Backend { } } - fn infer_member_declaration_kind( - &self, - class: &ClassInfo, - member_name: &str, - is_static: bool, - ) -> Option { - if is_static - && class - .constants - .iter() - .any(|c| c.name == member_name && c.visibility != crate::types::Visibility::Private) - { - return Some(MemberKind::Constant); - } - - if class.methods.iter().any(|m| { - m.name == member_name - && m.is_static == is_static - && !m.is_virtual - && m.visibility != crate::types::Visibility::Private - }) { - return Some(MemberKind::Method); - } - - if class.properties.iter().any(|p| { - p.name == member_name - && p.is_static == is_static - && !p.is_virtual - && p.visibility != crate::types::Visibility::Private - }) { - return Some(MemberKind::Property); - } - - None - } - - fn resolve_member_declaration_prototype( - &self, - uri: &str, - content: &str, - class: &ClassInfo, - member_name: &str, - kind: MemberKind, - class_loader: &dyn Fn(&str) -> Option>, - ) -> Option { - let search = MemberPrototypeSearch { - member_name, - kind, - uri, - content, - class_loader, - }; - - if let Some(loc) = self.find_member_prototype_in_traits(&class.used_traits, &search, 0) { - return Some(loc); - } - - let mut current = class.clone(); - for _ in 0..MAX_INHERITANCE_DEPTH { - let Some(parent_name) = current.parent_class else { - break; - }; - let Some(parent) = class_loader(&parent_name).map(Arc::unwrap_or_clone) else { - break; - }; - - if self.class_declares_member(&parent, &search) - && let Some(loc) = self.member_location(&parent_name, &parent, &search) - { - return Some(loc); - } - - if let Some(loc) = self.find_member_prototype_in_traits(&parent.used_traits, &search, 0) - { - return Some(loc); - } - - current = parent; - } - - if matches!(search.kind, MemberKind::Method | MemberKind::Constant) { - return self.find_member_prototype_in_interfaces(class, &search); - } - - None - } - - fn find_member_prototype_in_traits( - &self, - trait_names: &[crate::atom::Atom], - search: &MemberPrototypeSearch<'_>, - depth: usize, - ) -> Option { - if depth > MAX_INHERITANCE_DEPTH as usize { - return None; - } - - for trait_name in trait_names { - let Some(trait_info) = (search.class_loader)(trait_name).map(Arc::unwrap_or_clone) - else { - continue; - }; - if self.class_declares_member(&trait_info, search) - && let Some(loc) = self.member_location(trait_name, &trait_info, search) - { - return Some(loc); - } - if let Some(loc) = - self.find_member_prototype_in_traits(&trait_info.used_traits, search, depth + 1) - { - return Some(loc); - } - } - - None - } - - fn find_member_prototype_in_interfaces( - &self, - class: &ClassInfo, - search: &MemberPrototypeSearch<'_>, - ) -> Option { - let mut current = Some(class.clone()); - for _ in 0..MAX_INHERITANCE_DEPTH { - let cls = current?; - for iface_name in &cls.interfaces { - if let Some(loc) = self.find_member_prototype_in_interface(iface_name, search, 0) { - return Some(loc); - } - } - current = cls - .parent_class - .as_deref() - .and_then(|parent| (search.class_loader)(parent).map(Arc::unwrap_or_clone)); - } - - None - } - - fn find_member_prototype_in_interface( - &self, - iface_name: &str, - search: &MemberPrototypeSearch<'_>, - depth: usize, - ) -> Option { - if depth > MAX_INHERITANCE_DEPTH as usize { - return None; - } - let iface = (search.class_loader)(iface_name).map(Arc::unwrap_or_clone)?; - if self.class_declares_member(&iface, search) - && let Some(loc) = self.member_location(iface_name, &iface, search) - { - return Some(loc); - } - - for parent in &iface.interfaces { - if let Some(loc) = self.find_member_prototype_in_interface(parent, search, depth + 1) { - return Some(loc); - } - } - - if let Some(parent) = iface.parent_class - && let Some(loc) = self.find_member_prototype_in_interface(&parent, search, depth + 1) - { - return Some(loc); - } - - None - } - - fn class_declares_member(&self, class: &ClassInfo, search: &MemberPrototypeSearch<'_>) -> bool { - match search.kind { - MemberKind::Method => class.methods.iter().any(|m| { - m.name == search.member_name - && !m.is_virtual - && m.visibility != crate::types::Visibility::Private - }), - MemberKind::Property => class.properties.iter().any(|p| { - p.name == search.member_name - && !p.is_virtual - && p.visibility != crate::types::Visibility::Private - }), - MemberKind::Constant => class.constants.iter().any(|c| { - c.name == search.member_name && c.visibility != crate::types::Visibility::Private - }), - } - } - - fn member_location( - &self, - class_name: &str, - class: &ClassInfo, - search: &MemberPrototypeSearch<'_>, - ) -> Option { - let offset = class.member_name_offset(search.member_name, search.kind.as_str())?; - let (target_uri, target_content) = - self.find_class_file_content(class_name, search.uri, search.content)?; - let parsed_uri = Url::parse(&target_uri).ok()?; - Some(point_location( - parsed_uri, - crate::text_position::offset_to_position(&target_content, offset as usize), - )) - } - /// Return the declaration's own location for a symbol that has nowhere /// else to jump to. /// @@ -1175,7 +942,7 @@ impl Backend { // overridden class definition instead of the lexical class. if ssp_kind == SelfStaticParentKind::This && let Some(override_cls) = - self.resolve_closure_this_override(uri, content, cursor_offset) + self.resolve_closure_this_override(uri, content, cursor_offset) { let fqn = override_cls.fqn(); if let Some(loc) = @@ -1239,8 +1006,8 @@ impl Backend { for candidate in &candidates { if let Some(file_uri) = self.symbols.fqn_uri_index.read().get(candidate).cloned() && let Some(file_path) = Url::parse(&file_uri) - .ok() - .and_then(|u| u.to_file_path().ok()) + .ok() + .and_then(|u| u.to_file_path().ok()) && let Some(location) = self.resolve_class_in_file(&file_path, candidate) { return Some(location); diff --git a/tests/integration/definition_members.rs b/tests/integration/definition_members.rs index 1216ad062..0f9747da0 100644 --- a/tests/integration/definition_members.rs +++ b/tests/integration/definition_members.rs @@ -5811,3 +5811,88 @@ async fn definition_of_a_plain_parent_property_named_by_a_hook_call() { other => panic!("Expected Scalar location, got: {:?}", other), } } + +/// Regression for github #412: Ctrl+Click on a method name at its own +/// declaration site in a class that implements an interface must return the +/// concrete method's own location, not the interface declaration. +/// Editors detect "definition == cursor position" as the cue to show +/// usages; jumping to the interface makes the concrete method's usages +/// unreachable. The `implements` clause is the place that navigates to +/// the interface. +#[tokio::test] +async fn test_goto_definition_implements_method_declaration_returns_self_location() { + let (backend, dir) = create_psr4_workspace( + r#"{ + "autoload": { "psr-4": { "App\\": "src/" } } + }"#, + &[ + ( + "src/LoggerInterface.php", + concat!( + " locs, + Some(GotoDefinitionResponse::Scalar(loc)) => vec![loc], + other => panic!("Expected self-location, got: {other:?}"), + }; + assert_eq!(locations.len(), 1, "should return exactly one location"); + assert_eq!( + locations[0].uri, logger_uri, + "should return the concrete method's own location, not the interface declaration" + ); + assert_eq!( + locations[0].range.start.line, 3, + "should point back to the concrete method declaration line" + ); +} \ No newline at end of file From 29687a9e207405cf4b71180b04923feb2c577ed0 Mon Sep 17 00:00:00 2001 From: EranNL Date: Wed, 2 Sep 2026 21:00:21 +0200 Subject: [PATCH 2/4] Formatting --- src/definition/resolve.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/definition/resolve.rs b/src/definition/resolve.rs index d3e8284ac..3d7a6970e 100644 --- a/src/definition/resolve.rs +++ b/src/definition/resolve.rs @@ -47,7 +47,7 @@ impl Backend { let symbol = self.lookup_symbol_at_position(uri, content, position); if let Some(ref s) = symbol && let Some(resolved) = - self.resolve_from_symbol(&s.kind, uri, content, position, s.start) + self.resolve_from_symbol(&s.kind, uri, content, position, s.start) { return resolved; } @@ -55,7 +55,7 @@ impl Backend { // Laravel config fallback: declaration sites in config/*.php if self.resolved_class_cache.read().is_laravel() && let Some(loc) = - laravel::resolve_config_key_definition_fallback(self, uri, content, position) + laravel::resolve_config_key_definition_fallback(self, uri, content, position) { return vec![loc]; } @@ -942,7 +942,7 @@ impl Backend { // overridden class definition instead of the lexical class. if ssp_kind == SelfStaticParentKind::This && let Some(override_cls) = - self.resolve_closure_this_override(uri, content, cursor_offset) + self.resolve_closure_this_override(uri, content, cursor_offset) { let fqn = override_cls.fqn(); if let Some(loc) = @@ -1006,8 +1006,8 @@ impl Backend { for candidate in &candidates { if let Some(file_uri) = self.symbols.fqn_uri_index.read().get(candidate).cloned() && let Some(file_path) = Url::parse(&file_uri) - .ok() - .and_then(|u| u.to_file_path().ok()) + .ok() + .and_then(|u| u.to_file_path().ok()) && let Some(location) = self.resolve_class_in_file(&file_path, candidate) { return Some(location); From 4e8ad9c0b97b005e75cd345d8bd1245243df937c Mon Sep 17 00:00:00 2001 From: EranNL Date: Wed, 2 Sep 2026 21:02:20 +0200 Subject: [PATCH 3/4] Formatting --- tests/integration/definition_members.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/definition_members.rs b/tests/integration/definition_members.rs index 0f9747da0..55b88cdbc 100644 --- a/tests/integration/definition_members.rs +++ b/tests/integration/definition_members.rs @@ -5895,4 +5895,4 @@ async fn test_goto_definition_implements_method_declaration_returns_self_locatio locations[0].range.start.line, 3, "should point back to the concrete method declaration line" ); -} \ No newline at end of file +} From 17345245c88ec1ac16e9ca3ce795bbfd531c4069 Mon Sep 17 00:00:00 2001 From: EranNL Date: Wed, 2 Sep 2026 21:55:19 +0200 Subject: [PATCH 4/4] Formatting --- tests/integration/definition_members.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/integration/definition_members.rs b/tests/integration/definition_members.rs index 55b88cdbc..99c2ce742 100644 --- a/tests/integration/definition_members.rs +++ b/tests/integration/definition_members.rs @@ -5829,21 +5829,21 @@ async fn test_goto_definition_implements_method_declaration_returns_self_locatio ( "src/LoggerInterface.php", concat!( - "