From 6c9e3f5811fb96d33d23cf7cbc8d9ee5c700e529 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sun, 18 Oct 2026 11:37:02 +0700 Subject: [PATCH 01/20] Add --no-cache option to analyse command --- docs/cache.md | 10 +++ src/Cli/AnalyseCommand.php | 85 +++++++++++++++--------- src/Cli/Usage.php | 2 +- tests/Cli/StructArmedApplicationTest.php | 68 +++++++++++++++++++ 4 files changed, 134 insertions(+), 31 deletions(-) diff --git a/docs/cache.md b/docs/cache.md index cb988480..6fa5d023 100644 --- a/docs/cache.md +++ b/docs/cache.md @@ -53,3 +53,13 @@ You can also clear cache directly: ```bash vendor/bin/structarmed --clear-cache ``` + +## Disabling Cache + +Use `--no-cache` for a one-off run, such as a CI job that starts without a cache, or the first scan of a huge codebase. StructArmed then neither hashes the analysed files nor reads or writes any cache entry, so the run skips work that only a later run would benefit from. + +```bash +vendor/bin/structarmed analyse --no-cache +``` + +The existing cache is left untouched; clear it separately with `vendor/bin/structarmed --clear-cache`. diff --git a/src/Cli/AnalyseCommand.php b/src/Cli/AnalyseCommand.php index 385118a4..f421d2ec 100644 --- a/src/Cli/AnalyseCommand.php +++ b/src/Cli/AnalyseCommand.php @@ -45,6 +45,7 @@ * generate-baseline?: string, * no-progress?: true, * clear-cache?: true, + * no-cache?: true, * disable-parallel?: true, * fix?: true * } @@ -62,6 +63,7 @@ private const FLAG_OPTIONS = [ '--no-progress' => 'no-progress', '--clear-cache' => 'clear-cache', + '--no-cache' => 'no-cache', '--disable-parallel' => 'disable-parallel', '--fix' => 'fix', ]; @@ -146,34 +148,51 @@ public function run(array $arguments, string $basePath): int $start = microtime(true); $fileHashProvider = new FileHashProvider(); $analysisCacheMetadataFactory = new AnalysisCacheMetadataFactory($fileHashProvider); - $configHash = $analysisCacheMetadataFactory->fileHash($configFile); - $composerGeneratedVersionHash = $analysisCacheMetadataFactory->composerGeneratedVersionHash(); - $analysisResultCache = new AnalysisResultCache( - $basePath, - $fileHashProvider, - $architecture->getCacheDirectory(), - $configHash, - $composerGeneratedVersionHash - ); - $analysisNodeCacheNamespace = $analysisCacheMetadataFactory->analysisNodeCacheNamespace( - $basePath, - $configHash - ); - $analyser = new Analyser($basePath, $analysisResultCache, $analysisNodeCacheNamespace); - - if (isset($options['clear-cache']) || $analysisResultCache->shouldInvalidate()) { - $analysisResultCache->clear(); + $analysisResultCache = null; + $analysisNodeCacheNamespace = ''; + + // Without the cache, no file is hashed and no cache entry is read or + // written, which saves that work on a one-off run over a huge codebase. + if (! isset($options['no-cache'])) { + $configHash = $analysisCacheMetadataFactory->fileHash($configFile); + $analysisResultCache = new AnalysisResultCache( + $basePath, + $fileHashProvider, + $architecture->getCacheDirectory(), + $configHash, + $analysisCacheMetadataFactory->composerGeneratedVersionHash() + ); + $analysisNodeCacheNamespace = $analysisCacheMetadataFactory->analysisNodeCacheNamespace( + $basePath, + $configHash + ); + + if (isset($options['clear-cache']) || $analysisResultCache->shouldInvalidate()) { + $analysisResultCache->clear(); + } } + $analyser = new Analyser($basePath, $analysisResultCache, $analysisNodeCacheNamespace); $files = $analyser->filesForAnalysis($architecture, $scanPaths); - $metadata = $analysisCacheMetadataFactory->metadata($basePath, $configFile, $scanPaths, $files); - $cacheKey = $analysisCacheMetadataFactory->key($metadata); $progress = $reportType === 'console' && ! isset($options['no-progress']) ? $this->progressHandler ?? new ConsoleProgressBar() : null; $analyserOptions = isset($options['disable-parallel']) ? AnalyserOptions::sequential() : null; + $metadata = []; + $cacheKey = ''; + + $ruleViolationCollection = null; - $ruleViolationCollection = $analysisResultCache->load($cacheKey, $metadata); + if ($analysisResultCache instanceof AnalysisResultCache) { + $metadata = $analysisCacheMetadataFactory->metadata( + $basePath, + $configFile, + $scanPaths, + $files + ); + $cacheKey = $analysisCacheMetadataFactory->key($metadata); + $ruleViolationCollection = $analysisResultCache->load($cacheKey, $metadata); + } if (! $ruleViolationCollection instanceof RuleViolationCollection) { $ruleViolationCollection = $analyser->analyse( @@ -183,7 +202,7 @@ public function run(array $arguments, string $basePath): int $analyserOptions, $files ); - $analysisResultCache->store($cacheKey, $metadata, $ruleViolationCollection); + $analysisResultCache?->store($cacheKey, $metadata, $ruleViolationCollection); } $elapsed = microtime(true) - $start; @@ -215,16 +234,9 @@ public function run(array $arguments, string $basePath): int } $violationCountBeforePass = $ruleViolationCollection->count(); - $analysisResultCache->clear(); + $analysisResultCache?->clear(); $files = $analyser->filesForAnalysis($architecture, $scanPaths); - $metadata = $analysisCacheMetadataFactory->metadata( - $basePath, - $configFile, - $scanPaths, - $files - ); - $cacheKey = $analysisCacheMetadataFactory->key($metadata); $unfilteredRuleViolationCollection = $analyser->analyse( $architecture, $scanPaths, @@ -232,7 +244,20 @@ public function run(array $arguments, string $basePath): int $analyserOptions, $files ); - $analysisResultCache->store($cacheKey, $metadata, $unfilteredRuleViolationCollection); + + if ($analysisResultCache instanceof AnalysisResultCache) { + $metadata = $analysisCacheMetadataFactory->metadata( + $basePath, + $configFile, + $scanPaths, + $files + ); + $analysisResultCache->store( + $analysisCacheMetadataFactory->key($metadata), + $metadata, + $unfilteredRuleViolationCollection + ); + } try { $ruleViolationCollection = $this->resolveRuleViolationCollection( diff --git a/src/Cli/Usage.php b/src/Cli/Usage.php index 8e80bdef..0b649e29 100644 --- a/src/Cli/Usage.php +++ b/src/Cli/Usage.php @@ -14,7 +14,7 @@ public static function render(): string structarmed init [--preset=ddd|mvc|psr4|psr1|psr12|per|psr15|yagni|codequality|all] structarmed analyse|analyze [path ...] [--config=path/to/structarmed.php] [-d|--basepath=path/to/project] [--report=console|json|github] [--no-progress] - [--clear-cache] [--disable-parallel] [--fix] [--generate-baseline=structarmed-baseline.php] + [--clear-cache] [--no-cache] [--disable-parallel] [--fix] [--generate-baseline=structarmed-baseline.php] structarmed --clear-cache [--config=path/to/structarmed.php] [-d|--basepath=path/to/project] TXT; diff --git a/tests/Cli/StructArmedApplicationTest.php b/tests/Cli/StructArmedApplicationTest.php index 616696aa..0f0699ab 100644 --- a/tests/Cli/StructArmedApplicationTest.php +++ b/tests/Cli/StructArmedApplicationTest.php @@ -658,6 +658,45 @@ public function testAnalyseCommandStoresResultInConfiguredCacheDirectory(): void } } + public function testAnalyseCommandWithNoCacheOptionDoesNotWriteCache(): void + { + $basePath = $this->createProjectDirectoryWithViolation(); + $cacheDirectory = $basePath . '/var/cache/structarmed'; + + file_put_contents($basePath . '/structarmed.php', <<<'PHP' +cacheDirectory('var/cache/structarmed') + ->layer('Source', 'src/') + ->rule('source.must_be_final', new MustBeFinalRule('Source')); +PHP); + + try { + [$exitCode, $output] = $this->runApplication( + [ + 'structarmed', + 'analyse', + '--config=' . $basePath . '/structarmed.php', + '--no-cache', + '--no-progress', + ], + $basePath + ); + + $this->assertSame(1, $exitCode, $output); + $this->assertStringContainsString('1 violation(s) found', $output); + $this->assertDirectoryDoesNotExist($cacheDirectory); + } finally { + $this->removeTempDirectory($basePath); + } + } + public function testAnalyseCommandRendersJsonReport(): void { $basePath = $this->createProjectDirectory(); @@ -731,6 +770,35 @@ public function testAnalyseCommandFixesFixableViolations(): void } } + public function testAnalyseCommandFixesFixableViolationsWithNoCacheOption(): void + { + $basePath = $this->createProjectDirectoryWithImplicitMethodVisibilityViolation(); + + try { + [$exitCode, $output] = $this->runApplication( + [ + 'structarmed', + 'analyze', + '--config=' . $basePath . '/structarmed.php', + '--fix', + '--no-cache', + '--no-progress', + ], + $basePath + ); + + $this->assertSame(0, $exitCode, $output); + $this->assertStringContainsString('1 violation has been fixed.', $this->withoutAnsi($output)); + $this->assertStringContainsString('No violations found', $output); + $this->assertStringContainsString( + ' public function handle(): void', + (string) file_get_contents($basePath . '/src/Foo.php') + ); + } finally { + $this->removeTempDirectory($basePath); + } + } + public function testAnalyseCommandCountsEveryViolationResolvedByOneFix(): void { $basePath = $this->createProjectDirectory(); From 8e31398616a274301ff42835c235e3a2ff68ec38 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sun, 18 Oct 2026 11:49:08 +0700 Subject: [PATCH 02/20] avoid use --clear-cache along with --no-cache --- docs/cache.md | 2 +- src/Cli/AnalyseCommand.php | 6 ++ tests/Cli/StructArmedApplicationTest.php | 92 ++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 1 deletion(-) diff --git a/docs/cache.md b/docs/cache.md index 6fa5d023..85945b72 100644 --- a/docs/cache.md +++ b/docs/cache.md @@ -62,4 +62,4 @@ Use `--no-cache` for a one-off run, such as a CI job that starts without a cache vendor/bin/structarmed analyse --no-cache ``` -The existing cache is left untouched; clear it separately with `vendor/bin/structarmed --clear-cache`. +The existing cache is left untouched. `--no-cache` cannot be combined with `--clear-cache`; clear the cache separately with `vendor/bin/structarmed --clear-cache`. diff --git a/src/Cli/AnalyseCommand.php b/src/Cli/AnalyseCommand.php index f421d2ec..586ce8f5 100644 --- a/src/Cli/AnalyseCommand.php +++ b/src/Cli/AnalyseCommand.php @@ -102,6 +102,12 @@ public function run(array $arguments, string $basePath): int return 1; } + if (isset($options['no-cache'], $options['clear-cache'])) { + echo "Error: --no-cache and --clear-cache cannot be used together.\n"; + + return 1; + } + $workingDirectory = $basePath; if (isset($options['basepath'])) { diff --git a/tests/Cli/StructArmedApplicationTest.php b/tests/Cli/StructArmedApplicationTest.php index 0f0699ab..5cbf3267 100644 --- a/tests/Cli/StructArmedApplicationTest.php +++ b/tests/Cli/StructArmedApplicationTest.php @@ -697,6 +697,98 @@ public function testAnalyseCommandWithNoCacheOptionDoesNotWriteCache(): void } } + public function testAnalyseCommandWithNoCacheOptionKeepsExistingCache(): void + { + $basePath = $this->createProjectDirectoryWithViolation(); + $cacheDirectory = $basePath . '/var/cache/structarmed'; + + file_put_contents($basePath . '/structarmed.php', <<<'PHP' +cacheDirectory('var/cache/structarmed') + ->layer('Source', 'src/') + ->rule('source.must_be_final', new MustBeFinalRule('Source')); +PHP); + + try { + [$cachedExitCode, $cachedOutput] = $this->runApplication( + ['structarmed', 'analyse', '--config=' . $basePath . '/structarmed.php', '--no-progress'], + $basePath + ); + $cacheContents = $this->directoryContents($cacheDirectory); + + // A cached run would now rewrite the node cache of the changed file. + file_put_contents($basePath . '/src/Foo.php', 'runApplication( + [ + 'structarmed', + 'analyse', + '--config=' . $basePath . '/structarmed.php', + '--no-cache', + '--no-progress', + ], + $basePath + ); + + $this->assertSame(1, $cachedExitCode, $cachedOutput); + $this->assertNotSame([], $cacheContents); + $this->assertSame(0, $exitCode, $output); + $this->assertStringContainsString('No violations found', $output); + $this->assertSame($cacheContents, $this->directoryContents($cacheDirectory)); + } finally { + $this->removeTempDirectory($basePath); + } + } + + public function testAnalyseCommandRejectsNoCacheWithClearCacheOption(): void + { + $basePath = $this->createProjectDirectory(); + $cacheDirectory = $basePath . '/var/cache/structarmed'; + + try { + mkdir($cacheDirectory, 0777, true); + file_put_contents($cacheDirectory . '/_metadata.json', '{}'); + + [$exitCode, $output] = $this->runApplication( + [ + 'structarmed', + 'analyse', + '--config=' . $basePath . '/structarmed.php', + '--no-cache', + '--clear-cache', + ], + $basePath + ); + + $this->assertSame(1, $exitCode, $output); + $this->assertSame("Error: --no-cache and --clear-cache cannot be used together.\n", $output); + $this->assertFileExists($cacheDirectory . '/_metadata.json'); + } finally { + $this->removeTempDirectory($basePath); + } + } + + /** + * @return array + */ + private function directoryContents(string $directory): array + { + $contents = []; + + foreach (glob($directory . '/*.json') ?: [] as $file) { + $contents[$file] = (string) file_get_contents($file); + } + + return $contents; + } + public function testAnalyseCommandRendersJsonReport(): void { $basePath = $this->createProjectDirectory(); From 45f907d1222b59d2fe11d599d4e210173d85ff61 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Mon, 19 Oct 2026 21:48:53 +0700 Subject: [PATCH 03/20] update docs on --no-cache --- docs/cache.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/cache.md b/docs/cache.md index 85945b72..b0c64b53 100644 --- a/docs/cache.md +++ b/docs/cache.md @@ -56,10 +56,13 @@ vendor/bin/structarmed --clear-cache ## Disabling Cache -Use `--no-cache` for a one-off run, such as a CI job that starts without a cache, or the first scan of a huge codebase. StructArmed then neither hashes the analysed files nor reads or writes any cache entry, so the run skips work that only a later run would benefit from. +Use `--no-cache` to run analysis without caching. StructArmed skips file hashing and neither reads nor writes cache entries. Skipping this work can make analysis faster on very huge codebases. ```bash vendor/bin/structarmed analyse --no-cache ``` +{: .important } +Keep in mind that every run with `--no-cache` repeats the full analysis, so similar runs will take roughly the same amount of time. With caching enabled, later runs can reuse cached results and finish faster. + The existing cache is left untouched. `--no-cache` cannot be combined with `--clear-cache`; clear the cache separately with `vendor/bin/structarmed --clear-cache`. From afc14cb99af6b3505bb19b9f2a2b1dd5526b7801 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 22 Oct 2026 21:21:01 +0700 Subject: [PATCH 04/20] fix: Remove the empty temp cache directory after a parallel --no-cache run --- .../Parallel/ParallelAnalysisNodeExtractor.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/Analyser/Parallel/ParallelAnalysisNodeExtractor.php b/src/Analyser/Parallel/ParallelAnalysisNodeExtractor.php index d50ab13d..dbccf114 100644 --- a/src/Analyser/Parallel/ParallelAnalysisNodeExtractor.php +++ b/src/Analyser/Parallel/ParallelAnalysisNodeExtractor.php @@ -13,6 +13,7 @@ use Boundwize\StructArmed\Cache\AnalysisResultCache; use Boundwize\StructArmed\Cache\CachePathFactory; use Boundwize\StructArmed\Progress\ProgressHandlerInterface; +use FilesystemIterator; use RuntimeException; use function array_fill; @@ -306,6 +307,15 @@ public function extract( } } + // when --no-cache is used and no files in the cache directory yet, remove dir cache + // as means only for temporary file for worker needs + if (! $this->analysisResultCache instanceof AnalysisResultCache) { + $fileSystemIterator = new FilesystemIterator($cacheDirectory); + if (! $fileSystemIterator->valid()) { + rmdir($cacheDirectory); + } + } + if ($failures !== []) { throw new RuntimeException(implode("\n", $failures)); } From 8e52af9a28ca96efe755a1e8d5570f84b43eac14 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 22 Oct 2026 21:24:46 +0700 Subject: [PATCH 05/20] cs fix --- src/Analyser/Parallel/ParallelAnalysisNodeExtractor.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Analyser/Parallel/ParallelAnalysisNodeExtractor.php b/src/Analyser/Parallel/ParallelAnalysisNodeExtractor.php index dbccf114..6febfcf2 100644 --- a/src/Analyser/Parallel/ParallelAnalysisNodeExtractor.php +++ b/src/Analyser/Parallel/ParallelAnalysisNodeExtractor.php @@ -39,6 +39,7 @@ use function min; use function mkdir; use function proc_close; +use function rmdir; use function serialize; use function sprintf; use function stream_set_blocking; From 6a4f61f09831652c2cde78a50ff91ad25a799416 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 22 Oct 2026 21:33:24 +0700 Subject: [PATCH 06/20] skip MergePhpstanDocTagIntoNativeRector --- rector.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/rector.php b/rector.php index 597c9030..c79dc74e 100644 --- a/rector.php +++ b/rector.php @@ -3,6 +3,7 @@ declare(strict_types=1); use Rector\Config\RectorConfig; +use Rector\TypeDeclarationDocblocks\Rector\Property\MergePhpstanDocTagIntoNativeRector; return RectorConfig::configure() ->withPhpSets(php82: true) @@ -19,4 +20,7 @@ ->withParallel() ->withRootFiles() ->withPaths([__DIR__ . '/src', __DIR__ . '/tests']) - ->withImportNames(removeUnusedImports: true); + ->withImportNames(removeUnusedImports: true) + ->withSkip([ + MergePhpstanDocTagIntoNativeRector::class, + ]) From 41f83e854ad1d0999e87de0c1e23aa9e18b3c611 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 22 Oct 2026 21:34:28 +0700 Subject: [PATCH 07/20] fix comma --- rector.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rector.php b/rector.php index c79dc74e..a4687daa 100644 --- a/rector.php +++ b/rector.php @@ -23,4 +23,4 @@ ->withImportNames(removeUnusedImports: true) ->withSkip([ MergePhpstanDocTagIntoNativeRector::class, - ]) + ]); From 95f4d124971ad16f1c81906d616f54db8a0e1068 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 24 Oct 2026 14:20:32 +0700 Subject: [PATCH 08/20] fix: Stop treating .class.php files as PSR-4 aliases in Psr4NamespaceRule --- src/Rule/Rules/Composer/Psr4NamespaceRule.php | 2 -- tests/Rule/Composer/Psr4NamespaceRuleTest.php | 31 ++++++++++++++++--- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/Rule/Rules/Composer/Psr4NamespaceRule.php b/src/Rule/Rules/Composer/Psr4NamespaceRule.php index aa8fa274..71a95451 100644 --- a/src/Rule/Rules/Composer/Psr4NamespaceRule.php +++ b/src/Rule/Rules/Composer/Psr4NamespaceRule.php @@ -20,7 +20,6 @@ use function file_exists; use function ltrim; use function max; -use function preg_replace; use function sprintf; use function str_ends_with; use function str_replace; @@ -113,7 +112,6 @@ private function expectedClassNames(string $file): array } $relativeClass = substr($relativeClass, 0, -4); - $relativeClass = (string) preg_replace('/\.class$/i', '', $relativeClass); $relativeClass = str_replace('/', '\\', $relativeClass); $className = $namespace . ltrim($relativeClass, '\\'); diff --git a/tests/Rule/Composer/Psr4NamespaceRuleTest.php b/tests/Rule/Composer/Psr4NamespaceRuleTest.php index 73893a52..d7179f7c 100644 --- a/tests/Rule/Composer/Psr4NamespaceRuleTest.php +++ b/tests/Rule/Composer/Psr4NamespaceRuleTest.php @@ -16,6 +16,7 @@ use function file_put_contents; use function json_encode; use function mkdir; +use function sprintf; #[CoversClass(Psr4NamespaceRule::class)] final class Psr4NamespaceRuleTest extends TestCase @@ -205,19 +206,41 @@ public function testPassesWhenRelativeFileIsNotPhpFile(): void $this->assertNotInstanceOf(RuleViolation::class, $psr4NamespaceRule->evaluate($this->makeNode('Foo', $file))); } - public function testStripsClassSuffixFromExpectedClassName(): void + public function testDoesNotTreatClassPhpSuffixAsPhpFile(): void { $basePath = $this->makeTempProject(); - $file = $basePath . '/tests/Foo.class.php'; + mkdir($basePath . '/tests/Service'); - file_put_contents($file, 'assertNotInstanceOf( RuleViolation::class, - $psr4NamespaceRule->evaluate($this->makeNode('App\\Tests\\Foo', $file)) + $psr4NamespaceRule->evaluate($this->makeNode('App\\Tests\\Foo', $basePath . '/tests/Foo.php')) ); + + foreach ($this->classPhpSuffixFiles() as $relativeFile => [$className, $expectedClassName]) { + $file = $basePath . '/tests/' . $relativeFile; + file_put_contents($file, 'evaluate($this->makeNode($className, $file)); + + $this->assertInstanceOf(RuleViolation::class, $violation, $relativeFile); + $this->assertSame( + sprintf('Class [%s] must match PSR-4 class [%s]', $className, $expectedClassName), + $violation->message, + $relativeFile + ); + } + } + + /** @return iterable */ + private function classPhpSuffixFiles(): iterable + { + yield 'Foo.class.php' => ['App\\Tests\\Foo', 'App\\Tests\\Foo.class']; + yield 'Bar.class.php' => ['App\\Tests\\BarClass', 'App\\Tests\\Bar.class']; + yield 'Service/Foo.class.php' => ['App\\Tests\\Service\\Foo', 'App\\Tests\\Service\\Foo.class']; } public function testFailsWhenTraitNameDoesNotMatchFilename(): void From 12e529656a2279c054747e0a051757cde3fbc079 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 24 Oct 2026 15:56:27 +0700 Subject: [PATCH 09/20] fix: Report files that map to invalid PSR-4 class names in Psr4NamespaceRule --- src/Rule/Rules/Composer/Psr4NamespaceRule.php | 17 +++++++++++------ tests/Rule/Composer/Psr4NamespaceRuleTest.php | 6 +++++- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/Rule/Rules/Composer/Psr4NamespaceRule.php b/src/Rule/Rules/Composer/Psr4NamespaceRule.php index 71a95451..1010c2ff 100644 --- a/src/Rule/Rules/Composer/Psr4NamespaceRule.php +++ b/src/Rule/Rules/Composer/Psr4NamespaceRule.php @@ -20,6 +20,7 @@ use function file_exists; use function ltrim; use function max; +use function preg_match; use function sprintf; use function str_ends_with; use function str_replace; @@ -29,6 +30,10 @@ final class Psr4NamespaceRule implements RuleInterface, ProjectRuleInterface { + private const IDENTIFIER = '[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*'; + + private const CLASS_NAME_PATTERN = '/^' . self::IDENTIFIER . '(?:\\\\' . self::IDENTIFIER . ')*$/D'; + private ?string $projectBasePath = null; /** @var array>> */ @@ -67,13 +72,13 @@ public function evaluate(ClassNode $classNode): ?RuleViolation return null; } + $expectedClassName = array_key_first($expectedClassNames); + $format = preg_match(self::CLASS_NAME_PATTERN, $expectedClassName) === 1 + ? '%s [%s] must match PSR-4 class [%s]' + : '%s [%s] is in a file that maps to invalid PSR-4 class name [%s]'; + return new RuleViolation( - message: sprintf( - '%s [%s] must match PSR-4 class [%s]', - $classNode->getType(), - $classNode->className, - array_key_first($expectedClassNames) - ), + message: sprintf($format, $classNode->getType(), $classNode->className, $expectedClassName), file: $classNode->file, line: $classNode->line, className: $classNode->className, diff --git a/tests/Rule/Composer/Psr4NamespaceRuleTest.php b/tests/Rule/Composer/Psr4NamespaceRuleTest.php index d7179f7c..43ee72b5 100644 --- a/tests/Rule/Composer/Psr4NamespaceRuleTest.php +++ b/tests/Rule/Composer/Psr4NamespaceRuleTest.php @@ -228,7 +228,11 @@ public function testDoesNotTreatClassPhpSuffixAsPhpFile(): void $this->assertInstanceOf(RuleViolation::class, $violation, $relativeFile); $this->assertSame( - sprintf('Class [%s] must match PSR-4 class [%s]', $className, $expectedClassName), + sprintf( + 'Class [%s] is in a file that maps to invalid PSR-4 class name [%s]', + $className, + $expectedClassName + ), $violation->message, $relativeFile ); From 6fdbf83c281615df8b6256f3b9e614352b0f55dd Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 29 Oct 2026 21:25:25 +0700 Subject: [PATCH 10/20] feat: Add Architecture::import() to compose config from multiple files --- docs/configuration.md | 29 ++++++++++++ src/Architecture.php | 44 ++++++++++++++++++ src/Cache/AnalysisCacheMetadataFactory.php | 11 +++++ src/Cli/AnalyseCommand.php | 5 +- src/PHPUnit/StructArmedExtension.php | 5 +- tests/ArchitectureTest.php | 53 ++++++++++++++++++++++ tests/Cache/AnalysisResultCacheTest.php | 34 ++++++++++++++ 7 files changed, 179 insertions(+), 2 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 5d95f6c3..eaa3af50 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -79,6 +79,35 @@ Use `replaceRule()` to swap a preset rule's configuration: `replaceRule()` throws `RuleNotFoundException` if the target key does not exist, so typos are caught immediately. +## Importing Config Files + +Use `import()` to split a large config into per-module files: + +```php +import(__DIR__ . '/module/Foo/structarmed.php') + ->import(__DIR__ . '/module/Bar/structarmed.php'); +``` + +Each imported file returns a callable that receives the root `Architecture`: + +```php +layer('Foo', 'module/Foo/src/'); +}; +``` + +Paths in imported files are still resolved from the project root. Imported files can import other files. Changing any imported file invalidates the analysis cache, just like changing the main config. + ## Custom Extensions Use [Custom Rules And Presets](../custom-rules-and-presets/) when you want to add project-specific rules or package reusable rule sets. diff --git a/src/Architecture.php b/src/Architecture.php index 14d3b1fd..3603bbb4 100644 --- a/src/Architecture.php +++ b/src/Architecture.php @@ -19,7 +19,9 @@ use function array_merge; use function array_unique; use function array_values; +use function is_callable; use function is_int; +use function realpath; use function sprintf; /** @@ -112,6 +114,9 @@ final class Architecture private ?string $baseline = null; + /** @var list config files applied through import(), nested imports included */ + private array $importedFiles = []; + private function __construct() { } @@ -121,6 +126,39 @@ public static function define(): self return new self(); } + /** + * Apply another config file, which returns a callable that receives this + * Architecture: + * + * return Architecture::define() + * ->import(__DIR__ . '/module/Foo/structarmed.php') + * ->import(__DIR__ . '/module/Bar/structarmed.php'); + * + * // module/Foo/structarmed.php + * return static function (Architecture $architecture): void { + * $architecture->layer('Foo', 'module/Foo/src/'); + * }; + * + * Imported files are part of the config cache key, so changing one + * invalidates the analysis cache. + */ + public function import(string $file): self + { + $configure = require $file; + + if (! is_callable($configure)) { + throw new InvalidArgumentException( + sprintf('Imported configuration [%s] must return a callable.', $file) + ); + } + + $configure($this); + + $this->importedFiles[] = realpath($file) ?: $file; + + return $this; + } + // ------------------------------------------------------------------------- // Layer definition // ------------------------------------------------------------------------- @@ -492,6 +530,12 @@ public function getBaseline(): ?string return $this->baseline; } + /** @return list */ + public function getImportedFiles(): array + { + return $this->importedFiles; + } + private function registerPendingSkip(string $skip): void { if (isset($this->rules[$skip])) { diff --git a/src/Cache/AnalysisCacheMetadataFactory.php b/src/Cache/AnalysisCacheMetadataFactory.php index a76833c8..5f7a04a0 100644 --- a/src/Cache/AnalysisCacheMetadataFactory.php +++ b/src/Cache/AnalysisCacheMetadataFactory.php @@ -68,6 +68,17 @@ public function fileHash(string $path): string return $this->fileHashProvider->hash($path); } + /** + * Files merged through Architecture::import() shape the config as much as + * the config file itself, so a change to any of them must change the hash. + * + * @param list $importedFiles + */ + public function configHash(string $configPath, array $importedFiles): string + { + return $this->filesHash([$configPath, ...$importedFiles]); + } + /** * Cached analysis nodes store resolved layer assignments, which depend on the * composer.json PSR-4 mappings as well as the config, so both hashes must diff --git a/src/Cli/AnalyseCommand.php b/src/Cli/AnalyseCommand.php index 586ce8f5..c2290c00 100644 --- a/src/Cli/AnalyseCommand.php +++ b/src/Cli/AnalyseCommand.php @@ -160,7 +160,10 @@ public function run(array $arguments, string $basePath): int // Without the cache, no file is hashed and no cache entry is read or // written, which saves that work on a one-off run over a huge codebase. if (! isset($options['no-cache'])) { - $configHash = $analysisCacheMetadataFactory->fileHash($configFile); + $configHash = $analysisCacheMetadataFactory->configHash( + $configFile, + $architecture->getImportedFiles() + ); $analysisResultCache = new AnalysisResultCache( $basePath, $fileHashProvider, diff --git a/src/PHPUnit/StructArmedExtension.php b/src/PHPUnit/StructArmedExtension.php index 6c22c258..3f3f8316 100644 --- a/src/PHPUnit/StructArmedExtension.php +++ b/src/PHPUnit/StructArmedExtension.php @@ -50,7 +50,10 @@ public function bootstrap( $start = microtime(true); $fileHashProvider = new FileHashProvider(); $analysisCacheMetadataFactory = new AnalysisCacheMetadataFactory($fileHashProvider); - $configHash = $analysisCacheMetadataFactory->fileHash($configFile); + $configHash = $analysisCacheMetadataFactory->configHash( + $configFile, + $architecture->getImportedFiles() + ); $composerGeneratedVersionHash = $analysisCacheMetadataFactory->composerGeneratedVersionHash(); $analysisResultCache = new AnalysisResultCache( $basePath, diff --git a/tests/ArchitectureTest.php b/tests/ArchitectureTest.php index 2d930065..cab662cc 100644 --- a/tests/ArchitectureTest.php +++ b/tests/ArchitectureTest.php @@ -13,16 +13,21 @@ use Boundwize\StructArmed\Preset\Presets\Psr4Preset; use Boundwize\StructArmed\Rule\Rules\Class_\MustBeFinalRule; use Boundwize\StructArmed\Rule\Rules\Composer\Psr4SourcePathsRule; +use Boundwize\StructArmed\Tests\Support\TemporaryDirectoryCleanupTrait; use InvalidArgumentException; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; +use function file_put_contents; +use function realpath; use function sprintf; #[CoversClass(Architecture::class)] final class ArchitectureTest extends TestCase { + use TemporaryDirectoryCleanupTrait; + public function testDefineReturnsNewInstance(): void { $this->assertInstanceOf(Architecture::class, Architecture::define()); @@ -450,4 +455,52 @@ public function testSkipPathsForRulesetAcceptsStringAndArray(): void $this->assertSame(['tests/', 'fixtures/', 'stubs/'], $architecture->getRulesetSkipPaths()); } + + public function testImportAppliesReturnedCallableAndTracksNestedFiles(): void + { + $directory = (string) realpath($this->makeTemporaryDirectory('structarmed-import')); + file_put_contents($directory . '/bar.php', <<<'PHP' + layer('Bar', 'module/Bar/src/'); + }; + + PHP); + file_put_contents($directory . '/foo.php', <<<'PHP' + layer('Foo', 'module/Foo/src/') + ->import(__DIR__ . '/bar.php'); + }; + + PHP); + + $architecture = Architecture::define() + ->layer('App', 'src/') + ->import($directory . '/foo.php'); + + $this->assertSame( + ['App' => 'src/', 'Foo' => 'module/Foo/src/', 'Bar' => 'module/Bar/src/'], + $architecture->getLayers() + ); + $this->assertSame([$directory . '/bar.php', $directory . '/foo.php'], $architecture->getImportedFiles()); + } + + public function testImportThrowsWhenFileDoesNotReturnCallable(): void + { + $path = $this->makeTemporaryFile('structarmed-import'); + file_put_contents($path, "expectException(InvalidArgumentException::class); + $this->expectExceptionMessage(sprintf('Imported configuration [%s] must return a callable.', $path)); + + Architecture::define()->import($path); + } } diff --git a/tests/Cache/AnalysisResultCacheTest.php b/tests/Cache/AnalysisResultCacheTest.php index cba0d7df..b0e25deb 100644 --- a/tests/Cache/AnalysisResultCacheTest.php +++ b/tests/Cache/AnalysisResultCacheTest.php @@ -2864,6 +2864,40 @@ public function testMetadataFileHashDoesNotChangeWhenOnlyFileMtimeChanges(): voi } } + public function testConfigHashChangesWhenImportedFileChanges(): void + { + $directory = $this->createTempDirectory(); + $config = $directory . '/structarmed.php'; + $imported = $directory . '/module.php'; + + file_put_contents($config, 'configHash($config, [$imported]); + + $this->assertNotSame( + (new AnalysisCacheMetadataFactory(new FileHashProvider()))->configHash($config, []), + $configHash + ); + + file_put_contents($imported, 'assertNotSame( + (new AnalysisCacheMetadataFactory(new FileHashProvider()))->configHash($config, [$imported]), + $configHash + ); + } finally { + foreach ([$config, $imported] as $file) { + if (file_exists($file)) { + unlink($file); + } + } + + $this->removeTempDirectory($directory); + } + } + public function testCacheMissesWhenComposerGeneratedVersionChanges(): void { $directory = $this->createTempDirectory(); From 97b23f87c9957e610e62aee8401d60d7e68dff47 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 29 Oct 2026 21:31:33 +0700 Subject: [PATCH 11/20] fix test on windows --- tests/ArchitectureTest.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/ArchitectureTest.php b/tests/ArchitectureTest.php index cab662cc..a2399d86 100644 --- a/tests/ArchitectureTest.php +++ b/tests/ArchitectureTest.php @@ -458,7 +458,7 @@ public function testSkipPathsForRulesetAcceptsStringAndArray(): void public function testImportAppliesReturnedCallableAndTracksNestedFiles(): void { - $directory = (string) realpath($this->makeTemporaryDirectory('structarmed-import')); + $directory = $this->makeTemporaryDirectory('structarmed-import'); file_put_contents($directory . '/bar.php', <<<'PHP' 'src/', 'Foo' => 'module/Foo/src/', 'Bar' => 'module/Bar/src/'], $architecture->getLayers() ); - $this->assertSame([$directory . '/bar.php', $directory . '/foo.php'], $architecture->getImportedFiles()); + $this->assertSame( + [realpath($directory . '/bar.php'), realpath($directory . '/foo.php')], + $architecture->getImportedFiles() + ); } public function testImportThrowsWhenFileDoesNotReturnCallable(): void From 9eb662a076882c04add480ff8dadda71b247ad97 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 29 Oct 2026 21:50:42 +0700 Subject: [PATCH 12/20] guard of circular import --- src/Architecture.php | 21 ++++- tests/ArchitectureTest.php | 36 +++++++- tests/Cli/StructArmedApplicationTest.php | 109 +++++++++++++++++++++++ 3 files changed, 162 insertions(+), 4 deletions(-) diff --git a/src/Architecture.php b/src/Architecture.php index 3603bbb4..aff87707 100644 --- a/src/Architecture.php +++ b/src/Architecture.php @@ -117,6 +117,9 @@ final class Architecture /** @var list config files applied through import(), nested imports included */ private array $importedFiles = []; + /** @var array config files whose import() is still running */ + private array $importing = []; + private function __construct() { } @@ -152,9 +155,23 @@ public function import(string $file): self ); } - $configure($this); + $importedFile = realpath($file) ?: $file; + + // only imports still in progress are tracked, so a file imported twice + // through separate branches is applied again rather than rejected + if (isset($this->importing[$importedFile])) { + throw new InvalidArgumentException(sprintf('Circular import of configuration [%s].', $file)); + } + + $this->importing[$importedFile] = true; + + try { + $configure($this); + } finally { + unset($this->importing[$importedFile]); + } - $this->importedFiles[] = realpath($file) ?: $file; + $this->importedFiles[] = $importedFile; return $this; } diff --git a/tests/ArchitectureTest.php b/tests/ArchitectureTest.php index a2399d86..9147da8e 100644 --- a/tests/ArchitectureTest.php +++ b/tests/ArchitectureTest.php @@ -482,16 +482,18 @@ public function testImportAppliesReturnedCallableAndTracksNestedFiles(): void PHP); + // bar.php again after foo.php imported it: a repeated import is not a cycle $architecture = Architecture::define() ->layer('App', 'src/') - ->import($directory . '/foo.php'); + ->import($directory . '/foo.php') + ->import($directory . '/bar.php'); $this->assertSame( ['App' => 'src/', 'Foo' => 'module/Foo/src/', 'Bar' => 'module/Bar/src/'], $architecture->getLayers() ); $this->assertSame( - [realpath($directory . '/bar.php'), realpath($directory . '/foo.php')], + [realpath($directory . '/bar.php'), realpath($directory . '/foo.php'), realpath($directory . '/bar.php')], $architecture->getImportedFiles() ); } @@ -506,4 +508,34 @@ public function testImportThrowsWhenFileDoesNotReturnCallable(): void Architecture::define()->import($path); } + + public function testImportThrowsOnCircularImport(): void + { + $directory = $this->makeTemporaryDirectory('structarmed-import'); + file_put_contents($directory . '/foo.php', <<<'PHP' + import(__DIR__ . '/bar.php'); + }; + + PHP); + file_put_contents($directory . '/bar.php', <<<'PHP' + import(__DIR__ . '/foo.php'); + }; + + PHP); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessageMatches('#^Circular import of configuration \[.+/foo\.php\]\.$#'); + + Architecture::define()->import($directory . '/foo.php'); + } } diff --git a/tests/Cli/StructArmedApplicationTest.php b/tests/Cli/StructArmedApplicationTest.php index 69d1ec9b..41435104 100644 --- a/tests/Cli/StructArmedApplicationTest.php +++ b/tests/Cli/StructArmedApplicationTest.php @@ -462,6 +462,115 @@ class Foo } } + public function testAnalyseCommandDetectsNestedImportedConfigChangesWithoutClearCache(): void + { + $basePath = $this->createTempDirectory(); + mkdir($basePath . '/src'); + + file_put_contents($basePath . '/src/Foo.php', <<<'PHP' +layer('Source', []) + ->import(__DIR__ . '/foo.php'); +PHP); + + file_put_contents($basePath . '/foo.php', <<<'PHP' +import(__DIR__ . '/bar.php'); +}; +PHP); + + file_put_contents($basePath . '/bar.php', <<<'PHP' +runApplication( + [ + 'structarmed', + 'analyse', + 'src', + '--config=' . $basePath . '/structarmed.php', + '--clear-cache', + '--no-progress', + ], + $basePath + ); + + $this->assertSame(0, $firstExitCode, $firstOutput); + $this->assertStringContainsString('No violations found', $firstOutput); + + file_put_contents($basePath . '/bar.php', <<<'PHP' +rule('source.must_be_final', new MustBeFinalRule('Source')); +}; +PHP); + + [$secondExitCode, $secondOutput] = $this->runApplication( + [ + 'structarmed', + 'analyse', + 'src', + '--config=' . $basePath . '/structarmed.php', + '--no-progress', + ], + $basePath + ); + + $this->assertSame(1, $secondExitCode, $secondOutput); + $this->assertStringContainsString('Class [App\Foo] must be declared final', $secondOutput); + } finally { + unlink($basePath . '/foo.php'); + unlink($basePath . '/bar.php'); + $this->removeTempDirectory($basePath); + } + } + public function testAnalyseCommandResolvesProjectPathsAgainstBasePathOption(): void { $basePath = (string) realpath($this->createTempDirectory()); From 2a73e637cc45a0819b77451908c4c5913bdcacfb Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 29 Oct 2026 21:54:46 +0700 Subject: [PATCH 13/20] move require --- src/Architecture.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Architecture.php b/src/Architecture.php index aff87707..267bec8b 100644 --- a/src/Architecture.php +++ b/src/Architecture.php @@ -147,14 +147,6 @@ public static function define(): self */ public function import(string $file): self { - $configure = require $file; - - if (! is_callable($configure)) { - throw new InvalidArgumentException( - sprintf('Imported configuration [%s] must return a callable.', $file) - ); - } - $importedFile = realpath($file) ?: $file; // only imports still in progress are tracked, so a file imported twice @@ -163,6 +155,14 @@ public function import(string $file): self throw new InvalidArgumentException(sprintf('Circular import of configuration [%s].', $file)); } + $configure = require $file; + + if (! is_callable($configure)) { + throw new InvalidArgumentException( + sprintf('Imported configuration [%s] must return a callable.', $file) + ); + } + $this->importing[$importedFile] = true; try { From af3750171eb0cf16ae1e42c2d638c1ba5b539487 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 29 Oct 2026 22:03:09 +0700 Subject: [PATCH 14/20] add test warm cache --- tests/Cli/StructArmedApplicationTest.php | 26 ++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/Cli/StructArmedApplicationTest.php b/tests/Cli/StructArmedApplicationTest.php index 41435104..5efe797e 100644 --- a/tests/Cli/StructArmedApplicationTest.php +++ b/tests/Cli/StructArmedApplicationTest.php @@ -465,6 +465,22 @@ class Foo public function testAnalyseCommandDetectsNestedImportedConfigChangesWithoutClearCache(): void { $basePath = $this->createTempDirectory(); + $progress = new class implements ProgressHandlerInterface { + public bool $started = false; + + public function start(int $total): void + { + $this->started = true; + } + + public function advance(string $file): void + { + } + + public function finish(): void + { + } + }; mkdir($basePath . '/src'); file_put_contents($basePath . '/src/Foo.php', <<<'PHP' @@ -538,6 +554,16 @@ class Foo $this->assertSame(0, $firstExitCode, $firstOutput); $this->assertStringContainsString('No violations found', $firstOutput); + [$warmExitCode, $warmOutput] = $this->runAnalyseCommand( + ['src', '--config=' . $basePath . '/structarmed.php'], + $basePath, + $progress + ); + + $this->assertSame(0, $warmExitCode, $warmOutput); + $this->assertStringContainsString('No violations found', $warmOutput); + $this->assertFalse($progress->started); + file_put_contents($basePath . '/bar.php', <<<'PHP' Date: Tue, 20 Oct 2026 09:46:06 +0700 Subject: [PATCH 15/20] feat: Detect @api and @internal docblock tags on ClassNode --- src/Analyser/AnalysisNodeCollector.php | 15 +++++++ src/Analyser/ClassNode.php | 4 ++ src/Cache/AnalysisResultCache.php | 20 +++++++-- tests/Analyser/AnalysisNodeCollectorTest.php | 43 ++++++++++++++++++++ tests/Cache/AnalysisResultCacheTest.php | 21 ++++++++++ 5 files changed, 100 insertions(+), 3 deletions(-) diff --git a/src/Analyser/AnalysisNodeCollector.php b/src/Analyser/AnalysisNodeCollector.php index 0531c24b..899fb257 100644 --- a/src/Analyser/AnalysisNodeCollector.php +++ b/src/Analyser/AnalysisNodeCollector.php @@ -9,6 +9,7 @@ use Boundwize\StructArmed\Util\PhpParser\ObjectBoundAnonymousFunction; use Boundwize\StructArmed\Util\PhpParser\UnconditionallyDeclaredFunctions; use Boundwize\StructArmed\Util\PhpParser\VisibilityFlagChecker; +use PhpParser\Comment\Doc; use PhpParser\ConstExprEvaluationException; use PhpParser\ConstExprEvaluator; use PhpParser\Node; @@ -1689,6 +1690,8 @@ enumBackingType: $classLike instanceof Enum_ && $classLike->scalarType instan : null, nonClassDependencies: $analysis['nonClassDependencies'], constantFetches: $analysis['constantFetches'], + isApi: $this->hasDocBlockTag($classLike, 'api'), + isInternal: $this->hasDocBlockTag($classLike, 'internal'), ); } @@ -1829,6 +1832,18 @@ private function resolveClassName(ClassLike $classLike): string : (string) $classLike->name; } + /** + * Whether the docblock carries the $tag tag, e.g. `@api`; an inline + * `{@internal ...}` comment does not count. + */ + private function hasDocBlockTag(ClassLike $classLike, string $tag): bool + { + $docComment = $classLike->getDocComment(); + + return $docComment instanceof Doc + && preg_match('/(?getText()) === 1; + } + /** * @return array{ * dependencies: list, diff --git a/src/Analyser/ClassNode.php b/src/Analyser/ClassNode.php index 2a1ca7b7..67a81c97 100644 --- a/src/Analyser/ClassNode.php +++ b/src/Analyser/ClassNode.php @@ -41,6 +41,8 @@ final class ClassNode * @param string|null $enumBackingType Backing type for a backed enum, null otherwise * @param list $nonClassDependencies Dependencies only ever used as a function or constant name * @param list $constantFetches Global and namespaced constants fetched within this class + * @param bool $isApi Whether the docblock marks it as @api + * @param bool $isInternal Whether the docblock marks it as @internal */ public function __construct( public readonly string $className, @@ -75,6 +77,8 @@ public function __construct( public readonly ?string $enumBackingType = null, public readonly array $nonClassDependencies = [], public readonly array $constantFetches = [], + public readonly bool $isApi = false, + public readonly bool $isInternal = false, ) { $this->layers = $layers ?: array_filter([$this->layer]); } diff --git a/src/Cache/AnalysisResultCache.php b/src/Cache/AnalysisResultCache.php index 3b1637b0..dd15a30e 100644 --- a/src/Cache/AnalysisResultCache.php +++ b/src/Cache/AnalysisResultCache.php @@ -69,7 +69,7 @@ final class AnalysisResultCache * their shape, naming, or reference semantics change. It is recorded in * the metadata marker, so older caches are cleared on their next use. */ - public const FORMAT_VERSION = 17; + public const FORMAT_VERSION = 18; private readonly string $cacheDirectory; @@ -976,8 +976,8 @@ private function functionLikeBodyFromArray(array $node, string $file): ?array /** * The file is not stored: the payload belongs to one file, known when - * loading. Empty lists — most of a typical class's — are left out and - * default on load. + * loading. Empty lists — most of a typical class's — and the rarely set + * isApi and isInternal flags are left out and default on load. * * @return array */ @@ -997,6 +997,14 @@ private function classNodeToArray(ClassNode $classNode): array 'enumBackingType' => $classNode->enumBackingType, ]; + if ($classNode->isApi) { + $node['isApi'] = true; + } + + if ($classNode->isInternal) { + $node['isInternal'] = true; + } + $lists = [ 'dependencies' => $classNode->dependencies, 'nonClassDependencies' => $classNode->nonClassDependencies, @@ -1040,6 +1048,8 @@ private function classNodeFromArray(array $node, string $file): ?ClassNode $isTrait = $node['isTrait'] ?? null; $isEnum = $node['isEnum'] ?? null; $isReadonly = $node['isReadonly'] ?? null; + $isApi = $node['isApi'] ?? false; + $isInternal = $node['isInternal'] ?? false; $dependencies = $node['dependencies'] ?? []; $nonClassDependencies = $node['nonClassDependencies'] ?? []; $constantFetches = $node['constantFetches'] ?? []; @@ -1065,6 +1075,8 @@ private function classNodeFromArray(array $node, string $file): ?ClassNode || ! is_bool($isTrait) || ! is_bool($isEnum) || ! is_bool($isReadonly) + || ! is_bool($isApi) + || ! is_bool($isInternal) || ! $this->isStringArray($dependencies) || ! $this->isStringArray($nonClassDependencies) || ! $this->isStringArray($constantFetches) @@ -1125,6 +1137,8 @@ enumCases: $enumCases, enumBackingType: $enumBackingType, nonClassDependencies: array_values($nonClassDependencies), constantFetches: array_values($constantFetches), + isApi: $isApi, + isInternal: $isInternal, ); } diff --git a/tests/Analyser/AnalysisNodeCollectorTest.php b/tests/Analyser/AnalysisNodeCollectorTest.php index bfc2fe8f..47023bb8 100644 --- a/tests/Analyser/AnalysisNodeCollectorTest.php +++ b/tests/Analyser/AnalysisNodeCollectorTest.php @@ -29,6 +29,8 @@ use PHPUnit\Framework\TestCase; use function array_column; +use function array_combine; +use function array_map; #[CoversClass(AnonymousClassNode::class)] #[CoversClass(AnalysisNodeCollector::class)] @@ -910,6 +912,47 @@ public function testCollectsAbstractClass(): void $this->assertFalse($classNode->isFinal); } + public function testCollectsApiAndInternalDocBlockTags(): void + { + $classNodes = $this->collectNodes(<<<'CODE_SAMPLE' + assertSame( + [ + 'PublicFacade' => [true, false], + 'InternalContract' => [false, true], + 'Untagged' => [false, false], + 'WithoutDocBlock' => [false, false], + ], + array_combine( + array_column($classNodes, 'className'), + array_map( + static fn (ClassNode $classNode): array => [$classNode->isApi, $classNode->isInternal], + $classNodes + ) + ) + ); + } + public function testCollectsInterface(): void { $classNode = $this->collect(' [ + [ + 'nodes' => [ + [ + 'className' => Foo::class, + 'line' => 1, + 'layer' => null, + 'extends' => null, + 'isAbstract' => false, + 'isFinal' => true, + 'isInterface' => false, + 'isTrait' => false, + 'isEnum' => false, + 'isReadonly' => false, + 'isApi' => 'yes', + ], + ], + ], + ]; yield 'node has invalid string array key' => [ [ 'nodes' => [ @@ -2992,6 +3011,8 @@ className: Foo::class, functionCalls: ['sprintf'], superglobals: ['_SERVER'], constantFetches: ['PHP_EOL'], + isApi: true, + isInternal: true, ); } From 3df62d1d49c16f07698fddad2cc326324e4779ac Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Tue, 20 Oct 2026 10:03:10 +0700 Subject: [PATCH 16/20] update doc --- docs/custom-rules-and-presets.md | 46 ++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/docs/custom-rules-and-presets.md b/docs/custom-rules-and-presets.md index 13a1ff44..2246e5a8 100644 --- a/docs/custom-rules-and-presets.md +++ b/docs/custom-rules-and-presets.md @@ -121,6 +121,52 @@ return Architecture::define() ); ``` +## Reading @api And @internal Tags In A Custom Rule + +Each `ClassNode` records whether the docblock of its class, interface, trait, or enum declaration carries an `@api` or `@internal` tag: + +| Flag | Meaning | +| --- | --- | +| `$classNode->isApi` | The class-like docblock has an `@api` tag | +| `$classNode->isInternal` | The class-like docblock has an `@internal` tag | + +A tag followed by a description, such as `@internal only used by the analyser`, counts. Longer tag names such as `@apiVersion` do not, and neither does the phpDocumentor inline `{@internal ...}}` form, which marks part of the description as internal rather than the class-like itself. Member docblocks are not read. + +```php +isInLayer('Core'); + } + + public function evaluate(ClassNode $classNode): ?RuleViolation + { + if ($classNode->isApi || $classNode->isInternal) { + return null; + } + + return new RuleViolation( + message: sprintf('%s [%s] must be annotated @api or @internal', $classNode->getType(), $classNode->className), + file: $classNode->file, + line: $classNode->line, + className: $classNode->className, + layer: $classNode->layer, + ); + } +} +``` + ## Reading Usage Flags In A Custom Rule Usage flags are opt-in because collecting them costs extra analysis time. Before reading a usage flag, a custom rule must declare that it needs the data by implementing the corresponding marker interface instead of the plain `Boundwize\StructArmed\Rule\RuleInterface`. Each marker extends `RuleInterface`, so no other change is needed: From 367a6400ed7d2d43f596003987430774b998d6ff Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 29 Oct 2026 23:08:28 +0700 Subject: [PATCH 17/20] Add ApiMayNotBeInternalRule and ApiMayNotExtendInternalRule and register to StabilityPreset --- docs/available-rules.md | 2 + docs/cli.md | 1 + docs/custom-rules-and-presets.md | 4 +- docs/presets.md | 3 + docs/quick-start.md | 4 + src/Analyser/AnalysisNodeCollector.php | 7 +- src/Cli/InitCommand.php | 4 +- src/Cli/Usage.php | 2 +- src/Preset/Preset.php | 13 ++ src/Preset/Presets/StabilityPreset.php | 50 ++++++++ .../Rules/Class_/ApiMayNotBeInternalRule.php | 43 +++++++ .../Class_/ApiMayNotExtendInternalRule.php | 57 +++++++++ structarmed.php | 2 +- tests/Analyser/AnalyserTest.php | 44 +++++++ tests/Analyser/AnalysisNodeCollectorTest.php | 2 + tests/Cli/InitCommandTest.php | 8 +- ...ructArmedApplicationCommandRoutingTest.php | 2 +- tests/Cli/StructArmedApplicationTest.php | 8 +- tests/Preset/PresetTest.php | 26 ++++ .../Class_/ApiMayNotBeInternalRuleTest.php | 70 +++++++++++ .../ApiMayNotExtendInternalRuleTest.php | 117 ++++++++++++++++++ 21 files changed, 459 insertions(+), 10 deletions(-) create mode 100644 src/Preset/Presets/StabilityPreset.php create mode 100644 src/Rule/Rules/Class_/ApiMayNotBeInternalRule.php create mode 100644 src/Rule/Rules/Class_/ApiMayNotExtendInternalRule.php create mode 100644 tests/Rule/Class_/ApiMayNotBeInternalRuleTest.php create mode 100644 tests/Rule/Class_/ApiMayNotExtendInternalRuleTest.php diff --git a/docs/available-rules.md b/docs/available-rules.md index 91dfcd9f..9f5b7909 100644 --- a/docs/available-rules.md +++ b/docs/available-rules.md @@ -76,6 +76,8 @@ Namespace: `Boundwize\StructArmed\Rule\Rules\Class_`. | Rule | Constructor | Checks | |---|---|---| | `AnonymousClassMayNotHaveEmptyParenthesesRule` | `new AnonymousClassMayNotHaveEmptyParenthesesRule(layer: 'Source')` | Anonymous classes that pass no constructor argument omit the parentheses after `class` (`new class {}`, not `new class () {}`), per [PER Coding Style](https://www.php-fig.org/per/coding-style/#8-anonymous-classes). Supports `--fix` by removing the empty parentheses. | +| `ApiMayNotBeInternalRule` | `new ApiMayNotBeInternalRule(layer: 'Source')` | Class-likes in a layer are not tagged both `@api` and `@internal`, which contradict each other. | +| `ApiMayNotExtendInternalRule` | `new ApiMayNotExtendInternalRule(layer: 'Source')` | Class-likes tagged `@api` in a layer do not extend a scanned class or interface tagged `@internal`, directly or through any parent, which would expose its members through the public API. Unscanned parents are skipped, as their docblock is unknown. | | `ClassConstantNameMustBeUpperCaseRule` | `new ClassConstantNameMustBeUpperCaseRule(layer: 'Domain')` | Class, interface, and trait constants use upper case with underscore separators. Enums are skipped (PER Coding Style recommends PascalCase enum constants). | | `ClassImplementingInterfaceMustHaveSuffixRule` | `new ClassImplementingInterfaceMustHaveSuffixRule(layer: 'HTTP', interface: MiddlewareInterface::class, suffix: 'Middleware')` | Classes implementing a specific interface use the required suffix. | | `ClassNameMustBeStudlyCapsRule` | `new ClassNameMustBeStudlyCapsRule(layer: 'Source')` | Class names use StudlyCaps. | diff --git a/docs/cli.md b/docs/cli.md index e32fed27..e0561b93 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -27,6 +27,7 @@ vendor/bin/structarmed init --preset=mvc vendor/bin/structarmed init --preset=ddd vendor/bin/structarmed init --preset=yagni vendor/bin/structarmed init --preset=codequality +vendor/bin/structarmed init --preset=stability vendor/bin/structarmed init --preset=all ``` diff --git a/docs/custom-rules-and-presets.md b/docs/custom-rules-and-presets.md index 2246e5a8..39776110 100644 --- a/docs/custom-rules-and-presets.md +++ b/docs/custom-rules-and-presets.md @@ -130,7 +130,9 @@ Each `ClassNode` records whether the docblock of its class, interface, trait, or | `$classNode->isApi` | The class-like docblock has an `@api` tag | | `$classNode->isInternal` | The class-like docblock has an `@internal` tag | -A tag followed by a description, such as `@internal only used by the analyser`, counts. Longer tag names such as `@apiVersion` do not, and neither does the phpDocumentor inline `{@internal ...}}` form, which marks part of the description as internal rather than the class-like itself. Member docblocks are not read. +A tag counts when it starts a docblock line, also when a description follows, such as `@internal only used by the analyser`. A tag name mentioned within the description does not count, and neither do longer tag names such as `@apiVersion` or the phpDocumentor inline `{@internal ...}}` form, which marks part of the description as internal rather than the class-like itself. Member docblocks are not read. + +The built-in `ApiMayNotBeInternalRule` and `ApiMayNotExtendInternalRule` read both flags. The example below enforces that every class-like in a layer declares one of the two tags: ```php getDocComment(); return $docComment instanceof Doc - && preg_match('/(?getText()) === 1; + && preg_match('#^[/*\s]*@' . $tag . '\b#m', $docComment->getText()) === 1; } /** diff --git a/src/Cli/InitCommand.php b/src/Cli/InitCommand.php index ea3b27da..8853a1ca 100644 --- a/src/Cli/InitCommand.php +++ b/src/Cli/InitCommand.php @@ -90,6 +90,7 @@ private function presetConfig(string $preset): ?string 'psr15' => ' ->withPreset(Preset::PSR15());', 'yagni' => ' ->withPreset(Preset::YAGNI());', 'codequality' => ' ->withPreset(Preset::CODEQUALITY());', + 'stability' => ' ->withPreset(Preset::STABILITY());', 'all' => " ->withPresets(\n" . " Preset::PSR4(),\n" . " Preset::PSR1(),\n" @@ -99,7 +100,8 @@ private function presetConfig(string $preset): ?string . " Preset::DDD(),\n" . " Preset::MVC(),\n" . " Preset::YAGNI(),\n" - . " Preset::CODEQUALITY()\n" + . " Preset::CODEQUALITY(),\n" + . " Preset::STABILITY()\n" . " );", default => null, }; diff --git a/src/Cli/Usage.php b/src/Cli/Usage.php index 0b649e29..8ec0ffa9 100644 --- a/src/Cli/Usage.php +++ b/src/Cli/Usage.php @@ -11,7 +11,7 @@ public static function render(): string return <<<'TXT' Usage: structarmed --version - structarmed init [--preset=ddd|mvc|psr4|psr1|psr12|per|psr15|yagni|codequality|all] + structarmed init [--preset=ddd|mvc|psr4|psr1|psr12|per|psr15|yagni|codequality|stability|all] structarmed analyse|analyze [path ...] [--config=path/to/structarmed.php] [-d|--basepath=path/to/project] [--report=console|json|github] [--no-progress] [--clear-cache] [--no-cache] [--disable-parallel] [--fix] [--generate-baseline=structarmed-baseline.php] diff --git a/src/Preset/Preset.php b/src/Preset/Preset.php index e810a360..10621f0d 100644 --- a/src/Preset/Preset.php +++ b/src/Preset/Preset.php @@ -12,6 +12,7 @@ use Boundwize\StructArmed\Preset\Presets\Psr15Preset; use Boundwize\StructArmed\Preset\Presets\Psr1Preset; use Boundwize\StructArmed\Preset\Presets\Psr4Preset; +use Boundwize\StructArmed\Preset\Presets\StabilityPreset; use Boundwize\StructArmed\Preset\Presets\YagniPreset; /** @@ -27,6 +28,7 @@ * ->withPreset(Preset::PSR15()) * ->withPreset(Preset::YAGNI()) * ->withPreset(Preset::CODEQUALITY()) + * ->withPreset(Preset::STABILITY()) * ->withPresets(Preset::DDD(), Preset::MVC()) */ final class Preset @@ -124,6 +126,17 @@ public static function CODEQUALITY( ); } + /** + * @param list|null $sourcePaths + */ + public static function STABILITY( + ?array $sourcePaths = null, + ): StabilityPreset { + return new StabilityPreset( + sourcePaths: $sourcePaths, + ); + } + public static function MVC( int $controllerMaxComplexity = 5, int $controllerMaxMethodLength = 20, diff --git a/src/Preset/Presets/StabilityPreset.php b/src/Preset/Presets/StabilityPreset.php new file mode 100644 index 00000000..f2f1ea90 --- /dev/null +++ b/src/Preset/Presets/StabilityPreset.php @@ -0,0 +1,50 @@ +|null $sourcePaths + */ + public function __construct( + private ?array $sourcePaths = null, + ) { + } + + public function apply(Architecture $architecture): void + { + $layerName = $this->resolveLayerName($architecture); + $architecture->layer($layerName, $this->sourcePaths ?? []); + + $architecture->rule( + self::API_MAY_NOT_BE_INTERNAL, + new ApiMayNotBeInternalRule($layerName) + ); + $architecture->rule( + self::API_MAY_NOT_EXTEND_INTERNAL, + new ApiMayNotExtendInternalRule($layerName) + ); + } +} diff --git a/src/Rule/Rules/Class_/ApiMayNotBeInternalRule.php b/src/Rule/Rules/Class_/ApiMayNotBeInternalRule.php new file mode 100644 index 00000000..278b1a42 --- /dev/null +++ b/src/Rule/Rules/Class_/ApiMayNotBeInternalRule.php @@ -0,0 +1,43 @@ +isApi && $classNode->isInLayer($this->layer); + } + + public function evaluate(ClassNode $classNode): ?RuleViolation + { + if (! $classNode->isInternal) { + return null; + } + + return new RuleViolation( + message: sprintf( + '%s [%s] must not be marked both @api and @internal', + $classNode->getType(), + $classNode->className + ), + file: $classNode->file, + line: $classNode->line, + className: $classNode->className, + layer: $classNode->layer, + ); + } +} diff --git a/src/Rule/Rules/Class_/ApiMayNotExtendInternalRule.php b/src/Rule/Rules/Class_/ApiMayNotExtendInternalRule.php new file mode 100644 index 00000000..78c54e83 --- /dev/null +++ b/src/Rule/Rules/Class_/ApiMayNotExtendInternalRule.php @@ -0,0 +1,57 @@ +isApi && $classNode->isInLayer($this->layer); + } + + public function evaluate(ClassNode $classNode): ?RuleViolation + { + // Direct and transitive parents; a class's parentInterfaces would add + // the interfaces it implements, which it does not extend + $parents = $classNode->isInterface ? $classNode->parentInterfaces : $classNode->parentClasses; + + foreach ($parents as $parent) { + // Only a scanned parent has a known docblock + $parentNode = $this->getDependencyNode($parent); + + if (! $parentNode instanceof ClassNode || ! $parentNode->isInternal) { + continue; + } + + return new RuleViolation( + message: sprintf( + '@api %s [%s] must not extend @internal %s [%s]', + $classNode->getType(), + $classNode->className, + $parentNode->getType(), + $parent + ), + file: $classNode->file, + line: $classNode->line, + className: $classNode->className, + layer: $classNode->layer, + ); + } + + return null; + } +} diff --git a/structarmed.php b/structarmed.php index 8479b684..cd96e607 100644 --- a/structarmed.php +++ b/structarmed.php @@ -57,7 +57,7 @@ __DIR__ . '/tests', ], ]) - ->withPresets(Preset::PER(), Preset::YAGNI(), Preset::CODEQUALITY()) + ->withPresets(Preset::PER(), Preset::YAGNI(), Preset::CODEQUALITY(), Preset::STABILITY()) ->rule( 'source.must_be_final', new MustBeFinalRule(layer: 'Source') diff --git a/tests/Analyser/AnalyserTest.php b/tests/Analyser/AnalyserTest.php index ad5afbf9..1aa5467d 100644 --- a/tests/Analyser/AnalyserTest.php +++ b/tests/Analyser/AnalyserTest.php @@ -33,6 +33,7 @@ use Boundwize\StructArmed\Rule\FunctionRuleInterface; use Boundwize\StructArmed\Rule\RuleInterface; use Boundwize\StructArmed\Rule\Rules\Class_\AnonymousClassMayNotHaveEmptyParenthesesRule; +use Boundwize\StructArmed\Rule\Rules\Class_\ApiMayNotExtendInternalRule; use Boundwize\StructArmed\Rule\Rules\Class_\MayNotExtendClassRule; use Boundwize\StructArmed\Rule\Rules\Class_\MayNotImplementInterfaceRule; use Boundwize\StructArmed\Rule\Rules\Class_\MustBeFinalRule; @@ -7061,6 +7062,49 @@ final class AuthTokenStore {} $this->assertStringContainsString('Auth', $violations[0]->message); } + public function testApiMayNotExtendInternalRuleReadsAncestorDocBlockViaAnalyser(): void + { + $basePath = $this->makeTempProject([ + 'src/Facade.php' => <<<'PHP' + <<<'PHP' + <<<'PHP' + layer('Core', 'src/') + ->rule('api.not_extend_internal', new ApiMayNotExtendInternalRule(layer: 'Core')); + + $ruleViolationCollection = (new Analyser($basePath)) + ->analyse($architecture, ['src/'], null, AnalyserOptions::sequential()); + + $violations = $ruleViolationCollection->forRule('api.not_extend_internal'); + $this->assertCount(1, $violations); + $this->assertSame( + '@api Class [App\\Facade] must not extend @internal Class [App\\InternalFacade]', + $violations[0]->message + ); + } + public function testAnalyserRulesetAllowsUnscannedDependencyWhenFirstMatchedLayerIsAllowed(): void { $basePath = $this->makeTempProject([ diff --git a/tests/Analyser/AnalysisNodeCollectorTest.php b/tests/Analyser/AnalysisNodeCollectorTest.php index 47023bb8..3db908a1 100644 --- a/tests/Analyser/AnalysisNodeCollectorTest.php +++ b/tests/Analyser/AnalysisNodeCollectorTest.php @@ -928,6 +928,8 @@ final class PublicFacade {} interface InternalContract {} /** + * Keeps the `@api` surface apart from @internal code. + * * @apiVersion 1.0 * {@internal not a tag on the class} */ diff --git a/tests/Cli/InitCommandTest.php b/tests/Cli/InitCommandTest.php index a9f5b365..1b9a9d45 100644 --- a/tests/Cli/InitCommandTest.php +++ b/tests/Cli/InitCommandTest.php @@ -84,6 +84,11 @@ public static function presetProvider(): iterable ' ->withPreset(Preset::CODEQUALITY());', ]; + yield 'stability' => [ + ['--preset=stability'], + ' ->withPreset(Preset::STABILITY());', + ]; + yield 'all' => [ ['--preset=all'], " ->withPresets(\n" @@ -95,7 +100,8 @@ public static function presetProvider(): iterable . " Preset::DDD(),\n" . " Preset::MVC(),\n" . " Preset::YAGNI(),\n" - . " Preset::CODEQUALITY()\n" + . " Preset::CODEQUALITY(),\n" + . " Preset::STABILITY()\n" . " );", ]; } diff --git a/tests/Cli/StructArmedApplicationCommandRoutingTest.php b/tests/Cli/StructArmedApplicationCommandRoutingTest.php index 8b3ca7c8..f1adfd1b 100644 --- a/tests/Cli/StructArmedApplicationCommandRoutingTest.php +++ b/tests/Cli/StructArmedApplicationCommandRoutingTest.php @@ -35,7 +35,7 @@ public function testApplicationPrintsUsageWithoutCommand(): void $this->assertSame(0, $exitCode); $this->assertStringContainsString('structarmed --version', $output); $this->assertStringContainsString( - 'structarmed init [--preset=ddd|mvc|psr4|psr1|psr12|per|psr15|yagni|codequality|all]', + 'structarmed init [--preset=ddd|mvc|psr4|psr1|psr12|per|psr15|yagni|codequality|stability|all]', $output ); $this->assertStringContainsString('structarmed analyse|analyze', $output); diff --git a/tests/Cli/StructArmedApplicationTest.php b/tests/Cli/StructArmedApplicationTest.php index 5efe797e..bfeba192 100644 --- a/tests/Cli/StructArmedApplicationTest.php +++ b/tests/Cli/StructArmedApplicationTest.php @@ -223,6 +223,11 @@ public static function presetProvider(): iterable ' ->withPreset(Preset::CODEQUALITY());', ]; + yield 'stability' => [ + ['--preset=stability'], + ' ->withPreset(Preset::STABILITY());', + ]; + yield 'all' => [ ['--preset=all'], " ->withPresets(\n" @@ -234,7 +239,8 @@ public static function presetProvider(): iterable . " Preset::DDD(),\n" . " Preset::MVC(),\n" . " Preset::YAGNI(),\n" - . " Preset::CODEQUALITY()\n" + . " Preset::CODEQUALITY(),\n" + . " Preset::STABILITY()\n" . " );", ]; } diff --git a/tests/Preset/PresetTest.php b/tests/Preset/PresetTest.php index 0040c224..5d0d61a5 100644 --- a/tests/Preset/PresetTest.php +++ b/tests/Preset/PresetTest.php @@ -15,7 +15,10 @@ use Boundwize\StructArmed\Preset\Presets\Psr1Preset; use Boundwize\StructArmed\Preset\Presets\Psr4Preset; use Boundwize\StructArmed\Preset\Presets\ResolvesSourceLayerNameTrait; +use Boundwize\StructArmed\Preset\Presets\StabilityPreset; use Boundwize\StructArmed\Preset\Presets\YagniPreset; +use Boundwize\StructArmed\Rule\Rules\Class_\ApiMayNotBeInternalRule; +use Boundwize\StructArmed\Rule\Rules\Class_\ApiMayNotExtendInternalRule; use Boundwize\StructArmed\Rule\Rules\Class_\ExtendedClassMustBeAbstractOrInstantiatedRule; use Boundwize\StructArmed\Rule\Rules\Class_\MayNotExtendClassRule; use Boundwize\StructArmed\Rule\Rules\Class_\MustBeUsedAbstractClassRule; @@ -37,6 +40,7 @@ #[CoversClass(Psr15Preset::class)] #[CoversClass(Psr4Preset::class)] #[CoversClass(ResolvesSourceLayerNameTrait::class)] +#[CoversClass(StabilityPreset::class)] #[CoversClass(YagniPreset::class)] final class PresetTest extends TestCase { @@ -105,6 +109,28 @@ public function testCodeQualityPresetRegistersSourceLayerAndRules(): void ); } + public function testStabilityPresetRegistersSourceLayerAndRules(): void + { + $architecture = Architecture::define(); + + Preset::STABILITY( + sourcePaths: ['src/'], + )->apply($architecture); + + $this->assertSame(['Source' => ['src/']], $architecture->getLayers()); + + $rules = $architecture->getRules(); + $this->assertCount(2, $rules); + $this->assertInstanceOf( + ApiMayNotBeInternalRule::class, + $rules[StabilityPreset::API_MAY_NOT_BE_INTERNAL] ?? null + ); + $this->assertInstanceOf( + ApiMayNotExtendInternalRule::class, + $rules[StabilityPreset::API_MAY_NOT_EXTEND_INTERNAL] ?? null + ); + } + public function testCodeQualityPresetUsesComposerSourcePathsByDefault(): void { $architecture = Architecture::define(); diff --git a/tests/Rule/Class_/ApiMayNotBeInternalRuleTest.php b/tests/Rule/Class_/ApiMayNotBeInternalRuleTest.php new file mode 100644 index 00000000..11e03782 --- /dev/null +++ b/tests/Rule/Class_/ApiMayNotBeInternalRuleTest.php @@ -0,0 +1,70 @@ +assertTrue($apiMayNotBeInternalRule->appliesTo($this->makeNode(isApi: true))); + $this->assertFalse($apiMayNotBeInternalRule->appliesTo($this->makeNode(isInternal: true))); + $this->assertFalse($apiMayNotBeInternalRule->appliesTo($this->makeNode(layer: 'Other', isApi: true))); + } + + public function testViolatesWhenMarkedBothApiAndInternal(): void + { + $apiMayNotBeInternalRule = new ApiMayNotBeInternalRule(layer: 'Core'); + + $violation = $apiMayNotBeInternalRule->evaluate( + $this->makeNode(isInterface: true, isApi: true, isInternal: true) + ); + + $this->assertInstanceOf(RuleViolation::class, $violation); + $this->assertSame( + 'Interface [App\Facade] must not be marked both @api and @internal', + $violation->message + ); + } + + public function testPassesWhenMarkedOnlyApi(): void + { + $apiMayNotBeInternalRule = new ApiMayNotBeInternalRule(layer: 'Core'); + + $this->assertNotInstanceOf( + RuleViolation::class, + $apiMayNotBeInternalRule->evaluate($this->makeNode(isApi: true)) + ); + } + + private function makeNode( + string $layer = 'Core', + bool $isInterface = false, + bool $isApi = false, + bool $isInternal = false, + ): ClassNode { + return new ClassNode( + className: 'App\Facade', + file: '/fake.php', + line: 1, + layer: $layer, + extends: null, + isAbstract: false, + isFinal: false, + isInterface: $isInterface, + isReadonly: false, + isApi: $isApi, + isInternal: $isInternal, + ); + } +} diff --git a/tests/Rule/Class_/ApiMayNotExtendInternalRuleTest.php b/tests/Rule/Class_/ApiMayNotExtendInternalRuleTest.php new file mode 100644 index 00000000..4e16a38c --- /dev/null +++ b/tests/Rule/Class_/ApiMayNotExtendInternalRuleTest.php @@ -0,0 +1,117 @@ +assertTrue($apiMayNotExtendInternalRule->appliesTo($this->makeNode('App\Facade', isApi: true))); + $this->assertFalse($apiMayNotExtendInternalRule->appliesTo($this->makeNode('App\Facade'))); + $this->assertFalse( + $apiMayNotExtendInternalRule->appliesTo($this->makeNode('App\Facade', layer: 'Other', isApi: true)) + ); + } + + public function testViolatesWhenClassExtendsInternalAncestor(): void + { + $apiMayNotExtendInternalRule = new ApiMayNotExtendInternalRule(layer: 'Core'); + $apiMayNotExtendInternalRule->injectClassNodeMap([ + 'App\BaseFacade' => $this->makeNode('App\BaseFacade'), + 'App\InternalFacade' => $this->makeNode('App\InternalFacade', isInternal: true), + ]); + + $violation = $apiMayNotExtendInternalRule->evaluate( + $this->makeNode('App\Facade', parentClasses: ['App\BaseFacade', 'App\InternalFacade'], isApi: true) + ); + + $this->assertInstanceOf(RuleViolation::class, $violation); + $this->assertSame( + '@api Class [App\Facade] must not extend @internal Class [App\InternalFacade]', + $violation->message + ); + } + + public function testViolatesWhenInterfaceExtendsInternalInterface(): void + { + $apiMayNotExtendInternalRule = new ApiMayNotExtendInternalRule(layer: 'Core'); + $apiMayNotExtendInternalRule->injectClassNodeMap([ + 'App\Public' => $this->makeNode('App\Public', isInterface: true), + 'App\Internal' => $this->makeNode('App\Internal', isInterface: true, isInternal: true), + ]); + + $violation = $apiMayNotExtendInternalRule->evaluate($this->makeNode( + 'App\Contract', + parentInterfaces: ['App\Public', 'App\Unscanned', 'App\Internal'], + isInterface: true, + isApi: true, + )); + + $this->assertInstanceOf(RuleViolation::class, $violation); + $this->assertSame( + '@api Interface [App\Contract] must not extend @internal Interface [App\Internal]', + $violation->message + ); + } + + public function testPassesWhenNoParentClassIsInternalOrScanned(): void + { + $apiMayNotExtendInternalRule = new ApiMayNotExtendInternalRule(layer: 'Core'); + $apiMayNotExtendInternalRule->injectClassNodeMap([ + 'App\BaseFacade' => $this->makeNode('App\BaseFacade', isApi: true), + 'App\InternalContract' => $this->makeNode('App\InternalContract', isInterface: true, isInternal: true), + ]); + + // A class implements interfaces rather than extending them + $this->assertNotInstanceOf( + RuleViolation::class, + $apiMayNotExtendInternalRule->evaluate($this->makeNode( + 'App\Facade', + parentClasses: ['App\BaseFacade', 'Vendor\Base'], + parentInterfaces: ['App\InternalContract'], + isApi: true, + )) + ); + } + + /** + * @param list $parentClasses + * @param list $parentInterfaces + */ + private function makeNode( + string $className, + string $layer = 'Core', + array $parentClasses = [], + array $parentInterfaces = [], + bool $isInterface = false, + bool $isApi = false, + bool $isInternal = false, + ): ClassNode { + return new ClassNode( + className: $className, + file: '/fake.php', + line: 1, + layer: $layer, + extends: $parentClasses[0] ?? null, + isAbstract: false, + isFinal: false, + isInterface: $isInterface, + isReadonly: false, + parentClasses: $parentClasses, + parentInterfaces: $parentInterfaces, + isApi: $isApi, + isInternal: $isInternal, + ); + } +} From ad7f321898cd1beadb4ebe1876b2b08f5875c388 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 29 Oct 2026 23:11:27 +0700 Subject: [PATCH 18/20] add check on class and interface --- docs/available-rules.md | 2 +- docs/presets.md | 2 +- src/Preset/Presets/StabilityPreset.php | 4 ++-- .../Rules/Class_/ApiMayNotExtendInternalRule.php | 5 +++++ .../Class_/ApiMayNotExtendInternalRuleTest.php | 15 ++++++++++++++- 5 files changed, 23 insertions(+), 5 deletions(-) diff --git a/docs/available-rules.md b/docs/available-rules.md index 9f5b7909..7d547c38 100644 --- a/docs/available-rules.md +++ b/docs/available-rules.md @@ -77,7 +77,7 @@ Namespace: `Boundwize\StructArmed\Rule\Rules\Class_`. |---|---|---| | `AnonymousClassMayNotHaveEmptyParenthesesRule` | `new AnonymousClassMayNotHaveEmptyParenthesesRule(layer: 'Source')` | Anonymous classes that pass no constructor argument omit the parentheses after `class` (`new class {}`, not `new class () {}`), per [PER Coding Style](https://www.php-fig.org/per/coding-style/#8-anonymous-classes). Supports `--fix` by removing the empty parentheses. | | `ApiMayNotBeInternalRule` | `new ApiMayNotBeInternalRule(layer: 'Source')` | Class-likes in a layer are not tagged both `@api` and `@internal`, which contradict each other. | -| `ApiMayNotExtendInternalRule` | `new ApiMayNotExtendInternalRule(layer: 'Source')` | Class-likes tagged `@api` in a layer do not extend a scanned class or interface tagged `@internal`, directly or through any parent, which would expose its members through the public API. Unscanned parents are skipped, as their docblock is unknown. | +| `ApiMayNotExtendInternalRule` | `new ApiMayNotExtendInternalRule(layer: 'Source')` | Classes and interfaces tagged `@api` in a layer do not extend a scanned class or interface tagged `@internal`, directly or through any parent, which would expose its members through the public API. Unscanned parents are skipped, as their docblock is unknown. | | `ClassConstantNameMustBeUpperCaseRule` | `new ClassConstantNameMustBeUpperCaseRule(layer: 'Domain')` | Class, interface, and trait constants use upper case with underscore separators. Enums are skipped (PER Coding Style recommends PascalCase enum constants). | | `ClassImplementingInterfaceMustHaveSuffixRule` | `new ClassImplementingInterfaceMustHaveSuffixRule(layer: 'HTTP', interface: MiddlewareInterface::class, suffix: 'Middleware')` | Classes implementing a specific interface use the required suffix. | | `ClassNameMustBeStudlyCapsRule` | `new ClassNameMustBeStudlyCapsRule(layer: 'Source')` | Class names use StudlyCaps. | diff --git a/docs/presets.md b/docs/presets.md index 157f0e7d..72e3a665 100644 --- a/docs/presets.md +++ b/docs/presets.md @@ -28,7 +28,7 @@ StructArmed ships with presets for common PHP standards and architecture styles. | `Preset::MVC()` | Layer isolation, thin controllers, model/view/service rules, return types for helper functions | | `Preset::YAGNI()` | Speculative-abstraction cleanup: interfaces must be implemented by a class or extended by another interface, abstract classes must be extended, traits must be used, extended classes that are never instantiated must be abstract, and named functions must be called — a dependency reference (type hint, `instanceof`, `::class`, static call, a class-name string, ...) also counts as usage within the scanned paths, while only instantiation (`new X`, `new self`/`static`/`parent`, or a constant class expression such as `new (X::class)`) keeps an extended class concrete. All rules support `--fix`, removing the unused declaration or adding the `abstract` modifier | | `Preset::CODEQUALITY()` | General readability conventions independent of any architecture style: closures and arrow functions that do not read `$this` must be declared `static`, and plain decimal numeric literals of `1_000_000` or more must group their digits with `_` separators (`1000500` becomes `1_000_500`). Both rules support `--fix`. Tune the literal threshold with `replaceRule(CodeQualityPreset::LARGE_NUMERIC_LITERALS_MUST_USE_SEPARATOR, new LargeNumericLiteralMustUseSeparatorRule(minimum: 1_000))` | -| `Preset::STABILITY()` | Keeps the stable `@api` surface apart from `@internal` code: a class-like must not be tagged both `@api` and `@internal`, and an `@api` class-like must not extend a scanned `@internal` class or interface, directly or through any parent. Class-likes without either tag are not checked | +| `Preset::STABILITY()` | Keeps the stable `@api` surface apart from `@internal` code: a class-like must not be tagged both `@api` and `@internal`, and an `@api` class or interface must not extend a scanned `@internal` one, directly or through any parent. Class-likes without either tag are not checked | ## Initialize Presets diff --git a/src/Preset/Presets/StabilityPreset.php b/src/Preset/Presets/StabilityPreset.php index f2f1ea90..9ba10573 100644 --- a/src/Preset/Presets/StabilityPreset.php +++ b/src/Preset/Presets/StabilityPreset.php @@ -11,8 +11,8 @@ /** * Stability preset: keeps the stable `@api` surface apart from `@internal` - * code — a class-like is not tagged both, and an `@api` class-like does not - * extend an `@internal` one, which would expose its members. + * code — a class-like is not tagged both, and an `@api` class or interface + * does not extend an `@internal` one, which would expose its members. * * Trade-off: only docblock tags of scanned class-likes are known. Class-likes * without either tag are not checked. diff --git a/src/Rule/Rules/Class_/ApiMayNotExtendInternalRule.php b/src/Rule/Rules/Class_/ApiMayNotExtendInternalRule.php index 78c54e83..87c487d7 100644 --- a/src/Rule/Rules/Class_/ApiMayNotExtendInternalRule.php +++ b/src/Rule/Rules/Class_/ApiMayNotExtendInternalRule.php @@ -20,6 +20,11 @@ public function __construct( public function appliesTo(ClassNode $classNode): bool { + // Traits and enums cannot extend + if (! $classNode->isClass() && ! $classNode->isInterface) { + return false; + } + return $classNode->isApi && $classNode->isInLayer($this->layer); } diff --git a/tests/Rule/Class_/ApiMayNotExtendInternalRuleTest.php b/tests/Rule/Class_/ApiMayNotExtendInternalRuleTest.php index 4e16a38c..541bb931 100644 --- a/tests/Rule/Class_/ApiMayNotExtendInternalRuleTest.php +++ b/tests/Rule/Class_/ApiMayNotExtendInternalRuleTest.php @@ -13,11 +13,20 @@ #[CoversClass(ApiMayNotExtendInternalRule::class)] final class ApiMayNotExtendInternalRuleTest extends TestCase { - public function testAppliesOnlyToApiClassLikesInConfiguredLayer(): void + public function testAppliesOnlyToApiClassesAndInterfacesInConfiguredLayer(): void { $apiMayNotExtendInternalRule = new ApiMayNotExtendInternalRule(layer: 'Core'); $this->assertTrue($apiMayNotExtendInternalRule->appliesTo($this->makeNode('App\Facade', isApi: true))); + $this->assertTrue( + $apiMayNotExtendInternalRule->appliesTo($this->makeNode('App\Facade', isInterface: true, isApi: true)) + ); + $this->assertFalse( + $apiMayNotExtendInternalRule->appliesTo($this->makeNode('App\Facade', isTrait: true, isApi: true)) + ); + $this->assertFalse( + $apiMayNotExtendInternalRule->appliesTo($this->makeNode('App\Facade', isEnum: true, isApi: true)) + ); $this->assertFalse($apiMayNotExtendInternalRule->appliesTo($this->makeNode('App\Facade'))); $this->assertFalse( $apiMayNotExtendInternalRule->appliesTo($this->makeNode('App\Facade', layer: 'Other', isApi: true)) @@ -95,6 +104,8 @@ private function makeNode( array $parentClasses = [], array $parentInterfaces = [], bool $isInterface = false, + bool $isTrait = false, + bool $isEnum = false, bool $isApi = false, bool $isInternal = false, ): ClassNode { @@ -108,6 +119,8 @@ className: $className, isFinal: false, isInterface: $isInterface, isReadonly: false, + isTrait: $isTrait, + isEnum: $isEnum, parentClasses: $parentClasses, parentInterfaces: $parentInterfaces, isApi: $isApi, From 9ee9b815277fbe3d3618183f6c86ecdf56865f85 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 29 Oct 2026 23:16:49 +0700 Subject: [PATCH 19/20] case fix --- docs/custom-rules-and-presets.md | 4 ++-- src/Analyser/AnalysisNodeCollector.php | 7 ++++--- src/Rule/AbstractLayerAwareRule.php | 19 ++++++++++++++++--- .../Class_/ApiMayNotExtendInternalRule.php | 2 +- tests/Analyser/AnalyserTest.php | 3 ++- tests/Analyser/AnalysisNodeCollectorTest.php | 2 ++ .../ApiMayNotExtendInternalRuleTest.php | 19 +++++++++++++++++++ tests/Rule/Layer/MayNotDependOnRuleTest.php | 11 +++++++++++ 8 files changed, 57 insertions(+), 10 deletions(-) diff --git a/docs/custom-rules-and-presets.md b/docs/custom-rules-and-presets.md index 39776110..0c6ffac7 100644 --- a/docs/custom-rules-and-presets.md +++ b/docs/custom-rules-and-presets.md @@ -130,7 +130,7 @@ Each `ClassNode` records whether the docblock of its class, interface, trait, or | `$classNode->isApi` | The class-like docblock has an `@api` tag | | `$classNode->isInternal` | The class-like docblock has an `@internal` tag | -A tag counts when it starts a docblock line, also when a description follows, such as `@internal only used by the analyser`. A tag name mentioned within the description does not count, and neither do longer tag names such as `@apiVersion` or the phpDocumentor inline `{@internal ...}}` form, which marks part of the description as internal rather than the class-like itself. Member docblocks are not read. +A tag counts when it starts a docblock line, also when a description follows, such as `@internal only used by the analyser`. A tag name mentioned within the description does not count, and neither do longer tag names such as `@apiVersion` and `@api-version` or the phpDocumentor inline `{@internal ...}}` form, which marks part of the description as internal rather than the class-like itself. Member docblocks are not read. The built-in `ApiMayNotBeInternalRule` and `ApiMayNotExtendInternalRule` read both flags. The example below enforces that every class-like in a layer declares one of the two tags: @@ -253,7 +253,7 @@ Trade-off: only usage within the scanned paths is known. A class-like used solel ## Reading Other Classes' Layers In A Custom Rule -A rule only receives the node it is evaluating. When a check depends on the layer of *another* class — the layer a dependency belongs to, for instance — extend `Boundwize\StructArmed\Rule\AbstractLayerAwareRule`. Before any class is evaluated, the analyser injects the map of every scanned class into its protected `$classNodeMap` property, keyed by fully qualified class name, and `getDependencyNode()` looks a class up in it: +A rule only receives the node it is evaluating. When a check depends on the layer of *another* class — the layer a dependency belongs to, for instance — extend `Boundwize\StructArmed\Rule\AbstractLayerAwareRule`. Before any class is evaluated, the analyser injects the map of every scanned class into its protected `$classNodeMap` property, keyed by fully qualified class name, and `getDependencyNode()` looks a class up in it, ignoring case as PHP does for class names: ```php getDocComment(); return $docComment instanceof Doc - && preg_match('#^[/*\s]*@' . $tag . '\b#m', $docComment->getText()) === 1; + && preg_match('#^[/*\s]*@' . $tag . '(?![\w-])#m', $docComment->getText()) === 1; } /** diff --git a/src/Rule/AbstractLayerAwareRule.php b/src/Rule/AbstractLayerAwareRule.php index 4faf2b40..65c7164e 100644 --- a/src/Rule/AbstractLayerAwareRule.php +++ b/src/Rule/AbstractLayerAwareRule.php @@ -6,6 +6,9 @@ use Boundwize\StructArmed\Analyser\ClassNode; +use function array_change_key_case; +use function strtolower; + /** * A rule that needs the layer of a class other than the one under evaluation, * such as the layer a dependency belongs to. The analyser injects the scanned @@ -16,15 +19,25 @@ abstract class AbstractLayerAwareRule /** @var array class name → class node */ protected array $classNodeMap = []; + /** @var array lowercased class name → class node */ + private array $lowercaseClassNodeMap = []; + /** @param array $classNodeMap */ public function injectClassNodeMap(array $classNodeMap): void { - $this->classNodeMap = $classNodeMap; + $this->classNodeMap = $classNodeMap; + $this->lowercaseClassNodeMap = array_change_key_case($classNodeMap); } - /** The scanned node of a dependency, or null when it lies outside the scanned paths */ + /** + * The scanned node of a dependency, or null when it lies outside the + * scanned paths. Class names are case-insensitive, so `extends internalbase` + * still finds the node declared as InternalBase. + */ protected function getDependencyNode(string $dependency): ?ClassNode { - return $this->classNodeMap[$dependency] ?? null; + return $this->classNodeMap[$dependency] + ?? $this->lowercaseClassNodeMap[strtolower($dependency)] + ?? null; } } diff --git a/src/Rule/Rules/Class_/ApiMayNotExtendInternalRule.php b/src/Rule/Rules/Class_/ApiMayNotExtendInternalRule.php index 87c487d7..640d7336 100644 --- a/src/Rule/Rules/Class_/ApiMayNotExtendInternalRule.php +++ b/src/Rule/Rules/Class_/ApiMayNotExtendInternalRule.php @@ -48,7 +48,7 @@ public function evaluate(ClassNode $classNode): ?RuleViolation $classNode->getType(), $classNode->className, $parentNode->getType(), - $parent + $parentNode->className ), file: $classNode->file, line: $classNode->line, diff --git a/tests/Analyser/AnalyserTest.php b/tests/Analyser/AnalyserTest.php index 1aa5467d..84d7b4e1 100644 --- a/tests/Analyser/AnalyserTest.php +++ b/tests/Analyser/AnalyserTest.php @@ -7078,7 +7078,8 @@ final class Facade extends BaseFacade {} namespace App; - abstract class BaseFacade extends InternalFacade {} + // Class names are case-insensitive + abstract class BaseFacade extends internalfacade {} PHP, 'src/InternalFacade.php' => <<<'PHP' injectClassNodeMap([ + 'App\InternalFacade' => $this->makeNode('App\InternalFacade', isInternal: true), + ]); + + // Class names are case-insensitive: `extends internalfacade` is valid PHP + $violation = $apiMayNotExtendInternalRule->evaluate( + $this->makeNode('App\Facade', parentClasses: ['App\BaseFacade', 'app\internalfacade'], isApi: true) + ); + + $this->assertInstanceOf(RuleViolation::class, $violation); + $this->assertSame( + '@api Class [App\Facade] must not extend @internal Class [App\InternalFacade]', + $violation->message + ); + } + public function testViolatesWhenInterfaceExtendsInternalInterface(): void { $apiMayNotExtendInternalRule = new ApiMayNotExtendInternalRule(layer: 'Core'); diff --git a/tests/Rule/Layer/MayNotDependOnRuleTest.php b/tests/Rule/Layer/MayNotDependOnRuleTest.php index 12d1380b..85b698dd 100644 --- a/tests/Rule/Layer/MayNotDependOnRuleTest.php +++ b/tests/Rule/Layer/MayNotDependOnRuleTest.php @@ -202,6 +202,17 @@ public function testPassesUsingClassNodeMapWhenDependencyNotInForbiddenLayer(): $this->assertNotInstanceOf(RuleViolation::class, $mayNotDependOnRule->evaluate($classNode)); } + public function testFindsDependencyNodeWrittenInAnotherCase(): void + { + // Path matching would miss the lowercased name; the node's layer catches it. + $mayNotDependOnRule = new MayNotDependOnRule(from: 'Domain', to: 'Infrastructure'); + $dependencyNode = $this->makeNode(layer: 'Infrastructure', className: 'App\Persistence\OrderRepository'); + $mayNotDependOnRule->injectClassNodeMap([$dependencyNode->className => $dependencyNode]); + $classNode = $this->makeNode('Domain', ['app\persistence\orderrepository']); + + $this->assertInstanceOf(RuleViolation::class, $mayNotDependOnRule->evaluate($classNode)); + } + public function testClassNodeMapTakesPrecedenceOverToPath(): void { // Path would match 'App\Infrastructure\A', but its node is in Domain — no violation. From 01c2a3868081ab16a62efb2eb3268160a7552844 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 29 Oct 2026 23:32:53 +0700 Subject: [PATCH 20/20] Fix cache directory removal race between concurrent runs with best effort rmdir() --- .../Parallel/ParallelAnalysisNodeExtractor.php | 11 ++++------- src/Cache/AnalysisResultCache.php | 9 +++------ 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/Analyser/Parallel/ParallelAnalysisNodeExtractor.php b/src/Analyser/Parallel/ParallelAnalysisNodeExtractor.php index b7a10ec3..2b0e9432 100644 --- a/src/Analyser/Parallel/ParallelAnalysisNodeExtractor.php +++ b/src/Analyser/Parallel/ParallelAnalysisNodeExtractor.php @@ -13,7 +13,6 @@ use Boundwize\StructArmed\Cache\AnalysisResultCache; use Boundwize\StructArmed\Cache\CachePathFactory; use Boundwize\StructArmed\Progress\ProgressHandlerInterface; -use FilesystemIterator; use RuntimeException; use function array_fill; @@ -336,13 +335,11 @@ public function extract( } } - // when --no-cache is used and no files in the cache directory yet, remove dir cache - // as means only for temporary file for worker needs + // when --no-cache is used, the directory only held worker temporary files: + // remove it best-effort, rmdir() refuses a non-empty directory atomically, + // so a concurrent run's files or an existing cache keep it in place if (! $this->analysisResultCache instanceof AnalysisResultCache) { - $fileSystemIterator = new FilesystemIterator($cacheDirectory); - if (! $fileSystemIterator->valid()) { - rmdir($cacheDirectory); - } + @rmdir($cacheDirectory); } if ($failures !== []) { diff --git a/src/Cache/AnalysisResultCache.php b/src/Cache/AnalysisResultCache.php index dd15a30e..6578bd94 100644 --- a/src/Cache/AnalysisResultCache.php +++ b/src/Cache/AnalysisResultCache.php @@ -17,7 +17,6 @@ use Boundwize\StructArmed\Composer\ComposerJsonProvider; use Boundwize\StructArmed\Rule\RuleViolation; use Boundwize\StructArmed\Rule\RuleViolationCollection; -use FilesystemIterator; use function array_fill_keys; use function array_is_list; @@ -177,11 +176,9 @@ public function clear(): void } } - // only remove the cache directory if it is empty - $fileSystemIterator = new FilesystemIterator($this->cacheDirectory); - if (! $fileSystemIterator->valid()) { - rmdir($this->cacheDirectory); - } + // only remove the cache directory if it is empty: rmdir() refuses a + // non-empty directory atomically, so a concurrent run's files keep it + @rmdir($this->cacheDirectory); } public function getCacheDirectory(): string