Skip to content

Commit 79a5025

Browse files
lucasacoutinhoAJenbo
authored andcommitted
feat: Return self-location at definition sites so editors can fall back
to Find References
1 parent 1f48613 commit 79a5025

5 files changed

Lines changed: 186 additions & 169 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4141
- **Faster startup.** Stub loading during initialization is significantly faster.
4242
- **More accurate generics resolution.** Type substitution and resolution for complex nested generic types is more correct, particularly for unions, intersections, array shapes, and deeply nested generic arguments.
4343
- **More accurate type predicates.** `NULL`, `Null`, and case variants of `null` are now handled consistently throughout type checking, matching PHP's case-insensitive treatment of type keywords.
44+
- **Go-to-definition at declaration sites returns the symbol's own location.** Class, member, and variable declaration names now return their own location instead of nothing, so editors that detect "definition == cursor" can automatically fall back to Find References. Contributed by @lucasacoutinho in https://github.com/AJenbo/phpantom_lsp/pull/76.
4445

4546
### Fixed
4647

src/definition/resolve.rs

Lines changed: 30 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -40,46 +40,9 @@ impl Backend {
4040
// Consult precomputed symbol map (retries one byte earlier for
4141
// end-of-token edge cases).
4242
let symbol = self.lookup_symbol_at_position(uri, content, position);
43-
let result = if let Some(ref symbol) = symbol {
44-
self.resolve_from_symbol(&symbol.kind, uri, content, position, symbol.start)
45-
} else {
46-
None
47-
};
48-
49-
// ── Self-reference guard ────────────────────────────────────
50-
// When the resolved location points back to the same file and
51-
// the cursor is already within (or touching) the target range,
52-
// the user is at the definition site. Suppress the jump so
53-
// that Ctrl+Click doesn't navigate to itself.
54-
//
55-
// Special case: zero-width (point) locations arise from
56-
// `offset_to_position` and similar helpers that return the
57-
// start of a construct (e.g. the `define` keyword) but the
58-
// cursor may be anywhere on the same line (e.g. on the
59-
// constant name inside the string argument). For these we
60-
// expand the check to the entire line.
61-
if let Some(ref loc) = result
62-
&& let Ok(parsed_uri) = Url::parse(uri)
63-
&& loc.uri == parsed_uri
64-
{
65-
let is_point = loc.range.start == loc.range.end;
66-
let within = if is_point {
67-
// Zero-width target: suppress when cursor is on the same line.
68-
position.line == loc.range.start.line
69-
} else {
70-
position.line >= loc.range.start.line
71-
&& position.line <= loc.range.end.line
72-
&& (position.line != loc.range.start.line
73-
|| position.character >= loc.range.start.character)
74-
&& (position.line != loc.range.end.line
75-
|| position.character <= loc.range.end.character)
76-
};
77-
if within {
78-
return None;
79-
}
80-
}
81-
82-
result
43+
symbol
44+
.as_ref()
45+
.and_then(|s| self.resolve_from_symbol(&s.kind, uri, content, position, s.start))
8346
}
8447

8548
/// Look up the symbol at the given byte offset in the precomputed
@@ -205,14 +168,19 @@ impl Backend {
205168
// outer-scope lookup.
206169
if def_kind != VarDefKind::ClosureCapture {
207170
// The cursor is on a variable at its definition
208-
// site (assignment LHS, parameter, foreach
209-
// binding, catch binding, etc.). GTD should not
210-
// trigger here — the user is already at the
211-
// definition. Type hints next to the variable
212-
// (e.g. `Throwable` in `catch (Throwable $it)`)
213-
// are separate symbol spans that the user can
214-
// click directly.
215-
return None;
171+
// site. Return the symbol's own location so
172+
// editors can fall back to Find References.
173+
let parsed_uri = Url::parse(uri).ok()?;
174+
let start =
175+
crate::util::offset_to_position(content, cursor_offset as usize);
176+
let end = crate::util::offset_to_position(
177+
content,
178+
cursor_offset as usize + 1 + name.len(),
179+
);
180+
return Some(Location {
181+
uri: parsed_uri,
182+
range: Range { start, end },
183+
});
216184
}
217185
}
218186

@@ -275,11 +243,20 @@ impl Backend {
275243
self.resolve_class_reference(uri, content, name, *is_fqn, cursor_offset)
276244
}
277245

278-
SymbolKind::ClassDeclaration { .. } | SymbolKind::MemberDeclaration { .. } => {
279-
// The cursor is on a class/interface/trait/enum declaration
280-
// name or a method/property/constant declaration name —
281-
// the user is already at the definition site.
282-
None
246+
SymbolKind::ClassDeclaration { name } | SymbolKind::MemberDeclaration { name, .. } => {
247+
// The cursor is on a declaration name. Return the
248+
// symbol's own location so that editors can detect
249+
// "definition == current position" and fall back to
250+
// Find References (e.g. VS Code's
251+
// editor.gotoLocation.alternativeDefinitionCommand).
252+
let parsed_uri = Url::parse(uri).ok()?;
253+
let start = crate::util::offset_to_position(content, cursor_offset as usize);
254+
let end =
255+
crate::util::offset_to_position(content, cursor_offset as usize + name.len());
256+
Some(Location {
257+
uri: parsed_uri,
258+
range: Range { start, end },
259+
})
283260
}
284261

285262
SymbolKind::FunctionCall { name, .. } => {

src/symbol_map/mod.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@ pub(crate) enum SymbolKind {
7676
is_fqn: bool,
7777
},
7878
/// Class/interface/trait/enum name at its *declaration* site
79-
/// (`class Foo`, `interface Bar`, etc.). Not navigable for
80-
/// go-to-definition (the cursor is already at the definition),
81-
/// but useful for document highlights and other features.
79+
/// (`class Foo`, `interface Bar`, etc.). Go-to-definition returns
80+
/// the symbol's own location so editors can fall back to
81+
/// Find References. Also useful for document highlights.
8282
ClassDeclaration { name: String },
8383

8484
/// Member name on the RHS of `->`, `?->`, or `::`.
@@ -120,9 +120,10 @@ pub(crate) enum SymbolKind {
120120

121121
/// A method, property, or constant name at its *declaration* site.
122122
///
123-
/// Not navigable for go-to-definition or hover (the cursor is
124-
/// already at the definition), but needed for find-references and
125-
/// rename so that the declaration site participates in the match.
123+
/// Go-to-definition returns the symbol's own location so editors
124+
/// can fall back to Find References. Also needed for
125+
/// find-references and rename so that the declaration site
126+
/// participates in the match.
126127
MemberDeclaration {
127128
/// The member name (e.g. `"save"`, `"name"`, `"MAX_SIZE"`).
128129
/// For properties this is the name WITHOUT the `$` prefix.

tests/integration/definition_members.rs

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2431,9 +2431,9 @@ async fn test_goto_definition_own_method_same_short_name_as_parent() {
24312431
// ─── GTD self-reference suppression ─────────────────────────────────────────
24322432

24332433
/// Ctrl+Click on a method name at its own declaration site should return
2434-
/// `None` rather than jumping to itself.
2434+
/// self-location so editors can fall back to Find References.
24352435
#[tokio::test]
2436-
async fn test_goto_definition_method_declaration_returns_none() {
2436+
async fn test_goto_definition_method_declaration_returns_self_location() {
24372437
let backend = create_test_backend();
24382438

24392439
let uri = Url::parse("file:///self_ref_method.php").unwrap();
@@ -2471,17 +2471,19 @@ async fn test_goto_definition_method_declaration_returns_none() {
24712471
};
24722472

24732473
let result = backend.goto_definition(params).await.unwrap();
2474-
assert!(
2475-
result.is_none(),
2476-
"GTD on a method name at its declaration site should return None, got: {:?}",
2477-
result
2478-
);
2474+
match result {
2475+
Some(GotoDefinitionResponse::Scalar(location)) => {
2476+
assert_eq!(location.uri, uri);
2477+
assert_eq!(location.range.start.line, 2, "should point back to line 2");
2478+
}
2479+
other => panic!("Expected self-location Scalar, got: {other:?}"),
2480+
}
24792481
}
24802482

24812483
/// Ctrl+Click on a class name at its own declaration site should return
2482-
/// `None` rather than jumping to itself.
2484+
/// self-location so editors can fall back to Find References.
24832485
#[tokio::test]
2484-
async fn test_goto_definition_class_declaration_returns_none() {
2486+
async fn test_goto_definition_class_declaration_returns_self_location() {
24852487
let backend = create_test_backend();
24862488

24872489
let uri = Url::parse("file:///self_ref_class.php").unwrap();
@@ -2516,11 +2518,13 @@ async fn test_goto_definition_class_declaration_returns_none() {
25162518
};
25172519

25182520
let result = backend.goto_definition(params).await.unwrap();
2519-
assert!(
2520-
result.is_none(),
2521-
"GTD on a class name at its declaration site should return None, got: {:?}",
2522-
result
2523-
);
2521+
match result {
2522+
Some(GotoDefinitionResponse::Scalar(location)) => {
2523+
assert_eq!(location.uri, uri);
2524+
assert_eq!(location.range.start.line, 1, "should point back to line 1");
2525+
}
2526+
other => panic!("Expected self-location Scalar, got: {other:?}"),
2527+
}
25242528
}
25252529

25262530
/// Ctrl+Click on a constant name inside its own `define()` call should

0 commit comments

Comments
 (0)