Skip to content

Commit 92b1df5

Browse files
committed
Empty subject string in type resolution
1 parent 42d4a6d commit 92b1df5

7 files changed

Lines changed: 247 additions & 25 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212
- **Null-safe method chain resolution.** Null-safe method calls (`$obj?->method()`) now resolve the return type correctly for variable type inference, including cross-file chains. Previously `?->` calls were ignored by the RHS resolution pipeline, losing the type for any variable assigned from a null-safe chain.
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.
15+
- **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.
1516
- **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.
1617
- **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.
1718
- **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
@@ -103,7 +103,6 @@ unlikely to move the needle for most users.
103103
| | **[Bugs](todo/bugs.md)** | | |
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 |
106-
| B6 | [Empty subject string in diagnostic messages](todo/bugs.md#b6-empty-subject-string-in-diagnostic-messages) | Low | Low |
107106
| B7 | [Overloaded built-in function signatures in stubs](todo/bugs.md#b7-overloaded-built-in-function-signatures-in-stubs) | Low | Low |
108107
| B8 | [`getCode`/`getMessage` not found through deep inheritance chains](todo/bugs.md#b8-getcodemessage-not-found-through-deep-inheritance-chains) | Low | Low |
109108
| | **[Completion](todo/completion.md)** | | |

docs/todo/bugs.md

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -79,29 +79,6 @@ within the same scope at the cursor offset.
7979

8080
---
8181

82-
#### B6. Empty subject string in type resolution
83-
84-
| | |
85-
|---|---|
86-
| **Impact** | Low |
87-
| **Effort** | Low |
88-
89-
The subject extraction produces an empty string for complex expressions
90-
like `($a ?: $b)?->property`, meaning the type engine has no subject
91-
to resolve. This manifests as "Cannot resolve type of ''" in
92-
diagnostics, but the underlying issue is that `expr_to_subject_text`
93-
does not handle ternary-inside-nullable and similar compound patterns.
94-
95-
**Observed:** 5 cases in `shared` with empty subject strings.
96-
97-
**Fix:** Extend `expr_to_subject_text` to handle parenthesised ternary
98-
expressions, short ternary (`?:`), and null-coalesce (`??`) as subject
99-
bases. When the expression is too complex to represent as a subject
100-
string, the type engine should skip the access rather than attempt
101-
resolution with an empty key.
102-
103-
---
104-
10582
#### B7. Overloaded built-in function signatures not representable in stubs
10683

10784
| | |

src/diagnostics/unknown_members.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ impl Backend {
427427
// usually because the symbol map's subject_text
428428
// doesn't preserve full argument text, not because
429429
// the user is missing a type annotation.
430-
if !subject_text.contains('(') {
430+
if !subject_text.is_empty() && !subject_text.contains('(') {
431431
let range = match offset_range_to_lsp_range(
432432
content,
433433
span.start as usize,

src/symbol_map/extraction.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2377,6 +2377,30 @@ fn expr_to_subject_text(expr: &Expression<'_>) -> String {
23772377
format!("[{}]", parts.join(", "))
23782378
}
23792379

2380+
// Ternary `$a ? $b : $c` and short ternary `$a ?: $b`.
2381+
// For short ternary (`then` is None), the condition is the
2382+
// preferred branch; for full ternary, use the `then` branch.
2383+
// Either way we pick one branch so the type engine has
2384+
// something to resolve rather than an empty string.
2385+
Expression::Conditional(cond) => {
2386+
let preferred = cond.then.unwrap_or(cond.condition);
2387+
let text = expr_to_subject_text(preferred);
2388+
if !text.is_empty() {
2389+
return text;
2390+
}
2391+
// Fall back to the else branch.
2392+
expr_to_subject_text(cond.r#else)
2393+
}
2394+
2395+
// Null coalesce `$a ?? $b` — LHS is the preferred non-null value.
2396+
Expression::Binary(binary) if binary.operator.is_null_coalesce() => {
2397+
let text = expr_to_subject_text(binary.lhs);
2398+
if !text.is_empty() {
2399+
return text;
2400+
}
2401+
expr_to_subject_text(binary.rhs)
2402+
}
2403+
23802404
Expression::ArrayAccess(access) => {
23812405
let base = expr_to_subject_text(access.array);
23822406
if base.is_empty() {

tests/completion_variables.rs

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16889,6 +16889,138 @@ async fn test_completion_nullsafe_method_chain_cross_file() {
1688916889
}
1689016890
}
1689116891

16892+
// ─── Ternary / null-coalesce RHS resolution (B6) ────────────────────────────
16893+
16894+
/// Short ternary as RHS: `$p = $cond ?: new Printer()` should resolve `$p`.
16895+
#[tokio::test]
16896+
async fn test_completion_short_ternary_rhs_instantiation() {
16897+
let backend = create_test_backend();
16898+
16899+
let uri = Url::parse("file:///short_ternary.php").unwrap();
16900+
let text = concat!(
16901+
"<?php\n",
16902+
"class Printer {\n",
16903+
" public function print(): void {}\n",
16904+
"}\n",
16905+
"class Demo {\n",
16906+
" public function run(): void {\n",
16907+
" $p = null ?: new Printer();\n",
16908+
" $p->\n",
16909+
" }\n",
16910+
"}\n",
16911+
);
16912+
16913+
let open_params = DidOpenTextDocumentParams {
16914+
text_document: TextDocumentItem {
16915+
uri: uri.clone(),
16916+
language_id: "php".to_string(),
16917+
version: 1,
16918+
text: text.to_string(),
16919+
},
16920+
};
16921+
backend.did_open(open_params).await;
16922+
16923+
let completion_params = CompletionParams {
16924+
text_document_position: TextDocumentPositionParams {
16925+
text_document: TextDocumentIdentifier { uri },
16926+
position: Position {
16927+
line: 7,
16928+
character: 12,
16929+
},
16930+
},
16931+
work_done_progress_params: WorkDoneProgressParams::default(),
16932+
partial_result_params: PartialResultParams::default(),
16933+
context: None,
16934+
};
16935+
16936+
let result = backend.completion(completion_params).await.unwrap();
16937+
assert!(
16938+
result.is_some(),
16939+
"Completion should return results for short ternary RHS"
16940+
);
16941+
16942+
match result.unwrap() {
16943+
CompletionResponse::Array(items) => {
16944+
let names: Vec<&str> = items
16945+
.iter()
16946+
.filter(|i| i.kind == Some(CompletionItemKind::METHOD))
16947+
.map(|i| i.filter_text.as_deref().unwrap())
16948+
.collect();
16949+
assert!(
16950+
names.contains(&"print"),
16951+
"Should include 'print' from Printer via ternary, got: {:?}",
16952+
names
16953+
);
16954+
}
16955+
_ => panic!("Expected CompletionResponse::Array"),
16956+
}
16957+
}
16958+
16959+
/// Full ternary as RHS with instantiation on both branches.
16960+
#[tokio::test]
16961+
async fn test_completion_full_ternary_rhs_instantiation() {
16962+
let backend = create_test_backend();
16963+
16964+
let uri = Url::parse("file:///full_ternary.php").unwrap();
16965+
let text = concat!(
16966+
"<?php\n",
16967+
"class Renderer {\n",
16968+
" public function render(): string { return ''; }\n",
16969+
"}\n",
16970+
"class Page {\n",
16971+
" public function show(bool $flag): void {\n",
16972+
" $r = $flag ? new Renderer() : new Renderer();\n",
16973+
" $r->\n",
16974+
" }\n",
16975+
"}\n",
16976+
);
16977+
16978+
let open_params = DidOpenTextDocumentParams {
16979+
text_document: TextDocumentItem {
16980+
uri: uri.clone(),
16981+
language_id: "php".to_string(),
16982+
version: 1,
16983+
text: text.to_string(),
16984+
},
16985+
};
16986+
backend.did_open(open_params).await;
16987+
16988+
let completion_params = CompletionParams {
16989+
text_document_position: TextDocumentPositionParams {
16990+
text_document: TextDocumentIdentifier { uri },
16991+
position: Position {
16992+
line: 7,
16993+
character: 12,
16994+
},
16995+
},
16996+
work_done_progress_params: WorkDoneProgressParams::default(),
16997+
partial_result_params: PartialResultParams::default(),
16998+
context: None,
16999+
};
17000+
17001+
let result = backend.completion(completion_params).await.unwrap();
17002+
assert!(
17003+
result.is_some(),
17004+
"Completion should return results for full ternary RHS"
17005+
);
17006+
17007+
match result.unwrap() {
17008+
CompletionResponse::Array(items) => {
17009+
let names: Vec<&str> = items
17010+
.iter()
17011+
.filter(|i| i.kind == Some(CompletionItemKind::METHOD))
17012+
.map(|i| i.filter_text.as_deref().unwrap())
17013+
.collect();
17014+
assert!(
17015+
names.contains(&"render"),
17016+
"Should include 'render' from Renderer via ternary, got: {:?}",
17017+
names
17018+
);
17019+
}
17020+
_ => panic!("Expected CompletionResponse::Array"),
17021+
}
17022+
}
17023+
1689217024
// ─── Nullable type resolution (B1) ──────────────────────────────────────────
1689317025

1689417026
/// A parameter typed `?ClassName` should resolve to `ClassName` for completion.

tests/hover.rs

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7986,3 +7986,92 @@ class Demo {
79867986
text
79877987
);
79887988
}
7989+
7990+
// ─── Ternary / null-coalesce subject extraction (B6) ────────────────────────
7991+
7992+
#[test]
7993+
fn hover_short_ternary_member_access() {
7994+
let backend = create_test_backend();
7995+
let uri = "file:///b6_short_ternary.php";
7996+
let content = r#"<?php
7997+
class Gadget {
7998+
public string $label = '';
7999+
}
8000+
class B6Demo {
8001+
public function run(?Gadget $a, Gadget $b): void {
8002+
($a ?: $b)->label;
8003+
}
8004+
}
8005+
"#;
8006+
8007+
// Hover on `label` in `($a ?: $b)->label` (line 6, character 21)
8008+
let hover = hover_at(&backend, uri, content, 6, 21);
8009+
assert!(
8010+
hover.is_some(),
8011+
"hover should resolve the member through short ternary subject"
8012+
);
8013+
let text = hover_text(hover.as_ref().unwrap());
8014+
assert!(
8015+
text.contains("label"),
8016+
"hover should mention 'label', got: {}",
8017+
text
8018+
);
8019+
}
8020+
8021+
#[test]
8022+
fn hover_null_coalesce_member_access() {
8023+
let backend = create_test_backend();
8024+
let uri = "file:///b6_null_coalesce.php";
8025+
let content = r#"<?php
8026+
class Sensor {
8027+
public int $value = 0;
8028+
}
8029+
class B6Demo2 {
8030+
public function run(?Sensor $a, Sensor $b): void {
8031+
($a ?? $b)->value;
8032+
}
8033+
}
8034+
"#;
8035+
8036+
// Hover on `value` in `($a ?? $b)->value` (line 6, character 21)
8037+
let hover = hover_at(&backend, uri, content, 6, 21);
8038+
assert!(
8039+
hover.is_some(),
8040+
"hover should resolve the member through null-coalesce subject"
8041+
);
8042+
let text = hover_text(hover.as_ref().unwrap());
8043+
assert!(
8044+
text.contains("value"),
8045+
"hover should mention 'value', got: {}",
8046+
text
8047+
);
8048+
}
8049+
8050+
#[test]
8051+
fn hover_full_ternary_member_access() {
8052+
let backend = create_test_backend();
8053+
let uri = "file:///b6_full_ternary.php";
8054+
let content = r#"<?php
8055+
class Engine {
8056+
public function start(): void {}
8057+
}
8058+
class B6Demo3 {
8059+
public function run(bool $flag, Engine $a, Engine $b): void {
8060+
($flag ? $a : $b)->start();
8061+
}
8062+
}
8063+
"#;
8064+
8065+
// Hover on `start` in `($flag ? $a : $b)->start()` (line 6, character 28)
8066+
let hover = hover_at(&backend, uri, content, 6, 28);
8067+
assert!(
8068+
hover.is_some(),
8069+
"hover should resolve the member through full ternary subject"
8070+
);
8071+
let text = hover_text(hover.as_ref().unwrap());
8072+
assert!(
8073+
text.contains("start"),
8074+
"hover should mention 'start', got: {}",
8075+
text
8076+
);
8077+
}

0 commit comments

Comments
 (0)