Skip to content

Commit e010571

Browse files
committed
Merge composer and self scanner, less misses and fater
1 parent 0d8b4cb commit e010571

9 files changed

Lines changed: 211 additions & 324 deletions

File tree

docs/ARCHITECTURE.md

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,7 @@ At resolution time, `merge_traits_into` loads the `UnitEnum` or `BackedEnum` stu
567567

568568
PSR-4 mappings come exclusively from the project's own `composer.json`. Vendor PSR-4 (`vendor/composer/autoload_psr4.php`) is not loaded. The Composer classmap is the sole source of truth for vendor code.
569569

570-
**Design principle:** if the classmap is missing or stale, vendor classes fail to resolve visibly rather than being silently papered over by PSR-4. This makes the problem obvious to the user (fix: run `composer dump-autoload`). User PSR-4 roots are walked by Go-to-implementation (Phase 5) and Find References because user files change between `dump-autoload` runs.
570+
**Design principle:** vendor PSR-4 mappings are not loaded separately. The Composer classmap (augmented by the self-scanner, see below) is the sole source of truth for vendor code. User PSR-4 roots are walked by Go-to-implementation (Phase 5) and Find References because user files change between `dump-autoload` runs.
571571

572572
### Self-Generated Classmap
573573

@@ -577,16 +577,18 @@ Scanning is parallelised using a two-phase approach: directory walks collect fil
577577

578578
The indexing strategy is configurable via `[indexing] strategy` in `.phpantom.toml`:
579579

580-
- **`"composer"`** (default) — use Composer's classmap when available and complete; fall back to self-scan when it is missing or incomplete (i.e. the project's own PSR-4 namespaces have no entries in the classmap).
581-
- **`"self"`** — always self-scan, ignoring Composer's classmap entirely.
580+
- **`"composer"`** (default) — merged classmap + self-scan. Load Composer's classmap (if it exists) as a skip set, then self-scan all PSR-4 and vendor directories for anything the classmap missed. Whatever the classmap already covers is a free performance win; whatever it's missing, we find ourselves. No completeness heuristic needed.
581+
- **`"self"`** — always self-scan, ignoring Composer's classmap entirely. Equivalent to the merged approach with an empty skip set.
582582
- **`"full"`** — same as `"self"` for now; reserved for future background indexing.
583-
- **`"none"`** — no proactive scanning; uses Composer's classmap if present but never falls back to self-scan.
583+
- **`"none"`** — no proactive scanning; uses Composer's classmap if present but never self-scans to fill gaps.
584+
585+
The merged pipeline works in three steps: (1) load `autoload_classmap.php` into a `HashMap<String, PathBuf>`, (2) collect the classmap's file paths into a `HashSet<PathBuf>` skip set, (3) self-scan all PSR-4 and vendor directories, skipping files already in the skip set. The result is a merged index: classmap entries for everything Composer already knew about, plus self-scanned entries for everything it missed. When the classmap is complete (the common case), the self-scanner walks directories but skips every file, finishing almost instantly. When the classmap is empty or absent, it falls back to a full self-scan. When the classmap is partial (e.g. vendor classes only), vendor files are skipped and only user code is scanned. Every state of the classmap helps.
584586

585587
When self-scanning with a `composer.json` present, the scanner reads `autoload.psr-4`, `autoload-dev.psr-4`, `autoload.classmap`, and `autoload-dev.classmap` to determine which directories to walk. PSR-4 directories are filtered: only classes whose FQN matches the namespace prefix plus the relative file path are included. Vendor packages are discovered from `vendor/composer/installed.json` (both Composer 1 and 2 formats); the JSON packages array is borrowed rather than cloned to avoid allocating a copy of the entire vendor manifest. All directory walkers (full-scan, PSR-4 scanner, vendor package scanner, and go-to-implementation file collector) use the `ignore` crate for gitignore-aware traversal. Hidden directories are skipped automatically, and `.gitignore` rules are respected at every level. When no `composer.json` exists at all, the scanner falls back to walking all `.php` files under the workspace root.
586588

587589
The result is a `HashMap<String, PathBuf>` in the same format as the existing `Backend.classmap`. Everything downstream (resolution, diagnostics, go-to-definition) works unchanged.
588590

589-
**Redundant I/O elimination:** `init_single_project` parses `composer.json` once and passes the pre-parsed `serde_json::Value` to both `build_self_scan_composer` and `is_classmap_incomplete_with_json`. Previously each function re-read and re-parsed the file independently.
591+
**Redundant I/O elimination:** `init_single_project` parses `composer.json` once and passes the pre-parsed `serde_json::Value` to `build_self_scan_composer`. Previously each function re-read and re-parsed the file independently.
590592

591593
**Vendor dir detection:** the `config.vendor-dir` setting is read from `composer.json` once during `initialized` (via `parse_composer_json`, which returns both the PSR-4 mappings and the vendor dir name). The absolute vendor directory path is cached on `Backend.vendor_dir_paths` and a `file://` URI prefix is stored in `Backend.vendor_uri_prefixes` for fast vendor-file detection at runtime. Both fields are `Vec`s to support monorepo workspaces with multiple subprojects (see below). For single-project workspaces, each collection has exactly one entry.
592594

docs/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
5252
- **Two-phase diagnostic publishing.** Cheap diagnostics (unused import dimming, deprecated strikethrough) are published immediately. Expensive diagnostics (unknown classes, unknown members, unknown functions) arrive in a second pass. Previously all diagnostics waited for the slowest collector to finish, so unused imports could take several seconds to grey out in large files.
5353
- **Concurrent read access to shared state.** All read-heavy maps now use `parking_lot::RwLock` instead of `std::sync::Mutex`, allowing multiple requests to read in parallel.
5454
- **Parallel workspace indexing.** Find References and other workspace-wide operations now parse files across multiple CPU cores. The workspace walk respects `.gitignore` rules instead of hardcoding directory names to skip. Self-scan classmap building, PSR-4 directory scanning, and vendor package scanning now read and scan files in parallel across all CPU cores. The byte-level PHP scanner uses `memchr` SIMD acceleration to skip comments, strings, and heredocs instead of scanning byte-by-byte. Redundant `composer.json` reads during initialization have been eliminated.
55+
- **Merged classmap + self-scan pipeline.** The Composer classmap and the self-scanner are no longer mutually exclusive. PHPantom always loads the Composer classmap (when present) and then self-scans all PSR-4 and vendor directories, skipping files the classmap already covers. Stale or incomplete classmaps are no longer a problem: whatever Composer already knew about is a free performance win, and whatever it missed is discovered automatically. The fragile completeness heuristic that decided whether to trust the classmap has been removed. Monorepo subprojects use the same merged pipeline.
5556

5657
### Fixed
5758

59+
- **Parallel file scanner panics no longer crash the server.** If a thread panics during workspace indexing (e.g. due to a malformed PHP file), the panic is caught and logged instead of killing the entire LSP process. Previously a single thread failure during class, PSR-4, or full-symbol scanning would propagate the panic and silently terminate PHP intelligence in the editor.
5860
- **Type alias array shape diagnostics no longer fire on object values.** When a method returns a `@phpstan-type` alias that expands to an array shape containing object values (e.g. `array{pen: Pen}`), accessing a method on the object value no longer triggers a false diagnostic. Type aliases are now expanded before walking array shape segments in the resolution pipeline.
5961
- **Inline array-element function calls resolve correctly in diagnostics.** `end($obj->items)->method()` no longer produces a false "unknown member" diagnostic. Previously the argument text for property access expressions was lost during symbol map extraction, causing the resolver to fall back to the native `mixed|false` return type instead of extracting the element type from the array's generic annotation.
6062
- **Eloquent Builder scope chain diagnostics no longer flicker.** Scope methods on Builder chains (e.g. `Bakery::where(...)->fresh()->get()`) no longer produce false diagnostics that flip on and off between edits. The diagnostic now checks the pre-resolved class returned by the completion pipeline before re-resolving through the cache, preventing stale cache entries from hiding context-specific virtual members.

docs/todo.md

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,17 +32,6 @@ with each step.
3232

3333
---
3434

35-
## Sprint 2 — Stabilise for 0.5.0 release
36-
37-
| # | Item | Effort | Domain | Doc Link |
38-
|---|---|---|---|---|
39-
| 106 | Thread panic in parallel file scanners crashes server | Low | Bug Fixes | [bugs.md §0](todo/bugs.md#0-thread-panic-in-parallel-file-scanners-crashes-the-server) |
40-
| 107 | Analyse sequential write-lock acquisition in `parse_and_cache_content_versioned` | Low | Bug Fixes | [bugs.md §1](todo/bugs.md#1-sequential-write-lock-acquisition-in-parse_and_cache_content_versioned) |
41-
42-
**After Sprint 2:** Tag 0.5.0, write release notes, publish.
43-
44-
---
45-
4635
## Sprint 3 — Quick wins: close the visible gaps
4736

4837
Every item here is Low or Low-Medium effort and directly removes
@@ -64,7 +53,6 @@ feature surface grows.
6453
| 22 | Selection Ranges (`textDocument/selectionRange`) | Low | LSP Features | [lsp-features.md §13](todo/lsp-features.md#13-selection-ranges-textdocumentselectionrange) |
6554
| 81 | Work-done progress for GTI and Find References | Low | LSP Features | [lsp-features.md §18](todo/lsp-features.md#18-work-done-progress-for-gti-and-find-references) |
6655
| 101 | Argument count diagnostic | Low | Diagnostics | [diagnostics.md §7](todo/diagnostics.md#7-argument-count-diagnostic) |
67-
| 105 | Merged classmap + self-scan | Low | Indexing | [indexing.md §1.5](todo/indexing.md#phase-15-merged-classmap--self-scan) |
6856
| 88 | Early-exit and `Cow` return in `apply_substitution` | Low | Performance | [performance.md §7](todo/performance.md#7-recursive-string-substitution-in-apply_substitution) |
6957
| 87 | Reference-counted `ClassInfo` (`Arc<ClassInfo>`) | Medium | Performance | [performance.md §2](todo/performance.md#2-reference-counted-classinfo-arcclassinfo) |
7058

docs/todo/bugs.md

Lines changed: 2 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -14,71 +14,7 @@ Items are ordered by **impact** (descending), then **effort** (ascending).
1414

1515
---
1616

17-
## 0. Thread panic in parallel file scanners crashes the server
18-
19-
**Impact: High — Effort: Low**
20-
21-
The three `scan_files_parallel_*` functions in `classmap_scanner.rs`
22-
join spawned threads with `.join().unwrap()`. If any spawned thread
23-
panics (e.g. due to a malformed file encountered during the byte-level
24-
scan), the `unwrap()` propagates the panic to the caller. Because
25-
these functions run during server initialization (outside any
26-
`catch_unwind` guard), a single thread panic kills the entire LSP
27-
process. The user's editor silently loses PHP intelligence with no
28-
error message.
29-
30-
The `scan_content`, `find_classes`, and `find_symbols` functions
31-
process arbitrary user and vendor files from disk. While they are
32-
designed to be robust, a panic in any of them (or in future code
33-
changes) would be fatal.
34-
35-
### Fix
36-
37-
Replace `.join().unwrap()` with `.join().unwrap_or_default()` (or
38-
`.join().ok()` with appropriate flattening) and log an error for
39-
any thread that panicked. The affected scan results are simply
40-
skipped, matching the existing behaviour of `fs::read` failures
41-
which are silently ignored.
42-
43-
Three call sites:
44-
- `scan_files_parallel_classes` (L407)
45-
- `scan_files_parallel_psr4` (L467)
46-
- `scan_files_parallel_full` (L537)
47-
48-
---
49-
50-
## 1. Sequential write-lock acquisition in `parse_and_cache_content_versioned`
51-
52-
**Impact: Low-Medium — Effort: Low**
53-
54-
`parse_and_cache_content_versioned` in `resolution.rs` acquires write
55-
locks on `ast_map`, `use_map`, `namespace_map`, `fqn_index`, and
56-
`resolved_class_cache` in sequence. Each lock is acquired and released
57-
individually (not nested), so there is no deadlock risk with the
58-
current `parking_lot` implementation. However, holding one write lock
59-
while acquiring the next creates brief windows where readers of the
60-
earlier-released lock see updated data but readers of the
61-
not-yet-written lock see stale data.
62-
63-
In practice this is unlikely to cause user-visible issues because
64-
all five writes complete within microseconds and all consumers
65-
re-read the maps on each request. But the pattern is fragile: if a
66-
future change adds a reader that checks two of these maps for
67-
consistency within the same request, it could observe a partial
68-
update.
69-
70-
### Analysis needed
71-
72-
Audit the code paths that read multiple maps in a single request
73-
to confirm no consumer relies on cross-map consistency. If none
74-
does, document the invariant ("these maps are eventually consistent
75-
within a single `update_ast` call but not atomically consistent")
76-
and close the item. If a consumer is found, batch the writes under
77-
a single coordination mechanism.
78-
79-
---
80-
81-
## 2. Native type hints not considered in virtual property specificity ranking
17+
## 0. Native type hints not considered in virtual property specificity ranking
8218

8319
**Impact: Low-Medium — Effort: Medium**
8420

@@ -100,4 +36,4 @@ but to the general type resolution pipeline. Fixing it here would not help
10036
because the native vs docblock decision happens upstream in the parser.
10137

10238
This is out of scope for the virtual member specificity work but worth
103-
tracking as a separate improvement to `resolve_effective_type`.
39+
tracking as a separate improvement to `resolve_effective_type`.

docs/todo/indexing.md

Lines changed: 17 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ Four indexing strategies, selectable via `.phpantom.toml`:
5959

6060
```toml
6161
[indexing]
62-
# "composer" (default) - use composer classmap, self-scan on fallback
62+
# "composer" (default) - merged classmap + self-scan
6363
# "self" - always self-scan, ignore composer classmap
6464
# "full" - background-parse all project files for rich intelligence
6565
# "none" - no proactive scanning
@@ -68,17 +68,18 @@ strategy = "composer"
6868

6969
### `"composer"` (default)
7070

71-
Use Composer's classmap when available and complete. Fall back to
72-
self-scan when the classmap is missing or incomplete. This is the
73-
zero-config experience. Phase 1.5 will merge these into a single
74-
pipeline where the classmap acts as a skip set for the self-scanner.
71+
Merged classmap + self-scan. Load Composer's classmap (if it exists)
72+
as a skip set, then self-scan all PSR-4 and vendor directories for
73+
anything the classmap missed. Whatever the classmap already covers is
74+
a free performance win; whatever it's missing, we find ourselves. No
75+
completeness heuristic needed. This is the zero-config experience.
7576

7677
### `"self"`
7778

7879
Always build the classmap ourselves. Ignores `autoload_classmap.php`
79-
entirely. For users who prefer PHPantom's own scanner or who are
80-
actively editing `composer.json` dependencies. After Phase 1.5, this
81-
is equivalent to the merged approach with an empty skip set.
80+
entirely. Equivalent to the merged approach with an empty skip set.
81+
For users who prefer PHPantom's own scanner or who are actively
82+
editing `composer.json` dependencies.
8283

8384
### `"full"`
8485

@@ -94,8 +95,7 @@ completion items. Memory usage grows proportionally to project size.
9495
No proactive file scanning. Still uses Composer's classmap if present,
9596
still resolves classes on demand when the user triggers completion or
9697
hover, still has embedded stubs. The only difference from `"composer"`
97-
is that it never falls back to self-scan when the classmap is missing
98-
or incomplete. This is essentially what PHPantom does today.
98+
is that it never self-scans to fill gaps.
9999

100100
---
101101

@@ -109,70 +109,13 @@ configuration, and user feedback messages.
109109
---
110110

111111
## Phase 1.5: Merged classmap + self-scan
112-
**Impact: High · Effort: Low**
113-
114-
Instead of treating the Composer classmap and the self-scanner as
115-
mutually exclusive strategies (use the classmap when it looks complete,
116-
fall back to self-scan when it doesn't), merge them into a single
117-
pipeline that always runs:
118-
119-
1. **Load the Composer classmap** if it exists. Don't validate it,
120-
don't check whether it's complete, don't check whether it's stale.
121-
Just parse `autoload_classmap.php` into the `HashMap<String, PathBuf>`
122-
as today.
123-
2. **Self-scan all PSR-4 directories** (and vendor packages), but
124-
**skip every file that already appears in the classmap**. The
125-
classmap values are absolute paths, so the skip check is a
126-
`HashSet::contains` on each walked path.
127-
3. The result is a merged index: classmap entries for everything
128-
Composer already knew about, plus self-scanned entries for
129-
everything it missed.
130-
131-
### Why this is better
132-
133-
- **No staleness problem.** If the classmap is out of date (new
134-
classes added, old classes removed, files moved), the self-scanner
135-
picks up the difference. We never serve stale data.
136-
- **No completeness check.** The current code inspects the classmap
137-
to decide whether it's "complete enough" to trust. That heuristic
138-
is fragile and was the source of the old `composer dump-autoload -o`
139-
prompts. With the merged approach, we don't care. Whatever the
140-
classmap has is a free performance win; whatever it's missing, we
141-
find ourselves.
142-
- **Faster in every scenario.** When the classmap is complete (the
143-
common case), the self-scanner walks directories but skips every
144-
file, finishing almost instantly. When the classmap is empty or
145-
absent, we fall back to a full self-scan exactly as today. When
146-
the classmap is partial (e.g. vendor classes only), we skip vendor
147-
files and scan only user code. Every state of the classmap helps.
148-
- **Less code.** The `"composer"` and `"self"` strategies collapse
149-
into one path. The completeness-checking logic in `initialized`
150-
can be removed. The `"none"` strategy remains for users who want
151-
no proactive scanning at all.
152-
153-
### Implementation
154-
155-
1. In `initialized`, always load `autoload_classmap.php` when it
156-
exists and collect the file paths into a `HashSet<PathBuf>`.
157-
2. Pass the skip set to `scan_psr4_directories` and
158-
`scan_vendor_packages`. Before reading and scanning a file,
159-
check whether its canonical path is in the skip set. If so,
160-
skip it.
161-
3. Remove the "is the classmap complete?" branching. The pipeline
162-
is always: load classmap → self-scan with skip set → merge.
163-
4. Remove the `"self"` strategy mode (or keep it as an alias for
164-
the merged approach with an empty skip set, ignoring the
165-
classmap entirely). Update `"composer"` to mean "merged."
166-
5. Update the strategy mode documentation and log messages.
167-
168-
### Effect on Phase 2
169-
170-
Phase 2 (staleness detection and auto-refresh) becomes much simpler.
171-
Single-file `did_save` refresh still applies (re-scan the saved file,
172-
update its classmap entries). But the expensive "did the whole
173-
classmap go stale?" question disappears. On `composer.lock` change,
174-
re-running the merged pipeline with the new classmap as the skip set
175-
is sufficient.
112+
113+
No outstanding items. Implemented: the Composer classmap and the
114+
self-scanner run as a single merged pipeline. The classmap file paths
115+
serve as a skip set for the self-scanner, so whatever Composer already
116+
covers is a free performance win and whatever it missed is discovered
117+
automatically. The completeness heuristic has been removed. Monorepo
118+
subprojects use the same merged pipeline.
176119

177120
---
178121

0 commit comments

Comments
 (0)