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..3d7a6970e 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. /// @@ -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. /// diff --git a/tests/integration/definition_members.rs b/tests/integration/definition_members.rs index 1216ad062..99c2ce742 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" + ); +}