From 55b8e87fef21daf03b2b1d7ae223c50ef6e0a968 Mon Sep 17 00:00:00 2001 From: Florian DAVID Date: Sat, 23 May 2026 23:56:08 +0200 Subject: [PATCH] 0.5.0: honour neon excludePaths (closes #1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Warm worker was force-analyzing files the CLI phpstan would skip, producing false positives on test files whose lifecycle bootstrap (setUpBeforeClass / tearDownAfterClass) wasn't loaded. PhpstanRunner now: - Caches excludePaths.{analyse,analyseAndScan} at boot via the existing dump-parameters --json round-trip - Short-circuits analyse() with [] when the file matches - Check fires BEFORE ensureWorker() on warm calls — excluded files pay no worker boot cost - fnmatch handles absolute + relative globs + any-suffix match on absolute paths (so `tests/unit/*` matches /repo/tests/unit/Foo.php) TDD: 5 unit tests written first (RED), then implemented (GREEN). Closes #1. Co-Authored-By: Max --- CHANGELOG.md | 10 +++ bin/mcp-phpstan-warm | 2 +- src/PhpstanRunner.php | 94 ++++++++++++++++++++ tests/Unit/PhpstanRunnerExcludeTest.php | 113 ++++++++++++++++++++++++ 4 files changed, 218 insertions(+), 1 deletion(-) create mode 100644 tests/Unit/PhpstanRunnerExcludeTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index fa3f8e2..1011ef1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,16 @@ This project follows [Semantic Versioning](https://semver.org/). ## [Unreleased] +## [0.5.0] — 2026-05-23 + +### Fixed + +- **Honour neon `excludePaths` (closes [#1](https://github.com/Digital-Process-Tools/mcp-phpstan-warm/issues/1)).** The warm worker previously force-analysed files the CLI `phpstan analyse` would skip — producing false positives on test files whose lifecycle methods (`setUpBeforeClass`, `tearDownAfterClass`) weren't loaded with the project bootstrap. `PhpstanRunner` now caches `excludePaths.analyse` + `excludePaths.analyseAndScan` at boot via the same `dump-parameters --json` round-trip as `ignoreErrors`, and short-circuits `analyse()` with an empty errors list when the file matches an exclude glob. Matching covers absolute globs, relative globs (`tests/unit/*`) against any suffix of an absolute path, and `fnmatch` against both raw and realpath-resolved candidates. Check fires BEFORE `ensureWorker()` on warm calls so excluded files pay no boot cost. + +### Added + +- Unit tests `PhpstanRunnerExcludeTest` (5 cases) — absolute glob match, relative glob match, empty-list passthrough, short-circuit with allowlist, short-circuit without allowlist (legacy callers). + ## [0.4.0] — 2026-05-23 ### Security diff --git a/bin/mcp-phpstan-warm b/bin/mcp-phpstan-warm index 68ac3bd..8fbda0b 100755 --- a/bin/mcp-phpstan-warm +++ b/bin/mcp-phpstan-warm @@ -52,7 +52,7 @@ if ($config !== null) { putenv('MCP_PHPSTAN_PATHS=' . $paths); $server = Server::builder() - ->setServerInfo('mcp-phpstan-warm', '0.4.0') + ->setServerInfo('mcp-phpstan-warm', '0.5.0') ->setDiscovery(dirname(__DIR__), ['src']) ->build(); diff --git a/src/PhpstanRunner.php b/src/PhpstanRunner.php index 6e68771..ed73ea4 100644 --- a/src/PhpstanRunner.php +++ b/src/PhpstanRunner.php @@ -57,6 +57,18 @@ final class PhpstanRunner */ private ?array $ignoreErrors = null; + /** + * Cached excludePaths globs loaded once at boot via `phpstan dump-parameters`. + * Combined union of `excludePaths.analyse` + `excludePaths.analyseAndScan` from + * the neon config. Used by isExcluded() to honour neon excludes — without this, + * the warm worker force-analyzes files the CLI `phpstan analyse` would skip, + * producing false positives on test files whose lifecycle methods (setUpBeforeClass, + * tearDownAfterClass) aren't loaded with the right bootstrap. Closes issue #1. + * + * @var list|null null = not yet loaded + */ + private ?array $excludePaths = null; + public function isWarm(): bool { return $this->handshakeDone && $this->isWorkerAlive(); @@ -70,6 +82,17 @@ public function isWarm(): bool */ public function analyse(string $path): array { + // Issue #1: honour neon excludePaths so files the CLI phpstan would + // skip ("No files found to analyse.") don't get force-analysed by the + // warm worker. Test files excluded from the main analysis would + // otherwise produce confusing false positives because their + // lifecycle bootstrap isn't loaded. We check BEFORE ensureWorker() + // so an excluded file does not pay the worker boot cost — except + // on the very first call (excludePaths is populated by the boot). + if ($this->isExcluded($path)) { + return []; + } + $this->ensureWorker(); // Defence in depth: phpstan's worker may surface source-line context @@ -78,6 +101,12 @@ public function analyse(string $path): array // arbitrary files (e.g. /etc/passwd, ~/.ssh/*) via parse-error leaks. $this->assertPathAllowed($path); + // Re-check excludes after boot — on the cold first call, excludePaths + // was null above, so the early gate was a no-op. Now it is populated. + if ($this->isExcluded($path)) { + return []; + } + $request = json_encode(['action' => 'analyse', 'files' => [$path]]) . "\n"; $written = @fwrite($this->workerStream, $request); if ($written === false || $written === 0) { @@ -394,6 +423,71 @@ private function loadIgnoreErrors(): void } $this->ignoreErrors = array_values($decoded['ignoreErrors']); + + // Issue #1: extract excludePaths in the same pass — dump-parameters + // already gives us both. PHPStan stores them under + // `excludePaths.analyse` and `excludePaths.analyseAndScan`; we union + // them since a file in either list should be skipped by analyse(). + $excludes = []; + $ex = $decoded['excludePaths'] ?? null; + if (is_array($ex)) { + foreach (['analyse', 'analyseAndScan'] as $key) { + if (isset($ex[$key]) && is_array($ex[$key])) { + foreach ($ex[$key] as $glob) { + if (is_string($glob) && $glob !== '') { + $excludes[] = $glob; + } + } + } + } + } + $this->excludePaths = array_values(array_unique($excludes)); + } + + /** + * Match a file path against the cached excludePaths globs (issue #1). + * + * PHPStan's neon entries can be absolute or relative (`tests/*`). We match + * both shapes: + * - Absolute exclude vs absolute path: direct fnmatch. + * - Relative exclude vs absolute path: fnmatch against any path suffix. + * - Otherwise: fnmatch the input as-is. + * + * Returns false on an empty allowlist so legacy callers keep working. + */ + private function isExcluded(string $path): bool + { + if ($this->excludePaths === null || $this->excludePaths === []) { + return false; + } + + $real = realpath($path); + $candidates = [$path]; + if ($real !== false && $real !== $path) { + $candidates[] = $real; + } + + foreach ($this->excludePaths as $glob) { + $isAbsolute = ($glob !== '' && $glob[0] === DIRECTORY_SEPARATOR); + foreach ($candidates as $candidate) { + if (fnmatch($glob, $candidate)) { + return true; + } + // Relative glob like `tests/unit/*` should match the trailing + // segment of an absolute path. We test by stripping leading + // path components and re-matching. + if (!$isAbsolute) { + $parts = explode(DIRECTORY_SEPARATOR, ltrim($candidate, DIRECTORY_SEPARATOR)); + for ($i = 0; $i < count($parts); $i++) { + $tail = implode(DIRECTORY_SEPARATOR, array_slice($parts, $i)); + if (fnmatch($glob, $tail)) { + return true; + } + } + } + } + } + return false; } /** diff --git a/tests/Unit/PhpstanRunnerExcludeTest.php b/tests/Unit/PhpstanRunnerExcludeTest.php new file mode 100644 index 0000000..58b5ca4 --- /dev/null +++ b/tests/Unit/PhpstanRunnerExcludeTest.php @@ -0,0 +1,113 @@ +getProperty('excludePaths'); + $prop->setValue($runner, ['/abs/repo/tests/unit/*Test.php']); + + $m = $rc->getMethod('isExcluded'); + $m->setAccessible(true); + + self::assertTrue($m->invoke($runner, '/abs/repo/tests/unit/FooTest.php')); + self::assertFalse($m->invoke($runner, '/abs/repo/src/Foo.php')); + } + + public function testIsExcludedMatchesRelativeGlob(): void + { + // Real exclude entries are often relative like `tests/*` — must match + // both relative input AND absolute realpath that contains the suffix. + $runner = new PhpstanRunner(); + $rc = new \ReflectionClass($runner); + $rc->getProperty('excludePaths')->setValue($runner, ['tests/unit/*']); + $m = $rc->getMethod('isExcluded'); + $m->setAccessible(true); + + self::assertTrue($m->invoke($runner, '/repo/tests/unit/Bar.php')); + self::assertTrue($m->invoke($runner, 'tests/unit/Bar.php')); + self::assertFalse($m->invoke($runner, '/repo/src/Bar.php')); + } + + public function testIsExcludedEmptyListReturnsFalse(): void + { + $runner = new PhpstanRunner(); + $rc = new \ReflectionClass($runner); + $rc->getProperty('excludePaths')->setValue($runner, []); + $m = $rc->getMethod('isExcluded'); + $m->setAccessible(true); + + self::assertFalse($m->invoke($runner, '/anywhere/Foo.php')); + } + + public function testAnalyseShortCircuitsForExcludedPath(): void + { + // The fix path: when isExcluded returns true, analyse() must return an + // empty errors list WITHOUT booting the worker. We assert by leaving + // handshakeDone=false — if analyse() reached ensureWorker() it would + // try to spawn phpstan and the test would hang or fail loudly. + $tmp = sys_get_temp_dir() . '/mcp-phpstan-excl-' . bin2hex(random_bytes(4)) . '.php'; + file_put_contents($tmp, "getProperty('excludePaths')->setValue($runner, [$tmp]); + // Force the warm-state lie so ensureWorker() would early-return — + // belt-and-braces: if exclusion fails to short-circuit, we won't + // hang on a real worker spawn. + $rc->getProperty('handshakeDone')->setValue($runner, true); + + $errors = $runner->analyse($tmp); + self::assertSame([], $errors); + } finally { + @unlink($tmp); + } + } + + public function testAnalyseShortCircuitWithoutAllowedPathsConfig(): void + { + // The exclude check must run even when --paths allowlist is empty + // (legacy callers). Empty allowedPaths means assertPathAllowed is a + // no-op, so the exclude branch is the only protection against + // false positives from `tearDown`-only reads in excluded test files. + $tmp = sys_get_temp_dir() . '/mcp-phpstan-excl2-' . bin2hex(random_bytes(4)) . '.php'; + file_put_contents($tmp, "getProperty('excludePaths')->setValue($runner, [$tmp]); + $rc->getProperty('allowedPaths')->setValue($runner, []); // legacy mode + $rc->getProperty('handshakeDone')->setValue($runner, true); + + $errors = $runner->analyse($tmp); + self::assertSame([], $errors); + } finally { + @unlink($tmp); + } + } +}