Skip to content

Commit 3db99e7

Browse files
committed
Filter stub constants by @removed version when setting PHP version
1 parent c27ab05 commit 3db99e7

18 files changed

Lines changed: 385 additions & 137 deletions

File tree

docs/ARCHITECTURE.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -544,11 +544,12 @@ PHPantom detects the target PHP version during `initialized`:
544544

545545
The detected version is stored on the `Backend` as a `PhpVersion(major, minor)`.
546546

547-
When parsing stubs (both class stubs via `find_or_load_class` and function stubs via `find_or_load_function`), the PHP version is passed through `DocblockCtx.php_version` to the extraction functions. Three filtering points apply:
547+
When parsing stubs (both class stubs via `find_or_load_class` and function stubs via `find_or_load_function`), the PHP version is passed through `DocblockCtx.php_version` to the extraction functions. Four filtering points apply:
548548

549549
- **Function-level:** `extract_functions_from_statements` checks the function's `#[PhpStormStubsElementAvailable]` attribute. If the version range excludes the target, the entire function is skipped. This handles duplicate function definitions (e.g. `array_combine` has separate signatures for PHP ≤7.4 and ≥8.0).
550550
- **Method-level:** `extract_class_like_members` applies the same check to methods. For example, `SplFixedArray::__serialize` (from PHP 8.2) is excluded when targeting PHP 8.1.
551551
- **Parameter-level:** `extract_parameters` filters individual parameters. For example, `array_map`'s untyped `$arrays` parameter (PHP 5.3–7.4) is excluded when targeting PHP 8.0+, leaving only the typed `array $array`.
552+
- **Constant-level:** `set_php_version` calls `is_stub_constant_removed` to evict constants whose `@removed X.Y` tag indicates they were removed at or before the target version (e.g. `MCRYPT_ENCRYPT` with `@removed 7.2`).
552553

553554
The attribute supports named arguments (`from: '8.0'`, `to: '7.4'`) and a positional argument (`'8.1'` treated as `from`). Both bounds are inclusive. A missing bound means unbounded in that direction.
554555

docs/CHANGELOG.md

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

6060
### Fixed
6161

62+
- **Version-gated stub constants now filtered.** Constants with `@removed` tags (e.g. `MCRYPT_ENCRYPT`, removed in PHP 7.2) are now excluded from completion and resolution when the project targets a newer PHP version. Previously only classes and functions were filtered.
6263
- **Go-to-definition.** Fixed a potential deadlock when navigating to a vendor class that hadn't been parsed yet.
6364
- **LSP no longer freezes under heavy editor activity.** Server-to-client requests (diagnostic refresh, progress token creation) could deadlock the service loop when the editor was simultaneously sending bursts of open/close/hover messages. All server-to-client requests are now either fire-and-forget or time-bounded, long-running handlers are cancellation-safe, and the process exits cleanly if the service loop ever terminates unexpectedly.
6465
- **Rename class preserves `self`, `static`, and `parent` keywords.** Renaming a class no longer replaces occurrences of `self::`, `static::`, or `parent::` with the new class name.

docs/todo/refactor.md

Lines changed: 87 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -212,25 +212,7 @@ Each item must include:
212212

213213
---
214214

215-
## Outstanding items
216-
217-
### R3. Backward `@var` scanner in `docblock/tags.rs` 🟠
218-
219-
`src/docblock/tags.rs` L520–620 (`find_var_raw_type_in_source`) and L1000–1100
220-
(`find_iterable_raw_type_in_source`) scan source lines backward with
221-
`lines().rev()`, manually tracking brace depth and sibling scopes via character
222-
counting. This reimplements scope analysis that the forward walker and
223-
`scope_collector/` already provide. Called from `call_resolution.rs` (L1251,
224-
L2793) and `array_shape.rs` (L519), making them load-bearing.
225-
226-
**Action:** Replace with forward walker variable resolution or ensure the
227-
forward walker captures `@var` annotations so these backward scanners become
228-
unnecessary.
229-
230-
**Files:** `src/docblock/tags.rs`, `src/completion/call_resolution.rs`,
231-
`src/completion/array_shape.rs`
232-
233-
---
215+
# Outstanding items
234216

235217
## Intentional overlap (reference, not actionable)
236218

@@ -258,4 +240,90 @@ A minor inefficiency, not a parallel system.
258240
back to linear scan of `uri_classes_index`. Covers race conditions during initial
259241
indexing. Legitimate safety net.
260242

243+
### `DIAGNOSTIC_SCOPE` vs `HOVER_SCOPE_CACHE`
244+
245+
Two caches storing the same data type (`ScopeSnapshotMap`) for different consumers.
246+
`DIAGNOSTIC_SCOPE` is ephemeral/per-pass (build all scopes for the whole file,
247+
discard on drop). `HOVER_SCOPE_CACHE` is persistent/per-method with content-hash
248+
invalidation. The split is intentional: diagnostics check every line so they
249+
build all scopes upfront; hover checks one spot so it lazily caches per-method.
250+
However, the `is_diagnostic_scope_active()` guard causes ~14 behavioral forks
251+
in the forward walker, meaning bugs fixed for one consumer may not manifest for
252+
the other. Unifying them would eliminate the divergent walker behavior.
253+
254+
### Consumer-gated caches
255+
256+
`CALLABLE_TARGET_CACHE`, `PARSE_CACHE`, and `ACTIVE_RESOLVED_CACHE` are activated
257+
only during diagnostic/analyse passes. Completion and hover re-resolve callable
258+
targets and re-parse files from scratch. Extending these to all consumers would
259+
avoid redundant work.
260+
261+
### `MIXIN_CACHE` never invalidated
262+
263+
`src/virtual_members/phpdoc.rs` `MIXIN_CACHE` is a thread-local
264+
`HashMap<String, Arc<ClassInfo>>` that caches resolved mixin classes. It is used
265+
by all consumers (completion, hover, diagnostics) but is never cleared during
266+
normal LSP operation. If a mixin class definition changes (e.g. a method is added
267+
to `Illuminate\Database\Eloquent\Builder`), the cache serves stale data until the
268+
thread dies. Needs content-hash or generation-counter invalidation.
269+
270+
---
271+
272+
## Redundant backwards text walkers
273+
274+
These functions in `src/completion/source/helpers.rs` scan backward with `rfind`
275+
to find the most recent assignment to a variable. The forward walker's scope map
276+
already tracks this information during its top-to-bottom pass.
277+
278+
### `extract_closure_return_type_from_assignment`
279+
280+
Uses `rfind("$fn = ")` backward from cursor, then parses closure return type
281+
from raw text. The forward walker already processes closure assignments and
282+
extracts return types from the AST node's type hint. Callers in
283+
`call_resolution.rs` (L1272) and `rhs_resolution.rs` (L2695) should read the
284+
variable's type from the scope map instead.
285+
286+
### `extract_first_class_callable_return_type`
287+
288+
Uses `rfind("$fn = ")` backward, then resolves `Foo::bar(...)` callable return
289+
type from text. The forward walker already handles first-class callable
290+
assignments. Callers in `call_resolution.rs` (L1292) and `rhs_resolution.rs`
291+
(L2718) should use the scope map.
292+
293+
### `extract_function_return_from_source`
294+
295+
Uses `rfind("/**")` backward to get `@return` type for functions not yet in
296+
`global_functions`. This is a fallback for unindexed functions and is harder to
297+
replace, but could be eliminated once all reachable functions are guaranteed to
298+
be indexed before resolution runs.
299+
300+
---
301+
302+
## SymbolMap data re-extracted from AST
303+
304+
The SymbolMap is built once per file in `update_ast_inner` and stores symbol
305+
spans, variable definitions, scope boundaries, and call sites. Several features
306+
re-parse the AST to extract information the SymbolMap already has.
307+
308+
### Variable definition lookup
309+
310+
The SymbolMap has `var_defs` with `find_var_definition()`, but
311+
`definition/variable/mod.rs` ignores it and does its own full AST walk via
312+
`find_variable_definition_in_program()`. The SymbolMap's `var_defs` are used
313+
elsewhere (hover, forward walk) but the go-to-definition feature duplicates
314+
the work.
315+
316+
### Variables-in-scope collection
317+
318+
`collect_variables_in_scope` in `completion.rs` re-parses the AST to find all
319+
variables visible at the cursor. The SymbolMap's `var_defs` already has every
320+
variable definition site with scope boundaries; this could be a filtered query
321+
on `var_defs` instead of a full AST walk.
322+
323+
### Scope boundary detection
261324

325+
The SymbolMap stores `scopes` (function/closure boundaries) and provides
326+
`find_enclosing_scope()`. Yet several code action helpers and diagnostic modules
327+
manually walk the AST to find the enclosing function/method body (e.g.
328+
`build_scope_map` in extract variable, inline variable, undefined variable
329+
diagnostics).

src/completion/call_resolution.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -762,6 +762,7 @@ impl Backend {
762762
resolved_class_cache: Some(&self.resolved_class_cache),
763763
function_loader: Some(&function_loader_cl),
764764
scope_var_resolver: None,
765+
is_in_static_method: false,
765766
};
766767

767768
let parsed = SubjectExpr::parse(expr);

src/completion/handler.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -944,6 +944,7 @@ impl Backend {
944944
resolved_class_cache: Some(&self.resolved_class_cache),
945945
function_loader: Some(&function_loader),
946946
scope_var_resolver: None,
947+
is_in_static_method: false,
947948
};
948949
let mut resolved = super::resolver::resolve_target_classes(
949950
&target.subject,
@@ -976,6 +977,7 @@ impl Backend {
976977
resolved_class_cache: Some(&self.resolved_class_cache),
977978
function_loader: Some(&function_loader),
978979
scope_var_resolver: None,
980+
is_in_static_method: false,
979981
};
980982
resolved = super::resolver::resolve_target_classes(
981983
&target.subject,

src/completion/resolver.rs

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,11 @@ pub(crate) struct ResolutionCtx<'a> {
179179
/// `resolve_variable_types` which would trigger a full method-body
180180
/// re-walk.
181181
pub scope_var_resolver: ScopeVarResolverFn<'a>,
182+
/// Whether the cursor is inside a `static` method body.
183+
/// When `true`, `$this` is not available and `SubjectExpr::This`
184+
/// resolves to nothing. Precomputed from the `SymbolMap` at the
185+
/// call site to avoid re-parsing the AST.
186+
pub is_in_static_method: bool,
182187
}
183188

184189
/// Bundles the common parameters threaded through variable-type resolution.
@@ -243,6 +248,7 @@ impl<'a> VarResolutionCtx<'a> {
243248
function_loader: self.loaders.function_loader,
244249
resolved_class_cache: self.resolved_class_cache,
245250
scope_var_resolver: self.scope_var_resolver,
251+
is_in_static_method: false,
246252
}
247253
}
248254

@@ -449,25 +455,9 @@ fn resolve_target_classes_expr_inner_impl(
449455
match expr {
450456
// ── Keywords that always mean "current class" ────────────
451457
SubjectExpr::This => {
452-
// `$this` is not available inside static methods. Check the
453-
// AST to see whether the cursor is inside a static method body
454-
// and return nothing if so.
455-
if current_class.is_some() {
456-
let cursor = ctx.cursor_offset;
457-
let content = ctx.content;
458-
let in_static = crate::parser::with_parsed_program(
459-
content,
460-
"this_static_check",
461-
|program, _| {
462-
crate::util::is_offset_in_static_method_in_program(
463-
&program.statements,
464-
cursor,
465-
)
466-
},
467-
);
468-
if in_static {
469-
return vec![];
470-
}
458+
// `$this` is not available inside static methods.
459+
if current_class.is_some() && ctx.is_in_static_method {
460+
return vec![];
471461
}
472462

473463
// Check for `@param-closure-this` override: when the cursor

src/definition/implementation.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,9 @@ impl Backend {
478478
resolved_class_cache: Some(&self.resolved_class_cache),
479479
function_loader: Some(&function_loader),
480480
scope_var_resolver: None,
481+
is_in_static_method: false,
481482
};
483+
482484
let candidates = ResolvedType::into_arced_classes(
483485
crate::completion::resolver::resolve_target_classes(&subject, access_kind, &rctx),
484486
);

src/definition/member/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ impl Backend {
133133
resolved_class_cache: Some(&self.resolved_class_cache),
134134
function_loader: Some(&function_loader),
135135
scope_var_resolver: None,
136+
is_in_static_method: false,
136137
};
137138
let candidates = ResolvedType::into_arced_classes(
138139
crate::completion::resolver::resolve_target_classes(subject, access_kind, &rctx),

src/definition/type_definition.rs

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -47,17 +47,29 @@ impl Backend {
4747
let function_loader = self.function_loader(&ctx);
4848

4949
let resolved_types: Vec<PhpType> = match &symbol.kind {
50-
SymbolKind::Variable { name } => resolve_variable_type_names(
51-
name,
52-
content,
53-
offset,
54-
current_class,
55-
&ctx,
56-
&class_loader,
57-
&function_loader,
58-
)
59-
.into_iter()
60-
.collect(),
50+
SymbolKind::Variable { name } => {
51+
let in_static = self
52+
.symbol_maps
53+
.read()
54+
.get(uri)
55+
.is_some_and(|map| map.is_in_static_method(offset));
56+
// $this is not available in static methods.
57+
if name == "this" && in_static {
58+
Vec::new()
59+
} else {
60+
resolve_variable_type_names(
61+
name,
62+
content,
63+
offset,
64+
current_class,
65+
&ctx,
66+
&class_loader,
67+
&function_loader,
68+
)
69+
.into_iter()
70+
.collect()
71+
}
72+
}
6173

6274
SymbolKind::MemberAccess {
6375
subject_text,
@@ -83,6 +95,7 @@ impl Backend {
8395
&function_loader as &dyn Fn(&str) -> Option<FunctionInfo>,
8496
),
8597
scope_var_resolver: None,
98+
is_in_static_method: false,
8699
};
87100

88101
let candidates = ResolvedType::into_arced_classes(
@@ -276,18 +289,8 @@ fn resolve_variable_type_names(
276289
class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
277290
function_loader: &dyn Fn(&str) -> Option<FunctionInfo>,
278291
) -> Option<PhpType> {
279-
// $this resolves to the enclosing class, but not in static methods.
292+
// $this resolves to the enclosing class.
280293
if name == "this" {
281-
let in_static =
282-
crate::parser::with_parsed_program(content, "this_static_typedef", |program, _| {
283-
crate::util::is_offset_in_static_method_in_program(
284-
&program.statements,
285-
cursor_offset,
286-
)
287-
});
288-
if in_static {
289-
return None;
290-
}
291294
return current_class.map(|cc| PhpType::Named(cc.name.to_string()));
292295
}
293296

src/diagnostics/deprecated.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ impl Backend {
170170
resolved_class_cache: Some(cache),
171171
function_loader: Some(&function_loader),
172172
scope_var_resolver: None,
173+
is_in_static_method: false,
173174
};
174175

175176
resolve_variable_subject(subject_text, *is_static, &rctx)

0 commit comments

Comments
 (0)