diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 7a118ffd0..4d939c01e 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -23,6 +23,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. - **Renaming a namespace to one under a different autoload mapping moves its files to the right place.** The destination directory was worked out by cutting the *source* namespace's PSR-4 prefix off the destination name, which only holds when both sides sit under the same mapping. Renaming `App\Old` to `Lib\Domain` in a project mapping `App\` to `src/` and `Lib\` to `lib/` left the files under `src/` where the autoloader no longer looks, and a destination outside the autoload map entirely scattered them into a directory named after whatever was left of the name once the wrong prefix was cut. A destination name shorter than the prefix being cut crashed the request rather than renaming anything. The files now follow the destination to the mapping that actually covers it, a destination no mapping covers moves nothing and rewrites the declarations in place, and neither case can end the rename early. - **Import-class quick fixes are available on the first character of an unresolved class.** Invoking code actions from a normal-mode cursor now treats the cursor as a point inside the class name, rather than requiring a non-empty selection or a cursor farther into the name. - **A class named inside a `@phpstan-type` or `@phpstan-import-type` tag is a reference to it.** Both tags were read for their types — the aliases they declare resolve, expand through inheritance, and drive completion — but the class names written in them were never recorded as references, so everything downstream of that treated them as prose. They took no class highlighting (the tag name was coloured and the whole rest of the line came back as one flat comment), go-to-definition on them did nothing, they did not appear in find-references or document-highlight, and a class rename walked straight past them and left the alias pointing at a name that no longer exists. The type behind a `@phpstan-type` and the class after a `@phpstan-import-type`'s `from` are now recorded like any other docblock type. The alias names themselves are unaffected: `UserRow` in `@phpstan-type UserRow …` and `Row` in `… as Row` are not classes, so nothing claims them, and an alias referenced inside another alias is still not reported as an unknown class. The `@psalm-` spellings behave the same way. 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" + ); +}