Skip to content

Commit a789604

Browse files
committed
Fix inherited methods missing through deep stub chains
1 parent 92b1df5 commit a789604

6 files changed

Lines changed: 186 additions & 34 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1313
- **Nullable and generic types in class lookup.** Variables typed as `?ClassName` or `Collection<Item>` now resolve correctly across all code paths. Previously the `?` prefix and generic parameters were not stripped before class lookup, causing the type engine to treat them as unknown types. This fixes completion, hover, go-to-definition, and false-positive diagnostics for any variable whose type uses the nullable shorthand or carries generic arguments.
1414
- **`@see` references with qualified class names.** Docblock `@see Fully\Qualified\ClassName` references no longer have the file's namespace prepended, which previously produced doubled names like `App\Models\App\Models\User`. Qualified names in `@see` tags are now treated as fully-qualified.
1515
- **Ternary and null-coalesce member access.** Accessing a member on a ternary or null-coalesce expression (e.g. `($a ?: $b)->property`, `($x ?? $y)->method()`) now resolves correctly for hover, go-to-definition, and diagnostics. Previously the subject extraction produced an empty string, causing a confusing "Cannot resolve type of ''" hint and no hover information.
16+
- **Inherited methods missing through deep stub chains.** Methods like `getCode()` and `getMessage()` are now found on classes that inherit through multi-level chains where intermediate classes live in stubs (e.g. `QueryException``PDOException``RuntimeException``Exception`). Previously the inheritance chain broke when a stub file contained multiple namespace blocks, causing parent class names to be resolved against the wrong namespace.
1617
- **False-positive diagnostics for same-named variables in different methods.** When two methods in the same class both used a variable like `$order`, the diagnostic cache resolved it once and reused that type for every other method in the class. The second method saw the wrong type and flagged valid member accesses as unknown. Both the per-file subject cache and the cross-collector resolution cache are now scoped to the enclosing function/method/closure body, so each method resolves variables independently.
1718
- **False-positive diagnostics for `$this` inside traits.** Accessing host-class members via `$this->`, `self::`, `static::`, or `parent::` inside a trait method no longer produces "not found" warnings. Traits are incomplete by nature and expect the host class to provide these members.
1819
- **Type narrowing inside `return` statements.** `instanceof` checks in `&&` chains and ternary conditions now narrow the variable type when the expression is the operand of a `return` statement. Previously, `return $e instanceof QueryException && $e->errorInfo;` would flag `errorInfo` as unknown because narrowing only applied inside standalone expression statements and `if` conditions.

docs/todo.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,6 @@ unlikely to move the needle for most users.
104104
| B3 | [Trait static/self suppression not applied inside closures](todo/bugs.md#b3-trait-staticself-suppression-not-applied-inside-closures) | Medium | Low |
105105
| B4 | [Variable reassignment loses type when parameter name is reused](todo/bugs.md#b4-variable-reassignment-loses-type-when-parameter-name-is-reused) | Medium | Medium |
106106
| B7 | [Overloaded built-in function signatures in stubs](todo/bugs.md#b7-overloaded-built-in-function-signatures-in-stubs) | Low | Low |
107-
| B8 | [`getCode`/`getMessage` not found through deep inheritance chains](todo/bugs.md#b8-getcodemessage-not-found-through-deep-inheritance-chains) | Low | Low |
108107
| | **[Completion](todo/completion.md)** | | |
109108
| C1 | Array functions needing new code paths | Medium | High |
110109
| C10 | [Lazy documentation via `completionItem/resolve`](todo/completion.md#c10-lazy-documentation-via-completionitemresolve) | Medium | Medium |

docs/todo/bugs.md

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -114,25 +114,3 @@ this map before flagging. The map only needs entries for functions
114114
where the stub's single signature cannot represent the valid call
115115
forms.
116116

117-
---
118-
119-
#### B8. `getCode`/`getMessage` not found through deep inheritance chains
120-
121-
| | |
122-
|---|---|
123-
| **Impact** | Low |
124-
| **Effort** | Low |
125-
126-
Methods inherited from `Throwable` (like `getCode()` and
127-
`getMessage()`) are not found on `QueryException`, which inherits
128-
through `QueryException → PDOException → RuntimeException → Exception`.
129-
The chain involves both vendor classes and stub classes.
130-
131-
**Observed:** 3 diagnostics in `shared` for `getCode()` and
132-
`getMessage()` on `Illuminate\Database\QueryException`.
133-
134-
**Fix:** Investigate whether the inheritance chain breaks at the
135-
vendor-to-stub boundary (PDOException is in stubs, RuntimeException
136-
is in stubs). The chain resolution may stop walking when it crosses
137-
from a vendor class to a stub class, or the depth limit may be
138-
insufficient.

src/parser/mod.rs

Lines changed: 57 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -969,6 +969,23 @@ impl Backend {
969969
/// `#[PhpStormStubsElementAvailable]` whose version range excludes
970970
/// the target version are filtered out during extraction.
971971
pub fn parse_php_versioned(content: &str, php_version: Option<PhpVersion>) -> Vec<ClassInfo> {
972+
Self::parse_php_versioned_with_namespaces(content, php_version)
973+
.into_iter()
974+
.map(|(cls, _ns)| cls)
975+
.collect()
976+
}
977+
978+
/// Like [`parse_php_versioned`] but returns each class together with
979+
/// the namespace block it was declared in.
980+
///
981+
/// This is needed by [`parse_and_cache_content_versioned`](crate::resolution)
982+
/// so that multi-namespace stub files (e.g. PDO.php with both
983+
/// `namespace { }` and `namespace Pdo { }`) resolve parent class
984+
/// names against the correct namespace context.
985+
pub fn parse_php_versioned_with_namespaces(
986+
content: &str,
987+
php_version: Option<PhpVersion>,
988+
) -> Vec<(ClassInfo, Option<String>)> {
972989
with_parsed_program(content, "parse_php", |program, content| {
973990
let mut use_map = HashMap::new();
974991
Self::extract_use_statements_from_statements(program.statements.iter(), &mut use_map);
@@ -982,13 +999,46 @@ impl Backend {
982999
namespace,
9831000
};
9841001

985-
let mut classes = Vec::new();
986-
Self::extract_classes_from_statements(
987-
program.statements.iter(),
988-
&mut classes,
989-
Some(&doc_ctx),
990-
);
991-
classes
1002+
let mut result: Vec<(ClassInfo, Option<String>)> = Vec::new();
1003+
1004+
for statement in program.statements.iter() {
1005+
match statement {
1006+
Statement::Namespace(ns) => {
1007+
let block_ns: Option<String> = ns
1008+
.name
1009+
.as_ref()
1010+
.map(|ident| ident.value().to_string())
1011+
.filter(|n| !n.is_empty());
1012+
1013+
let mut block_classes = Vec::new();
1014+
Self::extract_classes_from_statements(
1015+
ns.statements().iter(),
1016+
&mut block_classes,
1017+
Some(&doc_ctx),
1018+
);
1019+
for cls in block_classes {
1020+
result.push((cls, block_ns.clone()));
1021+
}
1022+
}
1023+
Statement::Class(_)
1024+
| Statement::Interface(_)
1025+
| Statement::Trait(_)
1026+
| Statement::Enum(_) => {
1027+
let mut top_classes = Vec::new();
1028+
Self::extract_classes_from_statements(
1029+
std::iter::once(statement),
1030+
&mut top_classes,
1031+
Some(&doc_ctx),
1032+
);
1033+
for cls in top_classes {
1034+
result.push((cls, None));
1035+
}
1036+
}
1037+
_ => {}
1038+
}
1039+
}
1040+
1041+
result
9921042
})
9931043
}
9941044

src/resolution.rs

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -235,18 +235,54 @@ impl Backend {
235235
uri: &str,
236236
php_version: Option<PhpVersion>,
237237
) -> Option<Vec<Arc<ClassInfo>>> {
238-
let mut classes = Self::parse_php_versioned(content, php_version);
239238
let file_use_map = self.parse_use_statements(content);
240239
let file_namespace = self.parse_namespace(content);
241-
Self::resolve_parent_class_names(&mut classes, &file_use_map, &file_namespace);
240+
241+
// Parse classes with per-class namespace tracking so that
242+
// multi-namespace files (e.g. PDO.php with both `namespace { }`
243+
// and `namespace Pdo { }`) resolve parent names correctly.
244+
let classes_with_ns = Self::parse_php_versioned_with_namespaces(content, php_version);
245+
246+
// Group classes by their enclosing namespace and resolve parent
247+
// names once per group, mirroring the logic in `update_ast_inner`.
248+
let mut classes: Vec<ClassInfo> = Vec::with_capacity(classes_with_ns.len());
249+
let mut ns_groups: HashMap<Option<String>, Vec<usize>> = HashMap::new();
250+
for (i, (_cls, ns)) in classes_with_ns.iter().enumerate() {
251+
ns_groups.entry(ns.clone()).or_default().push(i);
252+
}
253+
254+
// Flatten into a single Vec, preserving original order.
255+
for (cls, _) in &classes_with_ns {
256+
classes.push(cls.clone());
257+
}
258+
259+
if ns_groups.len() <= 1 {
260+
// Single namespace (common case): resolve with file namespace.
261+
Self::resolve_parent_class_names(&mut classes, &file_use_map, &file_namespace);
262+
} else {
263+
// Multi-namespace file: resolve each group with its own
264+
// namespace context so that classes in `namespace { }` are
265+
// not polluted by a sibling `namespace Pdo { }` block.
266+
for (group_ns, indices) in &ns_groups {
267+
let mut group: Vec<ClassInfo> =
268+
indices.iter().map(|&i| classes[i].clone()).collect();
269+
Self::resolve_parent_class_names(&mut group, &file_use_map, group_ns);
270+
for (j, &idx) in indices.iter().enumerate() {
271+
classes[idx] = group[j].clone();
272+
}
273+
}
274+
}
242275

243276
// Set the per-class file_namespace so that classes loaded via
244277
// PSR-4 / classmap carry their namespace. This mirrors the
245278
// same assignment done in `update_ast_inner` for files opened
246279
// through `did_open` / `did_change`.
247-
for cls in &mut classes {
280+
for (i, cls) in classes.iter_mut().enumerate() {
248281
if cls.file_namespace.is_none() {
249-
cls.file_namespace = file_namespace.clone();
282+
cls.file_namespace = classes_with_ns[i]
283+
.1
284+
.clone()
285+
.or_else(|| file_namespace.clone());
250286
}
251287
}
252288

tests/completion_inheritance.rs

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1786,3 +1786,91 @@ async fn test_interface_virtual_members_visible_through_parent_chain() {
17861786
_ => panic!("Expected CompletionResponse::Array"),
17871787
}
17881788
}
1789+
1790+
// ─── Deep inheritance chain through stubs (B8) ──────────────────────────────
1791+
1792+
/// Methods inherited from `Exception` (like `getCode()`, `getMessage()`) should
1793+
/// be found on a class that extends through a multi-level chain where
1794+
/// intermediate classes live in stubs (e.g. PDOException → RuntimeException →
1795+
/// Exception).
1796+
#[tokio::test]
1797+
async fn test_completion_deep_inheritance_through_stubs() {
1798+
let backend = create_test_backend();
1799+
1800+
let uri = Url::parse("file:///deep_chain.php").unwrap();
1801+
let text = concat!(
1802+
"<?php\n",
1803+
"class QueryException extends \\PDOException {\n",
1804+
" public function getSql(): string { return ''; }\n",
1805+
"}\n",
1806+
"class DeepChainTest {\n",
1807+
" public function handle(QueryException $e): void {\n",
1808+
" $e->\n",
1809+
" }\n",
1810+
"}\n",
1811+
);
1812+
1813+
let open_params = DidOpenTextDocumentParams {
1814+
text_document: TextDocumentItem {
1815+
uri: uri.clone(),
1816+
language_id: "php".to_string(),
1817+
version: 1,
1818+
text: text.to_string(),
1819+
},
1820+
};
1821+
backend.did_open(open_params).await;
1822+
1823+
let completion_params = CompletionParams {
1824+
text_document_position: TextDocumentPositionParams {
1825+
text_document: TextDocumentIdentifier { uri },
1826+
position: Position {
1827+
line: 6,
1828+
character: 12,
1829+
},
1830+
},
1831+
work_done_progress_params: WorkDoneProgressParams::default(),
1832+
partial_result_params: PartialResultParams::default(),
1833+
context: None,
1834+
};
1835+
1836+
let result = backend.completion(completion_params).await.unwrap();
1837+
assert!(
1838+
result.is_some(),
1839+
"Completion should return results for QueryException"
1840+
);
1841+
1842+
match result.unwrap() {
1843+
CompletionResponse::Array(items) => {
1844+
let method_names: Vec<&str> = items
1845+
.iter()
1846+
.filter(|i| i.kind == Some(CompletionItemKind::METHOD))
1847+
.map(|i| i.filter_text.as_deref().unwrap())
1848+
.collect();
1849+
1850+
// Own method
1851+
assert!(
1852+
method_names.contains(&"getSql"),
1853+
"Should include own method 'getSql', got: {:?}",
1854+
method_names
1855+
);
1856+
1857+
// Methods from Exception (3 levels up: QueryException → PDOException → RuntimeException → Exception)
1858+
assert!(
1859+
method_names.contains(&"getMessage"),
1860+
"Should include 'getMessage' inherited from Exception through deep chain, got: {:?}",
1861+
method_names
1862+
);
1863+
assert!(
1864+
method_names.contains(&"getCode"),
1865+
"Should include 'getCode' inherited from Exception through deep chain, got: {:?}",
1866+
method_names
1867+
);
1868+
assert!(
1869+
method_names.contains(&"getTrace"),
1870+
"Should include 'getTrace' inherited from Exception through deep chain, got: {:?}",
1871+
method_names
1872+
);
1873+
}
1874+
_ => panic!("Expected CompletionResponse::Array"),
1875+
}
1876+
}

0 commit comments

Comments
 (0)