diff --git a/CLAUDE.md b/CLAUDE.md index a60e7031..dd6388ae 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -109,11 +109,12 @@ The cache's size limit evicts. #### Invalidation -The server never decides on its own that an answer is stale. -The editor is the only source of change events. +A change to a file's text needs no event, because the text is read when it is asked for. +An index over which files exist does need one. Two events invalidate: `workspace/didChangeWatchedFiles` for a path, and closing a document that was open. Both flow through `InvalidatableInterface::invalidate`, which fans out to every invalidatable in the wiring. -An open buffer is not an invalidation; it wins by composite order while it is open. +A client may not support `workspace/didChangeWatchedFiles`. +`PollingFileWatcher` stands in for that client only, and reports the same events through the same fan-out. Built-ins are never invalidated until the target environment can change. ## Handling Design or Specification Tensions diff --git a/deptrac.yaml b/deptrac.yaml index e5d9b95f..e2177323 100644 --- a/deptrac.yaml +++ b/deptrac.yaml @@ -36,6 +36,10 @@ deptrac: - name: Knowledge collectors: - { type: directory, value: src/Knowledge/.* } + - name: Message + collectors: + - type: classLike + value: ^Firehed\\PhpLsp\\BeforeMessageInterface$ - name: Parser collectors: - { type: directory, value: src/Parser/.* } @@ -62,13 +66,16 @@ deptrac: - Completion - Document - Domain + - Filesystem - Handler - Knowledge + - Message - Parser - Protocol - Repository - Resolution - Transport + - Watch Handler: - Cache # InvalidatableInterface: watched-file changes and close-after-edit drop cached on-disk state - Capability @@ -99,6 +106,12 @@ deptrac: - Parser - Repository - Watch # WatchedPathsSourceInterface: holders name the paths they depend on + Watch: + - Cache # InvalidatableInterface: the stand-in reports through the same fan-out + - Capability + - Domain + - Filesystem + - Message Repository: - Document # FileUri, the path/URI conversion authority - Domain diff --git a/docs/vim-ale.md b/docs/vim-ale.md index 021b1263..45a30bc8 100644 --- a/docs/vim-ale.md +++ b/docs/vim-ale.md @@ -29,6 +29,12 @@ call ale#linter#Define('php', { \}) ``` +## Changes on Disk + +ALE's own LSP client does not send `workspace/didChangeWatchedFiles`. +php-lsp notices this from the capabilities ALE declares and looks at the disk itself before each message. +A class created, deleted, or regenerated by `composer install` is seen on the next request. + ## Verifying the Connection 1. Open a PHP file diff --git a/phpstan.neon b/phpstan.neon index 02bfeabd..4e8c2269 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -267,10 +267,11 @@ parameters: - 'disk_total_space()' - 'parse_ini_file()' - 'move_uploaded_file()' - message: 'filesystem access is confined: SourceFileReader reads source, PhpDirectoryReader lists directories, ComposerAutoloadMap and ComposerMapBackend read Composer metadata, transport owns its streams' + message: 'filesystem access is confined: SourceFileReader reads source, PhpDirectoryReader lists directories, StatReader reads modification times, ComposerAutoloadMap and ComposerMapBackend read Composer metadata, transport owns its streams' allowIn: - src/Document/SourceFileReader.php - src/Filesystem/PhpDirectoryReader.php + - src/Filesystem/StatReader.php - src/Domain/ComposerAutoloadMap.php - src/Knowledge/ComposerMapBackend.php - src/Transport/* @@ -320,6 +321,7 @@ parameters: - src/Handler/DidChangeWatchedFilesHandler.php # the watched-file trigger - src/Handler/TextDocumentSyncHandler.php # the close-after-edit trigger - src/Knowledge/CompositeInvalidatable.php # the fan-out itself + - src/Watch/PollingFileWatcher.php # the trigger for a client without watched-file support - tests/* - method: diff --git a/src/BeforeMessageInterface.php b/src/BeforeMessageInterface.php new file mode 100644 index 00000000..ba6866d2 --- /dev/null +++ b/src/BeforeMessageInterface.php @@ -0,0 +1,13 @@ +handlers = [$lifecycleHandler, ...$handlers]; } @@ -137,7 +141,14 @@ public static function forProject( // is no static server capability for them), gated on the client declaring // support; the events invalidate cached workspace state (RFC 1 §5.2, §5.3). $watchedFilesRegistrar = new WatchedFilesRegistrar(new TransportClientConnection($transport)); - $lifecycleHandler = new LifecycleHandler($negotiator, [$watchedFilesRegistrar]); + $fileWatcher = new PollingFileWatcher( + $knowledge->watched, + $invalidator, + new PhpDirectoryReader(), + new StatReader(), + time(...), + ); + $lifecycleHandler = new LifecycleHandler($negotiator, [$watchedFilesRegistrar, $fileWatcher]); $handlers = [ new TextDocumentSyncHandler( @@ -173,7 +184,7 @@ public static function forProject( ), ]; - return new self($transport, $lifecycleHandler, $handlers, $parser); + return new self($transport, $lifecycleHandler, $handlers, $parser, $fileWatcher); } public function run(): int @@ -204,6 +215,8 @@ public function run(): int if ($error === null) { try { + $this->beforeMessage?->beforeMessage(); + // Inside the try because `supports()` is part of the // handler contract: a failure selecting a handler is a // handler failure, and must be answered rather than diff --git a/src/Watch/PollingFileWatcher.php b/src/Watch/PollingFileWatcher.php new file mode 100644 index 00000000..d1e9854d --- /dev/null +++ b/src/Watch/PollingFileWatcher.php @@ -0,0 +1,167 @@ + File -> how it last looked */ + private array $files = []; + + /** @var array> Root -> directory at or beneath it -> how it last looked */ + private array $roots = []; + + /** + * @param Closure(): int $now Unix time + */ + public function __construct( + private readonly WatchedPathsSourceInterface $watched, + private readonly InvalidatableInterface $invalidator, + private readonly PhpDirectoryReader $directories, + private readonly StatReader $stat, + private readonly Closure $now, + ) { + } + + public function beforeMessage(): void + { + if (!$this->standingIn) { + return; + } + + $paths = $this->watched->watchedPaths(); + $now = ($this->now)(); + $changed = [ + ...$this->changedAmong($paths->files, $now), + ...$this->changedUnder($paths->roots, $now), + ]; + + foreach (array_unique($changed) as $path) { + $this->invalidator->invalidate(FileUri::fromPath($path)); + } + } + + public function onInitialized(SessionCapabilities $capabilities): void + { + $this->standingIn = !$capabilities->watchedFilesDynamicRegistration; + } + + /** + * @param list $files + * @return list + */ + private function changedAmong(array $files, int $now): array + { + $changed = []; + $snapshots = []; + foreach ($files as $file) { + $stamp = $this->stat->stamp($file); + if (array_key_exists($file, $this->files) && $this->files[$file]->mayDifferFrom($stamp)) { + $changed[] = $file; + } + $snapshots[$file] = new Snapshot($stamp, $now); + } + $this->files = $snapshots; + + return $changed; + } + + /** + * @param list $roots + * @return list + */ + private function changedUnder(array $roots, int $now): array + { + $this->roots = array_intersect_key($this->roots, array_flip($roots)); + + $changed = []; + foreach ($roots as $root) { + if (!array_key_exists($root, $this->roots)) { + $this->roots[$root] = $this->snapshotsUnder($root, $now); + continue; + } + foreach ($this->roots[$root] as $directory => $before) { + $changed = [...$changed, ...$this->changedIn($root, $directory, $before, $now)]; + } + } + + return $changed; + } + + /** + * @return list + */ + private function changedIn(string $root, string $directory, Snapshot $before, int $now): array + { + $stamp = $this->stat->stamp($directory); + if (!$before->mayDifferFrom($stamp)) { + return []; + } + + $listing = $this->directories->read($directory); + if ($listing === null) { + // The root stays watched so its return is seen; anything beneath it + // is found again through its parent. + if ($directory === $root) { + $this->roots[$root][$directory] = new Snapshot(null, $now); + } else { + unset($this->roots[$root][$directory]); + } + + return $before->files; + } + + $this->roots[$root][$directory] = new Snapshot($stamp, $now, $listing->files); + $changed = [ + ...array_diff($listing->files, $before->files), + ...array_diff($before->files, $listing->files), + ]; + + foreach ($listing->directories as $child) { + if (array_key_exists($child, $this->roots[$root])) { + continue; + } + foreach ($this->snapshotsUnder($child, $now) as $path => $snapshot) { + $this->roots[$root][$path] = $snapshot; + $changed = [...$changed, ...$snapshot->files]; + } + } + + return $changed; + } + + /** + * @return array The directory and every directory beneath it + */ + private function snapshotsUnder(string $directory, int $now): array + { + $snapshots = [$directory => new Snapshot($this->stat->stamp($directory), $now)]; + foreach ($this->directories->walk($directory) as $listing) { + $snapshots[$listing->path] = new Snapshot($this->stat->stamp($listing->path), $now, $listing->files); + } + + return $snapshots; + } +} diff --git a/src/Watch/Snapshot.php b/src/Watch/Snapshot.php new file mode 100644 index 00000000..6fc6a779 --- /dev/null +++ b/src/Watch/Snapshot.php @@ -0,0 +1,41 @@ + $files For a directory, the PHP files directly inside + */ + public function __construct( + public ?PathStamp $stamp, + public int $seenAt, + public array $files = [], + ) { + } + + /** + * The filesystem reports whole seconds, so a path last looked at during the + * second it was modified may have been modified again since: an unchanged + * stamp proves nothing within its own second. + */ + public function mayDifferFrom(?PathStamp $current): bool + { + if ($this->stamp === null || $current === null) { + return $this->stamp !== $current; + } + + return $current->modifiedAt !== $this->stamp->modifiedAt + || $current->size !== $this->stamp->size + || $current->modifiedAt >= $this->seenAt; + } +} diff --git a/tests/Filesystem/StatReaderTest.php b/tests/Filesystem/StatReaderTest.php new file mode 100644 index 00000000..2a7cdcba --- /dev/null +++ b/tests/Filesystem/StatReaderTest.php @@ -0,0 +1,66 @@ +path = $path; + } + + protected function tearDown(): void + { + @unlink($this->path); + } + + public function testAStampCarriesTheModificationTimeAndSize(): void + { + copy($this->fixturePath('src/Domain/User.php'), $this->path); + touch($this->path, 1_000_000); + + $stamp = (new StatReader())->stamp($this->path); + + self::assertNotNull($stamp, 'a file that exists has a stamp'); + self::assertSame(1_000_000, $stamp->modifiedAt, 'the stamp carries when the path last changed'); + self::assertSame(filesize($this->fixturePath('src/Domain/User.php')), $stamp->size, 'and how large it is'); + } + + public function testAChangeMadeAfterAnEarlierReadIsSeen(): void + { + $reader = new StatReader(); + touch($this->path, 1_000_000); + $reader->stamp($this->path); + + touch($this->path, 2_000_000); + + self::assertSame( + 2_000_000, + $reader->stamp($this->path)?->modifiedAt, + 'PHP remembers stat results within a process; a stamp must not be served from that memory', + ); + } + + public function testAPathThatDoesNotExistHasNoStamp(): void + { + unlink($this->path); + + self::assertNull((new StatReader())->stamp($this->path), 'absence is an answer, not an error'); + } +} diff --git a/tests/ServerTest.php b/tests/ServerTest.php index fe5b96a2..d9f1d6bd 100644 --- a/tests/ServerTest.php +++ b/tests/ServerTest.php @@ -182,6 +182,69 @@ public function testDidChangeWatchedFilesInvalidatesWorkspaceStateThroughTheComp } } + /** + * [LSP] leaves `workspace/didChangeWatchedFiles` optional for a client. With + * one that never sends it, a class created on disk must still reach + * completion: the composed server stands in for the missing notification. + */ + public function testAFileCreatedOnDiskIsSeenWithoutWatchedFilesSupport(): void + { + $root = $this->createProject(''); + $consumerUri = 'file://' . $root . '/src/consumer.php'; + $sprocket = $root . '/src/Sprocket.php'; + + try { + $input = $this->buildMessages( + $this->initializeJson(1), + $this->initializedJson(), + $this->notificationJson('textDocument/didOpen', [ + 'textDocument' => [ + 'uri' => $consumerUri, + 'languageId' => 'php', + 'version' => 1, + 'text' => "classCompletionAt(2, $consumerUri), + $this->notificationJson('textDocument/didChange', [ + 'textDocument' => ['uri' => $consumerUri, 'version' => 2], + 'contentChanges' => [['text' => "classCompletionAt(3, $consumerUri), + $this->requestJson(4, 'shutdown'), + $this->notificationJson('exit'), + ); + + $outputBuffer = new WritableBuffer(); + // The class appears on disk after the first completion has built the + // name list, and nothing tells the server. + $appear = static function (Message $message) use ($sprocket): void { + if ($message->method === 'textDocument/didChange') { + file_put_contents($sprocket, "createTransport($input, $outputBuffer, $appear); + $server = Server::forProject($transport, new ServerInfo('test', '1.0'), $this->buildContainer(), $root); + + $server->run(); + + $responses = $this->decodeResponses($outputBuffer->buffer()); + self::assertContains( + 'Widget', + $this->completionLabels($this->responseWithId($responses, 2)), + 'the first completion must build the name list for it to be able to go stale', + ); + self::assertContains( + 'Sprocket', + $this->completionLabels($this->responseWithId($responses, 3)), + 'a class created on disk is offered on the next request', + ); + } finally { + @unlink($sprocket); + $this->removeProject($root); + } + } + public function testUnknownMethodReturnsError(): void { $input = $this->buildMessages( @@ -887,6 +950,15 @@ private function completionAt(int $id, string $uri): string ]); } + private function classCompletionAt(int $id, string $uri): string + { + // The consumer's `$w = new Xxx` sits at line 2; the cursor follows the prefix. + return $this->requestJson($id, 'textDocument/completion', [ + 'textDocument' => ['uri' => $uri], + 'position' => ['line' => 2, 'character' => strlen('$w = new Xxx')], + ]); + } + /** * @param array $response * @return list diff --git a/tests/Watch/PollingFileWatcherTest.php b/tests/Watch/PollingFileWatcherTest.php new file mode 100644 index 00000000..84d9927b --- /dev/null +++ b/tests/Watch/PollingFileWatcherTest.php @@ -0,0 +1,326 @@ + */ + private array $invalidated = []; + + private WatchedPaths $watched; + + private PollingFileWatcher $watcher; + + protected function setUp(): void + { + $root = tempnam(sys_get_temp_dir(), 'php-lsp-watch-'); + self::assertNotFalse($root, 'a temp path must be obtainable'); + unlink($root); + self::assertTrue(mkdir($root . '/src/Nested', 0777, true), 'the watched tree must be creatable'); + // macOS hands out temp paths through a symlink; the reader reports real ones. + $this->root = (string) realpath($root); + + $this->place('src/Widget.php'); + $this->place('src/Nested/Gadget.php'); + $this->place('bootstrap.php'); + $this->settle(); + + $this->watched = new WatchedPaths( + roots: [$this->root . '/src'], + files: [$this->root . '/bootstrap.php'], + ); + + $source = self::createStub(WatchedPathsSourceInterface::class); + $source->method('watchedPaths')->willReturnCallback(fn(): WatchedPaths => $this->watched); + + $invalidator = self::createStub(InvalidatableInterface::class); + $invalidator->method('invalidate')->willReturnCallback(function (string $uri): void { + $this->invalidated[] = $uri; + }); + + $this->watcher = new PollingFileWatcher( + $source, + $invalidator, + new PhpDirectoryReader(), + new StatReader(), + fn(): int => $this->now, + ); + $this->watcher->onInitialized(new SessionCapabilities(watchedFilesDynamicRegistration: false)); + $this->watcher->beforeMessage(); + } + + protected function tearDown(): void + { + self::remove($this->root); + } + + public function testTheFirstLookReportsNothing(): void + { + self::assertSame([], $this->invalidated, 'what is there when watching starts is not a change'); + } + + public function testNothingChangedReportsNothing(): void + { + $this->later(); + + self::assertSame([], $this->invalidated, 'an unchanged tree produces no events'); + } + + public function testAWatchedFileThatChangedIsReported(): void + { + touch($this->root . '/bootstrap.php', self::LONG_AGO + 500); + + $this->later(); + + self::assertSame([$this->uri('bootstrap.php')], $this->invalidated, 'a changed file is one event for it'); + } + + public function testAWatchedFileThatWasDeletedIsReported(): void + { + unlink($this->root . '/bootstrap.php'); + + $this->later(); + + self::assertSame([$this->uri('bootstrap.php')], $this->invalidated, 'a deleted file is one event for it'); + } + + public function testAWatchedFileThatAppearsIsReported(): void + { + $this->watched = $this->watched->with(new WatchedPaths(files: [$this->root . '/late.php'])); + $this->later(); + self::assertSame([], $this->invalidated, 'a file newly watched, and absent, is a baseline and not a change'); + + $this->place('late.php'); + $this->later(); + + self::assertSame([$this->uri('late.php')], $this->invalidated, 'a created file is one event for that file'); + } + + public function testAFileAddedUnderARootIsReported(): void + { + $this->place('src/Sprocket.php'); + touch($this->root . '/src', self::LONG_AGO + 500); + + $this->later(); + + self::assertSame([$this->uri('src/Sprocket.php')], $this->invalidated, 'only the new file is reported'); + } + + public function testAFileRemovedUnderARootIsReported(): void + { + unlink($this->root . '/src/Nested/Gadget.php'); + touch($this->root . '/src/Nested', self::LONG_AGO + 500); + + $this->later(); + + self::assertSame( + [$this->uri('src/Nested/Gadget.php')], + $this->invalidated, + 'a directory beneath the root is watched as the root is', + ); + } + + public function testAChangeToAFilesContentUnderARootIsNotReported(): void + { + touch($this->root . '/src/Widget.php', self::LONG_AGO + 500); + + $this->later(); + + self::assertSame([], $this->invalidated, 'under a root only a file\'s existence is watched'); + } + + public function testAnEntryThatIsNotPhpIsNotReported(): void + { + copy($this->fixturePath('composer.json'), $this->root . '/src/.Widget.php.swp'); + touch($this->root . '/src', self::LONG_AGO + 500); + + $this->later(); + + self::assertSame([], $this->invalidated, 'an editor\'s swap file changes the directory and no PHP file'); + } + + public function testADirectoryAddedUnderARootIsWatchedFromThenOn(): void + { + mkdir($this->root . '/src/Fresh'); + $this->place('src/Fresh/First.php'); + $this->settle(); + touch($this->root . '/src', self::LONG_AGO + 500); + + $this->later(); + self::assertSame([$this->uri('src/Fresh/First.php')], $this->invalidated, 'a new directory\'s files are new'); + + $this->invalidated = []; + $this->place('src/Fresh/Second.php'); + touch($this->root . '/src/Fresh', self::LONG_AGO + 700); + $this->later(); + + self::assertSame( + [$this->uri('src/Fresh/Second.php')], + $this->invalidated, + 'the new directory is watched like any other', + ); + } + + public function testADirectoryRemovedUnderARootReportsItsFiles(): void + { + unlink($this->root . '/src/Nested/Gadget.php'); + rmdir($this->root . '/src/Nested'); + touch($this->root . '/src', self::LONG_AGO + 500); + + $this->later(); + + self::assertSame([$this->uri('src/Nested/Gadget.php')], $this->invalidated, 'its files went with it'); + } + + public function testARootThatDoesNotExistYetIsSeenWhenItAppears(): void + { + $this->watched = new WatchedPaths(roots: [$this->root . '/lib']); + $this->later(); + + mkdir($this->root . '/lib'); + $this->place('lib/Arrival.php'); + touch($this->root . '/lib', self::LONG_AGO + 500); + $this->later(); + + self::assertSame( + [$this->uri('lib/Arrival.php')], + $this->invalidated, + 'Composer maps a prefix to a directory whether or not the directory exists yet', + ); + } + + public function testARootThatDisappearsIsSeenAgainWhenItReturns(): void + { + $this->watched = new WatchedPaths(roots: [$this->root . '/src/Nested']); + $this->later(); + + unlink($this->root . '/src/Nested/Gadget.php'); + rmdir($this->root . '/src/Nested'); + $this->later(); + self::assertSame([$this->uri('src/Nested/Gadget.php')], $this->invalidated, 'the files went with the root'); + + $this->invalidated = []; + mkdir($this->root . '/src/Nested'); + $this->place('src/Nested/Gadget.php'); + touch($this->root . '/src/Nested', self::LONG_AGO + 500); + $this->later(); + + self::assertSame([$this->uri('src/Nested/Gadget.php')], $this->invalidated, 'a branch checkout can do this'); + } + + public function testAChangeInTheSameSecondAsTheLastLookIsStillSeen(): void + { + // The filesystem reports whole seconds. A directory last looked at during + // the second it was modified may have been modified again since. + touch($this->root . '/src', $this->now); + $this->watcher->beforeMessage(); + $this->invalidated = []; + + $this->place('src/Sprocket.php'); + touch($this->root . '/src', $this->now); + $this->watcher->beforeMessage(); + + self::assertSame( + [$this->uri('src/Sprocket.php')], + $this->invalidated, + 'an unchanged modification time proves nothing within its own second', + ); + } + + public function testARootNoLongerWatchedIsForgotten(): void + { + $this->watched = new WatchedPaths(); + $this->later(); + + $this->place('src/Sprocket.php'); + touch($this->root . '/src', self::LONG_AGO + 500); + unlink($this->root . '/bootstrap.php'); + $this->later(); + + self::assertSame([], $this->invalidated, 'what no holder needs watched produces no events'); + } + + public function testAClientThatReportsChangesItselfIsNotSecondGuessed(): void + { + $this->watcher->onInitialized(new SessionCapabilities(watchedFilesDynamicRegistration: true)); + touch($this->root . '/bootstrap.php', self::LONG_AGO + 500); + + $this->later(); + + self::assertSame( + [], + $this->invalidated, + 'workspace/didChangeWatchedFiles is the route; this stands in only where a client lacks it', + ); + } + + private function later(): void + { + $this->now += 10; + $this->watcher->beforeMessage(); + } + + private function place(string $relative): void + { + self::assertTrue( + copy($this->fixturePath('src/Domain/User.php'), $this->root . '/' . $relative), + "{$relative} must be writable", + ); + touch($this->root . '/' . $relative, self::LONG_AGO); + } + + /** + * Writing into a directory stamps it with the real time; put every + * directory back to a time well before the test's clock. + */ + private function settle(): void + { + foreach ((new PhpDirectoryReader())->walk($this->root) as $listing) { + touch($listing->path, self::LONG_AGO); + } + } + + private function uri(string $relative): string + { + return FileUri::fromPath($this->root . '/' . $relative); + } + + private static function remove(string $directory): void + { + $entries = scandir($directory); + foreach ($entries === false ? [] : array_diff($entries, ['.', '..']) as $entry) { + $path = $directory . '/' . $entry; + is_dir($path) ? self::remove($path) : unlink($path); + } + rmdir($directory); + } +}