diff --git a/composer.json b/composer.json index 8569b61..844688f 100644 --- a/composer.json +++ b/composer.json @@ -29,6 +29,7 @@ "require": { "php": "^8.2", "composer-runtime-api": "^2.0", + "boundwize/jsonrecast": "^0.0.2", "fidry/cpu-core-counter": "^1.3", "nikic/php-parser": "^5.7" }, diff --git a/docs/assets/no-violation.svg b/docs/assets/no-violation.svg index 80814e7..2cc26b3 100644 --- a/docs/assets/no-violation.svg +++ b/docs/assets/no-violation.svg @@ -14,7 +14,7 @@ prj-ddd vendor/bin/structarmed analyze - StructArmed 0.14.3 — Architecture Enforcement + StructArmed 0.14.4 — Architecture Enforcement =============================================== diff --git a/docs/assets/structarmed-showoff.svg b/docs/assets/structarmed-showoff.svg index d61c269..7d2da84 100644 --- a/docs/assets/structarmed-showoff.svg +++ b/docs/assets/structarmed-showoff.svg @@ -15,7 +15,7 @@ prj-ddd vendor/bin/structarmed analyze - StructArmed 0.14.3 — Architecture Enforcement + StructArmed 0.14.4 — Architecture Enforcement =============================================== diff --git a/docs/available-rules.md b/docs/available-rules.md index 6b0bad4..d5f5747 100644 --- a/docs/available-rules.md +++ b/docs/available-rules.md @@ -46,7 +46,7 @@ Namespace: `Boundwize\StructArmed\Rule\Rules\Composer`. | Rule | Constructor | Checks | |---|---|---| -| `Psr4DirectoryExistsRule` | `new Psr4DirectoryExistsRule()` | `composer.json` exists, is valid JSON, and every PSR-4 source path exists on disk. | +| `Psr4DirectoryExistsRule` | `new Psr4DirectoryExistsRule()` | `composer.json` exists, is valid JSON, and every PSR-4 source path exists on disk. Supports `--fix` by removing mappings for missing directories. | | `Psr4EmptyNamespacePrefixRule` | `new Psr4EmptyNamespacePrefixRule()` | `autoload` and `autoload-dev` PSR-4 mappings do not use an empty namespace prefix. | | `Psr4NamespaceRule` | `new Psr4NamespaceRule(layer: 'Source')` | A class name matches the namespace expected from its PSR-4 path. | | `Psr4RootPathRule` | `new Psr4RootPathRule()` | PSR-4 mappings do not point directly to the project root. | @@ -91,7 +91,7 @@ Namespace: `Boundwize\StructArmed\Rule\Rules\Class_`. `classNamePattern` and `excludePattern` are regular expressions matched against the fully-qualified class name. -`Psr1PhpTagsRule`, `Psr1Utf8WithoutBomRule`, `MustBeFinalRule`, `MustDeclareConstantVisibilityRule`, `MustDeclareMethodVisibilityRule`, and `MustDeclarePropertyVisibilityRule` implement `Boundwize\StructArmed\Rule\FixableInterface`, so StructArmed can automatically normalize invalid PHP opening tags, remove UTF-8 byte order marks, add the `final` class modifier, and add missing constant, method, or property visibility modifiers when you run `vendor/bin/structarmed analyse --fix`. +`Psr4DirectoryExistsRule`, `Psr1PhpTagsRule`, `Psr1Utf8WithoutBomRule`, `MustBeFinalRule`, `MustDeclareConstantVisibilityRule`, `MustDeclareMethodVisibilityRule`, and `MustDeclarePropertyVisibilityRule` implement `Boundwize\StructArmed\Rule\FixableInterface`, so StructArmed can automatically remove PSR-4 mappings for missing directories, normalize invalid PHP opening tags, remove UTF-8 byte order marks, add the `final` class modifier, and add missing constant, method, or property visibility modifiers when you run `vendor/bin/structarmed analyse --fix`. ## Layer Rules diff --git a/src/Rule/Fixer/JsonRecast/AbstractJsonRecastFixableRule.php b/src/Rule/Fixer/JsonRecast/AbstractJsonRecastFixableRule.php new file mode 100644 index 0000000..9b06fb7 --- /dev/null +++ b/src/Rule/Fixer/JsonRecast/AbstractJsonRecastFixableRule.php @@ -0,0 +1,31 @@ +fixerProcessor()->process( + $ruleViolation->file, + $this->createFixerVisitor($ruleViolation), + ); + } + + abstract protected function createFixerVisitor(RuleViolation $ruleViolation): NodeJsonVisitor; + + private function fixerProcessor(): JsonRecastFixerProcessor + { + static $processor; + + return $processor instanceof JsonRecastFixerProcessor + ? $processor + : ($processor = new JsonRecastFixerProcessor()); + } +} diff --git a/src/Rule/Fixer/JsonRecast/Composer/RemoveMissingPsr4PathVisitor.php b/src/Rule/Fixer/JsonRecast/Composer/RemoveMissingPsr4PathVisitor.php new file mode 100644 index 0000000..44da628 --- /dev/null +++ b/src/Rule/Fixer/JsonRecast/Composer/RemoveMissingPsr4PathVisitor.php @@ -0,0 +1,203 @@ + */ + private const AUTOLOAD_SECTIONS = ['autoload', 'autoload-dev']; + + /** @var array */ + private array $changedContainerPathKeys = []; + + public function __construct( + private readonly string $basePath, + ) { + } + + public function enterNode(NodeJson $nodeJson, NodeJsonPath $nodeJsonPath): ?int + { + if ($nodeJson instanceof ObjectItemNode && $this->isPsr4Mapping($nodeJsonPath)) { + if ( + $nodeJson->value instanceof StringNode + && ! $this->directoryExists($nodeJson->value->value) + ) { + $this->markContainerChanged($nodeJsonPath); + + return NodeJsonVisitor::REMOVE_NODE; + } + + return null; + } + + if (! $nodeJson instanceof ArrayItemNode || ! $this->isPsr4PathListItem($nodeJsonPath)) { + return null; + } + + if (! $nodeJson->value instanceof StringNode || $this->directoryExists($nodeJson->value->value)) { + return null; + } + + $this->markContainerChanged($this->parentPath($nodeJsonPath)); + + return NodeJsonVisitor::REMOVE_NODE; + } + + public function leaveNode(NodeJson $nodeJson, NodeJsonPath $nodeJsonPath): null|int + { + if ($nodeJson instanceof ObjectNode || $nodeJson instanceof ArrayNode) { + $this->flagChangedContainer($nodeJson, $nodeJsonPath); + + return null; + } + + if (! $nodeJson instanceof ObjectItemNode) { + return null; + } + + if ($this->isPsr4Section($nodeJson, $nodeJsonPath)) { + if ( + ! $nodeJson->value instanceof ObjectNode + || $nodeJson->value->items !== [] + || ! $this->hasChangedChild($nodeJson->value) + ) { + return null; + } + + $this->markContainerChanged($nodeJsonPath); + + return NodeJsonVisitor::REMOVE_NODE; + } + + if ($this->isEmptyComposerAutoloadItem($nodeJson, $nodeJsonPath)) { + return NodeJsonVisitor::REMOVE_NODE; + } + + if (! $this->isPsr4Mapping($nodeJsonPath)) { + return null; + } + + if ( + ! $nodeJson->value instanceof ArrayNode + || $nodeJson->value->items !== [] + || ! $this->hasChangedChild($nodeJson->value) + ) { + return null; + } + + $this->markContainerChanged($nodeJsonPath); + + return NodeJsonVisitor::REMOVE_NODE; + } + + private function isPsr4Section(ObjectItemNode $objectItemNode, NodeJsonPath $nodeJsonPath): bool + { + return $objectItemNode->key->value === 'psr-4' + && $this->isComposerAutoloadPath($nodeJsonPath); + } + + private function isEmptyComposerAutoloadItem(ObjectItemNode $objectItemNode, NodeJsonPath $nodeJsonPath): bool + { + return $nodeJsonPath->isRoot() + && $this->isComposerAutoloadKey($objectItemNode->key->value) + && $objectItemNode->value instanceof ObjectNode + && $objectItemNode->value->items === [] + && $this->hasChangedChild($objectItemNode->value); + } + + private function isPsr4Mapping(NodeJsonPath $nodeJsonPath): bool + { + return $nodeJsonPath->matches(['autoload', 'psr-4']) + || $nodeJsonPath->matches(['autoload-dev', 'psr-4']); + } + + private function isPsr4PathListItem(NodeJsonPath $nodeJsonPath): bool + { + $last = $nodeJsonPath->last(); + + return $last?->isArrayIndex() === true + && $this->isPsr4MappingValuePath($this->parentPath($nodeJsonPath)); + } + + private function isPsr4MappingValuePath(NodeJsonPath $nodeJsonPath): bool + { + $last = $nodeJsonPath->last(); + + return $last?->isObjectKey() === true + && $this->isPsr4Mapping($this->parentPath($nodeJsonPath)); + } + + private function isComposerAutoloadPath(NodeJsonPath $nodeJsonPath): bool + { + return $nodeJsonPath->matches(['autoload']) + || $nodeJsonPath->matches(['autoload-dev']); + } + + private function isComposerAutoloadKey(string $key): bool + { + return in_array($key, self::AUTOLOAD_SECTIONS, true); + } + + private function directoryExists(string $path): bool + { + return is_dir(Path::resolve(Path::normalise(trim($path)), $this->basePath)); + } + + private function markContainerChanged(NodeJsonPath $nodeJsonPath): void + { + $this->changedContainerPathKeys[$this->pathKey($nodeJsonPath)] = true; + } + + private function flagChangedContainer(NodeJson $nodeJson, NodeJsonPath $nodeJsonPath): void + { + if (! array_key_exists($this->pathKey($nodeJsonPath), $this->changedContainerPathKeys)) { + return; + } + + $nodeJson->setAttribute(self::CHILD_CHANGED, true); + } + + private function hasChangedChild(NodeJson $nodeJson): bool + { + return $nodeJson->getAttribute(self::CHILD_CHANGED) === true; + } + + private function parentPath(NodeJsonPath $nodeJsonPath): NodeJsonPath + { + return new NodeJsonPath(array_slice($nodeJsonPath->segments(), 0, -1)); + } + + private function pathKey(NodeJsonPath $nodeJsonPath): string + { + $key = ''; + + foreach ($nodeJsonPath->segments() as $nodeJsonPathSegment) { + $value = (string) $nodeJsonPathSegment->value; + $key .= ($nodeJsonPathSegment->isObjectKey() ? 'o' : 'a') . strlen($value) . ':' . $value; + } + + return $key; + } +} diff --git a/src/Rule/Fixer/JsonRecast/JsonRecastFixerProcessor.php b/src/Rule/Fixer/JsonRecast/JsonRecastFixerProcessor.php new file mode 100644 index 0000000..8d88171 --- /dev/null +++ b/src/Rule/Fixer/JsonRecast/JsonRecastFixerProcessor.php @@ -0,0 +1,38 @@ +file)); + } } diff --git a/tests/Rule/Composer/Psr4DirectoryExistsRuleTest.php b/tests/Rule/Composer/Psr4DirectoryExistsRuleTest.php index cfbe4f3..73a59bb 100644 --- a/tests/Rule/Composer/Psr4DirectoryExistsRuleTest.php +++ b/tests/Rule/Composer/Psr4DirectoryExistsRuleTest.php @@ -5,20 +5,33 @@ namespace Boundwize\StructArmed\Tests\Rule\Composer; use Boundwize\StructArmed\Architecture; +use Boundwize\StructArmed\Rule\FixableInterface; +use Boundwize\StructArmed\Rule\Fixer\JsonRecast\AbstractJsonRecastFixableRule; +use Boundwize\StructArmed\Rule\Fixer\JsonRecast\Composer\RemoveMissingPsr4PathVisitor; +use Boundwize\StructArmed\Rule\Fixer\JsonRecast\JsonRecastFixerProcessor; use Boundwize\StructArmed\Rule\Rules\Composer\Psr4DirectoryExistsRule; use Boundwize\StructArmed\Rule\RuleViolation; use Boundwize\StructArmed\Tests\Support\TemporaryDirectoryCleanupTrait; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; +use function file_get_contents; use function file_put_contents; use function mkdir; #[CoversClass(Psr4DirectoryExistsRule::class)] +#[CoversClass(AbstractJsonRecastFixableRule::class)] +#[CoversClass(JsonRecastFixerProcessor::class)] +#[CoversClass(RemoveMissingPsr4PathVisitor::class)] final class Psr4DirectoryExistsRuleTest extends TestCase { use TemporaryDirectoryCleanupTrait; + public function testIsFixable(): void + { + $this->assertInstanceOf(FixableInterface::class, new Psr4DirectoryExistsRule()); + } + public function testPassesWhenAllPsr4DirectoriesExistOnDisk(): void { $basePath = $this->makeTempProject(<<<'JSON' @@ -90,6 +103,135 @@ public function testPassesWhenNoPsr4PathsAreDeclared(): void ); } + public function testFixRemovesPsr4MappingsForMissingDirectories(): void + { + $basePath = $this->makeTempProject(<<<'JSON' +{ + "autoload": { + "psr-4": { + "App\\": "src/", + "Missing\\": "missing/", + "Mixed\\": ["src/", "missing-tests/"], + "Gone\\": ["missing-one/", "missing-two/"] + } + }, + "autoload-dev": { + "psr-4": { + "ExistingTests\\": "tests/", + "MissingTests\\": "missing-tests/" + } + } +} +JSON, ['src', 'tests']); + $psr4DirectoryExistsRule = new Psr4DirectoryExistsRule(); + $violation = $psr4DirectoryExistsRule->evaluateProject($basePath, Architecture::define()); + + $this->assertInstanceOf(RuleViolation::class, $violation); + $this->assertTrue($psr4DirectoryExistsRule->fix($violation)); + + $this->assertSame(<<<'JSON' +{ + "autoload": { + "psr-4": { + "App\\": "src/", + "Mixed\\": ["src/"] + } + }, + "autoload-dev": { + "psr-4": { + "ExistingTests\\": "tests/" + } + } +} +JSON, file_get_contents($basePath . '/composer.json')); + $this->assertNotInstanceOf( + RuleViolation::class, + $psr4DirectoryExistsRule->evaluateProject($basePath, Architecture::define()) + ); + } + + public function testFixRemovesPsr4BlockWhenEveryMappingDirectoryIsMissing(): void + { + $basePath = $this->makeTempProject(<<<'JSON' +{ + "autoload": { + "psr-4": { + "View\\": "directory/not/exists" + } + } +} +JSON); + $psr4DirectoryExistsRule = new Psr4DirectoryExistsRule(); + $violation = $psr4DirectoryExistsRule->evaluateProject($basePath, Architecture::define()); + + $this->assertInstanceOf(RuleViolation::class, $violation); + $this->assertTrue($psr4DirectoryExistsRule->fix($violation)); + + $this->assertSame(<<<'JSON' +{ +} +JSON, file_get_contents($basePath . '/composer.json')); + $this->assertNotInstanceOf( + RuleViolation::class, + $psr4DirectoryExistsRule->evaluateProject($basePath, Architecture::define()) + ); + } + + public function testFixKeepsUnchangedEmptyPsr4Block(): void + { + $basePath = $this->makeTempProject(<<<'JSON' +{ + "autoload": { + "psr-4": { + } + }, + "autoload-dev": { + "psr-4": { + "View\\Tests\\": "directory/not/exists" + } + } +} +JSON); + $psr4DirectoryExistsRule = new Psr4DirectoryExistsRule(); + $violation = $psr4DirectoryExistsRule->evaluateProject($basePath, Architecture::define()); + + $this->assertInstanceOf(RuleViolation::class, $violation); + $this->assertTrue($psr4DirectoryExistsRule->fix($violation)); + + $this->assertSame(<<<'JSON' +{ + "autoload": { + "psr-4": { + } + } +} +JSON, file_get_contents($basePath . '/composer.json')); + $this->assertNotInstanceOf( + RuleViolation::class, + $psr4DirectoryExistsRule->evaluateProject($basePath, Architecture::define()) + ); + } + + public function testFixReturnsFalseWhenAllPsr4DirectoriesExist(): void + { + $basePath = $this->makeTempProject(<<<'JSON' +{ + "autoload": { + "psr-4": { + "App\\": "src/" + } + } +} +JSON, ['src']); + + $this->assertFalse((new Psr4DirectoryExistsRule())->fix(new RuleViolation( + message: 'PSR-4 source path(s) [src] declared in composer.json do not exist on disk', + file: $basePath . '/composer.json', + line: 1, + className: '', + ))); + } + /** * @param list $dirs */ diff --git a/tests/Rule/Fixer/JsonRecast/Composer/RemoveMissingPsr4PathVisitorTest.php b/tests/Rule/Fixer/JsonRecast/Composer/RemoveMissingPsr4PathVisitorTest.php new file mode 100644 index 0000000..6a973ac --- /dev/null +++ b/tests/Rule/Fixer/JsonRecast/Composer/RemoveMissingPsr4PathVisitorTest.php @@ -0,0 +1,41 @@ +assertNull($removeMissingPsr4PathVisitor->enterNode($arrayItemNode, new NodeJsonPath([ + NodeJsonPathSegment::objectKey('autoload'), + NodeJsonPathSegment::objectKey('psr-4'), + NodeJsonPathSegment::objectKey('App\\'), + ]))); + $this->assertNull($removeMissingPsr4PathVisitor->enterNode($arrayItemNode, new NodeJsonPath([ + NodeJsonPathSegment::objectKey('scripts'), + NodeJsonPathSegment::objectKey('psr-4'), + NodeJsonPathSegment::objectKey('App\\'), + NodeJsonPathSegment::arrayIndex(0), + ]))); + $this->assertNull($removeMissingPsr4PathVisitor->enterNode($arrayItemNode, new NodeJsonPath([ + NodeJsonPathSegment::objectKey('autoload'), + NodeJsonPathSegment::objectKey('classmap'), + NodeJsonPathSegment::objectKey('App\\'), + NodeJsonPathSegment::arrayIndex(0), + ]))); + } +} diff --git a/tests/Rule/Fixer/JsonRecast/JsonRecastFixerProcessorTest.php b/tests/Rule/Fixer/JsonRecast/JsonRecastFixerProcessorTest.php new file mode 100644 index 0000000..ed85f32 --- /dev/null +++ b/tests/Rule/Fixer/JsonRecast/JsonRecastFixerProcessorTest.php @@ -0,0 +1,57 @@ +temporaryJsonFile('{}'); + unlink($file); + + $this->assertFalse($this->process($file)); + } + + public function testProcessReturnsFalseWhenJsonCannotBeParsed(): void + { + $file = $this->temporaryJsonFile('{not json'); + + try { + $this->assertFalse($this->process($file)); + } finally { + unlink($file); + } + } + + private function process(string $file): bool + { + return (new JsonRecastFixerProcessor())->process( + $file, + new class extends NodeJsonVisitorAbstract { + }, + ); + } + + private function temporaryJsonFile(string $contents): string + { + $file = tempnam(sys_get_temp_dir(), 'structarmed-jsonrecast-fixer-'); + $this->assertIsString($file); + + file_put_contents($file, $contents); + + return $file; + } +}