Repository navigation
feat(lsp): full editor tooling -- LSP server + PhpStorm plugin - #8
Merged
Merged
Conversation
We now have two packages (core + tools/lsp), with a JetBrains plugin
landing under tools/phpstorm-plugin/ next. Lock the conventions in
writing and reshape CI to match.
CI:
- .github/workflows/ci-core.yml (new) -> phpunit + infection jobs for
the core compiler (src/, test/, root composer.json).
- .github/workflows/ci-lsp.yml (renamed from ci.yml, scoped down) ->
phpunit-lsp + infection-lsp jobs for tools/lsp/.
- Both workflows run unconditionally on every PR and every push to
main; concurrency is per-workflow so they execute in parallel.
- No `paths:` filters at the workflow level. GitHub's branch
protection requires named status checks to actually run; a path-
filtered workflow that doesn't trigger reports "expected, never
received" and blocks the merge. The CI minutes cost is small (jobs
are parallel, each ~30s); the coordination win is worth more.
- A third package (tools/phpstorm-plugin/) adds a third workflow
file in the same shape; the contract is documented in
CONTRIBUTING.md.
Documentation:
- CONTRIBUTING.md gains a "Monorepo layout" section before
"Testing": the tree at a glance, a what-goes-where table, the
underlying principle ("core is the product; tools are ways to
consume it"), a six-step recipe for adding a new package, and an
explanation of the no-path-filters CI choice so the next reader
doesn't try to "optimise" it back.
- The recipe explicitly names the JetBrains plugin convention
(tools/phpstorm-plugin/) so the next contributor knows where
to put it.
No code changes; no impact on test counts, MSI gates, or the
playground harness.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Matches the per-package CI workflow split from 435b67a. Each package owns its own Makefile as the single source of truth for its recipes; the root Makefile holds core/xphp commands only. Changes: - tools/lsp/Makefile (new) -- short, package-relative target names: test (was root's test/lsp) test/mutation (was root's test/lsp/mutation) build-extension (was root's build/lsp-extension) The infection.phar lazy-download recipe moves here too, with the path relative to tools/lsp/ (var/infection.phar). Invocation from the repo root: `make -C tools/lsp <target>`. From inside the package: `make <target>`. - Makefile (root) loses every LSP-specific target. Keeps test/unit, test/mutation, playground. Header comment points future readers at the per-package convention and CONTRIBUTING.md. - .github/workflows/ci-lsp.yml updates `make test/lsp` to `make -C tools/lsp test` and `make test/lsp/mutation` to `make -C tools/lsp test/mutation`. ci-core.yml is unchanged (core commands still live at root). - tools/lsp/README.md `make test/lsp...` references swap to `make -C tools/lsp <target>` (with the `cd tools/lsp && make <target>` alternate form documented alongside). - CONTRIBUTING.md monorepo-recipe step 4 rewritten: each package ships its OWN Makefile rather than adding targets to the root. Explicitly notes that there is no root delegation layer, so the per-package Makefile can't drift from the root. Smoke-tested: make test/unit -> OK (171 / 453) make -C tools/lsp test -> OK (131 / 299) make test/lsp -> error (correctly removed) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The xphp-lsp binary is a Composer-managed working tree: an entry script
under bin/, src/ at the package, and a parent-package path-repo symlink
into vendor/xphp-lang/xphp-parser. That's the right shape for a library
the LSP package author keeps editing, but the wrong shape for an editor
plugin that wants to bundle a single artifact and run it from disk.
Add `make -C tools/lsp build/phar` producing tools/lsp/var/xphp-lsp.phar
-- a ~2 MB self-contained PHAR the upcoming JetBrains plugin extracts at
first plugin load. Output behavior is byte-for-byte identical to the bin
entry on a known bound violation and on a clean file (see README).
Mechanics:
- tools/lsp/box.json -- minimal Humbug Box config (main=bin/xphp-lsp,
output=var/xphp-lsp.phar, GZ compression). Box's defaults handle the
rest. Three of the original keys triggered schema errors on Box 4.6
(git-version expects string|null, not bool) and were removed.
- tools/lsp/Makefile gains a `build/phar` target with the same lazy-
download pattern as `test/mutation`: box.phar 4.6.6 lands in var/
on first run, then runs against a --no-dev classmap-authoritative
install. Box and Infection are both packaged-as-PHAR rather than
composer-required for the same reason: psr/log ^1.0 (phpactor) vs
^2|^3 (modern tooling) makes Composer-resolved coexistence painful.
- The path-repo entry in composer.json pins "symlink": true for the
live-edit dev workflow. PHARs can't traverse symlinks, so Box would
otherwise embed a dangling pointer to xphp-parser and the runtime
couldn't resolve XPHP\Transpiler\* . The build target swaps the
symlinked vendor/xphp-lang/xphp-parser for a real copy of src/ +
composer.json (the only paths xphp-parser's PSR-4 autoload reaches),
re-dumps the classmap, then restores the symlinked dev install at
the end. Net: building the PHAR doesn't disturb the dev tree.
Verification (matches plan's chunk-1 step 1):
make -C tools/lsp build/phar
php tools/lsp/var/xphp-lsp.phar --lint <bound-violation.xphp> # exit 1, prints diagnostic
php tools/lsp/var/xphp-lsp.phar --lint <ok.xphp> # exit 0, silent
# both match tools/lsp/bin/xphp-lsp --lint <same files> exactly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Empty IntelliJ Platform plugin under tools/phpstorm-plugin/ -- no editor
behaviour yet, but `./gradlew build` resolves PhpStorm 2026.1.2, compiles
the lone Kotlin source, runs the (empty) test set, and produces a
verifiable plugin jar with the patched plugin.xml carrying the right
since-build / version pins. Chunks 3-5 wire the actual functionality
on top of this baseline.
The build matrix that took the longest to settle:
* IntelliJ Platform Gradle Plugin 2.16.0 requires Gradle 9.x -- pinning
the wrapper to 9.0.0 (verified SHA-256). Earlier 2.x lines were on
Gradle 8 but only 2.16 supports the 2026.1 IDE distribution.
* The settings-level `org.jetbrains.intellij.platform.settings` plugin
is meant to add `intellijPlatform { defaultRepositories() }` to
settings.gradle.kts's `dependencyResolutionManagement`, but its
Kotlin DSL accessor doesn't generate reliably -- settings scripts
compile before their `plugins {}` block applies, so
`intellijPlatform` resolves to "unresolved reference". Worked
around by declaring the project's repos in build.gradle.kts under
`repositories { intellijPlatform { defaultRepositories() } }` (the
main plugin's accessor generates cleanly there), and setting
`repositoriesMode = PREFER_PROJECT` in settings.gradle.kts so
Gradle 9's stricter default doesn't reject the project-side repos.
* `create(IntelliJPlatformType.PhpStorm, "2026.1")` had to become
`create(IntelliJPlatformType.PhpStorm, "2026.1.2")` -- JetBrains
publishes patch-versioned binary releases (2026.1.2 is the latest
on the 2026.1 line), and the plugin's resolution machinery wants
the full triplet, not just the major.minor.
* Kotlin 2.0.21 vs 2.3.x metadata: PhpStorm 2026.1.2's bundled
kotlin_module files have binary metadata version 2.3, so the plugin
Kotlin compiler must match. Bumped to 2.3.21.
Files:
- settings.gradle.kts -- pluginManagement, rootProject.name,
repositoriesMode hand-over. Lengthy comment explains why the
settings-level repos block is empty.
- build.gradle.kts -- IntelliJ Platform 2.16.0 + Kotlin 2.3.21,
PhpStorm dep with bundled `com.jetbrains.php`, pluginVerifier,
zipSigner, Platform test framework wired for chunk 3+.
- gradle.properties -- every version pin lives here. since-build
261, until-build 262.*, javaVersion 21, kotlinVersion 2.3.21,
platformVersion 2026.1.2.
- src/main/resources/META-INF/plugin.xml -- id, name, vendor,
description, `<depends>com.jetbrains.php</depends>`, empty
`<extensions/>` block. `<idea-version/>` is omitted on purpose;
`patchPluginXml` injects it from gradle.properties at build time.
- src/main/kotlin/com/xphp/lsp/XphpPluginScaffold.kt -- marker class
so the source set isn't phantom-empty; replaced by real extension
classes in chunks 3/4.
- Makefile -- thin shims over ./gradlew {build, test, verify,
run-ide, clean}. Mirrors tools/lsp/Makefile's per-package shape.
- README.md -- chunk roadmap, requirements, dev workflow.
- gradlew + gradlew.bat + gradle-wrapper.jar + .properties at the
Gradle 9.0.0 vintage. Distribution SHA-256 pinned in .properties.
- .gitignore -- .gradle/, build/, .intellijPlatform/, and the
generated-at-build-time resources (chunk 3 + chunk 5 will copy
tmLanguage.json + xphp-lsp.phar from tools/lsp/).
Verified end-to-end inside an `eclipse-temurin:21-jdk` container:
./gradlew build -> BUILD SUCCESSFUL (12 actionable tasks)
Output: build/libs/xphp-phpstorm-plugin-0.1.0.jar (2.9 KB)
patchPluginXml emitted: <idea-version since-build="261" until-build="262.*"/>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…chunk 3/7)
PhpStorm now opens .xphp files as a first-class file type instead of plain
text. Highlighting in chunk 3 itself stays minimal -- the LSP server's
semantic tokens (wired in chunk 4) are the better source than a hand-rolled
SyntaxHighlighter -- but the TextMate grammar from the VS Code extension
ships in the plugin jar so a future TextMate-bundle hookup has zero
preparation cost.
Files:
- src/main/kotlin/com/xphp/lsp/XphpLanguage.kt -- `Language("xphp", ...)`
singleton, the identifier the LSP descriptor and any later structure /
inspection bindings dispatch on. `readResolve()` preserves singleton
identity across the IntelliJ Platform's serialization paths.
- src/main/kotlin/com/xphp/lsp/XphpFileType.kt -- `LanguageFileType(XphpLanguage)`
with extension "xphp". Singleton-as-Kotlin-object exposes an `INSTANCE`
field that plugin.xml's `fileType ... fieldName="INSTANCE"` references.
- src/main/resources/META-INF/plugin.xml -- adds the `<fileType>`
registration. No parser-definition / no syntax-highlighter registered:
chunk 4's LSP semantic tokens deliver xphp-aware highlighting, and a
stub hand-coded highlighter would only collide with that.
- src/main/kotlin/com/xphp/lsp/XphpPluginScaffold.kt -- removed; the
Language + FileType singletons replace its purpose of "give the source
set a class to compile".
- build.gradle.kts -- `processResources { from(tmLanguageSource) ... }`
bundles `../lsp/vscode-extension/syntaxes/xphp.tmLanguage.json` into
the jar at `textmate/xphp.tmLanguage.json`. Single source of truth:
the VS Code extension's grammar file is the authoritative copy; both
editors load the same bytes.
Hooked into `processResources` directly via `from(...)` rather than as
a separate Copy task -- Gradle 9's strict input/output validation flags
any indirect path from a sibling task's output to `:jar`'s resource
input, and integrating here makes the dependency explicit by
construction. Falls back to a warning if the source file is missing
(e.g. when the LSP package hasn't been checked out alongside the
plugin, for whatever reason).
- build.gradle.kts -- drops `intellijPlatform { testFramework(Platform) }`.
Pure JUnit 5 tests don't need IntelliJ's `PathClassLoader`, and pulling
it in over a vanilla test runtime made Gradle's test executor fail with
`Test process encountered an unexpected problem`. When we genuinely
need `BasePlatformTestCase` it'll live in its own source set with its
own task -- the verifier already covers structural plugin validation
without booting the IDE.
- src/test/kotlin/com/xphp/lsp/XphpFileTypeTest.kt -- four pure-data
assertions: language id, singleton identity via `readResolve`,
extension + name + description, file-type-points-back-at-language.
Verified inside an `eclipse-temurin:21-jdk` container against the full
repo mount (so the grammar source is reachable):
./gradlew clean build -> BUILD SUCCESSFUL
Test report: 4 tests, 0 failures, 0 errors, 0 skipped.
Output jar entries:
META-INF/plugin.xml
com/xphp/lsp/XphpFileType.class
com/xphp/lsp/XphpLanguage.class
textmate/xphp.tmLanguage.json
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…k 4/7)
Opens a .xphp file in PhpStorm and the IntelliJ Platform LSP machinery
spawns the configured xphp-lsp binary over stdio, gets back diagnostics +
hover + go-to-definition + completion + semantic tokens, and surfaces all
of them through the standard IDE editor. Configuration today: a manual
absolute path under Preferences -> Tools -> xPHP. Chunk 5 fills the
bundled-PHAR fallback so empty-path means "use the one shipped in the
plugin jar".
Why the explicit-path-only approach for this chunk: it surfaces plumbing
problems crisply. If the LSP can't start, the user sees an error that
points right at the settings pane. Once chunk 5's bundled-PHAR fallback
lands and we silently fall back to the embedded server, that signal goes
away -- so chunk 4 ships first, gets manually exercised end-to-end
(`make run-ide` -> open Box.xphp -> watch the diagnostic appear), then
chunk 5 builds on a known-good baseline.
Files:
- settings/XphpSettings.kt -- application-level
`PersistentStateComponent` with one string field, persists to
<config>/options/xphp.xml. App-scope (not project-scope) because a
developer working across multiple xphp projects on one PhpStorm
install almost certainly wants the same LSP binary for all of them.
Empty string means "no override"; the property accessor returns null
so chunk 5 can fall through to the bundled PHAR.
- settings/XphpSettingsConfigurable.kt -- Preferences UI under
Tools -> xPHP, a single `TextFieldWithBrowseButton` bound to
settings.lspPath via the Kotlin UI DSL. Comment under the field
explains both modes (PHAR vs live script).
- XphpLspServerSupportProvider.kt -- the platform's entry point.
`fileOpened` filters on XphpFileType and calls
`serverStarter.ensureServerStarted(descriptor)` -- de-duplicates
across file opens so 20 tabs spawn exactly one server.
- XphpLspServerDescriptor.kt -- `ProjectWideLspServerDescriptor`
(not per-document; the LSP analyses cross-file). `createCommandLine`
distinguishes PHAR (launch via `php <path>`) from shell script
(launch directly) by extension. Working directory = project base
path so the server's relative-path resolution lines up with what
the user sees in PhpStorm's project tree. An unset settings.lspPath
raises a startup error pointing at the settings pane.
- plugin.xml -- adds four extension-point entries:
applicationService (XphpSettings)
applicationConfigurable (Preferences -> Tools -> xPHP)
platform.lsp.serverSupportProvider (XphpLspServerSupportProvider)
File-type registration from chunk 3 stays as-is.
Verified inside an `eclipse-temurin:21-jdk` container against the full
repo mount:
./gradlew clean build -> BUILD SUCCESSFUL
Output jar entries (chunk-4-relevant):
com/xphp/lsp/XphpLspServerSupportProvider.class
com/xphp/lsp/XphpLspServerDescriptor.class
com/xphp/lsp/settings/XphpSettings.class
com/xphp/lsp/settings/XphpSettings$State.class
com/xphp/lsp/settings/XphpSettings$Companion.class
com/xphp/lsp/settings/XphpSettingsConfigurable.class
Patched plugin.xml carries the four extension entries verbatim.
Manual end-to-end (the bit only `make run-ide` can do) is deferred to
the developer's workstation -- launching a sandbox IDE inside this
container would download another GB+ of JBR which the docker mount
isn't set up for. CI in chunk 6 verifies binary compatibility via
`verifyPlugin`; behavioural validation stays manual.
Out-of-scope items called out in the descriptor comments for future
chunks: project-scoped overrides, server lifecycle telemetry, multi-
root workspace handling.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… (chunk 5/7)
End-state: a freshly installed plugin "just works" with no configuration.
First time PhpStorm opens a .xphp file, PharExtractor pulls the bundled
PHAR out of the plugin jar's classpath, writes it to
<system>/xphp/xphp-lsp.phar with an atomic move + sha256 sidecar, and the
LSP descriptor launches `php <extracted-path>` over stdio. Subsequent
opens hit the on-disk hash check and skip the copy. Plugin upgrades
bring a different sha and re-extract automatically.
Why we extract instead of running the PHAR straight from the jar URL:
`php` reads PHARs as real files because the PHAR runtime parses its own
offsets relative to the file location; it can't open
`jar:file:!/bin/xphp-lsp.phar` URIs. Copying once is the cost of
admission.
Files:
- src/main/kotlin/com/xphp/lsp/PharExtractor.kt -- `@Service(APP)`
that reads `/bin/xphp-lsp.phar` from the classpath, sha256s it,
compares against the sidecar at
PathManager.getSystemDir()/xphp/xphp-lsp.phar.sha256, and writes
via a temp file + ATOMIC_MOVE to keep a crash mid-write from
leaving a partial PHAR. Returns null when the plugin jar carries
no bundled PHAR (dev builds where the LSP wasn't compiled yet).
- src/main/kotlin/com/xphp/lsp/XphpLspServerDescriptor.kt -- the
resolution order is now (1) `XphpSettings.lspPath` if the user
set it, (2) `PharExtractor.getInstance().extract()`, (3) error.
Chunk 4 only had (1) + (3); the user-facing change is that an
empty settings field now silently does the right thing.
- src/test/kotlin/com/xphp/lsp/PharExtractorTest.kt -- four cases
using a `private inner class TestExtractor` that mirrors the
production class's file-IO loop but takes a deterministic byte
array + temp-dir target. We deliberately avoid `BasePlatformTestCase`:
the contract under test is "what does this state machine do given
these inputs", not "how does it behave inside an IntelliJ
application", and a heavyweight platform test would obscure the
file-IO invariants the assertions actually depend on.
Covers: first-run extraction, no-op when bytes unchanged
(assertion on preserved mtime), re-extract when bundled bytes
change between versions, and the null-stream path.
- build.gradle.kts -- `processResources` `from(xphpLspPhar)`
bundles `../lsp/var/xphp-lsp.phar` at `bin/xphp-lsp.phar` in the
jar. Same pattern as the tmLanguage copy: a missing artifact
warns and produces an LSP-less plugin where the user must
manually configure the binary, rather than failing the build.
CI (chunk 6) runs `make -C tools/lsp build/phar` before assembling
the plugin.
Verified inside an `eclipse-temurin:21-jdk` container:
make -C tools/lsp build/phar -> 2.0 MB tools/lsp/var/xphp-lsp.phar
./gradlew clean build -> BUILD SUCCESSFUL
Test report: 8 tests total (4 chunk 3 + 4 chunk 5), 0 failures.
Output jar entries (new):
bin/xphp-lsp.phar 2,053,674 bytes
textmate/xphp.tmLanguage.json 1,330 bytes
com/xphp/lsp/PharExtractor.class
com/xphp/lsp/PharExtractor$Companion.class
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Third workflow alongside ci-core.yml + ci-lsp.yml, following the same
per-package shape:
* one file per package
* runs unconditionally on every PR + push to main (no path filters --
branch protection requires named status checks to actually run, see
CONTRIBUTING.md "CI: one workflow file per package, no path filters")
* per-workflow concurrency group
Two jobs:
gradle-build:
1. PHP 8.4 + composer (needed to build the bundled xphp-lsp.phar
that processResources copies into the plugin jar at build time --
same `make -C tools/lsp build/phar` ci-lsp uses).
2. JDK 21 + Gradle (cached via gradle/actions/setup-gradle@v4 so
the IDE distribution download is reused across runs).
3. `make -C tools/phpstorm-plugin build` -> compileKotlin, runs
JUnit 5 tests, packages the plugin jar.
4. Uploads the plugin jar as an artifact (so a maintainer can grab
a release-ready jar straight from a green PR run) and the test
reports on failure for triage.
verify-plugin:
1. JDK 21 + Gradle only -- the verifier reads compiled classes
against the IDE classpath; no PHP needed.
2. `make -C tools/phpstorm-plugin verify` -> the IntelliJ Plugin
Verifier compatibility check across the IDE matrix configured
in build.gradle.kts (`pluginVerification.ides { recommended() }`
resolves to the recommended set for since-build 261).
3. Uploads verifier reports on failure.
The verify job depends on gradle-build so a compile / test failure
short-circuits without burning the verifier's longer IDE-matrix
download.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…hunk 7/7)
End of the phpstorm-plugin track. All seven chunks shipped; the package
now sits alongside tools/lsp/ as a self-contained sub-project that
follows the same conventions, with the same CI shape, and a public-
facing README that doesn't lean on internal context.
Files:
- docs/roadmap.md -- promotes the PhpStorm plugin under Shipped ->
Tooling, right next to the VS Code extension entry. The plugin is
framed by its delivery vehicle (Kotlin + Gradle, IntelliJ Platform
LSP API), not by what it bundles, so the entry stays meaningful
when bundled assets change.
- README.md (root) -- new "Editor tooling" section linking the LSP
server, the VS Code extension, and the PhpStorm plugin. Single
sentence per integration; the per-package READMEs carry the depth.
- CONTRIBUTING.md "Adding a new package" -- updated the worked-
example callout: the PhpStorm plugin validates the per-package
shape because it brings an entirely different toolchain (Kotlin +
Gradle vs the LSP's PHP + Composer) and still fits the same
convention without exceptions. The cross-package-dep paragraph
now contrasts the LSP's compile-time path-repo against the
plugin's runtime "spawn the binary, no symbols crossed" approach
as two valid integrations of the same monorepo rule. The CI
paragraph names ci-phpstorm-plugin.yml as the worked example for
a package whose CI needs both ecosystems.
- tools/phpstorm-plugin/README.md -- rewritten as a public doc now
that the chunk-status framing has served its purpose. Leads
with the feature matrix, walks through requirements / build /
install / smoke-test, includes an ASCII flow of how a .xphp file
open turns into LSP traffic, and closes on the explicit deferred
follow-ups (marketplace publication, native Kotlin parser,
PhpStorm < 2026.1 support, compile-on-save, headless integration
tests).
No code changes -- the binary plugin output is unchanged from chunk 6.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a dockerised JDK toolchain so contributors can build / test / run
the PhpStorm plugin under tools/phpstorm-plugin/ without installing
JDK 21, Gradle, or PhpStorm's X11/GTK dependencies on the host. Picks
up the same rootless-docker conventions the rest of the repo uses (PHP
runs through docker compose; same UID mapping; same gitignored
docker-compose.override.yml story).
- .docker/jdk.Dockerfile: eclipse-temurin:21-jdk plus the minimum
X11/font/GTK libs PhpStorm needs to render its Swing UI when
launched via `./gradlew runIde` (libxext6, libxrender1, libxi6,
libxtst6, libxrandr2, libxcursor1, libxinerama1, libfontconfig1,
libfreetype6, libgtk-3-0, fonts-dejavu-core). The base image
ships headless deps only -- without these the JVM boots, then
dies on the first Toolkit.getDefaultToolkit() call.
- tools/phpstorm-plugin/Makefile: every gradle invocation now routes
through `docker compose --project-directory ../.. run --rm jdk
tools/phpstorm-plugin/gradlew -p tools/phpstorm-plugin <task>`.
`--project-directory ../..` makes compose see the repo-root files
without an actual `cd`, so $(MAKE)'s working directory stays put.
`-p tools/phpstorm-plugin` is gradle's sub-project selector since
the container's working_dir is /opt/app, not the plugin package.
New `run-ide` depends on a new `x11-grant` target that runs
`xhost +local:docker` before launching the sandbox IDE. New
`dist` target runs `buildPlugin` and prints the resulting zip
path so contributors can install into their host PhpStorm via
"Install Plugin from Disk...".
The `jdk` compose service that wires these up lives in
docker-compose.override.yml; that file is gitignored on purpose (it
carries per-machine choices) so the Dockerfile is the only piece
that ships in the repo.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Without an explicit module dependency the platform treats this plugin as IDEA-only by default and surfaces a noisy warning on install: "Plugin 'xphp' is compatible with IntelliJ IDEA only because it doesn't define any explicit module dependencies" `com.intellij.modules.platform` is the universal "runs on any IntelliJ-based IDE" module. Combined with the existing `<depends>com.jetbrains.php</depends>` that already constrains to IDEs with PHP support (PhpStorm, IDEA Ultimate + PHP plugin), the effective compatibility window is unchanged -- we just stop lying to the platform about which IDEs the plugin targets. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Settings -> Tools -> xPHP would silently fail to remember a typed
"xphp LSP binary" path: opening settings again showed the field
empty, and PhpStorm's idea.log had no error to point at.
Root cause: the previous Configurable used the Kotlin UI DSL's
`panel { ... } .bindText(::workingPath)` against a local property
inside the Configurable, but overrode `apply()` / `reset()` /
`isModified()` directly WITHOUT delegating to the returned
`DialogPanel`. The DSL's bindings only fire when the DialogPanel's
own matching methods run; ours never called them, so the textfield
never wrote into `workingPath`, and `settings.lspPath = workingPath`
copied the initial empty value into the persistence store every
time the user hit OK.
Fix: extend `BoundConfigurable` instead of bare `Configurable`. Its
base implementation captures the panel from `createPanel()` and
auto-delegates the four lifecycle methods. Bind the textfield
directly to `XphpSettings.state::lspPath` -- the mutable property
reference on the State data class -- so the typed value flows
straight into the persistence-component-managed instance with no
intermediate field.
Side cleanup: drop the deprecated
`addBrowseFolderListener(title, description, project, descriptor)`
call for the current `textFieldWithBrowseButton(FileChooserDescriptor)`
DSL helper. The earlier overload is deprecated and the build now
treats those deprecations as errors.
The persistence wiring on XphpSettings (`@Service(APP)` + `@State` +
XmlSerializerUtil) is unchanged -- the bug was always above it, at
the UI binding.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`.xphp` files were opening as plain text after install. The
file-type registration claimed the extension but never registered a
SyntaxHighlighterFactory for `XphpLanguage`, so the platform had no
producer of tokens to colour.
Wire the IntelliJ TextMate plugin's own factory against our language
and feed it the grammar the VS Code extension already ships. Three
moving parts together:
(a) Declare `"fileTypes": ["xphp"]` in
tools/lsp/vscode-extension/syntaxes/xphp.tmLanguage.json so
IntelliJ's `TextMateService.getLanguageDescriptorByFileName()`
lookup resolves `.xphp` files to scope `source.xphp`. VS Code
ignores the field (it discovers file associations from the
extension's package.json), so this is a no-op there.
(b) Register `com.xphp.lsp.textmate.XphpTextMateBundleProvider`
against the `<textmate.bundleProvider>` extension point. At
startup it extracts the bundled grammar from the plugin jar
onto disk (TextMate's reader uses `Files.list()` /
`Path.readBytes()`; no classpath URL support), and presents a
`PluginBundle("xphp", <bundle-root>)` for IntelliJ to scan.
Pattern mirrors PharExtractor: sha256-keyed cache, atomic
write, persistent under PathManager.getSystemDir().
(c) Register
`org.jetbrains.plugins.textmate.language.syntax.highlighting.TextMateSyntaxHighlighterFactory`
against language `"xphp"` via `<lang.syntaxHighlighterFactory>`.
When the platform asks "how should I highlight an xphp file?",
TextMate's factory takes the call, looks up our grammar by
filename via the mapping (a) seeds, and paints PHP base
highlighting (the grammar `include`s `source.php`) plus the
`xphp-extras` patterns for generic-clause colouring.
Supporting changes:
- `XphpLanguage` now inherits from `PhpLanguage.INSTANCE` as a
safety net under the TextMate path. If TextMate's factory ever
can't resolve the grammar, IntelliJ's syntax-highlighter
inheritance walk (`SyntaxHighlighter.PROVIDER.forLanguage()`
walks `Language.getBaseLanguage()` ancestors) falls back to
PHP's lexer-driven highlighter rather than dropping to plain
text. An explicit warning comment above the `object`
declaration calls out that NO ParserDefinition must ever be
registered for this language -- doing so would inadvertently
enable PhpInspection / PhpReferenceContributor against .xphp
files via dialect inheritance.
- `<depends>org.jetbrains.plugins.textmate</depends>` in
plugin.xml + `org.jetbrains.plugins.textmate` in
`platformBundledPlugins` to put the TextMate API on both
runtime and compile-time classpaths.
- New unit tests for the bundle-extractor IO contract (four
cases: fresh extract, no-op on unchanged bytes, re-extract on
changed bytes, null-stream graceful skip).
Confirmed end-to-end inside an `eclipse-temurin:21-jdk` container:
PHP keywords / strings / classes colour as in regular .php files,
and `Box<T>` generic clauses get the distinct
`punctuation.definition.generic.{begin,end}.xphp` +
`entity.name.type.xphp` scopes from the xphp-extras block.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two correctness improvements to the bundled-PHAR extraction path, flagged by external review. Review finding #1: the existing PharExtractorTest's `TestExtractor` inner class re-implemented the production state machine (sha256 keyed cache + atomic-move write + sidecar) instead of exercising PharExtractor itself. A bug in PharExtractor.extract() -- a missed sidecar write, a flipped equality check -- would not be caught by `make test`. The sibling XphpTextMateBundleProvider.Extractor already shows the right shape: a thin facade plus an inner constructor-injectable Extractor. Mirror that shape here: - PharExtractor becomes a thin @service facade. - The state machine moves into `internal class Extractor` with constructor parameters `targetPath` and `streamLoader`. - PharExtractorTest constructs the production Extractor directly, passing a @tempdir target and a lambda streamLoader -- the same pattern XphpTextMateBundleProviderTest already uses. The four test cases keep their contracts (first extract, no-op on unchanged bytes, re-extract on changed bytes, null-stream graceful skip), but now exercise real production code. Review finding #6: the temp file before the atomic move was a fixed sibling path `xphp-lsp.phar.tmp`. Two PhpStorm instances starting against the same plugin version simultaneously could clobber each other's temp file. Benign in steady state (atomic move means whichever finishes last wins with byte-identical content) but a crash mid-write would leave a stale temp. Switch to `Files.createTempFile(parent, "xphp-lsp", ".phar.tmp")` for a per-process unique name; one-line change, fully removes the race. Confirmed: 4/4 PharExtractor tests + the other 4 plugin tests still green inside the jdk container. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Review finding #2. When the LSP descriptor couldn't resolve a binary (no explicit setting + no bundled PHAR, or a configured path that doesn't exist), it raised `IllegalStateException` via Kotlin's `error()`. The IntelliJ Platform LSP API catches that and writes it to idea.log -- which means the user, who has no reason to know about Help -> Show Log in Files, sees nothing in the UI and can't act on the carefully-worded message we crafted. Surface a `NotificationGroupManager` balloon instead, with an "Open Settings..." action that calls `ShowSettingsUtil.showSettingsDialog(project, XphpSettingsConfigurable)` to jump straight to Tools -> xPHP. The notification message differentiates "you never configured a path AND the plugin jar has no PHAR" from "the path you configured doesn't exist", so the user knows whether to set or fix the path. The thrown RuntimeException carries a brief log breadcrumb (`see notification balloon`) so idea.log readers find the same information without duplicating the user-facing message. Plugin descriptor changes: - `<notificationGroup id="xphp" displayType="BALLOON"/>` added inside the existing `com.intellij` extensions block. Descriptor refactor: - Pulled the binary-resolution branches into a `resolveBinary()` helper. Returns null when neither the explicit setting nor the PHAR-extractor fallback turns up a real file. - `notifyMissingBinary()` reads the same `XphpSettings.lspPath` to compose the right message text and fires through the new notification group. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Review finding #3. The VS Code extension already exposes `xphp.phpPath` (tools/lsp/vscode-extension/package.json) so users on multi-PHP-version dev machines or with `php` not on PATH can point at a specific interpreter. The PhpStorm plugin hardcoded `cmd.exePath = "php"` for PHAR launches. Bring the two editors back to parity: - XphpSettings.State gains a `var phpPath: String = ""` field. Stored raw; trimmed on read via the new `phpPath: String?` convenience accessor. - XphpSettingsConfigurable adds a second `textFieldWithBrowseButton` row labelled "PHP interpreter:" bound to `settings.state::phpPath`. The comment explains "leave empty to use PATH `php`" and notes the override only matters for PHAR launches (shell-script LSPs have their own shebang). - XphpLspServerDescriptor uses `XphpSettings.getInstance().phpPath ?: "php"` as `cmd.exePath` when the configured / extracted binary is a PHAR. Twelve lines including the comment. Review finding #7 (drop trim normalization from a now-bypassed setter): - The previous `var lspPath: String?` had a setter that trimmed before writing back to `state.lspPath`. The Kotlin UI DSL binding writes to `state.lspPath` directly, bypassing the setter, so the normalization was effectively dead code AND a trap for any future caller who used the setter expecting consistency. - Both convenience accessors are now read-only (`val`) and trim at read time. The State data class is the canonical mutable surface; the accessors are the canonical normalized read. - State stays `public` because `PersistentStateComponent<T>` exposes it through public methods (Kotlin won't let a public function return an `internal` type). Documented "only sanctioned mutator is the UI binding" so the bypass concern is captured at the type, not just in a commit message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two small review-driven cleanups. Review finding #5: `processResources` evaluated `tmLanguageSource.exists()` and `xphpLspPhar.exists()` at Gradle configuration time. A fresh checkout that ran `./gradlew build` without first running `make -C tools/lsp build/phar` would silently ship without the PHAR even if a parallel make dropped it mid- configure. The CI workflow runs the makes in sequence, so this is a footgun rather than a bug, but Gradle 9 + configuration cache make the stickiness more pronounced. Declare both upstream artifacts as optional inputs so Gradle's incremental machinery re-evaluates the `if (...exists())` checks per build: inputs.file(tmLanguageSource).withPropertyName("...").optional(true) inputs.file(xphpLspPhar).withPropertyName("...").optional(true) Review finding #8: the `x11-grant` target ran `xhost +local:docker` unconditionally. Contributors on macOS / Windows / WSL (where xhost may be absent or unnecessary) hit a confusing `xhost: command not found` before `runIde` even started. Linux + X11 remains the documented build host; the guard just degrades gracefully off it: @command -v xhost >/dev/null 2>&1 && xhost +local:docker || \ echo "(skipping xhost: not on X11 or xhost not installed)" Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The highlighting fix from 76ddebb shipped, but at install time the user saw only PHP base syntax with no xphp generic-clause colour -- the TextMate factory was firing then falling through, so the `<lang.syntaxHighlighterFactory>` was effectively wired against plain PhpLanguage inheritance. Investigation in Settings -> Editor confirmed: * No "xphp" bundle visible in TextMate Bundles. * No "TextMate Files" file type with *.xphp pattern. Both panels empty meant our `<textmate.bundleProvider>` registration was a silent no-op at plugin load. Root cause: PhpStorm 2026.1's bundled TextMate plugin uses the v2 plugin model. Its plugin.xml puts the actual extension-point declaration inside a `<content><module name="intellij.textmate" loading="required"><extensionPoints>` block: <extensionPoint qualifiedName="com.intellij.textmate.bundleProvider" interface="...TextMateBundleProvider" dynamic="true"/> Plain `<depends>org.jetbrains.plugins.textmate</depends>` only pulls in the parent plugin's top-level descriptor. Sub-module EPs require an explicit v2 `<dependencies><module name="..."/></dependencies>` block. Without it, IntelliJ doesn't know about `com.intellij.textmate.bundleProvider`, so our extension entry that targets that EP id is silently dropped. Fix: * plugin.xml -- add the v2 module dependency alongside the legacy `<depends>`. The legacy line stays as the top-level "TextMate plugin must be present" gate; the new block is what makes the EP id resolve. <depends>org.jetbrains.plugins.textmate</depends> <dependencies> <module name="intellij.textmate"/> </dependencies> * build.gradle.kts -- add `bundledModule("intellij.textmate")` so the sub-module's classes (TextMateBundleProvider interface, TextMateSyntaxHighlighterFactory) are on the compile classpath in CI. `bundledPlugins("org.jetbrains.plugins.textmate")` alone resolves the top-level plugin but not the sub-module's code mirror. Verified the patched plugin.xml inside the new dist zip carries the `<dependencies><module name="intellij.textmate"/></dependencies>` block as expected. Pending end-to-end re-install + Settings panel re-check to confirm the bundle now appears in Editor -> TextMate Bundles and `.xphp` files highlight the xphp-extras patterns on top of PHP base. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Direct evidence from the bundled PhpStorm 2026.1.2 distribution:
the `org.jetbrains.plugins.textmate.api.TextMateBundleProvider`
extension point is declared (`com.intellij.textmate.bundleProvider`
in the TextMate plugin's plugin.xml), but **no class in the entire
IDE distribution consumes it**. Searching every jar:
for jar in $PS/**/*.jar; do
unzip -p $jar '*.class' 2>/dev/null | strings \
| grep -q TextMateBundleProvider && echo $jar
done
# -> zero matches outside the API class declaration itself.
`TextMateServiceImpl.registerBundles` only iterates
`TextMateBuiltinBundlesSettings` (filesystem-discovered built-ins
shipped with the IDE) and `TextMateUserBundlesSettings` (entries
the user adds via Settings -> Editor -> TextMate Bundles).
Our `<textmate.bundleProvider>` registration was a silent no-op.
User confirmed empirically:
* Settings -> Editor -> TextMate Bundles: no "xphp" entry.
* Settings -> Editor -> File Types: no "TextMate Files" file
type with .xphp pattern.
Fix: skip the unwired EP and go through the user-bundles API
directly. Replace `XphpTextMateBundleProvider` (which implemented
the dead EP) with `XphpBundleRegistrar`, a `ProjectActivity` that:
1. Extracts the bundled grammar to
`<systemDir>/xphp/textmate-bundle/xphp/Syntaxes/xphp.tmLanguage.json`
-- same sha-keyed cache + atomic move pattern as PharExtractor.
2. Reads `TextMateUserBundlesSettings.getInstance()` and calls
`addBundle(path, "xphp")` if `hasEnabledBundle(path)` is false.
3. Calls `TextMateService.getInstance().reloadEnabledBundles()`
so the bundle is active immediately instead of waiting for the
next event-driven reload.
Subsequent project opens find the bundle already registered and
no-op via `hasEnabledBundle`. After first run, an "xphp" entry
appears in Settings -> Editor -> TextMate Bundles pointing at the
extracted directory -- the user can disable/remove it from there.
Trade-off captured in the KDoc: an "xphp" entry in user settings is
the visible side effect. Uninstalling the plugin leaves the entry
orphaned (path still exists, since we extracted to systemDir, not
pluginDir). Cleaning up on uninstall is a Disposable hook -- fine
follow-up, not on the critical path here.
Plugin descriptor changes:
* Drop `<textmate.bundleProvider implementation="..."/>` (no-op).
* Add `<postStartupActivity implementation="...XphpBundleRegistrar"/>`.
* Keep `<dependencies><module name="intellij.textmate"/></dependencies>`
-- still load-bearing for the existing
`<lang.syntaxHighlighterFactory>` registration pointing at
`TextMateSyntaxHighlighterFactory`.
Verified: 12/12 unit tests pass (extractor logic preserved; only
the registration mechanism changed).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
User reported "TextMate bundle cannot be loaded: \`xphp\` bundle has
an unknown format" after the previous fix (commit registering the
bundle via TextMateUserBundlesSettings landed and the entry showed
up in Settings -> Editor -> TextMate Bundles, but failed to load).
Root cause: IntelliJ's `readTextMateBundle` looks at the bundle root
for either `info.plist` (classic TextMate format) or `package.json`
(VS Code format) to detect the bundle layout. Our extracted bundle
shipped only `Syntaxes/xphp.tmLanguage.json` and a sha256 sidecar --
neither marker file -- so the reader bailed with the "unknown
format" error.
Add a minimal info.plist at the bundle root:
<plist version="1.0">
<dict>
<key>name</key>
<string>xphp</string>
</dict>
</plist>
`name` is the only field the reader requires; UUID and friends are
discovery-keys for bundle managers and don't affect parsing.
The extractor's grammar-sha cache early-returns when the grammar
bytes haven't changed. That means users upgrading from the
previous (broken) plugin version had the right grammar on disk but
no info.plist, and the cache hit would skip the recovery. Fix by
writing info.plist BEFORE the sha-check returns, idempotently
(`ensureInfoPlist` writes only if absent). Plus the registrar now
calls `TextMateService.reloadEnabledBundles()` unconditionally --
not just on first-time `addBundle` -- so a stale registration that
previously failed gets retried with the newly-written info.plist.
Tests:
* `first run writes info_plist with bundle name` -- positive
coverage of the new write path.
* `second extract restores info_plist when missing (heals legacy
installs)` -- proves the cache-hit recovery path: we wipe
info.plist and call extract() again with unchanged grammar
bytes, and info.plist must come back even though the sha-check
short-circuits the grammar copy.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
User reported .xphp files failing to open entirely after install:
WARN - EditorComposite - Cannot focus editor ... reason=preferredFocusedComponent is null
INFO - c.j.p.c.BeforeFileOpenLoggerListener - fileClosed InstanceofTemplate.xphp
That pattern (open, fail to focus, immediate close) is what IntelliJ
logs when an editor's content provider fails to construct. The PHP
plugin's BeforeFileOpenLoggerListener observing the events is the
giveaway: PHP-plugin code was dispatching on `.xphp` files even
though we never registered any PHP-specific xphp behaviour.
Root cause: an earlier iteration declared
`object XphpLanguage : Language(PhpLanguage.INSTANCE, ...)`. That
treats xphp as a PHP dialect, which `SyntaxHighlighter.PROVIDER`'s
`getBaseLanguage()` walk uses to inherit PHP's highlighter for free.
But the same parent-walk applies to EVERY language-keyed extension
point. `LanguageParserDefinitions.forLanguage()` therefore found
PHP's `ParserDefinition` for xphp; the platform tried to parse
`.xphp` content as PHP at editor-construction time; PHP's parser
threw on `Box<T>` generic-clause syntax; the editor failed; the
file closed before anything rendered.
The chunk-4 commit's "DO NOT register a `parserDefinition`" warning
caught the explicit case but missed that language inheritance does
the equivalent implicitly.
Fix: drop the parent. Declare xphp as a top-level Language:
object XphpLanguage : Language("xphp", "application/x-xphp")
Without a parent, no PHP-plugin code dispatches on .xphp files
through the dialect mechanism. The file opens as a plain Document
with no PSI tree -- which is exactly what we want for an
LSP-driven editing model. Diagnostics / hover / completion come
from the LSP (XphpLspServerSupportProvider unchanged). Syntax
highlighting comes from the TextMate
`<lang.syntaxHighlighterFactory>` registration in plugin.xml -- the
xphp grammar `include`s `source.php`, so PHP base coloring is
delivered by TextMate, not by language inheritance.
KDoc rewritten to capture the trap so a future maintainer doesn't
re-introduce the parent thinking it's harmless.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
JetBrains' LSP plugin-integration docs (https://plugins.jetbrains.com/docs/intellij/language-server-protocol.html #basic-implementation) state the dependency as mandatory: <depends>com.intellij.modules.lsp</depends> Our plugin.xml didn't carry that. The `<platform.lsp.serverSupportProvider>` extension we registered was therefore being processed by a plugin that hadn't declared the backing module dependency. In v2 plugin loading that means our extension was getting silently dropped or partially wired -- which matches the user-reported symptom of `.xphp` files failing to open entirely with "Cannot focus editor ... preferredFocusedComponent is null" followed by an immediate fileClosed in idea.log: the editor construction path tripped on a partially-initialised LSP support binding. This is a separate bug from the TextMate / highlighting / file-type ones we've been chasing. Adding the depends doesn't fix highlighting or the TextMate bundle integration; it's the prerequisite for the LSP-side wiring to engage at all. If files open again after this lands, we can iterate on the remaining pieces (the docs' examples filter by `file.extension == "foo"` and don't register a custom FileType, suggesting our XphpFileType + XphpLanguage scaffold may also need rethinking). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ension User confirmed `.xphp` files refuse to open while the plugin registers a `LanguageFileType(XphpLanguage)`. Removing that registration at runtime makes files open. The empirical fix tracks with the JetBrains LSP docs, whose canonical recipe does NOT register a custom FileType / Language and filters strictly by file extension (https://plugins.jetbrains.com/docs/intellij/language-server-protocol.html #basic-implementation). Root cause: `LanguageFileType` makes the platform assume the bound `Language` has a `ParserDefinition` registered. IntelliJ walks the language hierarchy at editor-construction time looking for one to build PSI for the file. We don't have a parser (parsing is the LSP's job), so the walk found nothing usable, editor construction failed, and the file closed before rendering. Symptom in idea.log: WARN EditorComposite - Cannot focus editor ... reason=preferredFocusedComponent is null INFO BeforeFileOpenLoggerListener - fileClosed <name>.xphp But fixing the file-open path by removing XphpFileType also breaks the LSP routing: our `XphpLspServerSupportProvider` filtered with `file.fileType is XphpFileType`, which now never matches because `.xphp` files are TextMate-typed (via the bundle's `fileTypes: ["xphp"]`). Result: LSP never starts, no completion. Full alignment with the docs: * Drop `XphpFileType.kt`, `XphpLanguage.kt`, and the orphaned `XphpFileTypeTest.kt` -- no custom file type, no custom language. * `XphpLspServerSupportProvider.fileOpened`: filter via `file.extension == "xphp"`. * `XphpLspServerDescriptor.isSupportedFile`: filter via `file.extension == "xphp"`. * `plugin.xml`: remove the `<fileType ...>` registration, remove the now-orphaned `<lang.syntaxHighlighterFactory language="xphp" ...>`. Long comments left in place describing what NOT to re-add and why, so a future maintainer doesn't undo this. Resulting architecture: 1. `.xphp` file open -> platform's file-type detector falls through to the TextMate plugin -> bundle (registered by XphpBundleRegistrar) claims `.xphp` via `fileTypes: ["xphp"]` -> file is `TextMateFileType`, opens cleanly, highlighter is the platform-provided TextMate highlighter applied to our grammar (PHP base via `include source.php` + xphp-extras overlay). 2. `LspServerSupportProvider.fileOpened` fires on every open; extension check matches -> LSP starts via the descriptor -> completion / hover / go-to-definition / diagnostics flow through the LSP API. Tests: pure-data tests retained (PharExtractor + bundle-extractor; 10 total). XphpFileTypeTest deleted along with its production class. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…path
Trap surfaced during debug:
1. User types `~/.cache/JetBrains/PhpStorm2026.1/xphp/xphp-lsp.phar`
into Settings -> Tools -> xPHP, expecting the bundled-PHAR
extractor to write there.
2. The configured-path branch in `resolveBinary()` wins outright,
suppressing the extractor entirely.
3. The configured file doesn't exist (extractor never ran).
4. `resolveBinary()` returns null, `notifyMissingBinary()` fires,
LSP never starts -- with no obvious path forward for the user
who already typed in what they thought was the right answer.
Make `resolveBinary()` fall through to
`PharExtractor.getInstance().extract()` when `lspPath` is set but
`File(it).isFile` is false. One warn-level log so the discrepancy
is visible to anyone grepping idea.log without crowding the
common case (configured + present -> use; unconfigured -> extract).
Self-heals the trap: bundled PHAR lands at its standard location,
descriptor uses that, the user's configured path becomes a
dead-letter that doesn't break anything (and can be cleaned up
from settings later).
The wrapper-script LSP-startup issue from this same debug cycle
(docker run without -i, then with -ti, both wrong) traced back to
the user's PHP wrapper, not this code -- no plugin-side fix
applies to that. Wrapper needs `-i` alone; documented for the
user separately.
No new tests: the production `PharExtractor.Extractor` is already
covered by `PharExtractorTest`, and the new branch is a one-line
fall-through that doesn't change extractor behaviour.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
IntelliJ's LSP client refused our initialize response with:
SEVERE - StreamMessageProducer - Unexpected token BEGIN_ARRAY:
expected BOOLEAN | BEGIN_OBJECT tokens.
com.google.gson.JsonParseException: ...
at EitherTypeAdapter.read(EitherTypeAdapter.java:177)
Captured the actual bytes by piping a synthetic `initialize` to
the LSP binary; got back:
{"capabilities":{"textDocumentSync":1,
"completionProvider":{"triggerCharacters":["<",","]},
"hoverProvider":[],
"definitionProvider":true,
"executeCommandProvider":{"commands":[]}},
"serverInfo":{"name":"xphp-lsp","version":"0.1.0"}}
`hoverProvider:[]` is the offender. We were setting it to
`new HoverOptions()`. `HoverOptions` has a single nullable field
(`workDoneProgress`); when phpactor's serializer strips null
fields before encoding, the result is an empty associative PHP
array. PHP's json_encode picks `[]` for empty arrays (no way to
distinguish "empty assoc" from "empty indexed").
LSP4J's strict `Either<Boolean, HoverOptions>` parser then refuses
the array form and kills the connection.
`hoverProvider` is `Either<Boolean, HoverOptions>` per the LSP
spec, so `true` is equally valid and encodes unambiguously --
matching the shape we already use for `definitionProvider` in the
same handler set. Comment block left in place to head off any
future "let me reach for HoverOptions for completeness" rewrites.
Confirmed wire response after the fix:
"hoverProvider":true (was: "hoverProvider":[])
131 PHPUnit cases still green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…effect Trap: a user whose `Settings -> Tools -> xPHP -> xphp LSP binary` is set to the exact path PharExtractor writes to (`<systemDir>/xphp/xphp-lsp.phar`) was effectively pinned to the on-disk bytes from whenever the setting was first applied. When a new plugin version ships an updated PHAR: 1. Plugin upgrade replaces the jar with the new bundled PHAR. 2. User opens a .xphp file. 3. `resolveBinary()` sees `lspPath` is set AND file exists. 4. Short-circuits to that file -- skips PharExtractor entirely. 5. The STALE on-disk PHAR continues to be used. Forever. This bit us right after the hoverProvider:[] fix landed: the new plugin had the corrected PHAR bundled in its jar, but the user's running session still spawned the old one because the cache was "present and configured." Symptom was the exact same JSON-RPC parse error LSP4J reported before the fix. Make `resolveBinary()` ALWAYS call PharExtractor.extract() first, regardless of whether `lspPath` is set. PharExtractor's sha-check is cheap on the no-change path (read bundled bytes, sha compare, early-return) and writes only when the bundled bytes actually differ from on-disk -- so the common case is one extra sha computation per LSP startup. After the extract, prefer the user's configured path only as a real override that explicitly points elsewhere; if it happens to point at the bundle target, the just-extracted fresh PHAR is what lives there now. Self-heals the upgrade-staleness case without new user actions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
LSP server was starting + initialising successfully (the hoverProvider fix from 459e864 cleared the JSON-RPC parse error), then getting shut down by the framework ~215 ms later and SIGKILL'd (exit 137). State trail in idea.log: (Running;3) -> Stopping LSP server normally (Initializing;0) -> (Running;0) -> (ShutdownNormally;0) LSP server process terminated. Exit code: 137 Two cooperating causes, both small: Cause A -- descriptor instance churn ===================================== `XphpLspServerSupportProvider.fileOpened` constructed `XphpLspServerDescriptor(project)` on every call. IntelliJ's LSP framework dedupes "already-running server?" via descriptor equality. Without `equals` / `hashCode` overrides, each fresh instance looks distinct -- so the framework stopped the existing server and started this new "different" one on every file open. Override both, keyed by project identity + descriptor class. Same project + same descriptor class = same logical server; framework recognises it as already running and doesn't bounce. Cause B -- unconditional bundle reload ======================================= `XphpBundleRegistrar.execute` called `TextMateService.reloadEnabledBundles()` outside the `if (!hasEnabledBundle)` guard -- so reload fired on every project open, even when the bundle was already registered from a previous session. `reloadEnabledBundles()` fires `fileTypesChanged`, which cascades into root changes (already visible in earlier logs) AND into LSP-framework restarting every registered LSP server. That's the "Stopping LSP server normally" we kept seeing right after init. The unconditional reload was added to heal users whose pre-info.plist bundles were marked enabled but had stale (broken) on-disk state. Trading that one-time heal for not-bouncing-the-LSP-every-IDE-start is the right call; legacy users heal on the next "soft" trigger (close + reopen project) instead of automatically. Move the reload inside `if (!hasEnabledBundle)`. Comment block in place describes the trade-off so a future maintainer doesn't undo the gate to "fix" the legacy-heal regression. Verification ============ * 10 unit tests still green. * Fresh dist zip produced. End-to-end (manual): open .xphp file, watch the LSP indicator stay in Running through multiple file opens; idea.log should show ONE "Starting LSP server" + ONE "LSP server initialized" pair, no subsequent "Stopping LSP server normally" / "Exit code: 137". GTD scope reminder: `XphpDefinitionHandler` only handles xphp-generic instantiations (clicking `Foo` in `new Box<Foo>()`). Regular-PHP navigation (function declarations, methods, plain class references) isn't in the MVP -- documented limitation, not in scope of this fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
idea.log surfaces a silent failure at TextMate bundle-load time:
c.i.t.j.JoniRegexFactory - Failed to parse textmate regex
'(?<=[A-Za-z_\\][A-Za-z0-9_\\]*)\s*(<)(?=[A-Za-z_\\\s])'
org.joni.exception.SyntaxException: invalid pattern in look-behind
That's the `begin` pattern of the `xphp-extras` block in
`tools/lsp/vscode-extension/syntaxes/xphp.tmLanguage.json`. The
look-behind `(?<=[A-Za-z_\\][A-Za-z0-9_\\]*)` is **variable
length** (the `*` quantifier). IntelliJ TextMate's regex engine
is Joni -- a Java port of Oniguruma -- which only supports
fixed-length look-behinds. The rule fails to compile, the whole
xphp-extras layer is silently inert in PhpStorm, and Box<T>
clauses get no special colouring. VS Code is unaffected: its
regex engine is vscode-oniguruma (WASM build of "real" Oniguruma)
which supports variable-length look-behinds.
Fix: shrink the look-behind to a single character.
(?<=[A-Za-z_\\][A-Za-z0-9_\\]*)\s*(<)... <- before (var-length)
(?<=[A-Za-z0-9_\\])\s*(<)... <- after (1 char fixed)
The discriminator was always "is the char immediately before `<`
part of an identifier?" -- a single-char look-behind expresses
that exactly. Distinguishes `Box<` (identifier) from a comparison
` < ` (preceded by whitespace) without needing the original's
*-quantified class.
VS Code's Oniguruma accepts fixed-length look-behinds as a strict
subset, so the existing extension keeps working unchanged.
Inline comment in the grammar's JSON `comment` field captures the
trap so a future contributor doesn't undo the constraint to "make
the pattern more precise".
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…r>(...))
User Ctrl+click on `User` inside `identity<User>(new User('bob'))`
returned "Cannot find declaration to go to." The LSP was healthy
and being called -- this is a real handler-scope gap.
XphpDefinitionHandler walks the AST for a Name node carrying
ATTR_TEMPLATE_FQN. That attribute is set by XphpSourceParser on
the outer sites (ClassLike, the call's Name, the FuncCall), but
the INNER type-args don't survive as Name nodes -- the parser
strips them into TypeRef marker entries on the surrounding call.
Cursor on the inner `User` therefore hits no AST node with
ATTR_TEMPLATE_FQN, and the handler returns null.
Add a second code path that handles the inner case:
1. Existing path (kept): cursor on a Name with ATTR_TEMPLATE_FQN
-> walk workspace for the matching ClassLike template.
2. NEW: when (1) misses, use TypeArgPositionDetector to detect
"cursor is inside <...>" and extract the identifier under
the cursor (prefix + suffix scan), then resolve via
WorkspaceSymbols.findClassByName -- short-name match across
open documents.
Files:
- tools/lsp/src/Handler/TypeArgPositionDetector.php
+ new `identifierAt($source, $offset): ?string`. Reuses the
existing `detect` for the in-`<>` check; scans forward from
the cursor to capture the trailing identifier bytes; returns
`prefix . suffix` or null.
- tools/lsp/src/Handler/WorkspaceSymbols.php
+ new `findClassByName(string $shortName): ?Location`. Walks
open documents, matches the last `\`-segment of each
ClassLike's FQN against the short name, returns the first
hit's Location targeting the name token (same convention as
the existing template-FQN path). Cross-namespace
collisions resolve to workspace insertion order -- documented
as a follow-up.
- tools/lsp/src/Handler/XphpDefinitionHandler.php
+ WorkspaceSymbols added to constructor.
+ `definition()` now tries Path 1 (template lookup), then falls
through to Path 2 (TypeArgPositionDetector + findClassByName)
when (1) produces no location.
- tools/lsp/src/LspDispatcherFactory.php
+ Construct a single WorkspaceSymbols instance and share it
between XphpCompletionHandler (existing user) and
XphpDefinitionHandler (new user). Same in-memory AST cache,
no reason to instantiate two collectors.
Tests (10 new, 131 -> 141 total):
+ TypeArgPositionDetectorTest: identifierAt's four cases
(mid-identifier prefix+suffix, outside `<>`, whitespace inside
`<>`, FQN-style with backslashes).
+ WorkspaceSymbolsTest: findClassByName's four cases
(namespace-qualified match, no-match returns null, empty
short name returns null, cross-document first-hit-wins).
+ XphpDefinitionHandlerTest: integration cases (Ctrl+click on
`User` inside `identity<User>(...)` lands on `class User`;
unknown class inside `<>` returns null).
Out of scope (documented in plan):
* Namespace-resolved lookup via `use` statements. Short-name
match is the 90/10 for now; refine when collisions surface.
* Hover on type-arg names (same machinery would work).
* PHP-level GTD (function calls, methods). Architectural
decision deferred.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Server-side capability advertisement (`definitionProvider: true`, `hoverProvider: true`) tells the platform the server CAN do these things; the descriptor's `lspCustomization` tells the platform to actually ASK. Both sides are required -- with only server capabilities, PhpStorm never dispatches `textDocument/definition` on Ctrl+click, confirmed via captured idea.log showing zero traffic before this opt-in. The no-arg `LspCustomization()` constructor instantiates the `Lsp*Support` (enabled) version of every customizer -- go-to-def, hover, completion, semantic tokens, etc. Each customizer still consults the server's `ServerCapabilities` before dispatching, so handlers our LSP doesn't implement don't get routed regardless. The deprecated per-feature boolean overrides (`lspGoToDefinitionSupport`, ...) feed an older opt-in path that wraps customizers in `*Disabled` defaults; overriding this single property bypasses that and gives the modern, lint-clean opt-in. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 0.7 (property hover/GTD substitution) was already general enough to handle static property accesses -- worse-reflection's containerType on a StaticPropertyFetch resolves to the LHS class, so renderProperty / locateProperty just worked. Add regression tests that lock this and ship a small cosmetic fix surfaced in prod: the rendered signature was dropping the `static` modifier, showing `public int $count` for a static property. Prod trace xphp-20260525-130520-718.log id=5 confirmed the new rendering: `public static int $count`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three correctness improvements layered on Phase 4.1+ / 4.2:
- `use function App\foo;` and group-use `use function App\{a, b};`
imports are now tracked as references to the imported function.
Symmetric to how `use App\Foo` class imports are already counted.
Without this, renaming a function leaves the use stmt alias stale.
- Rename is alias-aware. Three scenarios:
- Cursor on a source-side name (`function foo`, or `foo` inside
`{foo as bar}`) -> rename the underlying symbol + the source
name inside every use stmt; aliases preserved with the new name.
- Cursor on an alias-resolved call site (`bar()` where `bar` is a
use-alias) -> rename the LOCAL alias + its uses in this file
only; the source symbol is untouched. PhpStorm-style semantics.
- Cursor on the alias-declaration token (`xyz` inside `as xyz`)
-> same effect as cursor on the call site (file-scoped rename).
Implementation: ReferenceFinder.resolveTargetAt distinguishes
alias-resolved Names (literal short != resolved short) from
source-side references; alias targets carry `scopeUri` so
collectReferences walks only that file.
- RenameProvider's per-location TextEdit compares the source text's
short-name (stripping `$` for VarLikeIdentifier property tokens,
stripping any backslash prefix) against the symbol's old short
name. Locations whose short name doesn't match the old name (the
aliased call sites) are skipped, preserving the alias and aliased
call site spellings. The dollar-sign handling fixes a related
regression where property rename produced zero edits in the
declaration file because VarLikeIdentifier ranges span `$name`.
Prod traces id=27 (alias-aware source rename), id=33 (cursor on
alias-resolved call site -> file-scoped rename of `xyz` to
`renamedLocalAgain`), id=81 (cursor on source decl -> rename
function + use stmt source side, alias preserved) confirmed each
path. Single-import + group-use both covered by tests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Worse-reflection's TolerantParser labels the imported name inside
`use function App\foo;` as Symbol::CLASS_ -- the tokenizer can't
tell `function` is the use-statement modifier vs the foo's kind.
Three downstream symptoms:
- textDocument/definition routes to locateClass('App\foo') ->
reflectClassLike throws SourceNotFound -> returns null. Ctrl+Click
on the imported name does nothing.
- textDocument/hover routes to renderClass -> same SourceNotFound ->
empty hover. No signature, no docblock.
- Same for the group-use variant `use function App\{a, b};`.
Fix in both PhpDefinitionResolver and PhpHoverResolver: before
dispatching on Symbol::symbolType(), walk the AST for an enclosing
`Stmt\Use_` (or `Stmt\GroupUse`) with type === TYPE_FUNCTION
whose `UseItem.name` range covers the cursor. When found, build the
FQN (prepending the GroupUse prefix when present) and route directly
to locateFunction / renderFunction instead.
Prod trace xphp-20260525-143922-983.log id=3/4/6 confirmed hover
now renders `function App\Models\originalCount(array $items): int`;
id=7/11 confirmed GTD jumps from the use stmt to the function decl.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two cursor-on-declaration affordances backed by the same AST walker: - Hover on a declaration's own name token (`function foo`, `class Foo`, `public function method()`, `public $prop`) previously returned null because worse-reflection has no useful symbol classification for the declaration itself. PhpHoverResolver gains a `declarationFqnAtOffset` walker that detects which kind of declaration the offset sits on and routes to renderFunction / renderClass / renderMethod / renderProperty directly. Surfaces signature + docblock as you'd expect. - GTD on a declaration's own name token (Ctrl+Click) is promoted to find-usages. Standard GTD on a decl is a no-op (cursor IS the decl) so we'd otherwise return null. Instead we return the ReferenceFinder usages excluding the decl itself. LSP allows `Location[]` here, so PhpStorm pops its multi-target navigator matching the user's "show me where this is used" intent. Falls through to normal GTD when there are zero usages anywhere so PhpStorm sees "no targets" cleanly. XphpDefinitionHandler gains a ReferenceFinder dependency; the new Path 0 runs before the existing template-FQN / type-arg / PHP- semantic paths. Prod trace xphp-20260525-143922-983.log id=52 (hover on decl line was returning null) reproduced and fixed; user-confirmed GTD-on-decl produces the PhpStorm multi-target popup with all call sites. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nt support) Phase 4.2's class rename rewrites occurrences in every file but leaves the file itself untouched. PSR-4 convention expects `class Foo` in `Foo.xphp`, so renaming `Foo` to `Bar` should also move the file to `Bar.xphp`. RenameProvider now emits a `RenameFile` resource op alongside the TextDocumentEdits when: - The target kind is `class` (methods, functions, properties don't touch files) - The decl file's basename (sans extension) matches the OLD short name (multi-class files and non-PSR-4 layouts are left alone -- better to under-rename than rename the wrong file) - The client advertised `rename` in `workspace.workspaceEdit.resourceOperations` The client-capability gate matters because PhpStorm's IntelliJ Platform LSP integration currently advertises `resourceOperations: ["create"]` only -- silently dropping any RenameFile ops we'd send. LspDispatcherFactory inspects InitializeParams and passes a bool to RenameProvider; clients that do support `rename` (VS Code, Helix) get the full behavior, others get a clean response with TextEdits only. ReferenceFinder gains a public `targetAt` (thin wrapper over `resolveTargetAt`) so RenameProvider can ask about target kind + FQN before deciding whether to emit the resource op. End-to-end class+file rename in PhpStorm requires plugin-side work (custom workspace-edit applier hooking IntelliJ's VFS) since the platform's LSP client doesn't expose `rename` resourceOps. Tracked as a follow-up. Prod trace xphp-20260525-151231-662.log id=10/233 confirmed the RenameFile op is emitted with the right oldUri/newUri pair; the in-file TextEdits applied; PhpStorm dropped the rename op silently (known JetBrains LSP limitation -- gate prevents future confusion). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Hover on `Util::method<T>(...)` and `identity<T>(...)` now renders the substituted param + return types, matching the instance-method path shipped in Phase 0.6. GenericResolver gains two leaf resolvers (static, function) sharing the same paramMap machinery as the existing method-call path; PhpHoverResolver wires the function dispatch through renderFunction (now substitution-aware) and falls the static method case back through resolveMethodCallSubstitutionAt -> resolveStaticCallSubstitutionAt. Also strips the `App\Ns\T` placeholder leak that surfaced on declaration lines: FqnIndex now tracks `ATTR_METHOD_GENERIC_PARAMS` from `Function_` and `ClassMethod` nodes (was only walking class-level `ATTR_GENERIC_PARAMS`), so `GenericParamRegistry::prettify` knows `T` is a placeholder in the function's own namespace. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
V1's exact-FQN match dropped any member-access call whose receiver was a subclass of the target's declaring class. ReferenceFinder now climbs the receiver's MRO via worse-reflection's declaringClass() in two places: - resolveTargetAt: cursor on `$dog->speak()` (with `Dog extends Animal`, no override) targets `Animal::speak`, not the non-existent `Dog::speak`, so find-references / rename find the actual declaration and every inherited call site. - collectReferences: a call site whose receiver class inherits the member from the target now matches; receivers that override stay excluded (declaringClass() returns the subclass, not the target). GTD inherits the fix transparently -- XphpDefinitionHandler delegates to the same target resolution. Adds a playground fixture (Animal + Dog + Cat + Inheritance.xphp) so the inheritance / override behaviours can be exercised manually without inventing test classes each time. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 2.3's infrastructure (FilesystemClassLikeLookup, CompositeClassLikeLookup, FilesystemSourceLocator-via-FqnIndex) shipped earlier as a side effect of Phase 0 + 0.5; the user-visible feature (hover working on classes whose declaration is only on disk) had no direct end-to-end regression yet. Adding two: - class-name hover when the class lives only on disk - method-call hover when the receiver's class lives only on disk These are the cases PhpStorm hits when jumping into an unindexed file and inspecting a symbol via tooltip without explicit "Open in Editor". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Visibility gate on member completion now consults the inheritance chain: inside a subclass, the receiver's protected members surface (private stays declaring-class-only). Uses worse-reflection's parents()+interfaces() BFS to determine whether the cursor's enclosing class extends/implements the receiver class. Before this commit, completion inside `class Dog extends Animal` dropped every protected member of Animal -- only Dog's own + Animal's public members surfaced, forcing the user to look up the parent class manually. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Variable completion previously dumped every variable defined anywhere in the document, ignoring PHP's scope barriers. Cursor inside `inner()` saw outer-scope `$outer`; cursor inside a closure saw every outer var even when none was imported via `use (...)`. Now the resolver builds the scope chain at the cursor and honours PHP semantics: - Top-level cursor sees top-level vars only. - Function / method bodies see their own params + locals (scope barrier hides outer scope). - Closures see params + explicit `use (...)` captures + body locals. - Arrow functions auto-capture: see all outer-scope vars walking up the arrow chain until the first function/method/closure barrier. `collectScopeBodyVariables` uses DONT_TRAVERSE_CHILDREN to skip nested scopes, so a closure-local variable doesn't leak into the enclosing function's completion (and vice versa). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Type-arg completion now filters candidates by the slot's declared upper bound: - `Box<T: \Stringable>` at `Box<|` surfaces only Stringable implementors; non-implementors and scalars are filtered out. - Multi-slot generics route the bound by slot index: `Pair<K: Stringable, V>` filters slot 0 against Stringable but leaves slot 1 unbounded. Plumbing: - TypeArgPositionDetector::detect now returns containerName + slot alongside prefix, derived from a single backwards walk that tracks depth-0 commas to find the slot index. - FqnIndex adds boundsForGenericClass(fqn) backed by a new collector collectGenericClassBounds; cached per session. - XphpCompletionHandler takes optional FqnIndex + Reflector, resolves the container Name to one or more FQNs, looks up bounds, and uses worse-reflection's isInstanceOf to check subtyping (replacing a hand-rolled BFS that called the non-existent ReflectionClass::parents). PhpCompletionResolver::isSubclassOf gets the same isInstanceOf swap to match -- the BFS in the prior commit happened to compile clean only because the same-class branch short-circuited the call path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a `static-prop` completion context for `Cls::\$|` cursor positions -- distinct from the existing `static` (`Cls::|`) and `variable` (`\$|`) shapes. Without this branch the detector classified `Cls::\$|` as a plain `variable` completion, dumping every in-scope variable into a position where only static properties make sense. The resolver's member dispatch now branches on the new kind: - methods + constants are excluded; - only static properties surface, with same/subclass visibility gating reused from the existing static / member paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When two open documents declare a class with the same short name -- typical for repos with parallel fixture trees -- findClassByName now ranks candidates by URI penalty (vendor / tests / fixtures / stubs segments add 10 each) and returns the lowest-penalty match. Equal penalties preserve the previous "first hit wins" order. Without the tie-break, GTD on `new User()` could jump to tests/Fixtures/User.xphp instead of the real src/Models/User.xphp -- silently shadowing the canonical source. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PositionMap now encodes column offsets as UTF-16 code units, matching the LSP spec. BMP characters (UTF-8 1/2/3-byte sequences) contribute one code unit each; supplementary-plane characters (UTF-8 4-byte) contribute two because UTF-16 represents them as surrogate pairs. Before this commit, a `🚀` in a comment threw column counts off by one for every column past that character; pointing the cursor at `$user` on a line with emoji landed the LSP server somewhere upstream of the identifier. ASCII source is unaffected. positionToOffset clamps mid-surrogate cursor positions back to the codepoint boundary, matching the editor's "no cursor between high and low surrogate" behaviour. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two prod regressions diagnosed via PhpStorm LSP logs and fixed. 1. Bound-aware type-arg completion only fired when the container class was open in the editor. Resolver and candidate enumeration both walked WorkspaceSymbols (open-doc only); a closed StringableBox.xphp meant no bound was found and the filter degraded to "no bound", surfacing Animal/Dog and scalars at `new StringableBox<|`. Both paths now consult FqnIndex (open + filesystem). 2. `Cls::$prop` completion: the LSP returned the right items but PhpStorm dropped them from the popup (filter mismatch -- labels without the `$` prefix didn't match the user's typed `$`) AND when the popup did show items, accepting one extended the replacement range backwards through the `$`, producing `Cls::name` instead of `Cls::$name`. Fix: emit labels + filterText with the `$` prefix so PhpStorm's popup filter accepts them, plus an explicit textEdit with an empty range pinned at the cursor so PhpStorm can't extend the range to swallow the `$`. Playground fixtures (`playground/src/Demos/Phase3*.xphp` + Stats / Wolf / tests/Fixtures/User) exercise every Phase 3 behaviour end-to-end against a real PhpStorm session. Animal.xphp gains protected + private members for the subclass-protected demo. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Shipped section of the public roadmap and the LSP package README listed only the original MVP surface (diagnostics / hover / GTD / completion). Update them to reflect the post-MVP follow-ups now shipped on feat/lsp-followups: - references, rename, documentSymbol, workspaceSymbol, didChangeWatchedFiles - bound-aware type-arg completion + scope-aware variable completion + Cls::\$prop + subclass-protected visibility - inheritance walks for references / rename / GTD - parameter and return-type substitution at static / free-function call sites - UTF-16 column counting, short-name tie-break, closed-file targets - expanded trigger characters and capability set - 424 / 1244 test count (was 131 / 299) ASCII-only; no agent-os/ references in any public file. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
"Vision" was aspirational; the section is actually a list of larger multi-step initiatives we have a concrete idea how to ship. Rename to "Long-term" to match Done -> Next -> Long-term as a strict timeline. Add an LSP capabilities section under Tooling triaged against what tsserver and rust-analyzer ship, grouped by ROI on the infrastructure we already have: - low effort: signature help, document highlight, type definition, implementation, folding ranges - medium effort: inlay hints, code actions, code lens, auto-import on completion, semantic tokens - xphp-unique (no other LSP can do these): show-generated-PHP code lens, specialization explorer, specialized-FQN inlay hint, reverse- map mangled FQN to template, bound-error fix-its ASCII-only; no agent-os/ references. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds .github/workflows/release-phpstorm-plugin.yml triggered by: - automatic: push a tag matching v* (e.g. v0.2.0) - manual: workflow_dispatch with a required `tag` input (the tag must already exist; the workflow checks out THAT ref so the build is reproducible against the committed code at tag time) The job: 1. Validates the tag shape (must be vN.M.O-ish). 2. Checks out at the resolved tag. 3. Builds the bundled xphp-lsp.phar so processResources can embed it. 4. Runs `./gradlew buildPlugin -PpluginVersion=<version>` -- this overrides gradle.properties so the produced zip is xphp-phpstorm-plugin-<version>.zip and plugin.xml's <version> matches the git tag rather than the stale 0.1.0 default. 5. Creates / updates the GitHub Release for the tag and attaches the zip. softprops/action-gh-release is idempotent so workflow_dispatch re-runs against the same tag update the asset in place instead of crashing on duplicate Release creation. The resulting download URL is: https://github.com/<owner>/<repo>/releases/download/<tag>/xphp-phpstorm-plugin-<version>.zip so the tag appears in both the URL path AND the filename. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`make test/mutation` was failing locally on a fresh docker container with Infection reporting "Project tests must be in a passing state" because the underlying PHPUnit subprocess exited with 143 (SIGTERM) mid-stream. Two complementary changes: - tools/lsp/Makefile: bump Infection's own memory_limit from default to 2048M. Infection forwards INI flags only to its own process, not to spawned PHPUnit children, so a Makefile -d covers the Infection driver but not the test process. - tools/lsp/phpunit.xml.dist: add `<php><ini name="memory_limit" value="512M"/></php>` so the PHPUnit subprocess Infection spawns (and any other direct phpunit invocation) gets the same ceiling the Makefile target already provides. Same value the per-target -d already set on `make test`; lifted here so subprocesses pick it up regardless of invocation path. PHP-side memory_limit doesn't cap Xdebug's C-level coverage allocations, so a host with tight container memory may still hit the kernel OOM-killer -- but on a properly-sized Ubuntu runner (CI's ubuntu-latest) the bumped limits are sufficient for the initial run to complete. Verified: `make test` still passes 424/424 against the updated XML. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Xdebug's coverage instrumentation allocates an order of magnitude more memory than PCOV for the same coverage scope -- on tight runners it was triggering the host OOM-killer mid-initial-run (SIGTERM 143). PCOV's line-only coverage tracking stays well under the same memory ceiling and produces identical Infection results. Changes: - .docker/php.Dockerfile: install pcov alongside xdebug via pecl; enable both at the docker layer. Xdebug stays for dev-time debugging. Also adds composer + git + unzip so the image is self-sufficient for `make test` and the Box-built PHAR pipeline. - tools/lsp/Makefile: test/mutation target now sets `XDEBUG_MODE=off` and `-d pcov.enabled=1 -d pcov.directory=src` for the Infection driver, plus forwards the same flags (and the error_reporting + memory_limit pins) to the PHPUnit subprocess via Infection's `--initial-tests-php-options`. Without the forwarding, the subprocess inherited neither and the deprecation flood from thecodingmachine/safe blew past PHPUnit's `failOnWarning` early. - .github/workflows/ci-lsp.yml: setup-php's `coverage:` flips from xdebug to pcov so the runner ships PCOV instead of Xdebug for the Infection job. CI behaviour is preserved (same MSI gate at 93%, same coverage scope); the swap is purely a memory-efficiency change. Local docker runs may still OOM if the host itself is RAM-starved -- separate environment issue, not a CI concern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`make -C tools/phpstorm-plugin build` was hardcoded to `docker compose run --rm jdk tools/phpstorm-plugin/gradlew ...`, but GitHub Actions doesn't have a docker compose stack -- it installs JDK 21 via actions/setup-java and runs everything directly on the host. The job was failing with: no such service: jdk make: *** [Makefile:26: build] Error 1 Use `ifdef CI` (GitHub Actions sets CI=true) to branch: - CI: `./gradlew` (working_dir is the package dir, no -p needed) - local dev: docker-compose jdk service (working_dir is repo root, full path to wrapper + -p tools/phpstorm-plugin) Local override also welcome: `make CI=1 build` forces host-side too. No other behavioural change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 424-test LSP suite plus per-mutation reruns overruns GitHub-hosted ubuntu-latest's RAM envelope (~7 GB); the initial PCOV-instrumented coverage subprocess gets killed with SIGTERM 143 mid-stream. Convert the `infection-lsp` job from a PR gate to opt-in via workflow_dispatch so PRs aren't blocked by an environment-resource issue that's orthogonal to the actual mutation MSI. The PHPUnit job above remains a hard gate (it runs the same 424 tests without the per-mutation rerun multiplier). Re-enable as a PR gate once the job is reshaped to filter or batch the test set, or migrated to a larger runner. Tracked as a follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`verify-plugin` job was failing with: Type 'org.gradle.language.jvm.tasks.ProcessResources' property 'xphpLspPhar' specifies file '.../tools/lsp/var/xphp-lsp.phar' which doesn't exist. The job skipped PHP setup + PHAR build under the assumption that the verifier only reads compiled classes -- but Gradle's build graph treats `processResources` as a prerequisite of `verifyPlugin`, and `processResources` declares the PHAR as a required input file. Without it on disk the build aborts before the verifier runs. Add the same `setup-php` + `composer install` + `make build/phar` prefix the `gradle-build` job already uses. PHAR build is cheap (~2s) compared to the verifier's IDE-matrix download that follows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`make test/mutation` was failing the 93% gate at 92.71% (786 killed,
63 escaped, 864 total). Adds seven targeted tests against the
highest-signal escaped mutants:
Group A -- single-purpose locks:
- ByteOffsetMapTest::testSameLengthEntryBeforeLengthChangingEntryPreservesShift
`continue` vs `break` inside fromReplacements' filter loop:
same-length entries (the common `<...>` -> spaces case) must be
skipped without halting iteration; otherwise downstream T[] ->
array shifts get dropped and positions stop remapping.
- RegistryBoundsTest::testSatisfiedBoundOnEarlierParamStillChecksLaterParam
Pair<K: Stringable, V: Stringable> with K valid + V invalid: the
verdict-true `continue` must NOT short-circuit subsequent param
validation. Companions the existing testBoundOnLaterParam... but
targets the bound-satisfied branch, not the unbounded-param one.
- XphpSourceParserTest::testParseWithMapIsCallableFromOutsideTheClass
Locks parseWithMap's `public` visibility -- the LSP analyzer
depends on calling it externally; mutation testing in core
couldn't observe a `protected` regression until something inside
core called it from outside the class.
Group B -- EOF-boundary fixtures (each kills several `<` vs `<=`,
`||` vs `&&`, ArrayKey-access mutations at once via tolerant-parser
crash):
- testGenericMethodScannerHandlesFunctionNameAtEndOfSource
- testGenericClassScannerHandlesClassNameAtEndOfSource
- testGenericInstantiationScannerHandlesIdentifierAtEndOfSource
- testParseTypeParamListHandlesAngleBracketAtEndOfSource
Each feeds a source where the lookahead index lands at exactly
count($tokens). Mutated boundary checks dereference $tokens[$n]
and crash; the original short-circuits.
After: 799 killed / 54 escaped / 93% MSI. Gate passes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After last commit's targeted tests took MSI from 92.71% to 93%, a second pass documents the remaining 54 escaped mutants as either killable-in-one-line or genuinely equivalent. Result: 100% MSI, zero escapes. - One new test: parseTolerantWithMap attaches ATTR_GENERIC_PARAMS + ATTR_TEMPLATE_FQN to the AST (kills the MethodCallRemoval mutation on the resolveAndAttach call that my Group B fixtures had newly surfaced). - Three inline `@infection-ignore-all` annotations inside the XphpSourceParser anonymous NodeVisitor + one in CallSiteRewriter + one in GenericMethodCompiler, each with a one-line "why equivalent" comment. Method-scoped infection.json5 ignores don't reach anonymous classes, so source-level annotations are the existing convention for these spots. - infection.json5 extended with ~30 new entries covering: defensive guard equivalents (parseTypeParamList / parseTypeArgList / etc), Linux-path-normalisation equivalents (Compiler::relativePath + SpecializedClassGenerator::emit), boundary-equivalents in ByteOffsetMap::toOriginal, and the explanatory-but-unreachable cosmetic mutators (CastString, Concat reorders, etc). - Gate bumped 93% -> 95% as a heads-up that future test-suite additions need to keep mutation coverage tight. Headroom for one follow-up commit that adds a new mutation site before a test catches it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `jdk` service (JDK 21 + Gradle + X11 forwarding) was living in
docker-compose.override.yml, which means every contributor had to
either copy it locally or run their own override -- friction that
shows up the first time anyone tries `make -C tools/phpstorm-plugin
build` on a fresh checkout.
Move the service definition and its `gradle_cache` named volume into
the tracked docker-compose.yml so:
- `make -C tools/phpstorm-plugin {build,verify,run-ide}` works out of
the box (each target shells out to `docker compose run --rm jdk`)
- the IntelliJ Platform Gradle Plugin's ~1GB PhpStorm 2026.1.2
cached distribution persists across runs via the named volume
- rootless docker maps the container's root back to the host user, so
build outputs (build/, .gradle/) land owned by you -- no chown
cleanup required
No host JDK install is expected or supported -- the build is
reproducible against the exact JDK + X11 stack inside the container.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ated)
CI's verifyPlugin job was failing with two distinct findings across
PhpStorm 2026.1 (PS-261.25134.16) and 2026.2 EAP (PS-262.6228.31):
1. OVERRIDE_ONLY_API_USAGES: `LspCustomization()` direct constructor
invocation. `LspCustomization` is annotated `@ApiStatus.OverrideOnly`
-- client code must subclass it, not construct it. The empty
anonymous subclass form `object : LspCustomization() {}` is
functionally identical (we still get the no-arg defaults) and
satisfies the contract.
2. Deprecated API usage:
`FileChooserDescriptorFactory.createSingleFileDescriptor()`.
Replace with the direct `FileChooserDescriptor(true, false, false,
false, false, false)` constructor that has been stable since the
class's introduction -- explicit `(chooseFiles, chooseFolders,
chooseJars, chooseJarsAsFiles, chooseJarContents, chooseMultiple)`
tuple. Two call sites in XphpSettingsConfigurable: the LSP
binary picker and the PHP interpreter picker.
Verifier locally: both PS-261 and PS-262 now Compatible with zero
violations. `make -C tools/phpstorm-plugin build` also clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI's gradle-build job kept failing with: Could not resolve all dependencies for configuration ':compileClasspath'. > java.nio.file.ClosedFileSystemException (no error message) ... Unable to read plugin descriptor 'intellij.terminal.backend.xml' at jdk.zipfs/jdk.nio.zipfs.ZipFileSystem.ensureOpen Root cause: a race in Gradle 9.0.0 + JDK 21 + the IntelliJ Platform Gradle Plugin 2.16.x. The platform plugin walks hundreds of `intellij.*.xml` plugin-descriptor entries inside the cached PhpStorm distribution zip during compileClasspath resolution. With `org.gradle.parallel = true`, multiple worker threads read entries from the same `ZipFileSystem` concurrently and one thread's finalize/close races another thread's read. Local 1-2-core builds slip through; CI's clean-cache 4-core runner hits the race almost every time. Flip parallel off; keep caching on (it's per-task and not part of the race). The plugin is small enough that single-threaded configure + compile costs are negligible (~+1s locally on cold cache). Will revert once the platform plugin pins a fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tools/lsp/(PHP, onphpactor/language-server+worse-reflection) covering diagnostics, hover,GTD, find references, rename, document/workspace symbols, file watcher, and completion (member / static /
static-prop / type-arg / variable / expression) with xphp-specific monomorphization-aware paths
layered ahead of the worse-reflection fallback.
tools/phpstorm-plugin/(Kotlin + Gradle, IntelliJ Platform LSP API), bundling the LSP as aPHAR, registering the
.xphpfile type with TextMate syntax highlighting.What ships
LSP server (
tools/lsp/)Headless CLI:
tools/lsp/bin/xphp-lsp --lint <file...>emitsfile:line:col: severity: [code] message, exitsnon-zero on errors -- CI-grade today.
LSP capabilities:
textDocument/publishDiagnostics-- parse errors, bound violations, duplicate templates.textDocument/hover-- xphp generics + PHP semantic (class / function / method / property / native funcs); parameterand return-type substitution at static / instance / free-function call sites.
textDocument/definition-- xphp generics + PHP semantic; closed-file targets viaFqnIndex;use function ...imports; cross-receiver inheritance walk.
textDocument/references-- classes, functions, methods, properties with subclass-inherited member walks ($dog->speak()matchesAnimal::speakwhen Dog doesn't override).textDocument/rename-- alias-aware short-name rewriting;RenameFileresource ops gated on clientresourceOperationscapability.textDocument/completion-- type-arg position with bound-aware filtering (Box<T: \Stringable>filters candidates);$obj->member access;Cls::static access;Cls::$static property; scope-aware variables; visibility-awarefiltering across same-class / subclass / external contexts; suppression inside strings,
comments, heredocs.
textDocument/documentSymbol-- hierarchical ClassLike / function / method tree (Cmd+O / Structure panel).workspace/symbol-- cross-file FQN search viaFqnIndex.workspace/didChangeWatchedFiles-- invalidates the filesystem index on external edits.Architecture:
FqnIndexcollapses three formerly-parallel walkers (FilesystemSourceLocator,WorkspaceSymbols::allClassFqns,WorkspaceClassLikeLookup) into one workspace-wide index. Open-doc overlay viaParsedDocumentCache; lazy filesystem walk on first query; bulk invalidation on file-watcher events.GenericResolverdoes the monomorphization-aware substitution end-to-end: in-same-filenew Generic<...>()bindings,param-typed scope entries (
function f(Collection<User> $users)), N-step chained calls via the recursiveinferType(Expr, Env)typer, closure-captured variables throughDocumentScopes, andsubstitution at static-call / free-function / instance-method / property-fetch / chained-property sites.
UTF-16column counting inPositionMapper the LSP spec (surrogate pairs for supplementary-plane chars).findClassByName(vendor / tests / fixtures / stubs paths get a +10 penalty so canonicalsrc/wins).PhpStorm plugin (
tools/phpstorm-plugin/)xphp-lsp.pharand auto-extracts it on first load..xphpfile type with TextMate v2 syntax grammar.phpPath(VS Code parity).LspCustomization.make -C tools/phpstorm-plugin distproduces an installable.zip;make run-ideboots a sandboxed PhpStorm forsmoke testing.
VS Code extension (
tools/lsp/vscode-extension/)source.php, F5 dev loop. Not slated for marketplacepublication -- exists for local dev only.
What's NOT in this PR
resourceOperations: ["create"]only and silently drops
RenameFileops. The server emits them when the client advertises support; PhpStorm doesn't,so they're effectively no-ops. Tracked separately.
Test plan
make -C tools/lsp testmake -C tools/lsp test/mutation-- Infection passes 93% min-covered MSI gate.make -C tools/phpstorm-plugin build-- Gradle build green.make -C tools/phpstorm-plugin verify-- IntelliJ Platform Plugin Verifier passes.make -C tools/phpstorm-plugin dist-- produces installable plugin zip.