Fixed go to usages of method goes to parent instead of own usages

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.

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<Arc<ClassInfo>>,
-}
-
 impl Backend {
     /// Handle a "go to definition" request.
     ///
@@ -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<MemberKind> {
-        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<Arc<ClassInfo>>,
-    ) -> Option<Location> {
-        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<Location> {
-        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<Location> {
-        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<Location> {
-        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<Location> {
-        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.
     ///

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!(
+                    "<?php\n",
+                    "namespace App;\n",
+                    "interface LoggerInterface {\n",
+                    "    public function log(string $message): void;\n",
+                    "}\n",
+                ),
+            ),
+            (
+                "src/FileLogger.php",
+                concat!(
+                    "<?php\n",
+                    "namespace App;\n",
+                    "class FileLogger implements LoggerInterface {\n",
+                    "    public function log(string $message): void {}\n",
+                    "}\n",
+                ),
+            ),
+        ],
+    );
+
+    let logger_path = dir.path().join("src/FileLogger.php");
+    let logger_uri = Url::from_file_path(&logger_path).unwrap();
+    let logger_content = std::fs::read_to_string(&logger_path).unwrap();
+
+    backend
+        .did_open(DidOpenTextDocumentParams {
+            text_document: TextDocumentItem {
+                uri: logger_uri.clone(),
+                language_id: "php".to_string(),
+                version: 1,
+                text: logger_content,
+            },
+        })
+        .await;
+
+    // Click on "log" in `    public function log(` on line 3 (0-indexed).
+    // "    public function " = 20 chars, so `log` starts at character 20.
+    let params = GotoDefinitionParams {
+        text_document_position_params: TextDocumentPositionParams {
+            text_document: TextDocumentIdentifier {
+                uri: logger_uri.clone(),
+            },
+            position: Position {
+                line: 3,
+                character: 20,
+            },
+        },
+        work_done_progress_params: WorkDoneProgressParams::default(),
+        partial_result_params: PartialResultParams::default(),
+    };
+
+    let result = backend.goto_definition(params).await.unwrap();
+    let locations = match result {
+        Some(GotoDefinitionResponse::Array(locs)) => 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"
+    );
+}