diff --git a/docs/available-rules.md b/docs/available-rules.md index 88bddfc4..194fca92 100644 --- a/docs/available-rules.md +++ b/docs/available-rules.md @@ -78,10 +78,14 @@ Namespace: `Boundwize\StructArmed\Rule\Rules\Class_`. | `ClassNameMustBeStudlyCapsRule` | `new ClassNameMustBeStudlyCapsRule(layer: 'Source')` | Class names use StudlyCaps. | | `ClassNameMustHaveSuffixRule` | `new ClassNameMustHaveSuffixRule(layer: 'Controller', suffix: 'Controller')` | Classes in a layer have the required suffix. | | `ClassNameMustNotHavePrefixRule` | `new ClassNameMustNotHavePrefixRule(layer: 'Model', prefix: 'Model')` | Classes in a layer do not use a forbidden prefix. | +| `ExtendedClassMustBeAbstractOrInstantiatedRule` | `new ExtendedClassMustBeAbstractOrInstantiatedRule(layer: 'Source')` | Classes another scanned class extends are declared `abstract` unless they are also instantiated (`new X`, a `new self`/`new static`/`new parent` resolving to them, or a dynamic `new $class` whose class name resolves from a constant expression such as `X::class` or a class-name string). Type hints, `instanceof`, and `::class` keep working on an abstract class, so they do not count — but when the scanned code contains a dynamic instantiation whose target cannot be resolved (a factory's `new $class` fed by call sites, a `ReflectionClass::newInstance*()` call, `unserialize()`, or `eval()`), every class conservatively counts as instantiated and the rule stays silent — the target name may come from environment or configuration, so no class can be proven safe to abstract. Supports `--fix` by adding the `abstract` modifier. | | `MaxDependencyCountRule` | `new MaxDependencyCountRule(layer: 'Controller', maxCount: 5)` | Constructor dependency count stays below the configured limit. | | `MayNotImplementInterfaceRule` | `new MayNotImplementInterfaceRule(layer: 'Domain', interface: JsonSerializable::class)` | Classes in a layer do not implement a forbidden interface. | | `MustBeFinalRule` | `new MustBeFinalRule(layer: 'Domain', classNamePattern: '/Entity$/')` | Matching classes in a layer are declared `final`. Classes extended by another scanned class are skipped (making them `final` would break the child). Supports `--fix`. | +| `MustBeUsedInterfaceRule` | `new MustBeUsedInterfaceRule(layer: 'Source')` | Interfaces are implemented by a scanned class (directly or through inheritance), extended by another scanned interface, or referenced as a dependency (type hint, `instanceof`, `::class`, a class-name string, ...). Supports `--fix` by removing the unused interface (and deleting its file when only boilerplate remains). | | `MustBeInterfaceRule` | `new MustBeInterfaceRule(layer: 'Contract', classNamePattern: '/Interface$/')` | Matching declarations in a layer are interfaces. | +| `MustBeUsedAbstractClassRule` | `new MustBeUsedAbstractClassRule(layer: 'Source')` | Abstract classes are extended by a scanned class or referenced as a dependency (type hint, `instanceof`, `::class`, static call, a class-name string, ...). Supports `--fix` by removing the unused abstract class (and deleting its file when only boilerplate remains). | +| `MustBeUsedTraitRule` | `new MustBeUsedTraitRule(layer: 'Source')` | Traits are used by a scanned class, trait, or enum, or referenced as a dependency (`::class`, static call, a class-name string, ...). Supports `--fix` by removing the unused trait (and deleting its file when only boilerplate remains). | | `MustDeclareConstantVisibilityRule` | `new MustDeclareConstantVisibilityRule(layer: 'Source')` | Class constants declare `public`, `protected`, or `private`. Supports `--fix`. | | `MustDeclareMethodVisibilityRule` | `new MustDeclareMethodVisibilityRule(layer: 'Source')` | Methods declare `public`, `protected`, or `private`. Supports `--fix`. | | `MustDeclarePropertyVisibilityRule` | `new MustDeclarePropertyVisibilityRule(layer: 'Source')` | Properties declare `public`, `protected`, or `private`. Supports `--fix`. | @@ -91,7 +95,7 @@ Namespace: `Boundwize\StructArmed\Rule\Rules\Class_`. `classNamePattern` and `excludePattern` are regular expressions matched against the fully-qualified class name. -`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`. +`Psr4DirectoryExistsRule`, `Psr1PhpTagsRule`, `Psr1Utf8WithoutBomRule`, `ExtendedClassMustBeAbstractOrInstantiatedRule`, `MustBeFinalRule`, `MustBeUsedInterfaceRule`, `MustBeUsedAbstractClassRule`, `MustBeUsedTraitRule`, `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` or `abstract` class modifier, remove unused interfaces, abstract classes, and traits (deleting their file when only `declare`/`namespace`/`use` boilerplate remains), and add missing constant, method, or property visibility modifiers when you run `vendor/bin/structarmed analyse --fix`. ## Layer Rules diff --git a/docs/presets.md b/docs/presets.md index 5b638e5c..c9b0ce0a 100644 --- a/docs/presets.md +++ b/docs/presets.md @@ -25,6 +25,7 @@ StructArmed ships with presets for common PHP standards and architecture styles. | `Preset::PSR4()` | Verifies configured source paths exist in composer.json `autoload` or `autoload-dev` PSR-4 mappings | | `Preset::DDD()` | Layer isolation, entity/VO/repository/event/service conventions | | `Preset::MVC()` | Layer isolation, thin controllers, model/view/service rules | +| `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, and extended classes that are never instantiated must be abstract — 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 dynamic `new $class` resolvable from a constant expression) keeps an extended class concrete. All rules support `--fix`, removing the unused declaration or adding the `abstract` modifier | ## Initialize Presets @@ -35,6 +36,7 @@ vendor/bin/structarmed init --preset=psr12 vendor/bin/structarmed init --preset=psr15 vendor/bin/structarmed init --preset=mvc vendor/bin/structarmed init --preset=ddd +vendor/bin/structarmed init --preset=yagni vendor/bin/structarmed init --preset=all ``` @@ -49,6 +51,7 @@ return Architecture::define() Preset::PSR15(), Preset::MVC(), Preset::DDD(), + Preset::YAGNI(), ); ``` diff --git a/src/Analyser/Analyser.php b/src/Analyser/Analyser.php index 33747a25..8da418c4 100644 --- a/src/Analyser/Analyser.php +++ b/src/Analyser/Analyser.php @@ -24,6 +24,8 @@ use Boundwize\StructArmed\Rule\RuleInterface; use Boundwize\StructArmed\Rule\RuleViolation; use Boundwize\StructArmed\Rule\RuleViolationCollection; +use Boundwize\StructArmed\Rule\UsedInterfaceAwareRuleInterface; +use Boundwize\StructArmed\Rule\UsedTraitAwareRuleInterface; use Boundwize\StructArmed\Util\Path; use function array_fill_keys; @@ -83,6 +85,8 @@ public function analyse( $classRules = []; $layerAwareRules = []; $hasExtendedClassAwareRule = false; + $hasUsedInterfaceAwareRule = false; + $hasUsedTraitAwareRule = false; foreach ($rules as $key => $rule) { if (array_key_exists($key, $skippedRuleKeys)) { @@ -101,6 +105,14 @@ public function analyse( $hasExtendedClassAwareRule = true; } + if ($rule instanceof UsedInterfaceAwareRuleInterface) { + $hasUsedInterfaceAwareRule = true; + } + + if ($rule instanceof UsedTraitAwareRuleInterface) { + $hasUsedTraitAwareRule = true; + } + if (! $rule instanceof ProjectRuleInterface) { continue; } @@ -152,6 +164,18 @@ public function analyse( $this->markExtendedClasses($classNodes, $extractionResult); } + if ($hasUsedInterfaceAwareRule) { + $this->markImplementedInterfaces($classNodes, $extractionResult); + } + + if ($hasExtendedClassAwareRule || $hasUsedInterfaceAwareRule || $hasUsedTraitAwareRule) { + $this->markReferencedClassLikes($classNodes, $extractionResult); + } + + if ($hasExtendedClassAwareRule) { + $this->markInstantiatedClasses($classNodes, $extractionResult); + } + if ($withFileAnalysis) { $fileAnalysisProvider = new FileAnalysisProvider( analyses: $extractionResult->fileAnalyses, @@ -688,6 +712,159 @@ private function markExtendedClasses(array $classNodes, ExtractionResult $extrac } } + /** + * Flag every interface that a scanned class implements (directly or through + * inheritance) or another scanned interface extends, using the recursive + * parent chain resolved by {@see withRecursiveParents()}. + * + * @param list $classNodes + */ + private function markImplementedInterfaces(array $classNodes, ExtractionResult $extractionResult): void + { + $implemented = []; + + foreach ($classNodes as $classNode) { + foreach ($classNode->parentInterfaces as $parentInterface) { + $implemented[strtolower($parentInterface)] = true; + } + } + + // Anonymous classes (`new class implements Foo {}`) have no ClassNode + // of their own, so the interfaces they implement are tracked separately. + foreach ($extractionResult->anonymousClassNodes as $anonymousClassNode) { + foreach ($anonymousClassNode->implements as $interface) { + $implemented[strtolower($interface)] = true; + } + } + + foreach ($classNodes as $classNode) { + // Only interfaces appear in parentInterfaces; classes, traits, and + // enums never do, so they are left with the default (not implemented). + if (isset($implemented[strtolower($classNode->className)])) { + $classNode->setImplemented(true); + } + } + } + + /** + * Flag every class-like that another scanned class-like (class, trait, or + * enum) uses — as a trait, or by referencing it as a dependency: a type + * hint, an instanceof check, a ::class constant, a static call, and so on. + * Self-references are ignored: a class-like cannot keep itself alive. + * Usage is a direct declaration, so no recursive chain is needed: a + * class-like used only by another unused one stays flagged as used until + * its user is removed, at which point the next run reports it. + * + * @param list $classNodes + */ + private function markReferencedClassLikes(array $classNodes, ExtractionResult $extractionResult): void + { + $used = []; + + foreach ($classNodes as $classNode) { + foreach ($classNode->traits as $trait) { + $used[strtolower($trait)] = true; + } + + // A node's own inheritance-clause names (and the imports that + // exist for them) are structural relations, not value references. + // Excluding them keeps "referenced" meaningful for the unresolved + // dynamic instantiation check below: a class extended by a child + // is not thereby a possible `new $class` target. The usage-aware + // deletion rules are unaffected — each combines this flag with its + // structural extended/implemented/trait marking. + $excludedKeys = [strtolower($classNode->className) => true]; + + if ($classNode->extends !== null) { + $excludedKeys[strtolower($classNode->extends)] = true; + } + + foreach ([$classNode->implements, $classNode->interfaceExtends, $classNode->traits] as $clauseNames) { + foreach ($clauseNames as $clauseName) { + $excludedKeys[strtolower($clauseName)] = true; + } + } + + foreach ($classNode->dependencies as $dependency) { + $dependencyKey = strtolower($dependency); + + if (! isset($excludedKeys[$dependencyKey])) { + $used[$dependencyKey] = true; + } + } + } + + // Anonymous classes (`new class { use Foo; }`) have no ClassNode of + // their own, so the traits they use are tracked separately. + foreach ($extractionResult->anonymousClassNodes as $anonymousClassNode) { + foreach ($anonymousClassNode->traits as $trait) { + $used[strtolower($trait)] = true; + } + } + + // References made outside any named class-like scope — procedural + // functions, top-level statements, top-level anonymous class bodies — + // have no ClassNode either, so they are tracked per file. + foreach ($extractionResult->fileReferences as $references) { + foreach ($references as $reference) { + $used[strtolower($reference)] = true; + } + } + + // An instantiation is also a reference; the sentinel is not a class + // name and marks nothing here. + foreach ($extractionResult->fileInstantiations as $instantiations) { + foreach ($instantiations as $instantiation) { + if ($instantiation !== ClassCollector::UNRESOLVED_INSTANTIATION) { + $used[strtolower($instantiation)] = true; + } + } + } + + foreach ($classNodes as $classNode) { + if (isset($used[strtolower($classNode->className)])) { + $classNode->setReferenced(true); + } + } + } + + /** + * Flag every concrete class that another scanned scope instantiates — + * `new X` (with self/static/parent already resolved), a dynamic + * `new $class` resolved from constant class-name values, and so on. + * No self-exclusion here: a class instantiating itself cannot become + * abstract either. + * + * A dynamic instantiation whose target could not be resolved statically — + * `new $class` on a function parameter, a ReflectionClass construction, + * unserialize(), eval() — may target any class: the name can come from + * the environment, configuration, or a payload, entirely outside the + * scanned code. No class can then be proven safe to abstract, so every + * class-like conservatively counts as instantiated. Resolvable + * construction keeps precise detection; unresolvable construction + * silences the concreteness fix. + * + * @param list $classNodes + */ + private function markInstantiatedClasses(array $classNodes, ExtractionResult $extractionResult): void + { + $instantiated = []; + + foreach ($extractionResult->fileInstantiations as $instantiations) { + foreach ($instantiations as $instantiation) { + $instantiated[strtolower($instantiation)] = true; + } + } + + $hasUnresolvedInstantiation = isset($instantiated[ClassCollector::UNRESOLVED_INSTANTIATION]); + + foreach ($classNodes as $classNode) { + if ($hasUnresolvedInstantiation || isset($instantiated[strtolower($classNode->className)])) { + $classNode->setInstantiated(true); + } + } + } + /** * @param list $classNodes * @return list @@ -852,6 +1029,8 @@ private function collectClassNodes( $classNodes = []; $fileAnalyses = []; $anonymousClassNodes = []; + $fileReferences = []; + $fileInstantiations = []; $filesToParse = []; foreach ($files as $file) { @@ -874,6 +1053,14 @@ private function collectClassNodes( $anonymousClassNodes[] = $cachedAnonymousClassNode; } + if ($cachedResult['fileReferences'] !== []) { + $fileReferences[$file] = $cachedResult['fileReferences']; + } + + if ($cachedResult['fileInstantiations'] !== []) { + $fileInstantiations[$file] = $cachedResult['fileInstantiations']; + } + $fileAnalyses[$file] = $cachedResult['fileAnalysis']; continue; @@ -896,6 +1083,14 @@ private function collectClassNodes( foreach ($cachedResult['anonymousClassNodes'] as $cachedAnonymousClassNode) { $anonymousClassNodes[] = $cachedAnonymousClassNode; } + + if ($cachedResult['fileReferences'] !== []) { + $fileReferences[$file] = $cachedResult['fileReferences']; + } + + if ($cachedResult['fileInstantiations'] !== []) { + $fileInstantiations[$file] = $cachedResult['fileInstantiations']; + } } $progressHandler?->start(count($filesToParse)); @@ -903,7 +1098,13 @@ private function collectClassNodes( if ($filesToParse === []) { $progressHandler?->finish(); - return new ExtractionResult($classNodes, $fileAnalyses, $anonymousClassNodes); + return new ExtractionResult( + $classNodes, + $fileAnalyses, + $anonymousClassNodes, + $fileReferences, + $fileInstantiations, + ); } $options = $analyserOptions ?? AnalyserOptions::parallel(); @@ -946,6 +1147,14 @@ private function collectClassNodes( $fileAnalyses[$file] = $fileAnalysis; } + foreach ($parsedResult->fileReferences as $file => $parsedFileReferences) { + $fileReferences[$file] = $parsedFileReferences; + } + + foreach ($parsedResult->fileInstantiations as $file => $parsedFileInstantiations) { + $fileInstantiations[$file] = $parsedFileInstantiations; + } + foreach ($classNodesByFile as $fileToParse => $fileClassNodes) { $this->analysisResultCache?->storeClassNodes( $fileToParse, @@ -953,12 +1162,20 @@ private function collectClassNodes( $fileClassNodes, $fileAnalyses[$fileToParse] ?? null, $anonymousClassNodesByFile[$fileToParse] ?? [], + $fileReferences[$fileToParse] ?? [], + $fileInstantiations[$fileToParse] ?? [], ); } $progressHandler?->finish(); - return new ExtractionResult($classNodes, $fileAnalyses, $anonymousClassNodes); + return new ExtractionResult( + $classNodes, + $fileAnalyses, + $anonymousClassNodes, + $fileReferences, + $fileInstantiations, + ); } /** diff --git a/src/Analyser/AnonymousClassNode.php b/src/Analyser/AnonymousClassNode.php index dcdac5af..c1f85c57 100644 --- a/src/Analyser/AnonymousClassNode.php +++ b/src/Analyser/AnonymousClassNode.php @@ -7,20 +7,24 @@ /** * An anonymous class declaration (`new class ... {}`). Anonymous classes never * become ClassNodes — they cannot be referenced by name and no rule targets - * them directly — but the class they extend is still extended within the - * scanned paths, which extended-class-aware rules must take into account. + * them directly — but the class they extend, the interfaces they implement, + * and the traits they use are still used within the scanned paths, which + * usage-aware rules must take into account. * * The usage example is on MustBeFinalRule, which must skip if target class is extended by an anonymous class. - * - * Note: Other properties like anonymous class's traits, implements, etc may come - * later if needed for future needed rules. */ final readonly class AnonymousClassNode { + /** + * @param string[] $implements Interface names this anonymous class implements + * @param string[] $traits Trait names this anonymous class uses + */ public function __construct( public string $file, public int $line, public ?string $extends, + public array $implements = [], + public array $traits = [], ) { } } diff --git a/src/Analyser/ClassCollector.php b/src/Analyser/ClassCollector.php index 8ccd532a..be9ed4c3 100644 --- a/src/Analyser/ClassCollector.php +++ b/src/Analyser/ClassCollector.php @@ -6,11 +6,17 @@ use Boundwize\StructArmed\LayerResolver\LayerResolverInterface; use Boundwize\StructArmed\Util\PhpParser\VisibilityFlagChecker; +use PhpParser\ConstExprEvaluationException; +use PhpParser\ConstExprEvaluator; use PhpParser\Node; +use PhpParser\Node\Expr; +use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\BinaryOp\BooleanAnd; use PhpParser\Node\Expr\BinaryOp\BooleanOr; +use PhpParser\Node\Expr\BinaryOp\Concat; use PhpParser\Node\Expr\BinaryOp\LogicalAnd; use PhpParser\Node\Expr\BinaryOp\LogicalOr; +use PhpParser\Node\Expr\ClassConstFetch; use PhpParser\Node\Expr\Empty_; use PhpParser\Node\Expr\Eval_; use PhpParser\Node\Expr\Exit_; @@ -18,6 +24,9 @@ use PhpParser\Node\Expr\Include_; use PhpParser\Node\Expr\Isset_; use PhpParser\Node\Expr\List_; +use PhpParser\Node\Expr\MethodCall; +use PhpParser\Node\Expr\New_; +use PhpParser\Node\Expr\NullsafeMethodCall; use PhpParser\Node\Expr\Print_; use PhpParser\Node\Expr\Ternary; use PhpParser\Node\Expr\Variable; @@ -26,6 +35,7 @@ use PhpParser\Node\Name; use PhpParser\Node\Name\FullyQualified; use PhpParser\Node\Param; +use PhpParser\Node\Scalar\String_; use PhpParser\Node\Stmt\Case_; use PhpParser\Node\Stmt\Catch_; use PhpParser\Node\Stmt\Class_; @@ -51,12 +61,15 @@ use PhpParser\Node\Stmt\While_; use PhpParser\NodeVisitorAbstract; +use function array_keys; use function array_pop; use function array_unique; use function array_values; use function count; +use function end; use function in_array; use function is_string; +use function preg_match; use function spl_object_id; use function strtolower; @@ -80,12 +93,84 @@ final class ClassCollector extends NodeVisitorAbstract 'null' => true, ]; + /** + * A string value shaped like a (possibly namespaced) class name, e.g. + * 'App\Contract' or 'stdClass'. Such values can reach `new $class` or + * `instanceof $class` at runtime, so they count as references. + */ + private const CLASS_LIKE_STRING_PATTERN = + '/^[A-Za-z_\x80-\xff][A-Za-z0-9_\x80-\xff]*+(?:\\\\[A-Za-z_\x80-\xff][A-Za-z0-9_\x80-\xff]*+)*+$/'; + + /** + * Sentinel recorded in the file instantiations when a dynamic `new` has no + * statically known class-name candidates (e.g. `new $class` on a function + * parameter). Can never collide with a real class name. The analyser then + * treats referenced class-likes as possibly instantiated, so a factory + * target passed in as `X::class` is not fixed into an abstract class. + */ + public const UNRESOLVED_INSTANTIATION = '*'; + + /** + * Method names of the ReflectionClass object-construction APIs. Calling + * any of them instantiates a class the collector cannot determine, so the + * call records {@see self::UNRESOLVED_INSTANTIATION}. Matching by method + * name alone over-approximates (any receiver type matches) — the safe + * direction for a fixer that would otherwise make a class abstract. + */ + private const REFLECTION_CONSTRUCTION_METHODS = [ + 'newinstance' => true, + 'newinstanceargs' => true, + 'newinstancewithoutconstructor' => true, + 'newlazyghost' => true, + 'newlazyproxy' => true, + ]; + /** @var list */ private array $nodes = []; /** @var list */ private array $anonymousClassNodes = []; + /** @var array> */ + private array $fileReferences = []; + + /** @var list */ + private array $currentFileReferences = []; + + /** @var array> */ + private array $fileInstantiations = []; + + /** @var list */ + private array $currentFileInstantiations = []; + + /** + * Constant class-name strings each variable may hold (a union across all + * of its resolvable assignments), so `new $variable` instantiations can be + * resolved to every possible target. + * + * @var array> + */ + private array $variableClassNames = []; + + /** + * Variables that received at least one statically unresolvable assignment, + * so `new $variable` cannot claim to know every possible target. + * + * @var array + */ + private array $variableUnknownAssignments = []; + + private readonly ConstExprEvaluator $constExprEvaluator; + + /** + * Stack of named class-likes currently being entered, so `new self`, + * `new static`, and `new parent` instantiations can be resolved to the + * class names they target. + * + * @var list + */ + private array $activeClassLikeScopes = []; + private string $currentFile = ''; /** @var list */ @@ -115,19 +200,39 @@ final class ClassCollector extends NodeVisitorAbstract public function __construct( private readonly LayerResolverInterface $layerResolver ) { + $this->constExprEvaluator = new ConstExprEvaluator(function (Expr $expr): string { + if ( + $expr instanceof ClassConstFetch + && $expr->name instanceof Identifier + && $expr->name->toLowerString() === 'class' + && $expr->class instanceof Name + ) { + $className = $this->resolveClassLikeName($expr->class); + + if ($className !== null) { + return $className; + } + } + + throw new ConstExprEvaluationException('Expression is not a resolvable class name.'); + }); } public function setCurrentFile(string $file): void { - $this->currentFile = $file; - $this->currentNamespaceUses = []; - $this->fileClassLikes = []; - $this->fileFunctions = []; - $this->classLikeAnalysis = []; - $this->classLikeMethods = []; - $this->activeClassLikeAnalyses = []; - $this->activeMethodIds = []; - $this->methodClassLikeAnalyses = []; + $this->currentFile = $file; + $this->currentFileReferences = []; + $this->currentFileInstantiations = []; + $this->variableClassNames = []; + $this->variableUnknownAssignments = []; + $this->currentNamespaceUses = []; + $this->fileClassLikes = []; + $this->fileFunctions = []; + $this->classLikeAnalysis = []; + $this->classLikeMethods = []; + $this->activeClassLikeAnalyses = []; + $this->activeMethodIds = []; + $this->methodClassLikeAnalyses = []; } /** @return list */ @@ -142,6 +247,30 @@ public function getAnonymousClassNodes(): array return $this->anonymousClassNodes; } + /** + * References to class-likes made outside any named class-like scope, per + * file — procedural functions, top-level statements, and top-level + * anonymous class bodies. + * + * @return array> + */ + public function getFileReferences(): array + { + return $this->fileReferences; + } + + /** + * Class-like instantiations (`new X`, with self/static/parent resolved to + * the class names they target), per file. `new` on an abstract class is + * fatal, so these are what an extended class needs to stay concrete. + * + * @return array> + */ + public function getFileInstantiations(): array + { + return $this->fileInstantiations; + } + public function enterNode(Node $node): null { if ($node instanceof Namespace_) { @@ -203,18 +332,43 @@ public function leaveNode(Node $node): null return null; } + // Both run on leave, once the NameResolver has resolved the nested + // name nodes (e.g. Base::class inside the assigned expression). + // + // A variable assigned a constant class-name value may feed a later + // `new $variable`. + if ($node instanceof Assign) { + $this->trackVariableClassName($node); + + return null; + } + + // Instantiations are tracked separately from plain references: + // `new` on an abstract class is fatal, so instantiation is the one + // usage that requires an extended class to stay concrete — type + // hints, instanceof checks, and ::class constants all keep working + // once a class becomes abstract. + if ($node instanceof New_) { + $this->collectInstantiation($node); + + return null; + } + if (! $node instanceof ClassLike) { return null; } if (! $node->name instanceof Identifier) { // Anonymous classes never become ClassNodes, but the class they - // extend is still extended within the scanned paths. + // extend, the interfaces they implement, and the traits they use + // are still used within the scanned paths. if ($node instanceof Class_) { $this->anonymousClassNodes[] = new AnonymousClassNode( - file: $this->currentFile, - line: $node->getStartLine(), - extends: $node->extends instanceof Name ? $node->extends->toString() : null, + file: $this->currentFile, + line: $node->getStartLine(), + extends: $node->extends instanceof Name ? $node->extends->toString() : null, + implements: $this->collectImplements($node), + traits: $this->collectTraits($node), ); } @@ -223,6 +377,7 @@ public function leaveNode(Node $node): null $this->fileClassLikes[] = $node; array_pop($this->activeClassLikeAnalyses); + array_pop($this->activeClassLikeScopes); return null; } @@ -234,10 +389,23 @@ public function afterTraverse(array $nodes): null $this->collectClassLike($fileClassLike); } + if ($this->currentFileReferences !== []) { + $this->fileReferences[$this->currentFile] = array_values(array_unique($this->currentFileReferences)); + $this->currentFileReferences = []; + } + + if ($this->currentFileInstantiations !== []) { + $this->fileInstantiations[$this->currentFile] = array_values( + array_unique($this->currentFileInstantiations) + ); + $this->currentFileInstantiations = []; + } + $this->fileClassLikes = []; $this->classLikeAnalysis = []; $this->classLikeMethods = []; $this->activeClassLikeAnalyses = []; + $this->activeClassLikeScopes = []; $this->activeMethodIds = []; $this->methodClassLikeAnalyses = []; @@ -254,6 +422,12 @@ private function startClassLikeAnalysis(ClassLike $classLike): void $this->classLikeAnalysis[$classLikeId] = $classLikeAnalysis; $this->activeClassLikeAnalyses[] = $classLikeAnalysis; + $this->activeClassLikeScopes[] = [ + 'name' => $this->resolveClassName($classLike), + 'extends' => $classLike instanceof Class_ && $classLike->extends instanceof Name + ? $classLike->extends->toString() + : null, + ]; foreach ($classLike->getMethods() as $classMethod) { $methodId = spl_object_id($classMethod); @@ -291,7 +465,59 @@ private function finishMethodAnalysis(ClassMethod $classMethod): void private function collectNodeAnalysis(Node $node): void { + // A class-name-shaped string literal may feed `new $class`, + // `$obj instanceof $class`, class_exists(), container ids, and so on. + // Whether it appears inside a class-like or in procedural code, treat + // it as a file-level reference so the named class-like stays alive. + if ($node instanceof String_) { + if ( + ! isset(self::KEYWORD_CONSTANTS[strtolower($node->value)]) + && preg_match(self::CLASS_LIKE_STRING_PATTERN, $node->value) === 1 + ) { + $this->currentFileReferences[] = $node->value; + } + + return; + } + + // Reflection construction APIs and unserialize() instantiate a class + // the collector cannot pin down statically, exactly like an + // unresolvable `new $class` — the unresolved marker keeps classes + // concrete. + if ( + ($node instanceof MethodCall || $node instanceof NullsafeMethodCall) + && $node->name instanceof Identifier + && isset(self::REFLECTION_CONSTRUCTION_METHODS[$node->name->toLowerString()]) + ) { + $this->currentFileInstantiations[] = self::UNRESOLVED_INSTANTIATION; + } + + if ( + $node instanceof FuncCall + && $node->name instanceof Name + && $node->name->toLowerString() === 'unserialize' + ) { + $this->currentFileInstantiations[] = self::UNRESOLVED_INSTANTIATION; + } + + // eval() can construct anything; it is additionally recorded as a + // language construct for in-class usage rules further down. + if ($node instanceof Eval_) { + $this->currentFileInstantiations[] = self::UNRESOLVED_INSTANTIATION; + } + if ($this->activeClassLikeAnalyses === []) { + // Outside any named class-like scope — procedural functions, + // top-level statements, top-level anonymous class bodies — a + // class-like reference still keeps the referenced class-like alive. + if ($node instanceof FullyQualified) { + $name = $node->toString(); + + if (! isset(self::KEYWORD_CONSTANTS[strtolower($name)])) { + $this->currentFileReferences[] = $name; + } + } + return; } @@ -398,6 +624,135 @@ private function collectNodeAnalysis(Node $node): void } } + private function collectInstantiation(New_ $new): void + { + $class = $new->class; + + if ($class instanceof Name) { + $className = $this->resolveClassLikeName($class); + + if ($className !== null) { + $this->currentFileInstantiations[] = $className; + } + + return; + } + + // Anonymous classes (`new class {}`) are tracked as + // AnonymousClassNodes; dynamic instantiations may still resolve below. + if (! $class instanceof Expr) { + return; + } + + // `new $class` instantiates any of the constant class-name values the + // variable may hold — conditional reassignments make every recorded + // possibility reachable at runtime. + if ($class instanceof Variable && is_string($class->name)) { + $possibleClassNames = array_keys($this->variableClassNames[$class->name] ?? []); + + foreach ($possibleClassNames as $possibleClassName) { + $this->currentFileInstantiations[] = $possibleClassName; + } + + // A variable with no candidates (e.g. a function parameter) or + // with an unresolvable assignment may target any class. + if ($possibleClassNames === [] || isset($this->variableUnknownAssignments[$class->name])) { + $this->currentFileInstantiations[] = self::UNRESOLVED_INSTANTIATION; + } + + return; + } + + // `new (X::class)` / `new ('App\X')` class expressions. + $className = $this->resolveClassNameExpr($class); + + if ($className !== null) { + $this->currentFileInstantiations[] = $className; + + return; + } + + $this->currentFileInstantiations[] = self::UNRESOLVED_INSTANTIATION; + } + + /** + * Resolve a class-like name node to a fully qualified name: either it is + * already fully qualified, or it is a self/static/parent keyword resolved + * against the enclosing class-like scope. Returns null when there is no + * scope to resolve against. + */ + private function resolveClassLikeName(Name $name): ?string + { + if ($name instanceof FullyQualified) { + return $name->toString(); + } + + // After name resolution only self, static, and parent survive as + // plain names. + $scope = end($this->activeClassLikeScopes); + + if ($scope === false) { + return null; + } + + $relativeName = $name->toLowerString(); + + if ($relativeName === 'self' || $relativeName === 'static') { + return $scope['name']; + } + + return $relativeName === 'parent' ? $scope['extends'] : null; + } + + /** + * Track `$variable = ` assignments so a + * later `new $variable` can be resolved. Every resolvable value is kept as + * a possibility — a conditional reassignment does not replace the earlier + * one, since either branch may run. Over-approximation is the safe + * direction: a recorded instantiation only keeps a class concrete or + * alive, while a missed one could let the fixer break runtime code. + */ + private function trackVariableClassName(Assign $assign): void + { + if (! $assign->var instanceof Variable || ! is_string($assign->var->name)) { + return; + } + + $className = $this->resolveClassNameExpr($assign->expr); + + if ($className === null) { + $this->variableUnknownAssignments[$assign->var->name] = true; + + return; + } + + $this->variableClassNames[$assign->var->name][$className] = true; + } + + /** + * Evaluate a constant expression to a class-name string: 'App\X' literals, + * X::class (including self/static/parent::class), and concatenations of + * those. Anything depending on runtime values resolves to null. + */ + private function resolveClassNameExpr(Expr $expr): ?string + { + if (! $expr instanceof String_ && ! $expr instanceof ClassConstFetch && ! $expr instanceof Concat) { + return null; + } + + try { + $value = $this->constExprEvaluator->evaluateSilently($expr); + } catch (ConstExprEvaluationException) { + return null; + } + + if (! is_string($value) || preg_match(self::CLASS_LIKE_STRING_PATTERN, $value) !== 1) { + return null; + } + + return $value; + } + private function addDependency(string $dependency): void { foreach ($this->activeClassLikeAnalyses as $activeClassLikeAnalysis) { diff --git a/src/Analyser/ClassNode.php b/src/Analyser/ClassNode.php index f4b88c74..68d7e081 100644 --- a/src/Analyser/ClassNode.php +++ b/src/Analyser/ClassNode.php @@ -60,6 +60,9 @@ public function __construct( public array $parentClasses = [], public array $parentInterfaces = [], public bool $isExtended = false, + public bool $isImplemented = false, + public bool $isReferenced = false, + public bool $isInstantiated = false, ) { $this->layers = $layers ?: array_filter([$this->layer]); } @@ -83,6 +86,48 @@ public function setExtended(bool $isExtended): void $this->isExtended = $isExtended; } + /** + * Whether another scanned class implements this interface (directly or + * through inheritance) or another scanned interface extends it. Computed by + * the analyser for rules implementing UsedInterfaceAwareRuleInterface; + * false otherwise. + */ + public function setImplemented(bool $isImplemented): void + { + $this->isImplemented = $isImplemented; + } + + /** + * Whether another scanned class-like references this class-like — as a + * trait it uses, or as a dependency (type hint, instanceof, ::class, + * static call, ...). Computed by the analyser when a usage-aware rule is + * active; false otherwise. + */ + public function setReferenced(bool $isReferenced): void + { + $this->isReferenced = $isReferenced; + } + + /** + * Whether another scanned scope instantiates this class — `new X`, or a + * `new self`/`new static`/`new parent` resolving to it. Instantiation is + * the one usage that requires a class to stay concrete. Computed by the + * analyser when a usage-aware rule is active; false otherwise. + * + * Only a concrete named class can be an instantiation target — `new` on + * an abstract class, interface, trait, or enum is fatal — so marking any + * other class-like as instantiated is ignored. (Anonymous classes never + * become ClassNodes in the first place.) + */ + public function setInstantiated(bool $isInstantiated): void + { + if ($isInstantiated && (! $this->isClass() || $this->isAbstract)) { + return; + } + + $this->isInstantiated = $isInstantiated; + } + public function shortName(): string { $parts = explode('\\', $this->className); diff --git a/src/Analyser/ClassNodeExtractor.php b/src/Analyser/ClassNodeExtractor.php index ca5c08fd..721d30c3 100644 --- a/src/Analyser/ClassNodeExtractor.php +++ b/src/Analyser/ClassNodeExtractor.php @@ -57,6 +57,8 @@ public function extract( $classCollector->getNodes(), $fileAnalyses, $classCollector->getAnonymousClassNodes(), + $classCollector->getFileReferences(), + $classCollector->getFileInstantiations(), ); } } diff --git a/src/Analyser/ExtractionResult.php b/src/Analyser/ExtractionResult.php index fb51c715..252641e6 100644 --- a/src/Analyser/ExtractionResult.php +++ b/src/Analyser/ExtractionResult.php @@ -10,11 +10,17 @@ * @param list $classNodes * @param array $fileAnalyses * @param list $anonymousClassNodes + * @param array> $fileReferences Class-like references made outside any + * named class-like scope, per file + * @param array> $fileInstantiations Class-like instantiations (`new X`, + * with self/static/parent resolved), per file */ public function __construct( public array $classNodes, public array $fileAnalyses, public array $anonymousClassNodes = [], + public array $fileReferences = [], + public array $fileInstantiations = [], ) { } } diff --git a/src/Analyser/Parallel/ClassNodeWorker.php b/src/Analyser/Parallel/ClassNodeWorker.php index c4231f4b..d2c75c8f 100644 --- a/src/Analyser/Parallel/ClassNodeWorker.php +++ b/src/Analyser/Parallel/ClassNodeWorker.php @@ -63,6 +63,8 @@ public static function run(string $inputFile, string $outputFile, mixed $outputS 'nodes' => $result->classNodes, 'fileAnalyses' => $result->fileAnalyses, 'anonymousClassNodes' => $result->anonymousClassNodes, + 'fileReferences' => $result->fileReferences, + 'fileInstantiations' => $result->fileInstantiations, 'error' => null, ])); @@ -72,6 +74,8 @@ public static function run(string $inputFile, string $outputFile, mixed $outputS 'nodes' => [], 'fileAnalyses' => [], 'anonymousClassNodes' => [], + 'fileReferences' => [], + 'fileInstantiations' => [], 'error' => sprintf('%s: %s', $throwable::class, $throwable->getMessage()), ])); diff --git a/src/Analyser/Parallel/ParallelClassNodeExtractor.php b/src/Analyser/Parallel/ParallelClassNodeExtractor.php index dcba6a7a..26d97fe6 100644 --- a/src/Analyser/Parallel/ParallelClassNodeExtractor.php +++ b/src/Analyser/Parallel/ParallelClassNodeExtractor.php @@ -137,6 +137,8 @@ public function extract( $nodes = []; $fileAnalyses = []; $anonymousClassNodes = []; + $fileReferences = []; + $fileInstantiations = []; $failure = null; while ($pending !== []) { @@ -244,6 +246,66 @@ public function extract( $anonymousClassNodes[] = $workerAnonClassNode; } + + $workerFileReferences = $result['fileReferences'] ?? []; + + if (! is_array($workerFileReferences)) { + throw new RuntimeException( + 'Parallel analysis worker returned invalid file references.' + ); + } + + foreach ($workerFileReferences as $file => $references) { + if (! is_string($file) || ! is_array($references)) { + throw new RuntimeException( + 'Parallel analysis worker returned invalid file references.' + ); + } + + $validReferences = []; + + foreach ($references as $reference) { + if (! is_string($reference)) { + throw new RuntimeException( + 'Parallel analysis worker returned invalid file references.' + ); + } + + $validReferences[] = $reference; + } + + $fileReferences[$file] = $validReferences; + } + + $workerFileInstantiations = $result['fileInstantiations'] ?? []; + + if (! is_array($workerFileInstantiations)) { + throw new RuntimeException( + 'Parallel analysis worker returned invalid file instantiations.' + ); + } + + foreach ($workerFileInstantiations as $file => $instantiations) { + if (! is_string($file) || ! is_array($instantiations)) { + throw new RuntimeException( + 'Parallel analysis worker returned invalid file instantiations.' + ); + } + + $validInstantiations = []; + + foreach ($instantiations as $instantiation) { + if (! is_string($instantiation)) { + throw new RuntimeException( + 'Parallel analysis worker returned invalid file instantiations.' + ); + } + + $validInstantiations[] = $instantiation; + } + + $fileInstantiations[$file] = $validInstantiations; + } } catch (RuntimeException $runtimeException) { $failure ??= $runtimeException->getMessage(); } finally { @@ -267,7 +329,7 @@ public function extract( throw new RuntimeException($failure); } - return new ExtractionResult($nodes, $fileAnalyses, $anonymousClassNodes); + return new ExtractionResult($nodes, $fileAnalyses, $anonymousClassNodes, $fileReferences, $fileInstantiations); } /** diff --git a/src/Cache/AnalysisResultCache.php b/src/Cache/AnalysisResultCache.php index d6eb2016..3f133ec6 100644 --- a/src/Cache/AnalysisResultCache.php +++ b/src/Cache/AnalysisResultCache.php @@ -186,7 +186,12 @@ private function ensureCacheInitialised(): void } /** - * @return array{classNodes: list, anonymousClassNodes: list}|null + * @return array{ + * classNodes: list, + * anonymousClassNodes: list, + * fileReferences: list, + * fileInstantiations: list + * }|null */ public function loadClassNodes(string $file, string $namespace): ?array { @@ -198,14 +203,23 @@ public function loadClassNodes(string $file, string $namespace): ?array $classNodes = $this->classNodesFromPayload($payload); $anonymousClassNodes = $this->anonymousClassNodesFromPayload($payload); + $fileReferences = $this->fileReferencesFromPayload($payload); + $fileInstantiations = $this->fileInstantiationsFromPayload($payload); - if ($classNodes === null || $anonymousClassNodes === null) { + if ( + $classNodes === null + || $anonymousClassNodes === null + || $fileReferences === null + || $fileInstantiations === null + ) { return null; } return [ 'classNodes' => $classNodes, 'anonymousClassNodes' => $anonymousClassNodes, + 'fileReferences' => $fileReferences, + 'fileInstantiations' => $fileInstantiations, ]; } @@ -213,6 +227,8 @@ public function loadClassNodes(string $file, string $namespace): ?array * @return array{ * classNodes: list, * anonymousClassNodes: list, + * fileReferences: list, + * fileInstantiations: list, * fileAnalysis: FileAnalysis * }|null */ @@ -226,17 +242,27 @@ public function loadClassNodesWithFileAnalysis(string $file, string $namespace): $classNodes = $this->classNodesFromPayload($payload); $anonymousClassNodes = $this->anonymousClassNodesFromPayload($payload); + $fileReferences = $this->fileReferencesFromPayload($payload); + $fileInstantiations = $this->fileInstantiationsFromPayload($payload); $fileAnalysis = is_array($payload['fileAnalysis'] ?? null) ? $this->fileAnalysisFromArray($payload['fileAnalysis']) : null; - if ($classNodes === null || $anonymousClassNodes === null || ! $fileAnalysis instanceof FileAnalysis) { + if ( + $classNodes === null + || $anonymousClassNodes === null + || $fileReferences === null + || $fileInstantiations === null + || ! $fileAnalysis instanceof FileAnalysis + ) { return null; } return [ 'classNodes' => $classNodes, 'anonymousClassNodes' => $anonymousClassNodes, + 'fileReferences' => $fileReferences, + 'fileInstantiations' => $fileInstantiations, 'fileAnalysis' => $fileAnalysis, ]; } @@ -285,6 +311,9 @@ private function classNodesFromPayload(array $payload): ?array /** * @param list $classNodes * @param list $anonymousClassNodes + * @param list $fileReferences Class-like references made outside any + * named class-like scope in this file + * @param list $fileInstantiations Class-like instantiations in this file */ public function storeClassNodes( string $file, @@ -292,6 +321,8 @@ public function storeClassNodes( array $classNodes, ?FileAnalysis $fileAnalysis = null, array $anonymousClassNodes = [], + array $fileReferences = [], + array $fileInstantiations = [], ): void { $this->ensureCacheInitialised(); @@ -299,6 +330,8 @@ public function storeClassNodes( 'metadata' => $this->fileMetadata($file, $namespace), 'nodes' => array_map($this->classNodeToArray(...), $classNodes), 'anonymousClassNodes' => array_map($this->anonymousClassNodeToArray(...), $anonymousClassNodes), + 'fileReferences' => $fileReferences, + 'fileInstantiations' => $fileInstantiations, ]; if ($fileAnalysis instanceof FileAnalysis) { @@ -382,15 +415,39 @@ className: $className, ); } + /** + * @param array $payload + * @return list|null + */ + private function fileReferencesFromPayload(array $payload): ?array + { + $fileReferences = $payload['fileReferences'] ?? []; + + return $this->isStringArray($fileReferences) ? array_values($fileReferences) : null; + } + + /** + * @param array $payload + * @return list|null + */ + private function fileInstantiationsFromPayload(array $payload): ?array + { + $fileInstantiations = $payload['fileInstantiations'] ?? []; + + return $this->isStringArray($fileInstantiations) ? array_values($fileInstantiations) : null; + } + /** * @return array */ private function anonymousClassNodeToArray(AnonymousClassNode $anonymousClassNode): array { return [ - 'file' => $anonymousClassNode->file, - 'line' => $anonymousClassNode->line, - 'extends' => $anonymousClassNode->extends, + 'file' => $anonymousClassNode->file, + 'line' => $anonymousClassNode->line, + 'extends' => $anonymousClassNode->extends, + 'implements' => $anonymousClassNode->implements, + 'traits' => $anonymousClassNode->traits, ]; } @@ -413,18 +470,26 @@ private function anonymousClassNodesFromPayload(array $payload): ?array return null; } - $file = $rawNode['file'] ?? null; - $line = $rawNode['line'] ?? null; - $extends = $rawNode['extends'] ?? null; + $file = $rawNode['file'] ?? null; + $line = $rawNode['line'] ?? null; + $extends = $rawNode['extends'] ?? null; + $implements = $rawNode['implements'] ?? []; + $traits = $rawNode['traits'] ?? []; if (! is_string($file) || ! is_int($line) || ($extends !== null && ! is_string($extends))) { return null; } + if (! $this->isStringArray($implements) || ! $this->isStringArray($traits)) { + return null; + } + $anonymousClassNodes[] = new AnonymousClassNode( - file: $file, - line: $line, - extends: $extends, + file: $file, + line: $line, + extends: $extends, + implements: $implements, + traits: $traits, ); } diff --git a/src/Cli/AnalyseCommand.php b/src/Cli/AnalyseCommand.php index db32f331..f3d82dc6 100644 --- a/src/Cli/AnalyseCommand.php +++ b/src/Cli/AnalyseCommand.php @@ -59,6 +59,15 @@ '--fix' => 'fix', ]; + /** + * Upper bound on fix/re-analyse passes. Fixers converge naturally (each + * pass removes or rewrites code), so this mainly guards against a fixer + * that keeps reporting success without resolving its violation. A cascade + * deeper than this cap (e.g. an unused-abstraction chain of more than + * ten levels) needs another --fix invocation to finish. + */ + private const MAX_FIX_PASSES = 10; + public function __construct(private ?ProgressHandlerInterface $progressHandler = null) { } @@ -172,9 +181,18 @@ public function run(array $arguments, string $basePath): int } if (isset($options['fix'])) { - $fixedCount = $this->fixViolations($architecture, $ruleViolationCollection); + // Removal fixers can cascade: deleting an unused child abstraction + // may leave its parent unused, so fix and re-analyse until a pass + // fixes nothing. The pass cap only guards against a fixer that + // reports success without resolving its violation. + for ($fixPass = 0; $fixPass < self::MAX_FIX_PASSES; $fixPass++) { + $passFixedCount = $this->fixViolations($architecture, $ruleViolationCollection); + + if ($passFixedCount === 0) { + break; + } - if ($fixedCount > 0) { + $fixedCount += $passFixedCount; $analysisResultCache->clear(); $files = $analyser->filesForAnalysis($architecture, $scanPaths); diff --git a/src/Cli/InitCommand.php b/src/Cli/InitCommand.php index 446ac9ef..be1ae3c7 100644 --- a/src/Cli/InitCommand.php +++ b/src/Cli/InitCommand.php @@ -87,13 +87,15 @@ private function presetConfig(string $preset): ?string 'psr12' => ' ->withPreset(Preset::PSR12());', 'psr15' => ' ->withPreset(Preset::PSR15());', 'psr4' => ' ->withPreset(Preset::PSR4());', + 'yagni' => ' ->withPreset(Preset::YAGNI());', 'all' => " ->withPresets(\n" . " Preset::PSR1(),\n" . " Preset::PSR12(),\n" . " Preset::PSR15(),\n" . " Preset::PSR4(),\n" . " Preset::DDD(),\n" - . " Preset::MVC()\n" + . " Preset::MVC(),\n" + . " Preset::YAGNI()\n" . " );", default => null, }; diff --git a/src/Cli/Usage.php b/src/Cli/Usage.php index 29890744..1752c038 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|psr1|psr12|psr15|psr4|all] + structarmed init [--preset=ddd|mvc|psr1|psr12|psr15|psr4|yagni|all] structarmed analyse|analyze [path ...] [--config=path/to/structarmed.php] [--report=console|json] [--no-progress] [--clear-cache] [--disable-parallel] [--fix] [--generate-baseline=structarmed-baseline.php] diff --git a/src/Preset/Preset.php b/src/Preset/Preset.php index a2c28e0c..5124f601 100644 --- a/src/Preset/Preset.php +++ b/src/Preset/Preset.php @@ -10,6 +10,7 @@ use Boundwize\StructArmed\Preset\Presets\Psr15Preset; use Boundwize\StructArmed\Preset\Presets\Psr1Preset; use Boundwize\StructArmed\Preset\Presets\Psr4Preset; +use Boundwize\StructArmed\Preset\Presets\YagniPreset; /** * Factory for built-in presets. @@ -21,6 +22,7 @@ * ->withPreset(Preset::PSR4()) * ->withPreset(Preset::PSR12()) * ->withPreset(Preset::PSR15()) + * ->withPreset(Preset::YAGNI()) * ->withPresets(Preset::DDD(), Preset::MVC()) */ final class Preset @@ -85,6 +87,17 @@ public static function DDD( ); } + /** + * @param list|null $sourcePaths + */ + public static function YAGNI( + ?array $sourcePaths = null, + ): YagniPreset { + return new YagniPreset( + sourcePaths: $sourcePaths, + ); + } + public static function MVC( int $controllerMaxComplexity = 5, int $controllerMaxMethodLength = 20, diff --git a/src/Preset/Presets/YagniPreset.php b/src/Preset/Presets/YagniPreset.php new file mode 100644 index 00000000..19c69d5f --- /dev/null +++ b/src/Preset/Presets/YagniPreset.php @@ -0,0 +1,68 @@ +|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::INTERFACE_MUST_BE_USED, + new MustBeUsedInterfaceRule($layerName) + ); + $architecture->rule( + self::ABSTRACT_CLASS_MUST_BE_USED, + new MustBeUsedAbstractClassRule($layerName) + ); + $architecture->rule( + self::TRAIT_MUST_BE_USED, + new MustBeUsedTraitRule($layerName) + ); + $architecture->rule( + self::EXTENDED_CLASS_MUST_BE_ABSTRACT_OR_INSTANTIATED, + new ExtendedClassMustBeAbstractOrInstantiatedRule($layerName) + ); + } +} diff --git a/src/Rule/Fixer/PhpParser/AbstractPhpParserFixableRule.php b/src/Rule/Fixer/PhpParser/AbstractPhpParserFixableRule.php index 692e6738..a6edc60c 100644 --- a/src/Rule/Fixer/PhpParser/AbstractPhpParserFixableRule.php +++ b/src/Rule/Fixer/PhpParser/AbstractPhpParserFixableRule.php @@ -17,11 +17,22 @@ final public function fix(RuleViolation $ruleViolation): bool return $this->fixerProcessor()->process( $ruleViolation->file, $nodeVisitor, + $this->shouldRemoveFileWhenEmpty(), ); } abstract protected function createFixerVisitor(RuleViolation $ruleViolation): NodeVisitor; + /** + * Whether the fixed file should be deleted when the fix leaves no code + * behind — only declare/namespace/use boilerplate. Rules whose fix removes + * whole declarations opt in by returning true. + */ + protected function shouldRemoveFileWhenEmpty(): bool + { + return false; + } + private function fixerProcessor(): PhpParserFixerProcessor { static $processor; diff --git a/src/Rule/Fixer/PhpParser/ClassLike/RemoveClassLikeVisitor.php b/src/Rule/Fixer/PhpParser/ClassLike/RemoveClassLikeVisitor.php new file mode 100644 index 00000000..4f97a9db --- /dev/null +++ b/src/Rule/Fixer/PhpParser/ClassLike/RemoveClassLikeVisitor.php @@ -0,0 +1,36 @@ +namespacedName)) { + return null; + } + + if ($node->namespacedName->toString() !== $this->className) { + return null; + } + + return NodeVisitor::REMOVE_NODE; + } +} diff --git a/src/Rule/Fixer/PhpParser/Class_/AddAbstractClassVisitor.php b/src/Rule/Fixer/PhpParser/Class_/AddAbstractClassVisitor.php new file mode 100644 index 00000000..e3ed3d62 --- /dev/null +++ b/src/Rule/Fixer/PhpParser/Class_/AddAbstractClassVisitor.php @@ -0,0 +1,37 @@ +isAbstract() || $node->isFinal() || $node->isAnonymous()) { + return null; + } + + if ($node->namespacedName?->toString() !== $this->className) { + return null; + } + + $node->flags |= Modifiers::ABSTRACT; + + return $node; + } +} diff --git a/src/Rule/Fixer/PhpParser/PhpParserFixerProcessor.php b/src/Rule/Fixer/PhpParser/PhpParserFixerProcessor.php index f8b634f4..e69db504 100644 --- a/src/Rule/Fixer/PhpParser/PhpParserFixerProcessor.php +++ b/src/Rule/Fixer/PhpParser/PhpParserFixerProcessor.php @@ -5,6 +5,12 @@ namespace Boundwize\StructArmed\Rule\Fixer\PhpParser; use PhpParser\Error; +use PhpParser\Node; +use PhpParser\Node\Stmt\Declare_; +use PhpParser\Node\Stmt\GroupUse; +use PhpParser\Node\Stmt\Namespace_; +use PhpParser\Node\Stmt\Nop; +use PhpParser\Node\Stmt\Use_; use PhpParser\NodeTraverser; use PhpParser\NodeVisitor; use PhpParser\NodeVisitor\CloningVisitor; @@ -15,10 +21,11 @@ use function file_get_contents; use function file_put_contents; use function is_file; +use function unlink; final readonly class PhpParserFixerProcessor { - public function process(string $file, NodeVisitor $nodeVisitor): bool + public function process(string $file, NodeVisitor $nodeVisitor, bool $removeFileWhenEmpty = false): bool { if (! is_file($file)) { return false; @@ -43,8 +50,52 @@ public function process(string $file, NodeVisitor $nodeVisitor): bool ->traverse((new NodeTraverser(new CloningVisitor())) ->traverse($originalStatements)); + // A fix that removes the last declaration leaves only boilerplate + // (declare/namespace/use); the whole file is dead weight at that point. + if ($removeFileWhenEmpty && $this->hasOnlyDeclarations($statements)) { + return unlink($file); + } + $fixedCode = (new Standard())->printFormatPreserving($statements, $originalStatements, $parser->getTokens()); return $fixedCode !== $code && file_put_contents($file, $fixedCode) !== false; } + + /** + * @param Node[] $statements + */ + private function hasOnlyDeclarations(array $statements): bool + { + foreach ($statements as $statement) { + // The block form `declare(...) { ... }` carries statements of its + // own, so only an empty-bodied declare counts as boilerplate. + if ($statement instanceof Declare_) { + if ($statement->stmts !== null && ! $this->hasOnlyDeclarations($statement->stmts)) { + return false; + } + + continue; + } + + if ( + $statement instanceof Use_ + || $statement instanceof GroupUse + || $statement instanceof Nop + ) { + continue; + } + + if ($statement instanceof Namespace_) { + if (! $this->hasOnlyDeclarations($statement->stmts)) { + return false; + } + + continue; + } + + return false; + } + + return true; + } } diff --git a/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php b/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php new file mode 100644 index 00000000..c605ce22 --- /dev/null +++ b/src/Rule/Rules/Class_/ExtendedClassMustBeAbstractOrInstantiatedRule.php @@ -0,0 +1,71 @@ +isClass() || $classNode->isAbstract) { + return false; + } + + if (! $classNode->isInLayer($this->layer)) { + return false; + } + + if ($this->classNamePattern !== null) { + return $classNode->nameMatches($this->classNamePattern, isFullName: true); + } + + return true; + } + + public function evaluate(ClassNode $classNode): ?RuleViolation + { + if (! $classNode->isExtended) { + return null; + } + + // Only instantiation (`new X`, or `new self`/`static`/`parent` + // resolving to X) requires the class to stay concrete — type hints, + // instanceof checks, and ::class constants keep working once the + // class becomes abstract. + if ($classNode->isInstantiated) { + return null; + } + + return new RuleViolation( + message: sprintf( + 'Extended class [%s] must be declared abstract or instantiated', + $classNode->className + ), + file: $classNode->file, + line: $classNode->line, + className: $classNode->className, + layer: $classNode->layer, + ); + } + + protected function createFixerVisitor(RuleViolation $ruleViolation): AddAbstractClassVisitor + { + return new AddAbstractClassVisitor($ruleViolation->className); + } +} diff --git a/src/Rule/Rules/Class_/MustBeUsedAbstractClassRule.php b/src/Rule/Rules/Class_/MustBeUsedAbstractClassRule.php new file mode 100644 index 00000000..2f9262b9 --- /dev/null +++ b/src/Rule/Rules/Class_/MustBeUsedAbstractClassRule.php @@ -0,0 +1,74 @@ +isClass() || ! $classNode->isAbstract) { + return false; + } + + if (! $classNode->isInLayer($this->layer)) { + return false; + } + + if ($this->classNamePattern !== null) { + return $classNode->nameMatches($this->classNamePattern, isFullName: true); + } + + return true; + } + + public function evaluate(ClassNode $classNode): ?RuleViolation + { + if ($classNode->isExtended) { + return null; + } + + // A dependency reference (instanceof, type hint, ::class, static + // call, ...) means removing the class would break the referencing code. + if ($classNode->isReferenced) { + return null; + } + + return new RuleViolation( + message: sprintf( + 'Abstract class [%s] must be extended by a class or referenced as a dependency', + $classNode->className + ), + file: $classNode->file, + line: $classNode->line, + className: $classNode->className, + layer: $classNode->layer, + ); + } + + protected function createFixerVisitor(RuleViolation $ruleViolation): RemoveClassLikeVisitor + { + return new RemoveClassLikeVisitor($ruleViolation->className); + } + + protected function shouldRemoveFileWhenEmpty(): bool + { + return true; + } +} diff --git a/src/Rule/Rules/Class_/MustBeUsedInterfaceRule.php b/src/Rule/Rules/Class_/MustBeUsedInterfaceRule.php new file mode 100644 index 00000000..4cd8d35c --- /dev/null +++ b/src/Rule/Rules/Class_/MustBeUsedInterfaceRule.php @@ -0,0 +1,75 @@ +isInterface) { + return false; + } + + if (! $classNode->isInLayer($this->layer)) { + return false; + } + + if ($this->classNamePattern !== null) { + return $classNode->nameMatches($this->classNamePattern, isFullName: true); + } + + return true; + } + + public function evaluate(ClassNode $classNode): ?RuleViolation + { + if ($classNode->isImplemented) { + return null; + } + + // A dependency reference (instanceof, type hint, ::class, ...) means + // removing the interface would break the referencing code. + if ($classNode->isReferenced) { + return null; + } + + return new RuleViolation( + message: sprintf( + 'Interface [%s] must be implemented by a class, extended by another interface,' + . ' or referenced as a dependency', + $classNode->className + ), + file: $classNode->file, + line: $classNode->line, + className: $classNode->className, + layer: $classNode->layer, + ); + } + + protected function createFixerVisitor(RuleViolation $ruleViolation): RemoveClassLikeVisitor + { + return new RemoveClassLikeVisitor($ruleViolation->className); + } + + protected function shouldRemoveFileWhenEmpty(): bool + { + return true; + } +} diff --git a/src/Rule/Rules/Class_/MustBeUsedTraitRule.php b/src/Rule/Rules/Class_/MustBeUsedTraitRule.php new file mode 100644 index 00000000..8c038f49 --- /dev/null +++ b/src/Rule/Rules/Class_/MustBeUsedTraitRule.php @@ -0,0 +1,68 @@ +isTrait) { + return false; + } + + if (! $classNode->isInLayer($this->layer)) { + return false; + } + + if ($this->classNamePattern !== null) { + return $classNode->nameMatches($this->classNamePattern, isFullName: true); + } + + return true; + } + + public function evaluate(ClassNode $classNode): ?RuleViolation + { + if ($classNode->isReferenced) { + return null; + } + + return new RuleViolation( + message: sprintf( + 'Trait [%s] must be used by a class, trait, or enum, or referenced as a dependency', + $classNode->className + ), + file: $classNode->file, + line: $classNode->line, + className: $classNode->className, + layer: $classNode->layer, + ); + } + + protected function createFixerVisitor(RuleViolation $ruleViolation): RemoveClassLikeVisitor + { + return new RemoveClassLikeVisitor($ruleViolation->className); + } + + protected function shouldRemoveFileWhenEmpty(): bool + { + return true; + } +} diff --git a/src/Rule/UsedInterfaceAwareRuleInterface.php b/src/Rule/UsedInterfaceAwareRuleInterface.php new file mode 100644 index 00000000..ca6a07a7 --- /dev/null +++ b/src/Rule/UsedInterfaceAwareRuleInterface.php @@ -0,0 +1,20 @@ +isReferenced. + * + * Trade-off: only usage within the scanned paths is known. A trait used solely + * by a consumer outside the scan is reported as if not used. + */ +interface UsedTraitAwareRuleInterface extends RuleInterface +{ +} diff --git a/structarmed.php b/structarmed.php index a96c55f6..baa6adcd 100644 --- a/structarmed.php +++ b/structarmed.php @@ -5,6 +5,7 @@ use Boundwize\StructArmed\Architecture; use Boundwize\StructArmed\Preset\Preset; use Boundwize\StructArmed\Preset\Presets\Psr1Preset; +use Boundwize\StructArmed\Rule\Rules\Class_\MustBeFinalRule; return Architecture::define() ->layer('Analyser', 'src/Analyser/') @@ -52,4 +53,8 @@ __DIR__ . '/tests/Analyser/Parallel/MockFunctions.php', ], ]) - ->withPresets(Preset::PSR1(), Preset::PSR12(), Preset::PSR4()); + ->withPresets(Preset::PSR1(), Preset::PSR12(), Preset::PSR4(), Preset::YAGNI()) + ->rule( + 'source.must_be_final', + new MustBeFinalRule(layer: 'Source') + ); diff --git a/tests/Analyser/AnalyserTest.php b/tests/Analyser/AnalyserTest.php index 1811639a..2e419f6f 100644 --- a/tests/Analyser/AnalyserTest.php +++ b/tests/Analyser/AnalyserTest.php @@ -19,6 +19,7 @@ use Boundwize\StructArmed\Preset\Presets\Psr15Preset; use Boundwize\StructArmed\Preset\Presets\Psr1Preset; use Boundwize\StructArmed\Preset\Presets\Psr4Preset; +use Boundwize\StructArmed\Preset\Presets\YagniPreset; use Boundwize\StructArmed\Progress\ProgressHandlerInterface; use Boundwize\StructArmed\Rule\FileAnalysisRuleInterface; use Boundwize\StructArmed\Rule\Rules\Class_\MustBeFinalRule; @@ -312,6 +313,702 @@ public function testMustBeFinalRuleFlagsExtendedClassWhenChildIsOutsideScannedPa $this->assertSame('App\BaseHandler', $violations[0]->className); } + public function testYagniPresetReportsOnlyUnusedAbstractions(): void + { + $consumer = 'makeTempProject([ + 'src/UsedInterface.php' => ' ' ' ' ' ' ' ' $consumer, + ]); + + $architecture = Architecture::define() + ->withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $ruleViolationCollection = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()); + + $interfaceViolations = $ruleViolationCollection->forRule(YagniPreset::INTERFACE_MUST_BE_USED); + $abstractViolations = $ruleViolationCollection->forRule(YagniPreset::ABSTRACT_CLASS_MUST_BE_USED); + $traitViolations = $ruleViolationCollection->forRule(YagniPreset::TRAIT_MUST_BE_USED); + + // BaseInterface is extended by ChildInterface, so only UnusedInterface + // and the never-implemented ChildInterface itself are reported. + $interfaceClassNames = array_map( + static fn (RuleViolation $ruleViolation): string => $ruleViolation->className, + $interfaceViolations + ); + sort($interfaceClassNames); + + $this->assertSame(['App\ChildInterface', 'App\UnusedInterface'], $interfaceClassNames); + + $this->assertCount(1, $abstractViolations); + $this->assertSame('App\UnusedBase', $abstractViolations[0]->className); + + $this->assertCount(1, $traitViolations); + $this->assertSame('App\UnusedTrait', $traitViolations[0]->className); + } + + public function testMustBeUsedInterfaceRuleRecognizesTransitiveImplementation(): void + { + $basePath = $this->makeTempProject([ + 'src/BaseInterface.php' => ' ' 'withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $ruleViolationCollection = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()); + + // Consumer implements ChildInterface, which transitively implements + // BaseInterface — neither interface is speculative. + $this->assertCount(0, $ruleViolationCollection->forRule(YagniPreset::INTERFACE_MUST_BE_USED)); + } + + public function testMustBeUsedTraitRuleRecognizesTraitUsedByAnotherTrait(): void + { + $basePath = $this->makeTempProject([ + 'src/InnerTrait.php' => ' ' 'withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $violations = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()) + ->forRule(YagniPreset::TRAIT_MUST_BE_USED); + + // InnerTrait is used by OuterTrait, which is used by Consumer. + $this->assertCount(0, $violations); + } + + public function testMustBeUsedTraitRuleRecognizesTraitUsedByEnum(): void + { + $basePath = $this->makeTempProject([ + 'src/Helper.php' => ' 'withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $violations = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()) + ->forRule(YagniPreset::TRAIT_MUST_BE_USED); + + $this->assertCount(0, $violations); + } + + public function testYagniRulesDoNotFlagAbstractionsReferencedAsDependencies(): void + { + $checker = 'makeTempProject([ + 'src/Contract.php' => ' ' ' $checker, + ]); + + $architecture = Architecture::define() + ->withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $ruleViolationCollection = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()); + + // instanceof checks, type hints, and ::class constants are references; + // removing the abstraction would break the referencing code. + $this->assertFalse($ruleViolationCollection->hasViolations()); + } + + public function testYagniRulesIgnoreSelfReferences(): void + { + $basePath = $this->makeTempProject([ + 'src/UnusedInterface.php' => ' ' 'withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $ruleViolationCollection = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()); + + $interfaceViolations = $ruleViolationCollection->forRule(YagniPreset::INTERFACE_MUST_BE_USED); + $abstractViolations = $ruleViolationCollection->forRule(YagniPreset::ABSTRACT_CLASS_MUST_BE_USED); + $traitViolations = $ruleViolationCollection->forRule(YagniPreset::TRAIT_MUST_BE_USED); + + // A class-like referencing itself cannot keep itself alive. + $this->assertCount(1, $interfaceViolations); + $this->assertSame('App\UnusedInterface', $interfaceViolations[0]->className); + $this->assertCount(1, $abstractViolations); + $this->assertSame('App\UnusedBase', $abstractViolations[0]->className); + $this->assertCount(1, $traitViolations); + $this->assertSame('App\UnusedTrait', $traitViolations[0]->className); + } + + public function testExtendedClassMustBeAbstractOrInstantiatedRuleFlagsUninstantiatedParent(): void + { + $basePath = $this->makeTempProject([ + 'src/BaseRepository.php' => ' 'withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $violations = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()) + ->forRule(YagniPreset::EXTENDED_CLASS_MUST_BE_ABSTRACT_OR_INSTANTIATED); + + // BaseRepository is only ever used as a parent class; extending it + // does not count as a reference, so it should be abstract. + $this->assertCount(1, $violations); + $this->assertSame('App\BaseRepository', $violations[0]->className); + } + + public function testExtendedClassMustBeAbstractOrInstantiatedRuleFlagsTypeHintedButUninstantiatedParent(): void + { + $consumer = 'makeTempProject([ + 'src/BaseRepository.php' => ' ' $consumer, + ]); + + $architecture = Architecture::define() + ->withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $violations = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()) + ->forRule(YagniPreset::EXTENDED_CLASS_MUST_BE_ABSTRACT_OR_INSTANTIATED); + + // Type hints, instanceof, and ::class keep working on an abstract + // class — only instantiation requires it to stay concrete. + $this->assertCount(1, $violations); + $this->assertSame('App\BaseRepository', $violations[0]->className); + } + + public function testExtendedClassMustBeAbstractOrInstantiatedRulePassesWhenParentIsInstantiated(): void + { + $factory = 'makeTempProject([ + 'src/BaseRepository.php' => ' ' $factory, + ]); + + $architecture = Architecture::define() + ->withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $violations = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()) + ->forRule(YagniPreset::EXTENDED_CLASS_MUST_BE_ABSTRACT_OR_INSTANTIATED); + + $this->assertCount(0, $violations); + } + + public function testExtendedClassMustBeAbstractOrInstantiatedRulePassesWhenChildInstantiatesParent(): void + { + // The extends clause itself must not count as a reference, but the + // child's `new BaseRepository()` must: making the parent abstract + // would fatal at that instantiation. + $child = 'makeTempProject([ + 'src/BaseRepository.php' => ' $child, + ]); + + $architecture = Architecture::define() + ->withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $violations = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()) + ->forRule(YagniPreset::EXTENDED_CLASS_MUST_BE_ABSTRACT_OR_INSTANTIATED); + + $this->assertCount(0, $violations); + } + + public function testExtendedClassMustBeAbstractOrInstantiatedRulePassesOnDynamicInstantiation(): void + { + // The constant-expression evaluator resolves `$class = X::class; + // new $class()` — dynamic instantiation still keeps the parent + // concrete. + $factory = 'makeTempProject([ + 'src/BaseRepository.php' => ' ' $factory, + ]); + + $architecture = Architecture::define() + ->withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $violations = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()) + ->forRule(YagniPreset::EXTENDED_CLASS_MUST_BE_ABSTRACT_OR_INSTANTIATED); + + $this->assertCount(0, $violations); + } + + public function testExtendedClassMustBeAbstractOrInstantiatedRulePassesOnConditionalDynamicInstantiation(): void + { + // `create(false)` instantiates Base at runtime even though the + // traversal sees Child assigned last — every possible value of the + // variable must keep its class concrete. + $factory = 'makeTempProject([ + 'src/Base.php' => ' ' $factory, + ]); + + $architecture = Architecture::define() + ->withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $violations = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()) + ->forRule(YagniPreset::EXTENDED_CLASS_MUST_BE_ABSTRACT_OR_INSTANTIATED); + + $this->assertCount(0, $violations); + } + + public function testExtendedClassMustBeAbstractOrInstantiatedRulePassesWhenFactoryInstantiatesUnknownClass(): void + { + // `make(Base::class)` instantiates Base at runtime, but the collector + // cannot connect the argument to `new $class`. The unresolved dynamic + // instantiation makes every referenced class count as possibly + // instantiated. + $functions = 'makeTempProject([ + 'src/Base.php' => ' ' $functions, + ]); + + $architecture = Architecture::define() + ->withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $violations = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()) + ->forRule(YagniPreset::EXTENDED_CLASS_MUST_BE_ABSTRACT_OR_INSTANTIATED); + + $this->assertCount(0, $violations); + } + + public function testExtendedClassMustBeAbstractOrInstantiatedRulePassesOnReflectionInstantiation(): void + { + // ReflectionClass::newInstance() constructs Base at runtime; the + // reflection construction call marks an unresolved instantiation, so + // the referenced Base stays concrete. + $bootstrap = 'newInstance();'; + + $basePath = $this->makeTempProject([ + 'src/Base.php' => ' ' $bootstrap, + ]); + + $architecture = Architecture::define() + ->withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $violations = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()) + ->forRule(YagniPreset::EXTENDED_CLASS_MUST_BE_ABSTRACT_OR_INSTANTIATED); + + $this->assertCount(0, $violations); + } + + public function testExtendedClassMustBeAbstractOrInstantiatedRuleSkipsAllWhenUnresolvedInstantiationExists(): void + { + // `make($_ENV['CLASS'])` can name any class — even one nothing in the + // scanned code references — so an unresolved dynamic instantiation + // must silence the rule entirely: no class can be proven safe to + // abstract. + $functions = 'makeTempProject([ + 'src/UnreferencedBase.php' => ' ' $functions, + ]); + + $architecture = Architecture::define() + ->withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $violations = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()) + ->forRule(YagniPreset::EXTENDED_CLASS_MUST_BE_ABSTRACT_OR_INSTANTIATED); + + $this->assertCount(0, $violations); + } + + public function testExtendedClassMustBeAbstractOrInstantiatedRuleSkipsAllWhenEvalIsUsed(): void + { + // eval() can construct any class the evaluated code names. + $bootstrap = 'makeTempProject([ + 'src/Base.php' => ' ' $bootstrap, + ]); + + $architecture = Architecture::define() + ->withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $violations = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()) + ->forRule(YagniPreset::EXTENDED_CLASS_MUST_BE_ABSTRACT_OR_INSTANTIATED); + + $this->assertCount(0, $violations); + } + + public function testExtendedClassMustBeAbstractOrInstantiatedRuleSkipsAllWhenUnserializeIsCalled(): void + { + // unserialize() constructs instances of whatever the payload names; + // abstracting a serialized concrete class breaks deserialization. + $bootstrap = 'makeTempProject([ + 'src/Base.php' => ' ' $bootstrap, + ]); + + $architecture = Architecture::define() + ->withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $violations = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()) + ->forRule(YagniPreset::EXTENDED_CLASS_MUST_BE_ABSTRACT_OR_INSTANTIATED); + + $this->assertCount(0, $violations); + } + + public function testExtendedClassMustBeAbstractOrInstantiatedRulePassesOnSelfAndParentInstantiation(): void + { + // `new self()` resolves to the class itself even when called through a + // subclass, and `new parent()` resolves to the extended class — both + // would fatal if the target became abstract. + $connection = 'makeTempProject([ + 'src/Connection.php' => $connection, + 'src/ConnectionPool.php' => $pool, + 'src/TunedPool.php' => $tuned, + ]); + + $architecture = Architecture::define() + ->withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $violations = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()) + ->forRule(YagniPreset::EXTENDED_CLASS_MUST_BE_ABSTRACT_OR_INSTANTIATED); + + // Connection is protected by both `new self()` and `new parent()`; + // ConnectionPool is extended but never referenced, so only it is + // reported. + $this->assertCount(1, $violations); + $this->assertSame('App\ConnectionPool', $violations[0]->className); + } + + public function testYagniRulesDoNotFlagAbstractionsReferencedByClassNameString(): void + { + $checker = 'makeTempProject([ + 'src/Contract.php' => ' ' ' $checker, + 'src/bootstrap.php' => 'withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $ruleViolationCollection = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()); + + // A class-name string value can reach `new $class` or `instanceof + // $class` at runtime, so it counts as a reference — in class bodies + // and procedural code alike. + $this->assertFalse($ruleViolationCollection->hasViolations()); + } + + public function testYagniRulesDoNotFlagAbstractionsReferencedByProceduralCode(): void + { + $functions = 'makeTempProject([ + 'src/Contract.php' => ' ' ' $functions, + ]); + + $architecture = Architecture::define() + ->withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $ruleViolationCollection = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()); + + // Type hints and ::class references in procedural code have no + // ClassNode of their own but still keep the abstractions alive. + $this->assertFalse($ruleViolationCollection->hasViolations()); + } + + public function testYagniRulesDoNotFlagAbstractionsReferencedByTopLevelAnonymousClassBody(): void + { + // Migration-style file: no named class at all; the reference lives in + // the anonymous class body, not in its extends/implements/use clauses. + $registration = 'makeTempProject([ + 'src/Contract.php' => ' $registration, + ]); + + $architecture = Architecture::define() + ->withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $ruleViolationCollection = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()); + + $this->assertCount(0, $ruleViolationCollection->forRule(YagniPreset::INTERFACE_MUST_BE_USED)); + } + + public function testYagniRulesRecognizeProceduralReferencesOnCachedRun(): void + { + $functions = 'makeTempProject([ + 'src/Contract.php' => ' ' ' $functions, + ]); + $analysisResultCache = new AnalysisResultCache($basePath, new FileHashProvider(), 'cache'); + + $architecture = Architecture::define() + ->withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $ruleViolationCollection = (new Analyser($basePath, $analysisResultCache, 'config')) + ->analyse($architecture, [], null, AnalyserOptions::sequential()); + $warmViolationCollection = (new Analyser($basePath, $analysisResultCache, 'config')) + ->analyse($architecture, [], null, AnalyserOptions::sequential()); + + // Procedural references must survive the class-node cache round-trip. + $this->assertFalse($ruleViolationCollection->hasViolations()); + $this->assertFalse($warmViolationCollection->hasViolations()); + } + + public function testYagniRulesRecognizeProceduralReferencesOnCachedRunWithFileAnalysis(): void + { + $functions = 'makeTempProject([ + 'src/Contract.php' => ' ' ' $functions, + ]); + $analysisResultCache = new AnalysisResultCache($basePath, new FileHashProvider(), 'cache'); + + // A file-analysis rule makes the warm run load class nodes through the + // file-analysis cache path, which must also restore file references. + $architecture = Architecture::define() + ->withPreset(Preset::YAGNI(sourcePaths: ['src/'])) + ->rule('psr1.php_tags', new Psr1PhpTagsRule(['src/'])); + + $ruleViolationCollection = (new Analyser($basePath, $analysisResultCache, 'config')) + ->analyse($architecture, [], null, AnalyserOptions::sequential()); + $warmViolationCollection = (new Analyser($basePath, $analysisResultCache, 'config')) + ->analyse($architecture, [], null, AnalyserOptions::sequential()); + + $this->assertFalse($ruleViolationCollection->hasViolations()); + $this->assertFalse($warmViolationCollection->hasViolations()); + } + + public function testYagniRulesDoNotFlagAbstractionsUsedByAnonymousClass(): void + { + $factory = 'makeTempProject([ + 'src/Contract.php' => ' ' $factory, + ]); + + $architecture = Architecture::define() + ->withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $ruleViolationCollection = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()); + + $this->assertCount(0, $ruleViolationCollection->forRule(YagniPreset::INTERFACE_MUST_BE_USED)); + $this->assertCount(0, $ruleViolationCollection->forRule(YagniPreset::TRAIT_MUST_BE_USED)); + } + + public function testYagniRulesDoNotFlagAnonymousClassUsageOnCachedRun(): void + { + $factory = 'makeTempProject([ + 'src/Contract.php' => ' ' $factory, + ]); + $analysisResultCache = new AnalysisResultCache($basePath, new FileHashProvider(), 'cache'); + + $architecture = Architecture::define() + ->withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $ruleViolationCollection = (new Analyser($basePath, $analysisResultCache, 'config')) + ->analyse($architecture, [], null, AnalyserOptions::sequential()); + $warmViolationCollection = (new Analyser($basePath, $analysisResultCache, 'config')) + ->analyse($architecture, [], null, AnalyserOptions::sequential()); + + // The anonymous-class implements/use lists must survive the class-node + // cache round-trip. + $this->assertFalse($ruleViolationCollection->hasViolations()); + $this->assertFalse($warmViolationCollection->hasViolations()); + } + + public function testYagniRulesFollowOriginalNamesWhenUsageIsAliased(): void + { + $consumer = 'makeTempProject([ + 'src/BaseHandler.php' => ' ' ' $consumer, + ]); + + $architecture = Architecture::define() + ->withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $ruleViolationCollection = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::sequential()); + + // Aliased imports resolve to the original names, so the abstractions + // count as used. + $this->assertFalse($ruleViolationCollection->hasViolations()); + } + + public function testYagniRulesRecognizeUsageWithParallelRunner(): void + { + $consumer = 'makeTempProject([ + 'src/UsedInterface.php' => ' ' ' ' ' ' $consumer, + 'src/functions.php' => 'withPreset(Preset::YAGNI(sourcePaths: ['src/'])); + + $ruleViolationCollection = (new Analyser($basePath)) + ->analyse($architecture, [], null, AnalyserOptions::parallel()); + + $this->assertFalse($ruleViolationCollection->hasViolations()); + } + public function testMustBeFinalRuleReportsSameExtendedClassesOnCachedRun(): void { $basePath = $this->makeTempProject([ diff --git a/tests/Analyser/ClassCollectorTest.php b/tests/Analyser/ClassCollectorTest.php index f3f1eeee..45c7b278 100644 --- a/tests/Analyser/ClassCollectorTest.php +++ b/tests/Analyser/ClassCollectorTest.php @@ -62,6 +62,277 @@ private function makeCollector(string $code): ClassCollector return $classCollector; } + public function testCollectsFileReferencesFromProceduralCode(): void + { + $code = 'makeCollector($code); + + $this->assertSame( + ['/fake/path/Foo.php' => ['App\Contract']], + $classCollector->getFileReferences() + ); + } + + public function testDoesNotCollectFileReferencesFromClassBodies(): void + { + $code = 'makeCollector($code); + + // References inside a named class-like land on its ClassNode + // dependencies, not in the file-level references. + $this->assertSame([], $classCollector->getFileReferences()); + } + + public function testCollectsClassNameShapedStringValuesAsFileReferences(): void + { + $code = 'makeCollector($code); + + $this->assertSame( + ['/fake/path/Foo.php' => ['App\Contract']], + $classCollector->getFileReferences() + ); + } + + public function testDoesNotCollectNonClassNameShapedStringValues(): void + { + $code = 'makeCollector($code); + + $this->assertSame([], $classCollector->getFileReferences()); + } + + public function testCollectsInstantiations(): void + { + $code = 'makeCollector($code); + + $this->assertSame( + ['/fake/path/Foo.php' => ['App\Service']], + $classCollector->getFileInstantiations() + ); + $this->assertSame([], $classCollector->getFileReferences()); + } + + public function testResolvesSelfStaticAndParentInstantiations(): void + { + $code = 'makeCollector($code); + + $this->assertSame( + ['/fake/path/Foo.php' => ['App\Repository', 'App\BaseRepository']], + $classCollector->getFileInstantiations() + ); + } + + public function testResolvesVariableClassNameInstantiations(): void + { + $code = 'makeCollector($code); + + $this->assertSame( + ['/fake/path/Foo.php' => ['App\Base', 'App\StringBase', 'App\Concatenated', 'App\Joined']], + $classCollector->getFileInstantiations() + ); + } + + public function testResolvesSelfClassConstantInstantiation(): void + { + $code = 'makeCollector($code); + + $this->assertSame( + ['/fake/path/Foo.php' => ['App\Registry']], + $classCollector->getFileInstantiations() + ); + } + + public function testCollectsAllPossibleClassNamesOnConditionalReassignment(): void + { + // Either branch may run at runtime, so both classes are instantiable. + $code = 'makeCollector($code); + + $this->assertSame( + ['/fake/path/Foo.php' => ['App\Base', 'App\Child']], + $classCollector->getFileInstantiations() + ); + } + + public function testKeepsEarlierClassNameOnUnresolvableReassignment(): void + { + $code = 'makeCollector($code); + + // The reassignment cannot be evaluated statically, but the earlier + // constant value may still reach the instantiation — keeping it plus + // the unresolved marker is the safe over-approximation. + $this->assertSame( + ['/fake/path/Foo.php' => ['App\Base', ClassCollector::UNRESOLVED_INSTANTIATION]], + $classCollector->getFileInstantiations() + ); + } + + public function testMarksUnresolvedInstantiationForUnknownDynamicClass(): void + { + // The class name flows in from a call site the collector cannot see. + $code = 'makeCollector($code); + + $this->assertSame( + ['/fake/path/Foo.php' => [ClassCollector::UNRESOLVED_INSTANTIATION]], + $classCollector->getFileInstantiations() + ); + } + + public function testMarksUnresolvedInstantiationForReflectionConstruction(): void + { + // ReflectionClass::newInstance() constructs an object of a class the + // collector cannot determine. + $code = 'newInstance() ?? $n?->newInstanceWithoutConstructor(); } }'; + + $classCollector = $this->makeCollector($code); + + $this->assertSame( + ['/fake/path/Foo.php' => [ClassCollector::UNRESOLVED_INSTANTIATION]], + $classCollector->getFileInstantiations() + ); + } + + public function testMarksUnresolvedInstantiationForUnserialize(): void + { + $code = 'makeCollector($code); + + $this->assertSame( + ['/fake/path/Foo.php' => [ClassCollector::UNRESOLVED_INSTANTIATION]], + $classCollector->getFileInstantiations() + ); + } + + public function testMarksUnresolvedInstantiationForEval(): void + { + $inClass = 'assertSame( + ['/fake/path/Foo.php' => [ClassCollector::UNRESOLVED_INSTANTIATION]], + $this->makeCollector($inClass)->getFileInstantiations() + ); + $this->assertSame( + ['/fake/path/Foo.php' => [ClassCollector::UNRESOLVED_INSTANTIATION]], + $this->makeCollector($procedural)->getFileInstantiations() + ); + } + + public function testDoesNotMarkUnresolvedInstantiationForOrdinaryMethodCalls(): void + { + $code = 'handle(); } }'; + + $classCollector = $this->makeCollector($code); + + $this->assertSame([], $classCollector->getFileInstantiations()); + } + + public function testMarksUnresolvedInstantiationForRuntimeClassExpression(): void + { + $code = 'class)(); } }'; + + $classCollector = $this->makeCollector($code); + + $this->assertSame( + ['/fake/path/Foo.php' => [ClassCollector::UNRESOLVED_INSTANTIATION]], + $classCollector->getFileInstantiations() + ); + } + + public function testIgnoresUnresolvableClassNameExpressions(): void + { + $code = 'kept = \'App\\Prop\'; return new $class(); } }'; + + $classCollector = $this->makeCollector($code); + + // Runtime-dependent concatenation, non-class-shaped strings, and + // property assignments resolve to no concrete candidate — only the + // unresolved marker remains. + $this->assertSame( + ['/fake/path/Foo.php' => [ClassCollector::UNRESOLVED_INSTANTIATION]], + $classCollector->getFileInstantiations() + ); + } + + public function testDoesNotCollectAnonymousInstantiations(): void + { + $code = 'makeCollector($code); + + // Anonymous classes are tracked as AnonymousClassNodes, and their + // known declaration does not make any named class instantiable. + $this->assertSame([], $classCollector->getFileInstantiations()); + } + + public function testIgnoresRelativeInstantiationOutsideClassScope(): void + { + // `new self` outside a class parses but cannot be resolved to a name; + // PHP itself rejects it at runtime. + $classCollector = $this->makeCollector('assertSame([], $classCollector->getFileInstantiations()); + } + public function testCollectsFinalClass(): void { $classNode = $this->collect('assertFalse($classNode->isExtended); } + public function testSetImplementedTogglesIsImplementedFlag(): void + { + $classNode = new ClassNode( + className: 'App\\Domain\\OrderRepositoryInterface', + file: '/src/OrderRepositoryInterface.php', + line: 5, + layer: 'Domain', + extends: null, + isAbstract: false, + isFinal: false, + isInterface: true, + isReadonly: false, + ); + + $this->assertFalse($classNode->isImplemented); + + $classNode->setImplemented(true); + $this->assertTrue($classNode->isImplemented); + + $classNode->setImplemented(false); + $this->assertFalse($classNode->isImplemented); + } + + public function testSetReferencedTogglesIsReferencedFlag(): void + { + $classNode = new ClassNode( + className: 'App\\Domain\\TimestampableTrait', + file: '/src/TimestampableTrait.php', + line: 5, + layer: 'Domain', + extends: null, + isAbstract: false, + isFinal: false, + isInterface: false, + isReadonly: false, + isTrait: true, + ); + + $this->assertFalse($classNode->isReferenced); + + $classNode->setReferenced(true); + $this->assertTrue($classNode->isReferenced); + + $classNode->setReferenced(false); + $this->assertFalse($classNode->isReferenced); + } + + public function testSetInstantiatedTogglesIsInstantiatedFlag(): void + { + $classNode = new ClassNode( + className: 'App\\Domain\\BaseRepository', + file: '/src/BaseRepository.php', + line: 5, + layer: 'Domain', + extends: null, + isAbstract: false, + isFinal: false, + isInterface: false, + isReadonly: false, + ); + + $this->assertFalse($classNode->isInstantiated); + + $classNode->setInstantiated(true); + $this->assertTrue($classNode->isInstantiated); + + $classNode->setInstantiated(false); + $this->assertFalse($classNode->isInstantiated); + } + + public function testSetInstantiatedIsIgnoredForNonInstantiableClassLikes(): void + { + $makeNode = static fn ( + bool $isAbstract = false, + bool $isInterface = false, + bool $isTrait = false, + bool $isEnum = false, + ): ClassNode => new ClassNode( + className: 'App\\Domain\\SomeClassLike', + file: '/src/SomeClassLike.php', + line: 5, + layer: 'Domain', + extends: null, + isAbstract: $isAbstract, + isFinal: false, + isInterface: $isInterface, + isReadonly: false, + isTrait: $isTrait, + isEnum: $isEnum, + ); + + $nonInstantiables = [ + 'abstract class' => $makeNode(isAbstract: true), + 'interface' => $makeNode(isInterface: true), + 'trait' => $makeNode(isTrait: true), + 'enum' => $makeNode(isEnum: true), + ]; + + foreach ($nonInstantiables as $kind => $classNode) { + $classNode->setInstantiated(true); + + // `new` on these class-likes is fatal, so they can never be an + // instantiation target. + $this->assertFalse($classNode->isInstantiated, $kind); + } + } + public function testDependsOnMatchesExistingClassesExactly(): void { $classNode = new ClassNode( diff --git a/tests/Analyser/Parallel/ParallelClassNodeExtractorTest.php b/tests/Analyser/Parallel/ParallelClassNodeExtractorTest.php index bf5a3ddc..7821c64b 100644 --- a/tests/Analyser/Parallel/ParallelClassNodeExtractorTest.php +++ b/tests/Analyser/Parallel/ParallelClassNodeExtractorTest.php @@ -7,7 +7,9 @@ use Boundwize\StructArmed\Analyser\ClassNode; use Boundwize\StructArmed\Analyser\Parallel\ParallelClassNodeExtractor; use Boundwize\StructArmed\Tests\Support\TemporaryDirectoryCleanupTrait; +use Iterator; use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use RuntimeException; @@ -451,4 +453,107 @@ public function testExtractThrowsWhenAnonymousClassNodeEntryIsInvalid(): void $GLOBALS['mock_tracked_tempnam_files'] = []; } } + + public function testExtractThrowsWhenFileReferencesPayloadIsNotAnArray(): void + { + $GLOBALS['mock_file_get_contents_payload'] = [ + 'nodes' => [], + 'fileAnalyses' => [], + 'anonymousClassNodes' => [], + 'fileReferences' => 'invalid', + 'error' => null, + ]; + + $dir = $this->makeTemporaryDirectory('structarmed-parallel-test'); + $file = $dir . '/Foo.php'; + file_put_contents($file, 'expectException(RuntimeException::class); + $this->expectExceptionMessage('Parallel analysis worker returned invalid file references.'); + + try { + $parallelClassNodeExtractor->extract([$file]); + } finally { + $GLOBALS['mock_file_get_contents_payload'] = null; + $GLOBALS['mock_tracked_tempnam_files'] = []; + } + } + + /** + * @return Iterator + */ + public static function invalidFileReferencesEntryProvider(): Iterator + { + yield 'entry not an array' => [['Foo.php' => 'invalid']]; + yield 'entry with non-string reference' => [['Foo.php' => [1]]]; + } + + /** + * @return Iterator + */ + public static function invalidFileInstantiationsProvider(): Iterator + { + yield 'not an array' => ['invalid']; + yield 'entry not an array' => [['Foo.php' => 'invalid']]; + yield 'entry with non-string instantiation' => [['Foo.php' => [1]]]; + } + + #[DataProvider('invalidFileInstantiationsProvider')] + public function testExtractThrowsWhenFileInstantiationsPayloadIsInvalid(mixed $invalidFileInstantiations): void + { + $GLOBALS['mock_file_get_contents_payload'] = [ + 'nodes' => [], + 'fileAnalyses' => [], + 'anonymousClassNodes' => [], + 'fileReferences' => [], + 'fileInstantiations' => $invalidFileInstantiations, + 'error' => null, + ]; + + $dir = $this->makeTemporaryDirectory('structarmed-parallel-test'); + $file = $dir . '/Foo.php'; + file_put_contents($file, 'expectException(RuntimeException::class); + $this->expectExceptionMessage('Parallel analysis worker returned invalid file instantiations.'); + + try { + $parallelClassNodeExtractor->extract([$file]); + } finally { + $GLOBALS['mock_file_get_contents_payload'] = null; + $GLOBALS['mock_tracked_tempnam_files'] = []; + } + } + + #[DataProvider('invalidFileReferencesEntryProvider')] + public function testExtractThrowsWhenFileReferencesEntryIsInvalid(mixed $invalidFileReferences): void + { + $GLOBALS['mock_file_get_contents_payload'] = [ + 'nodes' => [], + 'fileAnalyses' => [], + 'anonymousClassNodes' => [], + 'fileReferences' => $invalidFileReferences, + 'error' => null, + ]; + + $dir = $this->makeTemporaryDirectory('structarmed-parallel-test'); + $file = $dir . '/Foo.php'; + file_put_contents($file, 'expectException(RuntimeException::class); + $this->expectExceptionMessage('Parallel analysis worker returned invalid file references.'); + + try { + $parallelClassNodeExtractor->extract([$file]); + } finally { + $GLOBALS['mock_file_get_contents_payload'] = null; + $GLOBALS['mock_tracked_tempnam_files'] = []; + } + } } diff --git a/tests/Cache/AnalysisResultCacheTest.php b/tests/Cache/AnalysisResultCacheTest.php index 97f07db6..c784802d 100644 --- a/tests/Cache/AnalysisResultCacheTest.php +++ b/tests/Cache/AnalysisResultCacheTest.php @@ -603,9 +603,11 @@ public function testStoresAndLoadsAnonymousClassNodes(): void $classNodes = [$this->makeClassNode($sourceFile)]; $anonymousClassNodes = [ new AnonymousClassNode( - file: $sourceFile, - line: 7, - extends: 'App\BaseHandler', + file: $sourceFile, + line: 7, + extends: 'App\BaseHandler', + implements: ['App\Contract'], + traits: ['App\Helper'], ), ]; @@ -618,6 +620,8 @@ public function testStoresAndLoadsAnonymousClassNodes(): void $classNodes, null, $anonymousClassNodes, + ['App\ReferencedInFunction'], + ['App\InstantiatedInFunction'], ); $loaded = $analysisResultCache->loadClassNodes($sourceFile, 'config'); @@ -625,6 +629,8 @@ public function testStoresAndLoadsAnonymousClassNodes(): void $this->assertIsArray($loaded); $this->assertEquals($classNodes, $loaded['classNodes']); $this->assertEquals($anonymousClassNodes, $loaded['anonymousClassNodes']); + $this->assertSame(['App\ReferencedInFunction'], $loaded['fileReferences']); + $this->assertSame(['App\InstantiatedInFunction'], $loaded['fileInstantiations']); } finally { if (file_exists($sourceFile)) { unlink($sourceFile); @@ -658,6 +664,8 @@ public function testClassNodesLoadOldCachePayloadWithoutAnonymousClassNodes(): v $this->assertIsArray($loaded); $this->assertEquals($classNodes, $loaded['classNodes']); $this->assertSame([], $loaded['anonymousClassNodes']); + $this->assertSame([], $loaded['fileReferences']); + $this->assertSame([], $loaded['fileInstantiations']); } finally { if (file_exists($sourceFile)) { unlink($sourceFile); @@ -675,6 +683,66 @@ public static function corruptedAnonymousClassNodesProvider(): Iterator yield 'not an array' => ['invalid']; yield 'entry not an array' => [['invalid']]; yield 'entry with invalid field types' => [[['file' => 1, 'line' => 'x', 'extends' => null]]]; + yield 'entry with invalid implements' => [ + [['file' => '/Foo.php', 'line' => 7, 'extends' => null, 'implements' => ['App\Contract', 1]]], + ]; + yield 'entry with invalid traits' => [ + [['file' => '/Foo.php', 'line' => 7, 'extends' => null, 'traits' => 'invalid']], + ]; + } + + public function testLoadClassNodesRejectsCorruptedFileReferencesPayload(): void + { + $cacheDirectory = $this->createTempDirectory(); + $sourceFile = $cacheDirectory . '/Foo.php'; + $analysisResultCache = new AnalysisResultCache(__DIR__, new FileHashProvider(), $cacheDirectory); + + file_put_contents($sourceFile, 'storeClassNodes($sourceFile, 'config', [$this->makeClassNode($sourceFile)]); + + $cacheFile = $this->firstJsonFile($cacheDirectory); + $payload = json_decode((string) file_get_contents($cacheFile), true); + $this->assertIsArray($payload); + $payload['fileReferences'] = ['App\Contract', 1]; + file_put_contents($cacheFile, json_encode($payload, JSON_THROW_ON_ERROR)); + + $this->assertNull($analysisResultCache->loadClassNodes($sourceFile, 'config')); + } finally { + if (file_exists($sourceFile)) { + unlink($sourceFile); + } + + $this->removeTempDirectory($cacheDirectory); + } + } + + public function testLoadClassNodesRejectsCorruptedFileInstantiationsPayload(): void + { + $cacheDirectory = $this->createTempDirectory(); + $sourceFile = $cacheDirectory . '/Foo.php'; + $analysisResultCache = new AnalysisResultCache(__DIR__, new FileHashProvider(), $cacheDirectory); + + file_put_contents($sourceFile, 'storeClassNodes($sourceFile, 'config', [$this->makeClassNode($sourceFile)]); + + $cacheFile = $this->firstJsonFile($cacheDirectory); + $payload = json_decode((string) file_get_contents($cacheFile), true); + $this->assertIsArray($payload); + $payload['fileInstantiations'] = ['App\Base', 1]; + file_put_contents($cacheFile, json_encode($payload, JSON_THROW_ON_ERROR)); + + $this->assertNull($analysisResultCache->loadClassNodes($sourceFile, 'config')); + } finally { + if (file_exists($sourceFile)) { + unlink($sourceFile); + } + + $this->removeTempDirectory($cacheDirectory); + } } #[DataProvider('corruptedAnonymousClassNodesProvider')] diff --git a/tests/Cli/InitCommandTest.php b/tests/Cli/InitCommandTest.php index 8f08f368..2411e8b7 100644 --- a/tests/Cli/InitCommandTest.php +++ b/tests/Cli/InitCommandTest.php @@ -69,6 +69,11 @@ public static function presetProvider(): iterable ' ->withPreset(Preset::PSR4());', ]; + yield 'yagni' => [ + ['--preset=yagni'], + ' ->withPreset(Preset::YAGNI());', + ]; + yield 'all' => [ ['--preset=all'], " ->withPresets(\n" @@ -77,7 +82,8 @@ public static function presetProvider(): iterable . " Preset::PSR15(),\n" . " Preset::PSR4(),\n" . " Preset::DDD(),\n" - . " Preset::MVC()\n" + . " Preset::MVC(),\n" + . " Preset::YAGNI()\n" . " );", ]; } diff --git a/tests/Cli/StructArmedApplicationCommandRoutingTest.php b/tests/Cli/StructArmedApplicationCommandRoutingTest.php index fff2415a..b1df5d94 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|psr1|psr12|psr15|psr4|all]', + 'structarmed init [--preset=ddd|mvc|psr1|psr12|psr15|psr4|yagni|all]', $output ); $this->assertStringContainsString('structarmed analyse|analyze', $output); diff --git a/tests/Cli/StructArmedApplicationTest.php b/tests/Cli/StructArmedApplicationTest.php index b6b8c8d3..9292b847 100644 --- a/tests/Cli/StructArmedApplicationTest.php +++ b/tests/Cli/StructArmedApplicationTest.php @@ -172,6 +172,11 @@ public static function presetProvider(): iterable ' ->withPreset(Preset::PSR4());', ]; + yield 'yagni' => [ + ['--preset=yagni'], + ' ->withPreset(Preset::YAGNI());', + ]; + yield 'all' => [ ['--preset=all'], " ->withPresets(\n" @@ -180,7 +185,8 @@ public static function presetProvider(): iterable . " Preset::PSR15(),\n" . " Preset::PSR4(),\n" . " Preset::DDD(),\n" - . " Preset::MVC()\n" + . " Preset::MVC(),\n" + . " Preset::YAGNI()\n" . " );", ]; } @@ -570,6 +576,69 @@ public function testAnalyseCommandFixesFixableViolations(): void } } + public function testAnalyseCommandFixesCascadingYagniViolationsInOneRun(): void + { + $basePath = $this->createProjectDirectory(); + + // ChildInterface keeps BaseInterface "used" until the fixer removes it, + // which makes BaseInterface newly unused — a single --fix run must keep + // fixing until no violations remain. + file_put_contents($basePath . '/src/BaseInterface.php', <<<'PHP' +withPreset(Preset::YAGNI(sourcePaths: ['src/'])); +PHP); + + try { + [$exitCode, $output] = $this->runApplication( + [ + 'structarmed', + 'analyze', + '--config=' . $basePath . '/structarmed.php', + '--fix', + '--no-progress', + ], + $basePath + ); + + $this->assertSame(0, $exitCode, $output); + $this->assertStringContainsString('2 violations have been fixed.', $this->withoutAnsi($output)); + $this->assertStringContainsString('No violations found', $output); + $this->assertFileDoesNotExist($basePath . '/src/ChildInterface.php'); + $this->assertFileDoesNotExist($basePath . '/src/BaseInterface.php'); + } finally { + $this->removeTempDirectory($basePath); + } + } + public function testAnalyseCommandTracksComposerJsonProgressAsSingleFile(): void { $basePath = $this->createProjectDirectoryWithMissingComposerPsr4Path(); diff --git a/tests/Preset/PresetTest.php b/tests/Preset/PresetTest.php index 05c2c439..067698f2 100644 --- a/tests/Preset/PresetTest.php +++ b/tests/Preset/PresetTest.php @@ -13,6 +13,11 @@ use Boundwize\StructArmed\Preset\Presets\Psr1Preset; use Boundwize\StructArmed\Preset\Presets\Psr4Preset; use Boundwize\StructArmed\Preset\Presets\ResolvesSourceLayerNameTrait; +use Boundwize\StructArmed\Preset\Presets\YagniPreset; +use Boundwize\StructArmed\Rule\Rules\Class_\ExtendedClassMustBeAbstractOrInstantiatedRule; +use Boundwize\StructArmed\Rule\Rules\Class_\MustBeUsedAbstractClassRule; +use Boundwize\StructArmed\Rule\Rules\Class_\MustBeUsedInterfaceRule; +use Boundwize\StructArmed\Rule\Rules\Class_\MustBeUsedTraitRule; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; @@ -24,8 +29,48 @@ #[CoversClass(Psr15Preset::class)] #[CoversClass(Psr4Preset::class)] #[CoversClass(ResolvesSourceLayerNameTrait::class)] +#[CoversClass(YagniPreset::class)] final class PresetTest extends TestCase { + public function testYagniPresetRegistersSourceLayerAndRules(): void + { + $architecture = Architecture::define(); + + Preset::YAGNI( + sourcePaths: ['src/'], + )->apply($architecture); + + $this->assertSame(['Source' => ['src/']], $architecture->getLayers()); + + $rules = $architecture->getRules(); + $this->assertInstanceOf( + MustBeUsedInterfaceRule::class, + $rules[YagniPreset::INTERFACE_MUST_BE_USED] ?? null + ); + $this->assertInstanceOf( + MustBeUsedAbstractClassRule::class, + $rules[YagniPreset::ABSTRACT_CLASS_MUST_BE_USED] ?? null + ); + $this->assertInstanceOf( + MustBeUsedTraitRule::class, + $rules[YagniPreset::TRAIT_MUST_BE_USED] ?? null + ); + $this->assertInstanceOf( + ExtendedClassMustBeAbstractOrInstantiatedRule::class, + $rules[YagniPreset::EXTENDED_CLASS_MUST_BE_ABSTRACT_OR_INSTANTIATED] ?? null + ); + } + + public function testYagniPresetUsesComposerSourcePathsByDefault(): void + { + $architecture = Architecture::define(); + + Preset::YAGNI()->apply($architecture); + + // A null source path list defers to Composer-discovered PSR-4 paths. + $this->assertSame(['Source' => []], $architecture->getLayers()); + } + public function testPsr1PresetRegistersSourceLayerAndRules(): void { $architecture = Architecture::define(); diff --git a/tests/Rule/Class_/ExtendedClassMustBeAbstractOrInstantiatedRuleFixTest.php b/tests/Rule/Class_/ExtendedClassMustBeAbstractOrInstantiatedRuleFixTest.php new file mode 100644 index 00000000..6c7e6ccb --- /dev/null +++ b/tests/Rule/Class_/ExtendedClassMustBeAbstractOrInstantiatedRuleFixTest.php @@ -0,0 +1,51 @@ +makeTemporaryDirectory('structarmed-yagni-extended'); + $file = $temporaryDirectory . '/SomeBaseClass.php'; + + file_put_contents( + $file, + "assertTrue($extendedClassMustBeAbstractOrInstantiatedRule->fix(new RuleViolation( + message: 'Extended class [App\\SomeBaseClass] must be declared abstract' + . ' or referenced as a dependency', + file: $file, + line: 7, + className: 'App\\SomeBaseClass', + layer: 'Domain', + ))); + $this->assertFileExists($file); + $this->assertStringContainsString( + 'abstract class SomeBaseClass', + (string) file_get_contents($file) + ); + } +} diff --git a/tests/Rule/Class_/ExtendedClassMustBeAbstractOrInstantiatedRuleTest.php b/tests/Rule/Class_/ExtendedClassMustBeAbstractOrInstantiatedRuleTest.php new file mode 100644 index 00000000..a13f5e13 --- /dev/null +++ b/tests/Rule/Class_/ExtendedClassMustBeAbstractOrInstantiatedRuleTest.php @@ -0,0 +1,225 @@ +makeNode(isExtended: true); + + $violation = $extendedClassMustBeAbstractOrInstantiatedRule->evaluate($classNode); + + $this->assertInstanceOf(RuleViolation::class, $violation); + $this->assertStringContainsString('must be declared abstract', $violation->message); + } + + public function testPassesWhenClassIsNotExtended(): void + { + $extendedClassMustBeAbstractOrInstantiatedRule = new ExtendedClassMustBeAbstractOrInstantiatedRule( + layer: 'Domain' + ); + $classNode = $this->makeNode(isExtended: false); + + $this->assertNotInstanceOf( + RuleViolation::class, + $extendedClassMustBeAbstractOrInstantiatedRule->evaluate($classNode) + ); + } + + public function testPassesWhenExtendedClassIsInstantiated(): void + { + $extendedClassMustBeAbstractOrInstantiatedRule = new ExtendedClassMustBeAbstractOrInstantiatedRule( + layer: 'Domain' + ); + $classNode = $this->makeNode(isExtended: true, isInstantiated: true); + + $this->assertNotInstanceOf( + RuleViolation::class, + $extendedClassMustBeAbstractOrInstantiatedRule->evaluate($classNode) + ); + } + + public function testViolatesWhenExtendedClassIsOnlyReferencedButNotInstantiated(): void + { + // Type hints, instanceof, and ::class keep working on an abstract + // class, so a plain reference is not enough to keep it concrete. + $extendedClassMustBeAbstractOrInstantiatedRule = new ExtendedClassMustBeAbstractOrInstantiatedRule( + layer: 'Domain' + ); + $classNode = $this->makeNode(isExtended: true, isReferenced: true); + + $this->assertInstanceOf( + RuleViolation::class, + $extendedClassMustBeAbstractOrInstantiatedRule->evaluate($classNode) + ); + } + + public function testIsExtendedClassAware(): void + { + $this->assertInstanceOf( + ExtendedClassAwareRuleInterface::class, + new ExtendedClassMustBeAbstractOrInstantiatedRule(layer: 'Domain') + ); + } + + public function testIsFixable(): void + { + $this->assertInstanceOf( + FixableInterface::class, + new ExtendedClassMustBeAbstractOrInstantiatedRule(layer: 'Domain') + ); + } + + public function testCreatesAddAbstractClassFixerVisitor(): void + { + $extendedClassMustBeAbstractOrInstantiatedRule = new ExtendedClassMustBeAbstractOrInstantiatedRule( + layer: 'Domain' + ); + $reflectionMethod = new ReflectionMethod( + $extendedClassMustBeAbstractOrInstantiatedRule, + 'createFixerVisitor' + ); + $addAbstractClassVisitor = $reflectionMethod->invoke( + $extendedClassMustBeAbstractOrInstantiatedRule, + new RuleViolation( + message: 'Extended class [App\\BaseRepository] must be declared abstract' + . ' or referenced as a dependency', + file: '/src/BaseRepository.php', + line: 1, + className: 'App\\BaseRepository', + layer: 'Domain', + ) + ); + + $this->assertInstanceOf(AddAbstractClassVisitor::class, $addAbstractClassVisitor); + } + + public function testDoesNotApplyToWrongLayer(): void + { + $extendedClassMustBeAbstractOrInstantiatedRule = new ExtendedClassMustBeAbstractOrInstantiatedRule( + layer: 'Domain' + ); + $classNode = $this->makeNode(layer: 'Infrastructure'); + + $this->assertFalse($extendedClassMustBeAbstractOrInstantiatedRule->appliesTo($classNode)); + } + + public function testDoesNotApplyToAbstractClasses(): void + { + $extendedClassMustBeAbstractOrInstantiatedRule = new ExtendedClassMustBeAbstractOrInstantiatedRule( + layer: 'Domain' + ); + $classNode = $this->makeNode(isAbstract: true); + + $this->assertFalse($extendedClassMustBeAbstractOrInstantiatedRule->appliesTo($classNode)); + } + + public function testDoesNotApplyToInterfaces(): void + { + $extendedClassMustBeAbstractOrInstantiatedRule = new ExtendedClassMustBeAbstractOrInstantiatedRule( + layer: 'Domain' + ); + $classNode = $this->makeNode(isInterface: true); + + $this->assertFalse($extendedClassMustBeAbstractOrInstantiatedRule->appliesTo($classNode)); + } + + public function testDoesNotApplyToTraits(): void + { + $extendedClassMustBeAbstractOrInstantiatedRule = new ExtendedClassMustBeAbstractOrInstantiatedRule( + layer: 'Domain' + ); + $classNode = $this->makeNode(isTrait: true); + + $this->assertFalse($extendedClassMustBeAbstractOrInstantiatedRule->appliesTo($classNode)); + } + + public function testDoesNotApplyToEnums(): void + { + $extendedClassMustBeAbstractOrInstantiatedRule = new ExtendedClassMustBeAbstractOrInstantiatedRule( + layer: 'Domain' + ); + $classNode = $this->makeNode(isEnum: true); + + $this->assertFalse($extendedClassMustBeAbstractOrInstantiatedRule->appliesTo($classNode)); + } + + public function testAppliesToMatchingPattern(): void + { + $extendedClassMustBeAbstractOrInstantiatedRule = new ExtendedClassMustBeAbstractOrInstantiatedRule( + layer: 'Domain', + classNamePattern: '/Repository$/' + ); + $classNode = $this->makeNode(); + + $this->assertTrue($extendedClassMustBeAbstractOrInstantiatedRule->appliesTo($classNode)); + } + + public function testDoesNotApplyToNonMatchingPattern(): void + { + $extendedClassMustBeAbstractOrInstantiatedRule = new ExtendedClassMustBeAbstractOrInstantiatedRule( + layer: 'Domain', + classNamePattern: '/Service$/' + ); + $classNode = $this->makeNode(); + + $this->assertFalse($extendedClassMustBeAbstractOrInstantiatedRule->appliesTo($classNode)); + } + + public function testAppliesToLayerWhenNoPatternConfigured(): void + { + $extendedClassMustBeAbstractOrInstantiatedRule = new ExtendedClassMustBeAbstractOrInstantiatedRule( + layer: 'Domain' + ); + $classNode = $this->makeNode(); + + $this->assertTrue($extendedClassMustBeAbstractOrInstantiatedRule->appliesTo($classNode)); + } +} diff --git a/tests/Rule/Class_/MustBeUsedAbstractClassRuleFixTest.php b/tests/Rule/Class_/MustBeUsedAbstractClassRuleFixTest.php new file mode 100644 index 00000000..b3dd1d05 --- /dev/null +++ b/tests/Rule/Class_/MustBeUsedAbstractClassRuleFixTest.php @@ -0,0 +1,43 @@ +makeTemporaryDirectory('structarmed-yagni-abstract'); + $file = $temporaryDirectory . '/AbstractHandler.php'; + + file_put_contents( + $file, + "assertTrue($mustBeUsedAbstractClassRule->fix(new RuleViolation( + message: 'Abstract class [App\\AbstractHandler] must be extended by a class', + file: $file, + line: 7, + className: 'App\\AbstractHandler', + layer: 'Domain', + ))); + $this->assertFileDoesNotExist($file); + } +} diff --git a/tests/Rule/Class_/MustBeUsedAbstractClassRuleTest.php b/tests/Rule/Class_/MustBeUsedAbstractClassRuleTest.php new file mode 100644 index 00000000..0bed47aa --- /dev/null +++ b/tests/Rule/Class_/MustBeUsedAbstractClassRuleTest.php @@ -0,0 +1,176 @@ +makeNode(isExtended: true); + + $this->assertNotInstanceOf( + RuleViolation::class, + $mustBeUsedAbstractClassRule->evaluate($classNode) + ); + } + + public function testPassesWhenAbstractClassIsReferencedAsDependency(): void + { + $mustBeUsedAbstractClassRule = new MustBeUsedAbstractClassRule(layer: 'Domain'); + $classNode = $this->makeNode(isReferenced: true); + + $this->assertNotInstanceOf( + RuleViolation::class, + $mustBeUsedAbstractClassRule->evaluate($classNode) + ); + } + + public function testViolatesWhenAbstractClassIsNotExtended(): void + { + $mustBeUsedAbstractClassRule = new MustBeUsedAbstractClassRule(layer: 'Domain'); + $classNode = $this->makeNode(isExtended: false); + + $violation = $mustBeUsedAbstractClassRule->evaluate($classNode); + + $this->assertInstanceOf(RuleViolation::class, $violation); + $this->assertStringContainsString('must be extended', $violation->message); + } + + public function testIsExtendedClassAware(): void + { + $this->assertInstanceOf( + ExtendedClassAwareRuleInterface::class, + new MustBeUsedAbstractClassRule(layer: 'Domain') + ); + } + + public function testIsFixable(): void + { + $this->assertInstanceOf(FixableInterface::class, new MustBeUsedAbstractClassRule(layer: 'Domain')); + } + + public function testCreatesRemoveClassLikeFixerVisitor(): void + { + $mustBeUsedAbstractClassRule = new MustBeUsedAbstractClassRule(layer: 'Domain'); + $reflectionMethod = new ReflectionMethod( + $mustBeUsedAbstractClassRule, + 'createFixerVisitor' + ); + $removeClassLikeVisitor = $reflectionMethod->invoke( + $mustBeUsedAbstractClassRule, + new RuleViolation( + message: 'Abstract class [App\\AbstractHandler] must be extended by a class', + file: '/src/AbstractHandler.php', + line: 1, + className: 'App\\AbstractHandler', + layer: 'Domain', + ) + ); + + $this->assertInstanceOf(RemoveClassLikeVisitor::class, $removeClassLikeVisitor); + } + + public function testDoesNotApplyToWrongLayer(): void + { + $mustBeUsedAbstractClassRule = new MustBeUsedAbstractClassRule(layer: 'Domain'); + $classNode = $this->makeNode(layer: 'Infrastructure'); + + $this->assertFalse($mustBeUsedAbstractClassRule->appliesTo($classNode)); + } + + public function testDoesNotApplyToConcreteClasses(): void + { + $mustBeUsedAbstractClassRule = new MustBeUsedAbstractClassRule(layer: 'Domain'); + $classNode = $this->makeNode(isAbstract: false); + + $this->assertFalse($mustBeUsedAbstractClassRule->appliesTo($classNode)); + } + + public function testDoesNotApplyToInterfaces(): void + { + $mustBeUsedAbstractClassRule = new MustBeUsedAbstractClassRule(layer: 'Domain'); + $classNode = $this->makeNode(isInterface: true); + + $this->assertFalse($mustBeUsedAbstractClassRule->appliesTo($classNode)); + } + + public function testDoesNotApplyToTraits(): void + { + $mustBeUsedAbstractClassRule = new MustBeUsedAbstractClassRule(layer: 'Domain'); + $classNode = $this->makeNode(isTrait: true); + + $this->assertFalse($mustBeUsedAbstractClassRule->appliesTo($classNode)); + } + + public function testAppliesToLayerWhenNoPatternConfigured(): void + { + $mustBeUsedAbstractClassRule = new MustBeUsedAbstractClassRule(layer: 'Domain'); + $classNode = $this->makeNode(); + + $this->assertTrue($mustBeUsedAbstractClassRule->appliesTo($classNode)); + } + + public function testAppliesToMatchingPattern(): void + { + $mustBeUsedAbstractClassRule = new MustBeUsedAbstractClassRule( + layer: 'Domain', + classNamePattern: '/^App\\\\Domain\\\\Abstract/' + ); + $classNode = $this->makeNode(); + + $this->assertTrue($mustBeUsedAbstractClassRule->appliesTo($classNode)); + } + + public function testDoesNotApplyToNonMatchingPattern(): void + { + $mustBeUsedAbstractClassRule = new MustBeUsedAbstractClassRule( + layer: 'Domain', + classNamePattern: '/Base$/' + ); + $classNode = $this->makeNode(); + + $this->assertFalse($mustBeUsedAbstractClassRule->appliesTo($classNode)); + } +} diff --git a/tests/Rule/Class_/MustBeUsedInterfaceRuleFixTest.php b/tests/Rule/Class_/MustBeUsedInterfaceRuleFixTest.php new file mode 100644 index 00000000..152db619 --- /dev/null +++ b/tests/Rule/Class_/MustBeUsedInterfaceRuleFixTest.php @@ -0,0 +1,109 @@ +makeTemporaryDirectory('structarmed-yagni-interface'); + $file = $temporaryDirectory . '/UnusedInterface.php'; + + file_put_contents( + $file, + "assertTrue($mustBeUsedInterfaceRule->fix(new RuleViolation( + message: 'Interface [App\\UnusedInterface] must be implemented by a class' + . ' or extended by another interface', + file: $file, + line: 7, + className: 'App\\UnusedInterface', + layer: 'Domain', + ))); + $this->assertFileDoesNotExist($file); + } + + public function testFixKeepsFileWhenDeclareBlockContainsExecutableCode(): void + { + $temporaryDirectory = $this->makeTemporaryDirectory('structarmed-yagni-interface'); + $file = $temporaryDirectory . '/ticks.php'; + + // The block form `declare(ticks=1) { ... }` carries executable + // statements — removing the interface must not delete the file. + file_put_contents( + $file, + "assertTrue($mustBeUsedInterfaceRule->fix(new RuleViolation( + message: 'Interface [UnusedInterface] must be implemented by a class' + . ' or extended by another interface', + file: $file, + line: 7, + className: 'UnusedInterface', + layer: 'Domain', + ))); + $this->assertFileExists($file); + + $fixedCode = (string) file_get_contents($file); + + $this->assertStringNotContainsString('interface UnusedInterface', $fixedCode); + $this->assertStringContainsString("echo 'KEEP ME';", $fixedCode); + } + + public function testFixKeepsFileWhenOtherCodeRemains(): void + { + $temporaryDirectory = $this->makeTemporaryDirectory('structarmed-yagni-interface'); + $file = $temporaryDirectory . '/Contracts.php'; + + file_put_contents( + $file, + "assertTrue($mustBeUsedInterfaceRule->fix(new RuleViolation( + message: 'Interface [App\\UnusedInterface] must be implemented by a class' + . ' or extended by another interface', + file: $file, + line: 7, + className: 'App\\UnusedInterface', + layer: 'Domain', + ))); + $this->assertFileExists($file); + + $fixedCode = (string) file_get_contents($file); + + $this->assertStringNotContainsString('interface UnusedInterface', $fixedCode); + $this->assertStringContainsString('final class Order', $fixedCode); + } +} diff --git a/tests/Rule/Class_/MustBeUsedInterfaceRuleTest.php b/tests/Rule/Class_/MustBeUsedInterfaceRuleTest.php new file mode 100644 index 00000000..4506bf86 --- /dev/null +++ b/tests/Rule/Class_/MustBeUsedInterfaceRuleTest.php @@ -0,0 +1,164 @@ +makeNode(isImplemented: true); + + $this->assertNotInstanceOf( + RuleViolation::class, + $mustBeUsedInterfaceRule->evaluate($classNode) + ); + } + + public function testPassesWhenInterfaceIsReferencedAsDependency(): void + { + $mustBeUsedInterfaceRule = new MustBeUsedInterfaceRule(layer: 'Domain'); + $classNode = $this->makeNode(isReferenced: true); + + $this->assertNotInstanceOf( + RuleViolation::class, + $mustBeUsedInterfaceRule->evaluate($classNode) + ); + } + + public function testViolatesWhenInterfaceIsNotImplemented(): void + { + $mustBeUsedInterfaceRule = new MustBeUsedInterfaceRule(layer: 'Domain'); + $classNode = $this->makeNode(isImplemented: false); + + $violation = $mustBeUsedInterfaceRule->evaluate($classNode); + + $this->assertInstanceOf(RuleViolation::class, $violation); + $this->assertStringContainsString('must be implemented', $violation->message); + } + + public function testIsUsedInterfaceAware(): void + { + $this->assertInstanceOf( + UsedInterfaceAwareRuleInterface::class, + new MustBeUsedInterfaceRule(layer: 'Domain') + ); + } + + public function testIsFixable(): void + { + $this->assertInstanceOf(FixableInterface::class, new MustBeUsedInterfaceRule(layer: 'Domain')); + } + + public function testCreatesRemoveClassLikeFixerVisitor(): void + { + $mustBeUsedInterfaceRule = new MustBeUsedInterfaceRule(layer: 'Domain'); + $reflectionMethod = new ReflectionMethod($mustBeUsedInterfaceRule, 'createFixerVisitor'); + $removeClassLikeVisitor = $reflectionMethod->invoke( + $mustBeUsedInterfaceRule, + new RuleViolation( + message: 'Interface [App\\Unused] must be implemented by a class or extended by another interface', + file: '/src/Unused.php', + line: 1, + className: 'App\\Unused', + layer: 'Domain', + ) + ); + + $this->assertInstanceOf(RemoveClassLikeVisitor::class, $removeClassLikeVisitor); + } + + public function testDoesNotApplyToWrongLayer(): void + { + $mustBeUsedInterfaceRule = new MustBeUsedInterfaceRule(layer: 'Domain'); + $classNode = $this->makeNode(layer: 'Infrastructure'); + + $this->assertFalse($mustBeUsedInterfaceRule->appliesTo($classNode)); + } + + public function testDoesNotApplyToClasses(): void + { + $mustBeUsedInterfaceRule = new MustBeUsedInterfaceRule(layer: 'Domain'); + $classNode = $this->makeNode(isInterface: false); + + $this->assertFalse($mustBeUsedInterfaceRule->appliesTo($classNode)); + } + + public function testDoesNotApplyToTraits(): void + { + $mustBeUsedInterfaceRule = new MustBeUsedInterfaceRule(layer: 'Domain'); + $classNode = $this->makeNode(isInterface: false, isTrait: true); + + $this->assertFalse($mustBeUsedInterfaceRule->appliesTo($classNode)); + } + + public function testAppliesToLayerWhenNoPatternConfigured(): void + { + $mustBeUsedInterfaceRule = new MustBeUsedInterfaceRule(layer: 'Domain'); + $classNode = $this->makeNode(); + + $this->assertTrue($mustBeUsedInterfaceRule->appliesTo($classNode)); + } + + public function testAppliesToMatchingPattern(): void + { + $mustBeUsedInterfaceRule = new MustBeUsedInterfaceRule( + layer: 'Domain', + classNamePattern: '/Interface$/' + ); + $classNode = $this->makeNode(className: 'App\\Domain\\OrderRepositoryInterface'); + + $this->assertTrue($mustBeUsedInterfaceRule->appliesTo($classNode)); + } + + public function testDoesNotApplyToNonMatchingPattern(): void + { + $mustBeUsedInterfaceRule = new MustBeUsedInterfaceRule( + layer: 'Domain', + classNamePattern: '/Repository$/' + ); + $classNode = $this->makeNode(className: 'App\\Domain\\OrderRepositoryInterface'); + + $this->assertFalse($mustBeUsedInterfaceRule->appliesTo($classNode)); + } +} diff --git a/tests/Rule/Class_/MustBeUsedTraitRuleFixTest.php b/tests/Rule/Class_/MustBeUsedTraitRuleFixTest.php new file mode 100644 index 00000000..bf1d0b1e --- /dev/null +++ b/tests/Rule/Class_/MustBeUsedTraitRuleFixTest.php @@ -0,0 +1,43 @@ +makeTemporaryDirectory('structarmed-yagni-trait'); + $file = $temporaryDirectory . '/UnusedTrait.php'; + + file_put_contents( + $file, + "assertTrue($mustBeUsedTraitRule->fix(new RuleViolation( + message: 'Trait [App\\UnusedTrait] must be used by a class, trait, or enum', + file: $file, + line: 7, + className: 'App\\UnusedTrait', + layer: 'Domain', + ))); + $this->assertFileDoesNotExist($file); + } +} diff --git a/tests/Rule/Class_/MustBeUsedTraitRuleTest.php b/tests/Rule/Class_/MustBeUsedTraitRuleTest.php new file mode 100644 index 00000000..16b789af --- /dev/null +++ b/tests/Rule/Class_/MustBeUsedTraitRuleTest.php @@ -0,0 +1,142 @@ +makeNode(isReferenced: true); + + $this->assertNotInstanceOf(RuleViolation::class, $mustBeUsedTraitRule->evaluate($classNode)); + } + + public function testViolatesWhenTraitIsNotUsed(): void + { + $mustBeUsedTraitRule = new MustBeUsedTraitRule(layer: 'Domain'); + $classNode = $this->makeNode(isReferenced: false); + + $violation = $mustBeUsedTraitRule->evaluate($classNode); + + $this->assertInstanceOf(RuleViolation::class, $violation); + $this->assertStringContainsString('must be used', $violation->message); + } + + public function testIsUsedTraitAware(): void + { + $this->assertInstanceOf( + UsedTraitAwareRuleInterface::class, + new MustBeUsedTraitRule(layer: 'Domain') + ); + } + + public function testIsFixable(): void + { + $this->assertInstanceOf(FixableInterface::class, new MustBeUsedTraitRule(layer: 'Domain')); + } + + public function testCreatesRemoveClassLikeFixerVisitor(): void + { + $mustBeUsedTraitRule = new MustBeUsedTraitRule(layer: 'Domain'); + $reflectionMethod = new ReflectionMethod($mustBeUsedTraitRule, 'createFixerVisitor'); + $removeClassLikeVisitor = $reflectionMethod->invoke( + $mustBeUsedTraitRule, + new RuleViolation( + message: 'Trait [App\\UnusedTrait] must be used by a class, trait, or enum', + file: '/src/UnusedTrait.php', + line: 1, + className: 'App\\UnusedTrait', + layer: 'Domain', + ) + ); + + $this->assertInstanceOf(RemoveClassLikeVisitor::class, $removeClassLikeVisitor); + } + + public function testDoesNotApplyToWrongLayer(): void + { + $mustBeUsedTraitRule = new MustBeUsedTraitRule(layer: 'Domain'); + $classNode = $this->makeNode(layer: 'Infrastructure'); + + $this->assertFalse($mustBeUsedTraitRule->appliesTo($classNode)); + } + + public function testDoesNotApplyToClasses(): void + { + $mustBeUsedTraitRule = new MustBeUsedTraitRule(layer: 'Domain'); + $classNode = $this->makeNode(isTrait: false); + + $this->assertFalse($mustBeUsedTraitRule->appliesTo($classNode)); + } + + public function testDoesNotApplyToInterfaces(): void + { + $mustBeUsedTraitRule = new MustBeUsedTraitRule(layer: 'Domain'); + $classNode = $this->makeNode(isInterface: true, isTrait: false); + + $this->assertFalse($mustBeUsedTraitRule->appliesTo($classNode)); + } + + public function testAppliesToLayerWhenNoPatternConfigured(): void + { + $mustBeUsedTraitRule = new MustBeUsedTraitRule(layer: 'Domain'); + $classNode = $this->makeNode(); + + $this->assertTrue($mustBeUsedTraitRule->appliesTo($classNode)); + } + + public function testAppliesToMatchingPattern(): void + { + $mustBeUsedTraitRule = new MustBeUsedTraitRule(layer: 'Domain', classNamePattern: '/Trait$/'); + $classNode = $this->makeNode(); + + $this->assertTrue($mustBeUsedTraitRule->appliesTo($classNode)); + } + + public function testDoesNotApplyToNonMatchingPattern(): void + { + $mustBeUsedTraitRule = new MustBeUsedTraitRule(layer: 'Domain', classNamePattern: '/Helper$/'); + $classNode = $this->makeNode(); + + $this->assertFalse($mustBeUsedTraitRule->appliesTo($classNode)); + } +} diff --git a/tests/Rule/Fixer/PhpParser/ClassLike/RemoveClassLikeVisitorTest.php b/tests/Rule/Fixer/PhpParser/ClassLike/RemoveClassLikeVisitorTest.php new file mode 100644 index 00000000..07bf6803 --- /dev/null +++ b/tests/Rule/Fixer/PhpParser/ClassLike/RemoveClassLikeVisitorTest.php @@ -0,0 +1,80 @@ +namespacedName = new Name('App\\UnusedInterface'); + + $statements = (new NodeTraverser(new RemoveClassLikeVisitor('App\\UnusedInterface'))) + ->traverse([$interface]); + + $this->assertSame([], $statements); + } + + public function testRemovesMatchingAbstractClass(): void + { + $class = new Class_('AbstractHandler'); + $class->namespacedName = new Name('App\\AbstractHandler'); + + $statements = (new NodeTraverser(new RemoveClassLikeVisitor('App\\AbstractHandler'))) + ->traverse([$class]); + + $this->assertSame([], $statements); + } + + public function testRemovesMatchingTrait(): void + { + $trait = new Trait_('UnusedTrait'); + $trait->namespacedName = new Name('App\\UnusedTrait'); + + $statements = (new NodeTraverser(new RemoveClassLikeVisitor('App\\UnusedTrait'))) + ->traverse([$trait]); + + $this->assertSame([], $statements); + } + + public function testKeepsNonMatchingClassLike(): void + { + $interface = new Interface_('UsedInterface'); + $interface->namespacedName = new Name('App\\UsedInterface'); + + $statements = (new NodeTraverser(new RemoveClassLikeVisitor('App\\UnusedInterface'))) + ->traverse([$interface]); + + $this->assertSame([$interface], $statements); + } + + public function testKeepsAnonymousClass(): void + { + $class = new Class_(null); + + $statements = (new NodeTraverser(new RemoveClassLikeVisitor('App\\UnusedInterface'))) + ->traverse([$class]); + + $this->assertSame([$class], $statements); + } + + public function testDoesNotRemoveNonClassLikeNode(): void + { + $removeClassLikeVisitor = new RemoveClassLikeVisitor('App\\UnusedInterface'); + + $this->assertNull($removeClassLikeVisitor->leaveNode(new ClassMethod('save'))); + } +} diff --git a/tests/Rule/Fixer/PhpParser/Class_/AddAbstractClassVisitorTest.php b/tests/Rule/Fixer/PhpParser/Class_/AddAbstractClassVisitorTest.php new file mode 100644 index 00000000..758032c5 --- /dev/null +++ b/tests/Rule/Fixer/PhpParser/Class_/AddAbstractClassVisitorTest.php @@ -0,0 +1,80 @@ +namespacedName = new Name('App\\BaseRepository'); + + (new NodeTraverser($addAbstractClassVisitor))->traverse([$class]); + + $this->assertSame(Modifiers::ABSTRACT, $class->flags); + } + + public function testDoesNotChangeNonClassNode(): void + { + $addAbstractClassVisitor = new AddAbstractClassVisitor('App\\BaseRepository'); + + $this->assertNotInstanceOf(Node::class, $addAbstractClassVisitor->enterNode(new ClassMethod('save'))); + } + + public function testDoesNotChangeAlreadyAbstractClass(): void + { + $class = new Class_('BaseRepository', ['flags' => Modifiers::ABSTRACT]); + $addAbstractClassVisitor = new AddAbstractClassVisitor('App\\BaseRepository'); + $class->namespacedName = new Name('App\\BaseRepository'); + + (new NodeTraverser($addAbstractClassVisitor))->traverse([$class]); + + $this->assertSame(Modifiers::ABSTRACT, $class->flags); + } + + public function testDoesNotChangeFinalClass(): void + { + $class = new Class_('BaseRepository', ['flags' => Modifiers::FINAL]); + $addAbstractClassVisitor = new AddAbstractClassVisitor('App\\BaseRepository'); + $class->namespacedName = new Name('App\\BaseRepository'); + + (new NodeTraverser($addAbstractClassVisitor))->traverse([$class]); + + $this->assertSame(Modifiers::FINAL, $class->flags); + } + + public function testDoesNotChangeDifferentClass(): void + { + $class = new Class_('BaseRepository'); + $addAbstractClassVisitor = new AddAbstractClassVisitor('App\\BaseRepository'); + $class->namespacedName = new Name('App\\OtherRepository'); + + (new NodeTraverser($addAbstractClassVisitor))->traverse([$class]); + + $this->assertSame(0, $class->flags); + } + + public function testDoesNotChangeAnonymousClass(): void + { + $class = new Class_(null); + $addAbstractClassVisitor = new AddAbstractClassVisitor('App\\BaseRepository'); + + (new NodeTraverser($addAbstractClassVisitor))->traverse([$class]); + + $this->assertSame(0, $class->flags); + } +}