Skip to content

Commit ad2ec8a

Browse files
committed
fix: go-to-implementation with same-named interface and class
When an interface and its implementing class shared the same short name (e.g. App\Contracts\HttpClient and App\Foo\HttpClient), go-to-implementation returned no results or pointed to the interface itself instead of the concrete class. Two places in the implementation resolver compared by short name instead of fully-qualified name, causing false matches when classes in different namespaces shared a name: - class_implements_or_extends excluded any candidate whose short name matched the target, even when the FQNs differed. Now compares by FQN only. - locate_class_declaration looked up the class file by short name, so it returned the first file containing that name rather than the correct namespace. Now builds the FQN from the class's namespace before lookup. Closes #279
1 parent 517879c commit ad2ec8a

3 files changed

Lines changed: 65 additions & 7 deletions

File tree

AGENTS.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,11 @@ full set of CI checks, testing conventions, and code style rules.
3131
introduce regressions in startup time or memory usage.
3232
- **Run the full lint pipeline.** `cargo clippy` and `cargo fmt` must
3333
pass with zero warnings before every commit. Do not skip this.
34-
- **Update the changelog.** When a change affects user-visible
35-
behaviour, add an entry under `## [Unreleased]` in
36-
`docs/CHANGELOG.md`. Write for end users, not developers. Include
37-
`Contributed by @username` with the GitHub username of the author.
34+
- **Update the changelog.** Add an entry under `## [Unreleased]` in
35+
`docs/CHANGELOG.md` for bug fixes and new features. Skip purely
36+
internal refactors that don't change observable behaviour. Write
37+
for end users, not developers. Include `Contributed by @username`
38+
with the GitHub username of the author.
3839
- **Reference issues in commits.** When fixing a GitHub issue, include
3940
`Closes #123` in the commit message body.
4041
- **Prefer single tests.** Run individual tests (`cargo test test_name`)

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4040

4141
### Fixed
4242

43+
- **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.
4344
- **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.
4445
- **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.
4546
- **`class-string<static>` parameters no longer reject sibling subclass constants.** Static helper calls from a shared base class that pass concrete `::class` constants for sibling subclasses no longer report false argument-type mismatches against `class-string<static>`. Contributed by @calebdw.

src/definition/implementation.rs

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -994,8 +994,10 @@ impl Backend {
994994
// Build the FQN of the candidate class for comparison.
995995
let cls_fqn = crate::util::build_fqn(&cls.name, cls.file_namespace.as_deref());
996996

997-
// Skip the target class itself.
998-
if cls_fqn == target_fqn || cls.name == target_short {
997+
// Skip the target class itself — compare by FQN so that
998+
// classes in different namespaces that share the same short
999+
// name are not incorrectly excluded.
1000+
if cls_fqn == target_fqn {
9991001
return false;
10001002
}
10011003

@@ -1231,8 +1233,9 @@ impl Backend {
12311233
current_uri: &str,
12321234
current_content: &str,
12331235
) -> Option<Location> {
1236+
let cls_fqn = crate::util::build_fqn(&cls.name, cls.file_namespace.as_deref());
12341237
let (class_uri, class_content) =
1235-
self.find_class_file_content(&cls.name, current_uri, current_content)?;
1238+
self.find_class_file_content(&cls_fqn, current_uri, current_content)?;
12361239

12371240
if cls.keyword_offset == 0 {
12381241
return None;
@@ -1399,4 +1402,57 @@ mod tests {
13991402
);
14001403
assert_eq!(locations[0].uri, user_impl_uri);
14011404
}
1405+
1406+
#[test]
1407+
fn same_short_name_interface_and_implementation_found() {
1408+
let dir = tempfile::tempdir().expect("temp dir");
1409+
let src = dir.path().join("src");
1410+
fs::create_dir_all(src.join("Contracts")).expect("contracts dir");
1411+
fs::create_dir_all(src.join("Foo")).expect("foo dir");
1412+
1413+
let interface_php = concat!(
1414+
"<?php\n",
1415+
"namespace App\\Contracts;\n",
1416+
"interface HttpClient {}\n",
1417+
);
1418+
let impl_php = concat!(
1419+
"<?php\n",
1420+
"namespace App\\Foo;\n",
1421+
"use App\\Contracts\\HttpClient as HttpClientInterface;\n",
1422+
"class HttpClient implements HttpClientInterface {}\n",
1423+
);
1424+
1425+
let interface_path = src.join("Contracts/HttpClient.php");
1426+
let impl_path = src.join("Foo/HttpClient.php");
1427+
fs::write(&interface_path, interface_php).expect("interface file");
1428+
fs::write(&impl_path, impl_php).expect("impl file");
1429+
1430+
let backend = Backend::new_test_with_workspace(dir.path().to_path_buf(), Vec::new());
1431+
let mut config = Config::default();
1432+
config.indexing.strategy = Some(IndexingStrategy::Full);
1433+
backend.set_config(config);
1434+
1435+
let interface_uri = Url::from_file_path(&interface_path).expect("interface uri");
1436+
let impl_uri = Url::from_file_path(&impl_path).expect("impl uri");
1437+
backend.update_ast(interface_uri.as_str(), interface_php);
1438+
backend.update_ast(impl_uri.as_str(), impl_php);
1439+
1440+
let locations = backend
1441+
.resolve_implementation(
1442+
interface_uri.as_str(),
1443+
interface_php,
1444+
Position {
1445+
line: 2,
1446+
character: 12,
1447+
},
1448+
)
1449+
.expect("implementation with same short name should be found");
1450+
1451+
assert_eq!(
1452+
locations.len(),
1453+
1,
1454+
"should find exactly one implementation: {locations:?}",
1455+
);
1456+
assert_eq!(locations[0].uri, impl_uri);
1457+
}
14021458
}

0 commit comments

Comments
 (0)