Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
251 changes: 9 additions & 242 deletions src/definition/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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.
///
Expand Down
85 changes: 85 additions & 0 deletions tests/integration/definition_members.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
);
}
Loading