Skip to content

Commit 52f7d45

Browse files
committed
Argument checking no longer slows down quadratically with file size
1 parent 3804765 commit 52f7d45

9 files changed

Lines changed: 202 additions & 72 deletions

File tree

docs/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
5757
- **Class origin classification no longer re-scans the whole classmap after the fact.** Startup used to look up each discovered class's completion-ranking origin (project, explicit dependency, transitive dependency, core stub) by re-reading and re-parsing `installed.json` a second time and prefix-matching every class's file path against the package list on a single thread. The origin is now attached to a class the moment it is discovered during the already-parallel vendor scan, the same way it already worked for functions and constants, removing both the duplicate parse and the serial pass.
5858
- **Faster `assert()`/type-guard narrowing during the forward walk.** Every statement used to build a fresh resolution context (including a scope clone) for each in-scope variable to check whether it was an `assert()` or `@phpstan-assert`/`@psalm-assert` call, even for statements that could never be one. Non-call statements now skip that work entirely, cutting a measurable share of the walk on methods with many locals and many statements.
5959
- **Faster diagnostics on method/function calls that resolve to no concrete class.** Checking whether such a call's result was actually a bare `object`/`?object` (still valid for member access) used to re-resolve the callee's whole receiver chain and method signature a second time from scratch. That check now reuses the resolution already performed, roughly halving diagnostics time on files with many unresolved or missing-method call chains.
60+
- **Argument checking no longer slows down quadratically with file size.** The argument-count and argument-type checks know the byte offset of every call they inspect, but used to convert it into an editor line/column position and immediately back again before looking up the called function. Each of those conversions re-read the file from the beginning to count characters, so the cost of checking one call grew with the size of the file around it and a file with thousands of calls spent nearly all its time on offset arithmetic. The offset is now used directly. On a 370 KB file containing 2200 calls the two checks together drop from 13.7 seconds to 0.2, taking the whole file from 16.7 seconds to 3.4, and analysing the project it belongs to is close to three times faster.
6061

6162
### Removed
6263

6364
- **Bundled Zed extension.** PHPantom's plain-PHP wiring has merged into Zed's official PHP extension, so a separate PHPantom extension is no longer needed. See [Editor Setup](editor-setup.md) for the updated Zed configuration.
6465

6566
### Fixed
6667

68+
- **Parameter-name inlay hints in Blade templates are no longer filtered against the wrong positions.** Deciding which hints fall inside the visible part of the file compared each argument's position in the compiled PHP against a viewport still measured in Blade coordinates, so hints could go missing or appear for arguments that were scrolled out of view. The already-translated viewport is now used.
6769
- **`static`/`$this` return types are no longer flagged when passed where a `Stringable` object is accepted.** Passing a value typed `static(Foo)` or `$this(Foo)` to a `string` parameter reported a type mismatch even when `Foo` implements `Stringable`, which PHP accepts by calling `__toString()`. This hit any use of `SimpleXMLElement`, whose magic `__get` returns `static`, so code like `(string) $xml->Body->Message` reported a false positive on every argument passed to a `string` parameter.
6870
- **Inline `{@see}` references nested inside other docblock text are now found.** An inline `{@see Foo}` written in the description of another tag (`@param Type $x see {@see Foo}`), or nested inside another inline tag (`{@deprecated use {@see Bar} instead}`), was invisible to go-to-definition, find references, and rename: the previous scan located a reference by searching for the next `}` after `{@see `, which stopped at the first brace it met rather than the one that actually closed the tag. Inline `{@see}` references are now read from the same PHPDoc parse tree as everything else in the docblock.
6971
- **Docblock navigation works in `@method` and `@property` tags written across several lines.** A tag whose type wrapped onto a continuation line, such as a `@method Collection<int, Item> fetchAll(Filter $filter)` broken after the `<`, only ever had its first line read. Everything after it was invisible: go-to-definition, find references, and rename did nothing on the method or property name, on the type arguments, or on the parameter types, and the truncated first line was reported as a class named `Collection<` that resolved to nothing. Docblock positions now come from the PHPDoc grammar itself, so every name in such a tag is navigable wherever it sits.

docs/todo.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@ unlikely to move the needle for most users.
108108
| D5 | [External tool diagnostic suppression actions](todo/diagnostics.md#d5-external-tool-diagnostic-suppression-actions) | Low | Low |
109109
| D15 | [Unused parameter diagnostic](todo/diagnostics.md#d15-unused-parameter-diagnostic) | Low | Low |
110110
| | **[Bug Fixes](todo/bugs.md)** | | |
111+
| B117 | [Per-file callable-target caches are keyed by call-expression text alone](todo/bugs.md#b117-per-file-callable-target-caches-are-keyed-by-call-expression-text-alone) | Medium | Low-Medium |
112+
| B118 | [`analyze` on `examples/laravel` reports one error, but CI requires none](todo/bugs.md#b118-phpantom_lsp-analyze-on-exampleslaravel-reports-one-error-but-ci-requires-none) | Low | Low |
111113
| | **[Code Actions](todo/actions.md)** | | |
112114
| A40 | [Generate method from call](todo/actions.md#a40-generate-method-from-call) | Medium-High | Medium |
113115
| A41 | [Create class from non-existing name](todo/actions.md#a41-create-class-from-non-existing-name) | Medium | Medium |
@@ -199,7 +201,6 @@ unlikely to move the needle for most users.
199201
| P30 | [Evaluate migrating parse/resolve/docblock pipeline to `mago-hir`](todo/performance.md#p30-evaluate-migrating-parseresolvedocblock-pipeline-to-mago-hir) (parked — re-evaluated at mago 1.45.0, still no `mago-hir` consumers upstream) | Medium-High | High |
200202
| P47 | [The resolved-class cache lock caps concurrent class resolution](todo/performance.md#p47-the-resolved-class-cache-lock-caps-concurrent-class-resolution) | Medium | Medium-High |
201203
| P16 | [Pre-parsed stub format (eliminate raw PHP embedding)](todo/performance.md#p16-pre-parsed-stub-format-eliminate-raw-php-embedding) | High | Medium-High |
202-
| P25 | [`type_mismatch_argument` / `argument_count_mismatch` slow on large single files](todo/performance.md#p25-type_mismatch_argument-argument_count_mismatch-slow-on-large-single-files) | Medium | Medium |
203204
| P22 | [Signature change re-queues slow diagnostics for every open file](todo/performance.md#p22-signature-change-re-queues-slow-diagnostics-for-every-open-file) | Medium-High | Medium |
204205
| P11 | [Uncached base-resolution in `build_scope_methods_for_builder`](todo/performance.md#p11-uncached-base-resolution-in-build_scope_methods_for_builder) | Low-Medium | Low |
205206
| P3 | Parallel pre-filter in `find_implementors` | Low-Medium | Medium |

docs/todo/bugs.md

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,58 @@ pipeline so it produces correct data. Downstream consumers
77
(diagnostics, hover, completion, definition) should never need
88
to second-guess upstream output.
99

10-
No outstanding items.
10+
## B117. Per-file callable-target caches are keyed by call-expression text alone
11+
12+
**Impact: Medium · Effort: Low-Medium**
13+
14+
Both argument diagnostic passes memoize `resolve_callable_target`
15+
in a per-file `HashMap<String, Option<ResolvedCallableTarget>>`
16+
keyed only on the call expression, then resolve the entry using
17+
whichever call site happened to be reached first. Callable-target
18+
resolution is position-dependent (it picks the enclosing class via
19+
`find_class_at_offset` and resolves receiver variables against the
20+
scope at the cursor), so any expression whose meaning varies within
21+
the file gets the wrong target at every site after the first:
22+
23+
- `argument_count.rs` caches *every* expression, including
24+
variable-based calls. Two methods that both call
25+
`$parser->parse(…)` on different types share one cached
26+
signature, so the argument count is checked against the wrong
27+
method.
28+
- `type_errors/mod.rs` deliberately skips variable-based calls
29+
(`expr.starts_with('$')`), but `self::`, `static::`, and
30+
`parent::` do not start with `$` and are equally
31+
position-dependent. In a file declaring two classes that each
32+
have a zero-argument `self::make()`, the second class resolves
33+
to the first class's method.
34+
35+
Both are latent today because the wrong target usually has a
36+
compatible signature, and a wrong resolution can produce either a
37+
false positive or a silently missed diagnostic.
38+
39+
The cache key must cover what resolution actually depends on.
40+
Restricting the cache to position-independent expressions (plain
41+
function calls and `Fqn::method` where the class part is not
42+
`self`/`static`/`parent`) is the smallest sound fix; keying on
43+
`(expr, enclosing_class_start_offset)` keeps more hits for the
44+
keyword forms. Note that per-site resolution is no longer
45+
expensive now that the offset round trip is gone, so dropping the
46+
unsound entries costs little.
47+
48+
---
49+
50+
## B118. `phpantom_lsp analyze` on `examples/laravel` reports one error, but CI requires none
51+
52+
**Impact: Low · Effort: Low**
53+
54+
`docs/CONTRIBUTING.md` lists
55+
`phpantom_lsp analyze --project-root examples/laravel --no-colour`
56+
as a required check and states it "must report `[OK] No errors`",
57+
but `examples/laravel/app/Demo.php` intentionally contains
58+
`Artisan::call('does:not-exist')` to demonstrate the
59+
`invalid_laravel_command` diagnostic, so the command reports
60+
`Found 1 error`. Either the demo needs a form that shows the
61+
diagnostic without failing the check (the way `examples/demo.php`
62+
keeps its intentional diagnostics out of the analyze gate), or the
63+
documented expectation needs to name the diagnostics the Laravel
64+
example is expected to produce.

docs/todo/performance.md

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -592,42 +592,6 @@ regardless since it is a few lines.
592592

593593
---
594594

595-
## P25. `type_mismatch_argument` / `argument_count_mismatch` slow on large single files
596-
597-
**Impact: Medium · Effort: Medium**
598-
599-
On very large PHP files with thousands of call sites, the
600-
`type_mismatch_argument` and `argument_count_mismatch` diagnostic
601-
passes dominate the per-file time. On PDepend's
602-
`src/Source/Language/PHP/AbstractPHPParser.php` a single file spent
603-
~11.7s in `type_mismatch_argument` and ~2.9s in
604-
`argument_count_mismatch`:
605-
606-
```
607-
⚠ slow file (15.6s): src/Source/Language/PHP/AbstractPHPParser.php
608-
scope=1.9s, fast=0.0s, unknown_class=0.0s, unknown_member=0.1s,
609-
unknown_function=0.0s, argument_count_mismatch=2.9s,
610-
type_mismatch_argument=11.6s, deprecated_usage=0.1s,
611-
unknown_variable=0.8s, invalid_class_kind=0.0s
612-
```
613-
614-
Both passes walk every call site and call
615-
`resolve_callable_target_with_args` / `resolve_callable_target`
616-
per site. Variable-based calls and calls with argument text are
617-
resolved fresh at every site (only zero-arg static/function calls
618-
are cached), so a file with thousands of `$obj->method($args)`
619-
call sites re-resolves the receiver type once per site. Look at
620-
memoizing receiver-type resolution per (expression, offset) within
621-
a file, or sharing the forward-walk scope snapshot across both
622-
passes so the receiver type is computed once.
623-
624-
A per-request callable-target cache now memoizes the
625-
class::method → target lookup, but it sits downstream of
626-
receiver-type resolution — the per-site receiver re-resolution
627-
this item describes is still unaddressed.
628-
629-
---
630-
631595
## P46. `mago-phpdoc-syntax` cannot parse `@method static (…) name()`
632596

633597
**Impact: Low · Effort: Low (upstream)**

src/diagnostics/argument_count.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -264,11 +264,12 @@ impl Backend {
264264
let resolved = call_cache
265265
.entry(expr.clone())
266266
.or_insert_with(|| {
267-
let position = crate::text_position::offset_to_position(
267+
self.resolve_callable_target_at_offset(
268+
expr,
268269
content,
269-
call_site.args_start as usize,
270-
);
271-
self.resolve_callable_target(expr, content, position, &file_ctx)
270+
call_site.args_start,
271+
&file_ctx,
272+
)
272273
})
273274
.clone();
274275

src/diagnostics/type_errors/mod.rs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -543,26 +543,23 @@ impl Backend {
543543
// (method-level template subs depend on per-site args);
544544
// only zero-arg calls can be cached.
545545
let resolved = if is_variable_call || call_args_text.is_some() {
546-
let position = crate::text_position::offset_to_position(
547-
content,
548-
call_site.args_start as usize,
549-
);
550-
self.resolve_callable_target_with_args(
546+
self.resolve_callable_target_with_args_at_offset(
551547
expr,
552548
content,
553-
position,
549+
call_site.args_start,
554550
&file_ctx,
555551
call_args_text,
556552
)
557553
} else {
558554
call_cache
559555
.entry(expr.clone())
560556
.or_insert_with(|| {
561-
let position = crate::text_position::offset_to_position(
557+
self.resolve_callable_target_at_offset(
558+
expr,
562559
content,
563-
call_site.args_start as usize,
564-
);
565-
self.resolve_callable_target(expr, content, position, &file_ctx)
560+
call_site.args_start,
561+
&file_ctx,
562+
)
566563
})
567564
.clone()
568565
};

src/inlay_hints.rs

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,13 @@ impl Backend {
8282
continue;
8383
}
8484

85-
self.emit_parameter_hints(call_site, content, range, &ctx, &mut hints);
85+
self.emit_parameter_hints(
86+
call_site,
87+
content,
88+
(range_start, range_end),
89+
&ctx,
90+
&mut hints,
91+
);
8692
}
8793

8894
// ── Closure / arrow function hints ──────────────────────────
@@ -408,22 +414,23 @@ impl Backend {
408414
}
409415

410416
/// Emit parameter-name and by-reference hints for a single call site.
417+
///
418+
/// `range` is the requested viewport as byte offsets, already
419+
/// translated to virtual PHP coordinates by the caller so it can be
420+
/// compared against the symbol map's argument offsets.
411421
fn emit_parameter_hints(
412422
&self,
413423
call_site: &CallSite,
414424
content: &str,
415-
range: Range,
425+
range: (u32, u32),
416426
ctx: &FileContext,
417427
hints: &mut Vec<InlayHint>,
418428
) {
419-
// Build a synthetic position from the call site's start so that
420-
// resolve_callable_target has a cursor context.
421-
let position = offset_to_position(content, call_site.args_start as usize);
422-
423-
let resolved = match self.resolve_callable_target(
429+
// The call site's start offset gives the resolver its cursor context.
430+
let resolved = match self.resolve_callable_target_at_offset(
424431
&call_site.call_expression,
425432
content,
426-
position,
433+
call_site.args_start,
427434
ctx,
428435
) {
429436
Some(r) => r,
@@ -435,8 +442,7 @@ impl Backend {
435442
return;
436443
}
437444

438-
let range_start = position_to_offset(content, range.start);
439-
let range_end = position_to_offset(content, range.end);
445+
let (range_start, range_end) = range;
440446

441447
// Build a set of parameter names consumed by named arguments so
442448
// positional arguments can be mapped to the remaining parameters.
@@ -616,11 +622,10 @@ impl Backend {
616622
// @template parameters are inferred from the sibling arguments
617623
// and substituted into parameter type hints (e.g. turning
618624
// `callable(T): T` into `callable(int): int`).
619-
let position = offset_to_position(content, representative_offset.unwrap_or(0) as usize);
620-
let resolved = match self.resolve_callable_target_with_args(
625+
let resolved = match self.resolve_callable_target_with_args_at_offset(
621626
&site.parent_call_expression,
622627
content,
623-
position,
628+
representative_offset.unwrap_or(0),
624629
ctx,
625630
call_args_text,
626631
) {

0 commit comments

Comments
 (0)