fix: go-to-definition on overrides jumps to parent declaration

When the cursor is on a method definition that overrides a parent or
implements an interface method, go-to-definition now navigates to the
prototype declaration. Previously it returned call-site references
(find_references), which sent the user to a seemingly random usage.

Similarly, go-to-definition on a class declaration name now jumps to
the parent class when one exists, instead of returning usages.

The reverse-implementation lookup (already used by
textDocument/implementation) is now reused by the definition handler
for MemberDeclaration symbols. Methods and classes that do not
override anything still fall through to the usages behavior.

Closes #236

Author: calebdw

docs/CHANGELOG.md

@@ -41,6 +41,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **Go-to-definition on overriding methods jumps to the parent declaration.** When the cursor is on a method definition that overrides a parent or implements an interface method, go-to-definition now navigates to the prototype declaration instead of returning call-site references. Similarly, go-to-definition on a class name jumps to the parent class when one exists. Methods and classes that don't override anything still show usages as before. Contributed by @calebdw.
+- **Code lens navigation works on Cursor, VSCodium, Neovim, and other editors.** Clicking a code lens annotation (e.g. "overrides Parent::method") now uses the standard `window/showDocument` LSP request instead of editor-specific commands (`vscode.open`, `editor.action.showReferences`) that only worked on VS Code. Contributed by @calebdw.
 - **Go-to-implementation works when interface and class share the same short name.** An interface and its implementing class in different namespaces but with the same class name (e.g. `App\Contracts\HttpClient` and `App\Foo\HttpClient`) now resolves correctly instead of returning no results. Contributed by @calebdw.
 - **Static method calls resolve return types as accurately as instance calls.** `Foo::bar()` previously missed inference that `$foo->bar()` already had: return types behind a `@phpstan-type` alias, inherited return types substituted through a generic interface or trait, and the `__callStatic()` magic-method fallback. These now resolve the same way for both call styles.
 - **Facade static calls keep concrete method return types.** Static calls on Laravel-style facades now resolve missing methods through `getFacadeAccessor()` and facade `@mixin` targets before falling back to `__callStatic()`, so values like `Driver::details()` keep the concrete provider method return type instead of degrading to the facade's broad magic-call return. Contributed by @calebdw.

src/code_lens.rs

@@ -1,8 +1,6 @@
 //! Code Lens (`textDocument/codeLens`) support.
 //!
-//! Shows clickable annotations above methods that override a parent
-//! method or implement an interface method, linking to the prototype
-//! declaration.
+//! Shows override/implement annotations linking to the prototype declaration.
 
 use tower_lsp::lsp_types::*;
 
@@ -11,6 +9,17 @@ use crate::definition::member::MemberKind;
 use crate::text_position::offset_to_position;
 use crate::types::{ClassInfo, ClassLikeKind, MAX_INHERITANCE_DEPTH};
 
+fn line_indent(content: &str, byte_offset: usize) -> u32 {
+    let line_start = content[..byte_offset]
+        .rfind('\n')
+        .map(|i| i + 1)
+        .unwrap_or(0);
+    content[line_start..]
+        .chars()
+        .take_while(|c| *c == ' ' || *c == '\t')
+        .count() as u32
+}
+
 /// Information about a prototype (ancestor) method that a local method
 /// overrides or implements.
 struct Prototype {
@@ -41,32 +50,28 @@ impl Backend {
             let class_fqn = class.fqn();
 
             for method in &class.methods {
-                // Skip synthetic/stub methods with no real source position.
-                if method.name_offset == 0 {
-                    continue;
-                }
-
-                // Skip virtual methods (injected via @method tags, not
-                // actually declared in source).
-                if method.is_virtual {
+                if method.name_offset == 0
+                    || method.is_virtual
+                    || method.visibility == crate::types::Visibility::Private
+                {
                     continue;
                 }
 
-                if let Some(proto) =
-                    self.find_prototype(class, &class_fqn, &method.name, uri, content)
-                {
-                    let pos = offset_to_position(content, method.name_offset as usize);
-                    let range = Range {
-                        start: Position {
-                            line: pos.line,
-                            character: 0,
-                        },
-                        end: Position {
-                            line: pos.line,
-                            character: 0,
-                        },
-                    };
-
+                let pos = offset_to_position(content, method.name_offset as usize);
+                let indent = line_indent(content, method.name_offset as usize);
+                let range = Range {
+                    start: Position {
+                        line: pos.line,
+                        character: indent,
+                    },
+                    end: Position {
+                        line: pos.line,
+                        character: indent,
+                    },
+                };
+
+                let proto = self.find_prototype(class, &class_fqn, &method.name, uri, content);
+                if let Some(proto) = proto {
                     let icon = if proto.is_interface { "◆" } else { "↑" };
                     let title = format!("{} {}::{}", icon, proto.ancestor_name, method.name);
 
@@ -307,39 +312,14 @@ impl Backend {
     /// Build the LSP `Command` for a code lens that navigates to a target
     /// location.
     ///
-    /// Uses `editor.action.showReferences` (widely supported) by default,
-    /// and `vscode.open` when connected to a VS Code client.
+    /// Returns a custom command handled by `workspace/executeCommand`,
+    /// which uses `window/showDocument` to navigate.  This works across
+    /// all LSP clients without editor-specific command sniffing.
     fn build_code_lens_command(&self, title: String, uri: Url, position: Position) -> Command {
-        let client = self.client_name.lock();
-        if client.contains("Visual Studio Code") {
-            // VS Code: use vscode.open with a fragment for direct navigation.
-            let fragment = format!("L{},{}", position.line + 1, position.character + 1);
-            let mut target_uri = uri;
-            target_uri.set_fragment(Some(&fragment));
-            Command {
-                title,
-                command: "vscode.open".to_string(),
-                arguments: Some(vec![serde_json::json!(target_uri)]),
-            }
-        } else {
-            // All other editors: use editor.action.showReferences which is
-            // handled by most LSP clients (Zed, Neovim, Emacs, etc.).
-            let location = Location {
-                uri: uri.clone(),
-                range: Range {
-                    start: position,
-                    end: position,
-                },
-            };
-            Command {
-                title,
-                command: "editor.action.showReferences".to_string(),
-                arguments: Some(vec![
-                    serde_json::json!(uri),
-                    serde_json::json!(position),
-                    serde_json::json!([location]),
-                ]),
-            }
+        Command {
+            title,
+            command: "phpantom.navigateToPrototype".to_string(),
+            arguments: Some(vec![serde_json::json!(uri), serde_json::json!(position)]),
         }
     }
 

src/definition/implementation.rs

@@ -211,7 +211,7 @@ impl Backend {
     /// `public function handle()` in a class that implements `Handler`),
     /// this finds the interface/abstract method declaration and returns
     /// its location.
-    fn resolve_reverse_implementation(
+    pub(super) fn resolve_reverse_implementation(
         &self,
         uri: &str,
         content: &str,

src/definition/resolve.rs

@@ -266,41 +266,48 @@ impl Backend {
                 .resolve_class_reference(uri, content, name, *is_fqn, cursor_offset)
                 .map(|loc| vec![loc]),
 
-            SymbolKind::ClassDeclaration { name }
-            | SymbolKind::MemberDeclaration { name, .. }
-            | SymbolKind::NamespaceDeclaration { name } => {
-                // The cursor is on a declaration name, so "go to
-                // definition" has nowhere to jump — we are already at the
-                // definition.  Implement "Declaration or Usages" by
-                // returning the symbol's usages instead.
-                //
-                // VS Code achieves this by detecting "definition ==
-                // current position" and running Find References itself
-                // (editor.gotoLocation.alternativeDefinitionCommand), but
-                // PHPStorm simply navigates to the returned location and
-                // stops.  Returning the usages directly makes the feature
-                // work uniformly across clients.
-                let usages = self.find_references(uri, content, position, false);
-                if let Some(usages) = usages
-                    && !usages.is_empty()
+            SymbolKind::MemberDeclaration { name, .. } => {
+                // 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(locs) =
+                        self.resolve_reverse_implementation(uri, content, cls, name, &class_loader)
+                    && !locs.is_empty()
                 {
-                    return Some(usages);
+                    return Some(locs);
                 }
 
-                // No usages found: fall back to the declaration's own
-                // location so clients that rely on the self-location
-                // signal (VS Code) still trigger their reference fallback.
-                let parsed_uri = Url::parse(uri).ok()?;
-                let start =
-                    crate::text_position::offset_to_position(content, cursor_offset as usize);
-                let end = crate::text_position::offset_to_position(
-                    content,
-                    cursor_offset as usize + name.len(),
-                );
-                Some(vec![Location {
-                    uri: parsed_uri,
-                    range: Range { start, end },
-                }])
+                self.declaration_or_usages(uri, content, cursor_offset, name)
+            }
+
+            SymbolKind::ClassDeclaration { name } => {
+                // If this class extends a parent, jump to the parent
+                // class declaration.
+                let ctx = self.file_context(uri);
+                let current_class =
+                    crate::class_lookup::find_class_at_offset(&ctx.classes, cursor_offset);
+                if let Some(cls) = current_class
+                    && let Some(ref parent_name) = cls.parent_class
+                    && let Some(loc) = self.resolve_class_reference(
+                        uri,
+                        content,
+                        parent_name,
+                        parent_name.contains('\\'),
+                        cursor_offset,
+                    )
+                {
+                    return Some(vec![loc]);
+                }
+
+                self.declaration_or_usages(uri, content, cursor_offset, name)
+            }
+
+            SymbolKind::NamespaceDeclaration { name } => {
+                self.declaration_or_usages(uri, content, cursor_offset, name)
             }
 
             SymbolKind::FunctionCall { name, .. } => {
@@ -354,6 +361,27 @@ impl Backend {
     /// When `is_fqn` is `true`, the name is already fully-qualified
     /// (the original PHP source used a leading `\`) and should be used
     /// as-is without namespace resolution.
+    fn declaration_or_usages(
+        &self,
+        uri: &str,
+        content: &str,
+        cursor_offset: u32,
+        name: &str,
+    ) -> Option<Vec<Location>> {
+        // Return the declaration's own location.  Editors detect
+        // "definition == current position" and offer Find References
+        // as a fallback (e.g. VS Code's
+        // editor.gotoLocation.alternativeDefinitionCommand).
+        let parsed_uri = Url::parse(uri).ok()?;
+        let start = crate::text_position::offset_to_position(content, cursor_offset as usize);
+        let end =
+            crate::text_position::offset_to_position(content, cursor_offset as usize + name.len());
+        Some(vec![Location {
+            uri: parsed_uri,
+            range: Range { start, end },
+        }])
+    }
+
     pub(super) fn resolve_class_reference(
         &self,
         uri: &str,

src/server.rs

@@ -265,6 +265,10 @@ impl LanguageServer for Backend {
                 } else {
                     None
                 },
+                execute_command_provider: Some(ExecuteCommandOptions {
+                    commands: vec!["phpantom.navigateToPrototype".to_string()],
+                    ..ExecuteCommandOptions::default()
+                }),
                 ..ServerCapabilities::default()
             },
             server_info: Some(ServerInfo {
@@ -1357,6 +1361,31 @@ impl LanguageServer for Backend {
         .await
     }
 
+    async fn execute_command(
+        &self,
+        params: ExecuteCommandParams,
+    ) -> Result<Option<serde_json::Value>> {
+        if params.command == "phpantom.navigateToPrototype"
+            && let [uri_val, pos_val] = params.arguments.as_slice()
+            && let Ok(uri) = serde_json::from_value::<Url>(uri_val.clone())
+            && let Ok(position) = serde_json::from_value::<Position>(pos_val.clone())
+            && let Some(ref client) = self.client
+        {
+            let _ = client
+                .show_document(ShowDocumentParams {
+                    uri,
+                    external: Some(false),
+                    take_focus: Some(true),
+                    selection: Some(Range {
+                        start: position,
+                        end: position,
+                    }),
+                })
+                .await;
+        }
+        Ok(None)
+    }
+
     async fn document_link(&self, params: DocumentLinkParams) -> Result<Option<Vec<DocumentLink>>> {
         let uri = params.text_document.uri.to_string();
         let backend = self.clone_for_blocking();

tests/integration/code_lens.rs

@@ -295,13 +295,13 @@ class Handler extends Base {
     let lens = &lenses[0];
     // The method `process` in Handler is on line 6 (0-based)
     assert_eq!(lens.range.start.line, 6);
-    assert_eq!(lens.range.start.character, 0);
+    assert_eq!(lens.range.start.character, 4);
 }
 
 // ─── Code Lens Command ─────────────────────────────────────────────────────
 
 #[test]
-fn lens_command_uses_show_references_by_default() {
+fn lens_command_uses_navigate_to_prototype() {
     let backend = create_test_backend();
     let content = r#"<?php
 class Parent_ {
@@ -317,11 +317,10 @@ class Child extends Parent_ {
 
     assert_eq!(lenses.len(), 1);
     let cmd = lenses[0].command.as_ref().unwrap();
-    assert_eq!(cmd.command, "editor.action.showReferences");
+    assert_eq!(cmd.command, "phpantom.navigateToPrototype");
     assert!(cmd.arguments.is_some());
     let args = cmd.arguments.as_ref().unwrap();
-    // Should have 3 arguments: uri, position, locations[]
-    assert_eq!(args.len(), 3);
+    assert_eq!(args.len(), 2);
 }
 
 // ─── Multiple Interfaces ────────────────────────────────────────────────────

tests/integration/definition_members.rs

@@ -2597,7 +2597,7 @@ async fn test_goto_definition_method_declaration_returns_self_location() {
 /// Ctrl+Click on a class name at its own declaration site returns the
 /// self-location when the class has no usages, so editors can fall back
 /// to Find References.  (When there are usages, they are returned
-/// directly — see `test_goto_definition_interface_declaration_returns_usages`.)
+/// directly — see `test_goto_definition_interface_declaration_returns_own_location`.)
 #[tokio::test]
 async fn test_goto_definition_class_declaration_returns_self_location() {
     let backend = create_test_backend();
@@ -2649,7 +2649,7 @@ async fn test_goto_definition_class_declaration_returns_self_location() {
 /// navigates to the returned location, so we must return the usages
 /// directly rather than the self-location.
 #[tokio::test]
-async fn test_goto_definition_interface_declaration_returns_usages() {
+async fn test_goto_definition_interface_declaration_returns_own_location() {
     let (backend, dir) = create_psr4_workspace(
         r#"{
             "autoload": { "psr-4": { "Test\\": "src/" } }
@@ -2681,9 +2681,7 @@ async fn test_goto_definition_interface_declaration_returns_usages() {
     );
 
     let iface_path = dir.path().join("src/EventListenerInterface.php");
-    let test_path = dir.path().join("src/Test.php");
     let iface_uri = Url::from_file_path(&iface_path).unwrap();
-    let test_uri = Url::from_file_path(&test_path).unwrap();
     let iface_content = std::fs::read_to_string(&iface_path).unwrap();
 
     backend
@@ -2697,8 +2695,6 @@ async fn test_goto_definition_interface_declaration_returns_usages() {
         })
         .await;
 
-    // Click on "EventListenerInterface" in `interface EventListenerInterface`
-    // (line 2, inside the name).
     let params = GotoDefinitionParams {
         text_document_position_params: TextDocumentPositionParams {
             text_document: TextDocumentIdentifier {
@@ -2717,16 +2713,13 @@ async fn test_goto_definition_interface_declaration_returns_usages() {
     let locations = match result {
         Some(GotoDefinitionResponse::Array(locs)) => locs,
         Some(GotoDefinitionResponse::Scalar(loc)) => vec![loc],
-        other => panic!("Expected usage locations, got: {other:?}"),
+        other => panic!("Expected own location, got: {other:?}"),
     };
 
-    assert!(
-        locations.iter().any(|loc| loc.uri == test_uri),
-        "Expected the `implements EventListenerInterface` usage in Test.php, got: {locations:#?}"
-    );
-    assert!(
-        !locations.iter().any(|loc| loc.uri == iface_uri),
-        "Should not return the declaration's own location when usages exist: {locations:#?}"
+    assert_eq!(locations.len(), 1, "should return exactly one location");
+    assert_eq!(
+        locations[0].uri, iface_uri,
+        "should return the declaration's own location so the editor can offer Find References"
     );
 }