diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9e8e7b9b..d61bb134 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,6 +20,11 @@ jobs: - operating-system: ubuntu-latest php-versions: "8.2" coverage: pcov + exclude: + # Temporarily disabled: GitHub Actions macOS runners currently fail + # to set up PHP 8.5. Re-enable once the runner image is fixed. + - operating-system: macos-latest + php-versions: "8.5" steps: - name: Checkout diff --git a/composer.json b/composer.json index 87b1b9e3..e093d17f 100644 --- a/composer.json +++ b/composer.json @@ -31,7 +31,7 @@ "php": "^8.2", "composer-runtime-api": "^2.0", "boundwize/jsonrecast": "^1.0", - "fidry/cpu-core-counter": "^1.3", + "fidry/cpu-core-counter": "^1.4", "nikic/php-parser": "^5.9" }, "require-dev": { diff --git a/docs/available-rules.md b/docs/available-rules.md index cc7cfd4b..91dfcd9f 100644 --- a/docs/available-rules.md +++ b/docs/available-rules.md @@ -169,6 +169,7 @@ Namespace: `Boundwize\StructArmed\Rule\Rules\Usage`. |---|---|---| | `MayNotCallFunctionRule` | `new MayNotCallFunctionRule(layer: 'Domain', function: 'header')` | Classes in a layer do not call a forbidden function. | | `MayNotUseClassRule` | `new MayNotUseClassRule(layer: 'Domain', forbiddenClass: DateTime::class)` | Classes in a layer do not depend on a forbidden class. | +| `MayNotUseConstantRule` | `new MayNotUseConstantRule(layer: 'Domain', constant: 'PHP_EOL')` | Classes in a layer do not use a forbidden constant. | | `MayNotUseLanguageConstructRule` | `new MayNotUseLanguageConstructRule(layer: 'Domain', construct: 'echo')` | Classes in a layer do not use a forbidden language construct. | | `MayNotUseNamespaceRule` | `new MayNotUseNamespaceRule(layer: 'Domain', forbiddenNamespace: 'Doctrine\\ORM\\')` | Classes in a layer do not depend on a forbidden namespace. | | `MayNotUseSuperglobalsRule` | `new MayNotUseSuperglobalsRule(layer: 'Controller')` | Classes in a layer do not access superglobals directly. | @@ -176,4 +177,6 @@ Namespace: `Boundwize\StructArmed\Rule\Rules\Usage`. `MayNotUseClassRule` and `MayNotUseNamespaceRule` also accept `classNamePattern` when only matching classes should be checked. +`MayNotUseConstantRule` takes a global or namespaced constant name, such as `'PHP_EOL'` or `'Vendor\\Config\\DEBUG'`. An unqualified constant inside a namespace counts as both the constant of that namespace and the global constant of that name, as PHP only resolves it at runtime. + `MayNotUseLanguageConstructRule` accepts one of the following `construct` names: `echo`, `print`, `eval`, `isset`, `empty`, `unset`, `list`, `exit`, `die`, `include`, `include_once`, `require`, `require_once`. `die` is a pure alias of `exit`, so banning either spelling catches both. The `include` / `include_once` / `require` / `require_once` constructs are distinct and are matched exactly. diff --git a/docs/cli.md b/docs/cli.md index 9663ef2e..e32fed27 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -46,6 +46,42 @@ vendor/bin/structarmed analyse --config=path/to/structarmed.php vendor/bin/structarmed analyze --config=path/to/structarmed.php ``` +## Base Path + +The project root defaults to the directory the command runs in. Pass `--basepath` when StructArmed is installed somewhere else, for example in a `tools/structarmed` directory with its own `composer.json`: + +```text +composer.json +src/ +tests/ +tools/ +└── structarmed/ + ├── composer.json + ├── structarmed.php + └── vendor/ +``` + +```bash +cd tools/structarmed +vendor/bin/structarmed analyse --basepath=../../ +``` + +`-d` is a short alias for `--basepath`: + +```bash +vendor/bin/structarmed analyse -d ../../ +``` + +Everything relative to the project root now resolves against the base path: layer paths such as `->layer('Config', 'src/ConfigProvider.php')`, scan paths given on the command line, the `composer.json` read by the composer rules and PSR-4 layers, the cache directory, and baseline paths. The config file is discovered in the current directory first, then in the base path; `--config` keeps pointing to a path relative to the current directory. + +`--clear-cache` accepts the same option, so the cache of a project analysed through `--basepath` is cleared with: + +```bash +vendor/bin/structarmed --basepath=../../ --clear-cache +``` + +Options may be given before or after the command. + ## Auto-Fix Violations Use `--fix` to automatically apply fixes for violations produced by rules that implement `Boundwize\StructArmed\Rule\FixableInterface`. diff --git a/src/Analyser/AnalysisNodeCollector.php b/src/Analyser/AnalysisNodeCollector.php index e58af247..3bd78816 100644 --- a/src/Analyser/AnalysisNodeCollector.php +++ b/src/Analyser/AnalysisNodeCollector.php @@ -1176,8 +1176,26 @@ private function collectNodeAnalysis(Node $node): void // Entered before its name, so the FullyQualified branch above sees // the mark. if ($node instanceof ConstFetch) { - $node->name->setAttribute(self::NON_CLASS_NAME_ATTRIBUTE, true); - $this->collectKeywordConstant($node->name); + $name = $node->name; + $name->setAttribute(self::NON_CLASS_NAME_ATTRIBUTE, true); + $this->collectKeywordConstant($name); + + if ($this->activeClassLikeAnalyses !== [] && ! isset(self::KEYWORD_CONSTANTS[$name->toLowerString()])) { + // An unqualified fetch in a namespace is not a FullyQualified + // node: PHP fetches the namespaced constant when it exists and + // the global one otherwise, which is not known here, so both + // candidates are recorded. + $namespacedName = $name->getAttribute('namespacedName'); + $constant = $name->toString(); + + foreach ($this->activeClassLikeAnalyses as $activeClassLikeAnalysis) { + if ($namespacedName instanceof Name) { + $activeClassLikeAnalysis->constantFetches[$namespacedName->toString()] = true; + } + + $activeClassLikeAnalysis->constantFetches[$constant] = true; + } + } return; } @@ -1617,6 +1635,7 @@ enumBackingType: $classLike instanceof Enum_ && $classLike->scalarType instan ? $classLike->scalarType->toLowerString() : null, nonClassDependencies: $analysis['nonClassDependencies'], + constantFetches: $analysis['constantFetches'], ); } @@ -1760,6 +1779,7 @@ private function resolveClassName(ClassLike $classLike): string * @return array{ * dependencies: list, * nonClassDependencies: list, + * constantFetches: list, * functionCalls: string[], * superglobals: string[], * languageConstructs: string[], @@ -1787,6 +1807,7 @@ private function collectClassLikeAnalysis(ClassLikeAnalysis $classLikeAnalysis): strcasecmp(...) ) ), + 'constantFetches' => array_keys($classLikeAnalysis->constantFetches), 'functionCalls' => array_values(array_unique($functionCalls)), 'superglobals' => array_keys($classLikeAnalysis->superglobals), 'languageConstructs' => array_keys($classLikeAnalysis->languageConstructs), diff --git a/src/Analyser/ClassLikeAnalysis.php b/src/Analyser/ClassLikeAnalysis.php index 22f658a0..dc061afe 100644 --- a/src/Analyser/ClassLikeAnalysis.php +++ b/src/Analyser/ClassLikeAnalysis.php @@ -25,6 +25,14 @@ final class ClassLikeAnalysis */ public array $classDependencies = []; + /** + * The constants fetched by name, kept apart from the dependencies: a + * class-like or function of the same name is not a constant fetch. + * + * @var array + */ + public array $constantFetches = []; + /** @var list */ public array $functionCallNames = []; diff --git a/src/Analyser/ClassNode.php b/src/Analyser/ClassNode.php index a4f6b8d7..26e0851c 100644 --- a/src/Analyser/ClassNode.php +++ b/src/Analyser/ClassNode.php @@ -5,8 +5,10 @@ namespace Boundwize\StructArmed\Analyser; use function array_filter; +use function in_array; use function preg_match; use function strcasecmp; +use function strncasecmp; use function strrpos; use function substr; @@ -37,6 +39,7 @@ final class ClassNode * @param EnumCaseNode[] $enumCases Cases of this enum * @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 */ public function __construct( public readonly string $className, @@ -70,6 +73,7 @@ public function __construct( public readonly array $enumCases = [], public readonly ?string $enumBackingType = null, public readonly array $nonClassDependencies = [], + public readonly array $constantFetches = [], ) { $this->layers = $layers ?: array_filter([$this->layer]); } @@ -93,6 +97,36 @@ public function usesClass(string $class): bool return true; } + /** + * Whether the class fetches the constant $constant: a class-like or + * function of the same name does not count. + */ + public function usesConstant(string $constant): bool + { + $separatorPosition = strrpos($constant, '\\'); + + // a constant name is case-sensitive + if ($separatorPosition === false) { + return in_array($constant, $this->constantFetches, true); + } + + $namespaceLength = $separatorPosition + 1; + $name = substr($constant, $namespaceLength); + + // namespace names are case-insensitive; only the namespace is + // compared that way, the constant name after it stays case-sensitive + foreach ($this->constantFetches as $constantFetch) { + if ( + strncasecmp($constantFetch, $constant, $namespaceLength) === 0 + && substr($constantFetch, $namespaceLength) === $name + ) { + return true; + } + } + + return false; + } + public function isBackedEnum(): bool { return $this->isEnum && $this->enumBackingType !== null; diff --git a/src/Analyser/NodeQueryTrait.php b/src/Analyser/NodeQueryTrait.php index 59a21298..c00b63bc 100644 --- a/src/Analyser/NodeQueryTrait.php +++ b/src/Analyser/NodeQueryTrait.php @@ -32,14 +32,14 @@ public function isInLayer(string $layer): bool return in_array($layer, $this->layers, true); } - public function dependsOn(string $class, bool $isCaseSensitive = true): bool + public function dependsOn(string $dependency, bool $isCaseSensitive = true): bool { if ($isCaseSensitive) { - return in_array($class, $this->dependencies, true); + return in_array($dependency, $this->dependencies, true); } - foreach ($this->dependencies as $dependency) { - if (strcasecmp($dependency, $class) === 0) { + foreach ($this->dependencies as $existingDependency) { + if (strcasecmp($existingDependency, $dependency) === 0) { return true; } } diff --git a/src/Cache/AnalysisResultCache.php b/src/Cache/AnalysisResultCache.php index 1776884e..238960da 100644 --- a/src/Cache/AnalysisResultCache.php +++ b/src/Cache/AnalysisResultCache.php @@ -65,7 +65,7 @@ final class AnalysisResultCache * their shape or naming changes: it is recorded in the metadata marker, * so a cache written by an older format is cleared on its next use. */ - public const FORMAT_VERSION = 13; + public const FORMAT_VERSION = 14; private readonly string $cacheDirectory; @@ -970,6 +970,7 @@ private function classNodeToArray(ClassNode $classNode): array $lists = [ 'dependencies' => $classNode->dependencies, 'nonClassDependencies' => $classNode->nonClassDependencies, + 'constantFetches' => $classNode->constantFetches, 'implements' => array_values($classNode->implements), 'interfaceExtends' => array_values($classNode->interfaceExtends), 'parentClasses' => $classNode->parentClasses, @@ -1011,6 +1012,7 @@ private function classNodeFromArray(array $node, string $file): ?ClassNode $isReadonly = $node['isReadonly'] ?? null; $dependencies = $node['dependencies'] ?? []; $nonClassDependencies = $node['nonClassDependencies'] ?? []; + $constantFetches = $node['constantFetches'] ?? []; $implements = $node['implements'] ?? []; $interfaceExtends = $node['interfaceExtends'] ?? []; $parentClasses = $node['parentClasses'] ?? []; @@ -1035,6 +1037,7 @@ private function classNodeFromArray(array $node, string $file): ?ClassNode || ! is_bool($isReadonly) || ! $this->isStringArray($dependencies) || ! $this->isStringArray($nonClassDependencies) + || ! $this->isStringArray($constantFetches) || ! $this->isStringArray($implements) || ! $this->isStringArray($interfaceExtends) || ! $this->isStringArray($parentClasses) @@ -1091,6 +1094,7 @@ interfaceExtends: array_values($interfaceExtends), enumCases: $enumCases, enumBackingType: $enumBackingType, nonClassDependencies: array_values($nonClassDependencies), + constantFetches: array_values($constantFetches), ); } diff --git a/src/Cli/AnalyseCommand.php b/src/Cli/AnalyseCommand.php index 5ece1701..ebe1d571 100644 --- a/src/Cli/AnalyseCommand.php +++ b/src/Cli/AnalyseCommand.php @@ -42,6 +42,7 @@ * @phpstan-type CommandOptions array{ * report?: string, * config?: string, + * basepath?: string, * generate-baseline?: string, * no-progress?: true, * clear-cache?: true, @@ -54,6 +55,8 @@ private const VALUE_OPTIONS = [ '--report' => 'report', '--config' => 'config', + '--basepath' => 'basepath', + '-d' => 'basepath', '--generate-baseline' => 'generate-baseline', ]; @@ -98,6 +101,18 @@ public function run(array $arguments, string $basePath): int return 1; } + $workingDirectory = $basePath; + + if (isset($options['basepath'])) { + $basePath = Path::normalise(Path::resolve($options['basepath'], $workingDirectory), canonicalise: true); + + if (! is_dir($basePath)) { + echo sprintf("Error: base path [%s] not found.\n", $options['basepath']); + + return 1; + } + } + foreach ($scanPaths as $scanPath) { $fullScanPath = Path::resolve($scanPath, $basePath); @@ -123,7 +138,7 @@ public function run(array $arguments, string $basePath): int } try { - $configFile = $options['config'] ?? ConfigLoader::discover($basePath); + $configFile = $options['config'] ?? ConfigLoader::discover($workingDirectory, $basePath); $architecture = ConfigLoader::load($configFile); } catch (RuntimeException $runtimeException) { return $this->reportError($runtimeException); diff --git a/src/Cli/ClearCacheCommand.php b/src/Cli/ClearCacheCommand.php index 6585c726..3da1eccb 100644 --- a/src/Cli/ClearCacheCommand.php +++ b/src/Cli/ClearCacheCommand.php @@ -7,18 +7,24 @@ use Boundwize\StructArmed\Cache\AnalysisResultCache; use Boundwize\StructArmed\Cache\FileHashProvider; use Boundwize\StructArmed\Config\ConfigLoader; +use Boundwize\StructArmed\Util\Path; use RuntimeException; use function count; +use function explode; +use function is_dir; use function sprintf; -use function str_starts_with; -use function strlen; -use function substr; use const PHP_EOL; final readonly class ClearCacheCommand { + private const VALUE_OPTIONS = [ + '--config' => 'config', + '--basepath' => 'basepath', + '-d' => 'basepath', + ]; + /** * @param list $arguments */ @@ -28,15 +34,12 @@ public function run(array $arguments, string $basePath): int $counter = count($arguments); for ($i = 0; $i < $counter; $i++) { - $argument = $arguments[$i]; - - if (str_starts_with($argument, '--config=')) { - $options['config'] = substr($argument, strlen('--config=')); - continue; - } + $argument = $arguments[$i]; + $optionAndValue = explode('=', $argument, 2); + $option = $optionAndValue[0]; - if ($argument === '--config') { - $options['config'] = $arguments[++$i] ?? ''; + if (isset(self::VALUE_OPTIONS[$option])) { + $options[self::VALUE_OPTIONS[$option]] = $optionAndValue[1] ?? $arguments[++$i] ?? ''; continue; } @@ -46,10 +49,22 @@ public function run(array $arguments, string $basePath): int return 1; } + $workingDirectory = $basePath; + + if (isset($options['basepath'])) { + $basePath = Path::normalise(Path::resolve($options['basepath'], $workingDirectory), canonicalise: true); + + if (! is_dir($basePath)) { + echo sprintf("Error: base path [%s] not found.\n", $options['basepath']); + + return 1; + } + } + $cacheDirectory = null; try { - $configFile = $options['config'] ?? ConfigLoader::discover($basePath); + $configFile = $options['config'] ?? ConfigLoader::discover($workingDirectory, $basePath); $cacheDirectory = ConfigLoader::load($configFile)->getCacheDirectory(); } catch (RuntimeException $runtimeException) { if (isset($options['config'])) { diff --git a/src/Cli/StructArmedApplication.php b/src/Cli/StructArmedApplication.php index 90660ea6..5f7244e8 100644 --- a/src/Cli/StructArmedApplication.php +++ b/src/Cli/StructArmedApplication.php @@ -8,24 +8,39 @@ use Boundwize\StructArmed\Version; use function array_slice; +use function array_values; use function getcwd; use function in_array; use function sprintf; final readonly class StructArmedApplication { + private const COMMANDS = ['init', 'analyse', 'analyze', '--clear-cache', '--version', '-V', '--help', '-h']; + /** * @param list $argv */ public function run(array $argv, ?string $basePath = null): int { $basePath ??= (string) getcwd(); - $command = $argv[1] ?? null; + $arguments = array_slice($argv, 1); + $command = $arguments[0] ?? null; if ($command === '--internal-worker') { return AnalysisNodeWorker::run($argv[2] ?? '', $argv[3] ?? ''); } + // Options may precede the command: `structarmed --basepath=../../ --clear-cache`. + foreach ($arguments as $index => $argument) { + if (in_array($argument, self::COMMANDS, true)) { + $command = $argument; + unset($arguments[$index]); + break; + } + } + + $arguments = array_values($arguments); + if (in_array($command, ['--version', '-V'], true)) { echo sprintf("StructArmed %s\n", Version::current()); @@ -39,15 +54,15 @@ public function run(array $argv, ?string $basePath = null): int } if ($command === 'init') { - return (new InitCommand())->run(array_slice($argv, 2), $basePath); + return (new InitCommand())->run($arguments, $basePath); } if ($command === '--clear-cache') { - return (new ClearCacheCommand())->run(array_slice($argv, 2), $basePath); + return (new ClearCacheCommand())->run($arguments, $basePath); } if (in_array($command, ['analyse', 'analyze'], true)) { - return (new AnalyseCommand())->run(array_slice($argv, 2), $basePath); + return (new AnalyseCommand())->run($arguments, $basePath); } echo sprintf("Unknown command: %s\n\n", $command); diff --git a/src/Cli/Usage.php b/src/Cli/Usage.php index 7c89fd0b..8e80bdef 100644 --- a/src/Cli/Usage.php +++ b/src/Cli/Usage.php @@ -13,9 +13,9 @@ public static function render(): string structarmed --version structarmed init [--preset=ddd|mvc|psr4|psr1|psr12|per|psr15|yagni|codequality|all] structarmed analyse|analyze [path ...] [--config=path/to/structarmed.php] - [--report=console|json|github] [--no-progress] [--clear-cache] [--disable-parallel] - [--fix] [--generate-baseline=structarmed-baseline.php] - structarmed --clear-cache [--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] + structarmed --clear-cache [--config=path/to/structarmed.php] [-d|--basepath=path/to/project] TXT; } diff --git a/src/Config/ConfigLoader.php b/src/Config/ConfigLoader.php index 35833363..2b669d8b 100644 --- a/src/Config/ConfigLoader.php +++ b/src/Config/ConfigLoader.php @@ -35,16 +35,22 @@ public static function load(string $configPath): Architecture return $architecture; } - public static function discover(string $basePath): string + /** + * Searches the base paths in order, so the CLI's working directory wins over + * a --basepath project root that also holds a config file. + */ + public static function discover(string ...$basePaths): string { - $candidates = [ - $basePath . '/structarmed.php', - $basePath . '/structarmed.dist.php', - ]; - - foreach ($candidates as $candidate) { - if (file_exists($candidate)) { - return $candidate; + foreach ($basePaths as $basePath) { + $candidates = [ + $basePath . '/structarmed.php', + $basePath . '/structarmed.dist.php', + ]; + + foreach ($candidates as $candidate) { + if (file_exists($candidate)) { + return $candidate; + } } } diff --git a/src/Preset/Presets/DddPreset.php b/src/Preset/Presets/DddPreset.php index 2ba49ded..78d8411b 100644 --- a/src/Preset/Presets/DddPreset.php +++ b/src/Preset/Presets/DddPreset.php @@ -17,6 +17,7 @@ use Boundwize\StructArmed\Rule\Rules\Method\MustHaveReturnTypeRule; use Boundwize\StructArmed\Rule\Rules\Usage\MayNotCallFunctionRule; use Boundwize\StructArmed\Rule\Rules\Usage\MayNotUseClassRule; +use Boundwize\StructArmed\Rule\Rules\Usage\MayNotUseConstantRule; use Boundwize\StructArmed\Rule\Rules\Usage\MayNotUseLanguageConstructRule; use DateTime; use Exception; @@ -270,6 +271,13 @@ private function applySafetyRules(Architecture $architecture): self new MayNotImplementInterfaceRule(layer: 'Domain', interface: JsonSerializable::class) ); + foreach (['STDIN', 'STDOUT', 'STDERR'] as $constant) { + $architecture->rule( + sprintf('ddd.safety.domain_no_%s', strtolower($constant)), + new MayNotUseConstantRule(layer: 'Domain', constant: $constant) + ); + } + foreach (['Domain', 'Application'] as $layer) { $architecture->rule( sprintf('ddd.safety.%s_max_complexity', strtolower($layer)), diff --git a/src/Rule/RuleViolationCollection.php b/src/Rule/RuleViolationCollection.php index cfdcbe38..4f32a3c0 100644 --- a/src/Rule/RuleViolationCollection.php +++ b/src/Rule/RuleViolationCollection.php @@ -13,9 +13,6 @@ use function array_map; use function array_values; use function count; -use function json_encode; - -use const JSON_INVALID_UTF8_SUBSTITUTE; /** * @implements IteratorAggregate @@ -57,17 +54,6 @@ public function getIterator(): Traversable return new ArrayIterator($this->violations); } - /** @return RuleViolation[] */ - public function forLayer(string $layer): array - { - return array_values( - array_filter( - $this->violations, - static fn(RuleViolation $ruleViolation): bool => $ruleViolation->layer === $layer - ) - ); - } - /** @return RuleViolation[] */ public function forRule(string $ruleKey): array { @@ -87,9 +73,4 @@ public function toArray(): array $this->violations )); } - - public function toJson(): string - { - return (string) json_encode($this->toArray(), JSON_INVALID_UTF8_SUBSTITUTE); - } } diff --git a/src/Rule/Rules/Usage/MayNotUseConstantRule.php b/src/Rule/Rules/Usage/MayNotUseConstantRule.php new file mode 100644 index 00000000..ea6cba13 --- /dev/null +++ b/src/Rule/Rules/Usage/MayNotUseConstantRule.php @@ -0,0 +1,45 @@ +isInLayer($this->layer); + } + + public function evaluate(ClassNode $classNode): ?RuleViolation + { + if (! $classNode->usesConstant($this->constant)) { + return null; + } + + return new RuleViolation( + message: sprintf( + '%s [%s] must not use constant [%s]', + $classNode->getType(), + $classNode->className, + $this->constant + ), + file: $classNode->file, + line: $classNode->line, + className: $classNode->className, + layer: $classNode->layer, + ); + } +} diff --git a/tests/Analyser/AnalyserTest.php b/tests/Analyser/AnalyserTest.php index 62b0302c..66e720a1 100644 --- a/tests/Analyser/AnalyserTest.php +++ b/tests/Analyser/AnalyserTest.php @@ -565,8 +565,7 @@ public function testAnalyserReturnsNoViolationsForValidCode(): void $ruleViolationCollection = $analyser->analyse($architecture); // Order.php is a valid entity — should produce no layer violations - $this->assertEmpty($ruleViolationCollection->forLayer('Application')); - $this->assertEmpty($ruleViolationCollection->forLayer('Infrastructure')); + $this->assertTrue($ruleViolationCollection->isEmpty()); } public function testAnalyserDetectsViolationsInBadCode(): void diff --git a/tests/Analyser/AnalysisNodeCollectorTest.php b/tests/Analyser/AnalysisNodeCollectorTest.php index f0e9a57e..027d6da2 100644 --- a/tests/Analyser/AnalysisNodeCollectorTest.php +++ b/tests/Analyser/AnalysisNodeCollectorTest.php @@ -2349,6 +2349,66 @@ public function isEnabled(): bool $this->assertContains('App\Infrastructure\Config\FEATURE_ENABLED', $classNode->dependencies); } + public function testCollectsConstantFetchesApartFromDependencies(): void + { + $classNode = $this->collect( + <<<'PHP_WRAP' + assertSame( + ['App\Domain\PHP_EOL', 'PHP_EOL', 'PHP_INT_MAX', 'App\Infrastructure\Config\FEATURE_ENABLED'], + $classNode->constantFetches + ); + $this->assertNotContains('PHP_EOL', $classNode->dependencies); + } + + public function testCollectsUnqualifiedConstantFetchOfSameNamespaceConstant(): void + { + $classNode = $this->collect( + <<<'PHP' + assertTrue($classNode->usesConstant('Vendor\Config\DEBUG')); + $this->assertTrue($classNode->usesConstant('DEBUG')); + } + public function testCollectsFullyQualifiedDependencies(): void { $classNode = $this->collect('assertTrue($classNode->dependsOn('Vendor\\helper')); } + public function testUsesConstantMatchesNamespaceCaseInsensitivelyAndNameCaseSensitively(): void + { + $classNode = new ClassNode( + className: 'App\\Domain\\OrderService', + file: '/src/OrderService.php', + line: 5, + layer: 'Domain', + extends: null, + isAbstract: false, + isFinal: false, + isInterface: false, + isReadonly: false, + dependencies: ['STDOUT'], + constantFetches: ['STDIN', 'Vendor\\Config\\DEBUG'], + ); + + $this->assertTrue($classNode->usesConstant('STDIN')); + $this->assertFalse($classNode->usesConstant('stdin')); + $this->assertTrue($classNode->usesConstant('vendor\\config\\DEBUG')); + $this->assertFalse($classNode->usesConstant('Vendor\\Config\\debug')); + $this->assertFalse($classNode->usesConstant('Vendor\\DEBUG')); + $this->assertFalse($classNode->usesConstant('DEBUG')); + // a dependency of that name is a class-like or function, not a fetch + $this->assertFalse($classNode->usesConstant('STDOUT')); + } + public function testDependsOnDoesNotMatchNamespacePrefix(): void { $classNode = new ClassNode( diff --git a/tests/Cache/AnalysisResultCacheTest.php b/tests/Cache/AnalysisResultCacheTest.php index 2c0dbca8..8b04594f 100644 --- a/tests/Cache/AnalysisResultCacheTest.php +++ b/tests/Cache/AnalysisResultCacheTest.php @@ -2945,6 +2945,7 @@ className: Foo::class, ], functionCalls: ['sprintf'], superglobals: ['_SERVER'], + constantFetches: ['PHP_EOL'], ); } diff --git a/tests/Cli/StructArmedApplicationCommandRoutingTest.php b/tests/Cli/StructArmedApplicationCommandRoutingTest.php index fb8f6229..8b3ca7c8 100644 --- a/tests/Cli/StructArmedApplicationCommandRoutingTest.php +++ b/tests/Cli/StructArmedApplicationCommandRoutingTest.php @@ -79,6 +79,39 @@ public function testApplicationRejectsUnknownClearCacheOption(): void $this->assertStringContainsString('Unknown option: --bad-option', $output); } + public function testApplicationRejectsMissingClearCacheBasePath(): void + { + [$exitCode, $output] = $this->runApplication( + ['structarmed', '--clear-cache', '--basepath', 'missing'], + self::BASE_PATH + ); + + $this->assertSame(1, $exitCode); + $this->assertStringContainsString('Error: base path [missing] not found.', $output); + } + + public function testApplicationAcceptsOptionsBeforeClearCacheCommand(): void + { + [$exitCode, $output] = $this->runApplication( + ['structarmed', '--basepath=missing', '--clear-cache'], + self::BASE_PATH + ); + + $this->assertSame(1, $exitCode); + $this->assertStringContainsString('Error: base path [missing] not found.', $output); + } + + public function testApplicationAcceptsShortClearCacheBasePathOption(): void + { + [$exitCode, $output] = $this->runApplication( + ['structarmed', '--clear-cache', '-d=missing'], + self::BASE_PATH + ); + + $this->assertSame(1, $exitCode); + $this->assertStringContainsString('Error: base path [missing] not found.', $output); + } + public function testInitCommandRejectsUnknownOption(): void { [$exitCode, $output] = $this->runApplication(['structarmed', 'init', '--bad-option'], self::BASE_PATH); @@ -119,6 +152,28 @@ public function testAnalyseCommandRejectsMissingScanPath(): void $this->assertStringContainsString('Error: path [missing] not found.', $output); } + public function testAnalyseCommandRejectsMissingBasePath(): void + { + [$exitCode, $output] = $this->runApplication( + ['structarmed', 'analyse', '--basepath=missing'], + self::BASE_PATH + ); + + $this->assertSame(1, $exitCode); + $this->assertStringContainsString('Error: base path [missing] not found.', $output); + } + + public function testAnalyseCommandAcceptsShortBasePathOption(): void + { + [$exitCode, $output] = $this->runApplication( + ['structarmed', 'analyse', '-d', 'missing'], + self::BASE_PATH + ); + + $this->assertSame(1, $exitCode); + $this->assertStringContainsString('Error: base path [missing] not found.', $output); + } + public function testAnalyseCommandAcceptsAbsoluteScanPath(): void { [$exitCode, $output] = $this->runApplication( diff --git a/tests/Cli/StructArmedApplicationTest.php b/tests/Cli/StructArmedApplicationTest.php index ead2a5ab..147a9901 100644 --- a/tests/Cli/StructArmedApplicationTest.php +++ b/tests/Cli/StructArmedApplicationTest.php @@ -32,6 +32,7 @@ use function preg_replace; use function random_bytes; use function realpath; +use function rename; use function rmdir; use function serialize; use function str_replace; @@ -97,6 +98,41 @@ public function testApplicationClearsConfiguredCacheWithoutAnalyseCommand(): voi } } + public function testApplicationClearsConfiguredCacheFromBasePathOption(): void + { + $basePath = (string) realpath($this->createProjectDirectory()); + $toolsPath = $basePath . '/tools/structarmed'; + $cacheDirectory = $basePath . '/var/cache/structarmed'; + + try { + mkdir($toolsPath, 0777, true); + mkdir($cacheDirectory, 0777, true); + + file_put_contents($cacheDirectory . '/key.json', '{}'); + file_put_contents($toolsPath . '/structarmed.php', <<<'PHP' +cacheDirectory('var/cache/structarmed'); +PHP); + + [$exitCode, $output] = $this->runApplication( + ['structarmed', '--basepath=../../', '--clear-cache'], + $toolsPath + ); + + $this->assertSame(0, $exitCode, $output); + $this->assertStringContainsString('StructArmed cache cleared.', $output); + $this->assertDirectoryDoesNotExist($cacheDirectory); + } finally { + $this->removeTempDirectory($basePath); + } + } + public function testApplicationClearsConfiguredCacheWithSeparateConfigOption(): void { $basePath = $this->createProjectDirectory(); @@ -426,6 +462,92 @@ class Foo } } + public function testAnalyseCommandResolvesProjectPathsAgainstBasePathOption(): void + { + $basePath = (string) realpath($this->createTempDirectory()); + $toolsPath = $basePath . '/tools/structarmed'; + + mkdir($toolsPath, 0777, true); + mkdir($basePath . '/src/Exception', 0777, true); + file_put_contents($basePath . '/src/ConfigProvider.php', <<<'PHP' +layer('Config', 'src/ConfigProvider.php') + ->layer('Exception', 'src/Exception') + ->rule('config.must_be_final', new MustBeFinalRule('Config')) + ->rule('exception.must_be_final', new MustBeFinalRule('Exception')) + ->rule('composer.psr4_directory_exists', new Psr4DirectoryExistsRule()); +PHP); + + try { + [$exitCode, $output] = $this->runApplication( + ['structarmed', 'analyse', '--basepath=../../', '--no-progress'], + $toolsPath + ); + + $this->assertSame(1, $exitCode, $output); + $this->assertStringContainsString('Class [App\ConfigProvider] must be declared final', $output); + $this->assertStringContainsString('Class [App\Exception\NotFound] must be declared final', $output); + $this->assertStringContainsString('declared in composer.json do not exist on disk', $output); + $this->assertStringContainsString( + $this->normalisePath($basePath . '/src/ConfigProvider.php'), + $this->normalisePath($output) + ); + $this->assertStringContainsString( + $this->normalisePath($basePath . '/src/Exception/NotFound.php'), + $this->normalisePath($output) + ); + + // Without a config next to the tool, discovery falls back to the base path. + rename($toolsPath . '/structarmed.php', $basePath . '/structarmed.php'); + + [$fallbackExitCode, $fallbackOutput] = $this->runApplication( + ['structarmed', 'analyse', '-d', '../../', '--no-progress'], + $toolsPath + ); + + $this->assertSame(1, $fallbackExitCode, $fallbackOutput); + $this->assertStringContainsString('Class [App\ConfigProvider] must be declared final', $fallbackOutput); + } finally { + $this->removeTempDirectory($basePath); + } + } + public function testAnalyseCommandOnlyParsesNewFilesAfterCacheWarmup(): void { $basePath = $this->createProjectDirectory(); @@ -1807,6 +1929,10 @@ private function removeTempDirectory(string $basePath): void unlink($basePath . '/structarmed-custom.php'); } + if (file_exists($basePath . '/tools/structarmed/structarmed.php')) { + unlink($basePath . '/tools/structarmed/structarmed.php'); + } + if (file_exists($basePath . '/structarmed-baseline.php')) { unlink($basePath . '/structarmed-baseline.php'); } @@ -1827,6 +1953,14 @@ private function removeTempDirectory(string $basePath): void unlink($sourceFile); } + foreach (glob($basePath . '/src/Exception/*.php') ?: [] as $sourceFile) { + unlink($sourceFile); + } + + if (is_dir($basePath . '/src/Exception')) { + rmdir($basePath . '/src/Exception'); + } + if (is_dir($basePath . '/src/Domain')) { rmdir($basePath . '/src/Domain'); } @@ -1851,6 +1985,14 @@ private function removeTempDirectory(string $basePath): void rmdir($basePath . '/nested'); } + if (is_dir($basePath . '/tools/structarmed')) { + rmdir($basePath . '/tools/structarmed'); + } + + if (is_dir($basePath . '/tools')) { + rmdir($basePath . '/tools'); + } + if (is_dir($basePath)) { rmdir($basePath); } diff --git a/tests/Config/ConfigLoaderTest.php b/tests/Config/ConfigLoaderTest.php index fbe57257..9d58a5d7 100644 --- a/tests/Config/ConfigLoaderTest.php +++ b/tests/Config/ConfigLoaderTest.php @@ -53,6 +53,15 @@ public function testDiscoverFallsBackToDistConfig(): void $this->assertSame($basePath . '/structarmed.dist.php', ConfigLoader::discover($basePath)); } + public function testDiscoverFallsBackToNextBasePath(): void + { + $workingDirectory = $this->makeTempDir(); + $basePath = $this->makeTempDir(); + touch($basePath . '/structarmed.php'); + + $this->assertSame($basePath . '/structarmed.php', ConfigLoader::discover($workingDirectory, $basePath)); + } + private function writeTempConfig(string $body): string { $path = $this->makeTemporaryFile('structarmed-config'); diff --git a/tests/Preset/PresetTest.php b/tests/Preset/PresetTest.php index 06e1b7a8..0040c224 100644 --- a/tests/Preset/PresetTest.php +++ b/tests/Preset/PresetTest.php @@ -323,6 +323,7 @@ public function testDddPresetRegistersAllDefaultRules(): void ); $this->assertArrayHasKey('ddd.safety.domain_no_dd', $rules); $this->assertArrayHasKey('ddd.safety.application_no_exit', $rules); + $this->assertArrayHasKey('ddd.safety.domain_no_stderr', $rules); } public function testDddPresetCanSkipOptionalFinalRules(): void diff --git a/tests/Rule/RuleViolationTest.php b/tests/Rule/RuleViolationTest.php index 8eca2001..9aeb406f 100644 --- a/tests/Rule/RuleViolationTest.php +++ b/tests/Rule/RuleViolationTest.php @@ -10,8 +10,6 @@ use PHPUnit\Framework\TestCase; use function iterator_to_array; -use function json_decode; -use function json_encode; #[CoversClass(RuleViolation::class)] #[CoversClass(RuleViolationCollection::class)] @@ -163,33 +161,11 @@ public function testCollectionFiltersAndSerializesViolations(): void $this->assertFalse($collection->isEmpty()); $this->assertTrue($collection->hasViolations()); $this->assertCount(2, $collection); - $this->assertSame([$ruleViolation], $collection->forLayer('Domain')); $this->assertSame([$app], $collection->forRule('app.rule')); - $this->assertSame(json_encode($collection->toArray()), $collection->toJson()); - $this->assertSame($collection->toArray(), json_decode($collection->toJson(), true)); + $this->assertSame([$ruleViolation->toArray(), $app->toArray()], $collection->toArray()); $this->assertSame([$ruleViolation, $app], iterator_to_array($collection)); } - public function testCollectionSerializesInvalidUtf8Text(): void - { - $ruleViolationCollection = new RuleViolationCollection(); - $ruleViolationCollection->add(new RuleViolation( - message: "Invalid byte \xB1", - file: '/src/File.php', - line: 7, - className: 'App\\Domain\\File', - layer: 'Domain', - ruleKey: 'domain.rule', - )); - - $data = json_decode($ruleViolationCollection->toJson(), true); - - $this->assertIsArray($data); - $this->assertIsArray($data[0]); - $this->assertIsString($data[0]['message']); - $this->assertStringContainsString("\xEF\xBF\xBD", $data[0]['message']); - } - private function violation(string $ruleKey, string $layer): RuleViolation { return new RuleViolation( diff --git a/tests/Rule/Usage/MayNotUseConstantRuleTest.php b/tests/Rule/Usage/MayNotUseConstantRuleTest.php new file mode 100644 index 00000000..52b56164 --- /dev/null +++ b/tests/Rule/Usage/MayNotUseConstantRuleTest.php @@ -0,0 +1,138 @@ + $constantFetches */ + private function makeNode( + array $constantFetches, + string $layer = 'Domain', + bool $isTrait = false, + bool $isEnum = false, + ): ClassNode { + return new ClassNode( + className: 'App\\Domain\\OrderService', + file: '/fake.php', + line: 1, + layer: $layer, + extends: null, + isAbstract: false, + isFinal: true, + isInterface: false, + isReadonly: false, + isTrait: $isTrait, + isEnum: $isEnum, + constantFetches: $constantFetches, + ); + } + + public function testPassesWhenForbiddenConstantNotUsed(): void + { + $mayNotUseConstantRule = new MayNotUseConstantRule(layer: 'Domain', constant: 'PHP_EOL'); + $classNode = $this->makeNode(['PHP_INT_MAX']); + + $this->assertNotInstanceOf(RuleViolation::class, $mayNotUseConstantRule->evaluate($classNode)); + } + + public function testViolatesWhenForbiddenConstantIsUsed(): void + { + $mayNotUseConstantRule = new MayNotUseConstantRule(layer: 'Domain', constant: 'PHP_EOL'); + $classNode = $this->makeNode(['PHP_EOL']); + + $violation = $mayNotUseConstantRule->evaluate($classNode); + + $this->assertInstanceOf(RuleViolation::class, $violation); + $this->assertSame('Class [App\\Domain\\OrderService] must not use constant [PHP_EOL]', $violation->message); + } + + #[DataProvider('traitAndEnumKindProvider')] + public function testViolationMessageNamesTheClassLikeKind(string $expectedKind, bool $isTrait, bool $isEnum): void + { + $mayNotUseConstantRule = new MayNotUseConstantRule(layer: 'Domain', constant: 'PHP_EOL'); + $classNode = $this->makeNode(['PHP_EOL'], isTrait: $isTrait, isEnum: $isEnum); + + $violation = $mayNotUseConstantRule->evaluate($classNode); + + $this->assertInstanceOf(RuleViolation::class, $violation); + $this->assertSame( + $expectedKind . ' [App\\Domain\\OrderService] must not use constant [PHP_EOL]', + $violation->message + ); + } + + /** @return iterable */ + public static function traitAndEnumKindProvider(): iterable + { + yield 'trait' => ['Trait', true, false]; + yield 'enum' => ['Enum', false, true]; + } + + public function testConstantComparisonIsCaseSensitive(): void + { + $mayNotUseConstantRule = new MayNotUseConstantRule(layer: 'Domain', constant: 'PHP_EOL'); + $classNode = $this->makeNode(['php_eol']); + + $this->assertNotInstanceOf(RuleViolation::class, $mayNotUseConstantRule->evaluate($classNode)); + } + + public function testNamespaceComparisonIsCaseInsensitive(): void + { + $mayNotUseConstantRule = new MayNotUseConstantRule(layer: 'Domain', constant: 'Vendor\\Config\\DEBUG'); + + $this->assertInstanceOf( + RuleViolation::class, + $mayNotUseConstantRule->evaluate($this->makeNode(['vendor\\config\\DEBUG'])) + ); + $this->assertNotInstanceOf( + RuleViolation::class, + $mayNotUseConstantRule->evaluate($this->makeNode(['Vendor\\Config\\debug'])) + ); + } + + public function testPassesWhenOnlyAClassOrFunctionOfThatNameIsUsed(): void + { + $mayNotUseConstantRule = new MayNotUseConstantRule(layer: 'Domain', constant: 'STDIN'); + $classNode = new ClassNode( + className: 'App\\Domain\\OrderService', + file: '/fake.php', + line: 1, + layer: 'Domain', + extends: null, + isAbstract: false, + isFinal: true, + isInterface: false, + isReadonly: false, + dependencies: ['STDIN'], + functionCalls: ['STDIN'], + ); + + $this->assertNotInstanceOf(RuleViolation::class, $mayNotUseConstantRule->evaluate($classNode)); + } + + public function testDoesNotApplyToWrongLayer(): void + { + $mayNotUseConstantRule = new MayNotUseConstantRule(layer: 'Domain', constant: 'PHP_EOL'); + $classNode = $this->makeNode(['PHP_EOL'], layer: 'Infrastructure'); + + $this->assertFalse($mayNotUseConstantRule->appliesTo($classNode)); + } + + public function testAppliesToCorrectLayer(): void + { + $mayNotUseConstantRule = new MayNotUseConstantRule(layer: 'Domain', constant: 'PHP_EOL'); + $classNode = $this->makeNode([]); + + $this->assertTrue($mayNotUseConstantRule->appliesTo($classNode)); + } +}