diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..0403ecf --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,39 @@ +name: CI + +on: + push: + pull_request: + +permissions: + contents: read + +jobs: + tests: + name: Tests (PHP ${{ matrix.php }}) + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + php: ['8.1', '8.2', '8.3', '8.4'] + steps: + - uses: actions/checkout@v7 + - uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php }} + coverage: none + - run: composer update --no-interaction --no-progress --prefer-dist + - run: vendor/bin/phpunit + + static-analysis: + name: PHPStan and architecture + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: shivammathur/setup-php@v2 + with: + php-version: '8.4' + coverage: none + - run: composer update --no-interaction --no-progress --prefer-dist + - run: vendor/bin/phpstan analyse --no-progress --memory-limit=1G + - name: php-deps checks itself + run: php analyze.php --check --format=github --quiet diff --git a/README.md b/README.md index d98b828..cd1abdd 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,7 @@ **Static analysis tool for visualizing PHP class dependencies and call graphs.** +[![CI](https://github.com/furybee/php-deps/actions/workflows/ci.yml/badge.svg)](https://github.com/furybee/php-deps/actions/workflows/ci.yml) [![License: MIT](https://img.shields.io/badge/License-MIT-blue.svg)](LICENSE) [![PHP](https://img.shields.io/badge/PHP-8.1+-8892BF.svg)](https://www.php.net) [![Laravel](https://img.shields.io/badge/Laravel-Compatible-FF2D20.svg)](https://laravel.com) @@ -303,9 +304,13 @@ php-deps/ ```bash composer install -composer test +composer test # PHPUnit +composer analyse # PHPStan, level max on src and tests +php analyze.php --check # php-deps checks its own layers and cycles (php-deps.php) ``` +CI runs the tests on PHP 8.1 to 8.4, PHPStan and the self-check. + ## Performance Tested on a real Laravel project: diff --git a/composer.json b/composer.json index fac7ff3..f1e3b57 100644 --- a/composer.json +++ b/composer.json @@ -8,7 +8,10 @@ "nikic/php-parser": "^5.0" }, "require-dev": { - "phpunit/phpunit": "^10.5" + "phpunit/phpunit": "^10.5", + "phpstan/phpstan": "^2.1", + "phpstan/phpstan-phpunit": "^2.0", + "phpstan/extension-installer": "^1.4" }, "autoload": { "psr-4": { @@ -24,6 +27,13 @@ "analyze.php" ], "scripts": { - "test": "phpunit" + "test": "phpunit", + "analyse": "phpstan analyse --memory-limit=1G" + }, + "config": { + "allow-plugins": { + "phpstan/extension-installer": true + }, + "sort-packages": true } } diff --git a/php-deps.php b/php-deps.php new file mode 100644 index 0000000..951bdf2 --- /dev/null +++ b/php-deps.php @@ -0,0 +1,19 @@ + ['src'], + 'layers' => [ + 'Console' => ['DepsGraph\Console\**'], + 'Architecture' => ['DepsGraph\Architecture\**'], + 'Metrics' => ['DepsGraph\Metrics\**'], + 'Analysis' => ['DepsGraph\*'], + ], + 'rules' => [ + 'Analysis' => [], + 'Metrics' => ['Analysis'], + 'Architecture' => ['Metrics', 'Analysis'], + ], + 'cycles' => ['level' => 'class'], +]; diff --git a/phpstan.neon.dist b/phpstan.neon.dist new file mode 100644 index 0000000..03d18f1 --- /dev/null +++ b/phpstan.neon.dist @@ -0,0 +1,8 @@ +parameters: + level: max + paths: + - src + - tests + - analyze.php + excludePaths: + - tests/Fixtures diff --git a/src/Analyzer.php b/src/Analyzer.php index fe7b732..4609a29 100644 --- a/src/Analyzer.php +++ b/src/Analyzer.php @@ -11,6 +11,10 @@ use PhpParser\Parser; use PhpParser\ParserFactory; +/** + * @phpstan-import-type ClassData from ClassVisitor + * @phpstan-import-type Graph from GraphBuilder + */ final class Analyzer { private readonly string $basePath; @@ -39,7 +43,7 @@ public function __construct( } /** - * @return array{root: string, nodes: array, edges: array, stats: array} + * @return Graph */ public function analyze(): array { @@ -62,7 +66,7 @@ public function analyze(): array /** * @param string[] $files - * @return array> + * @return array */ private function extractClasses(array $files): array { @@ -115,7 +119,7 @@ private function traverseFiles(array $files, NodeVisitorAbstract $visitor, \Clos } /** - * @return string[] + * @return list */ private function findPhpFiles(): array { @@ -137,6 +141,7 @@ private function findPhpFiles(): array ), ); + /** @var \SplFileInfo $file */ foreach ($iterator as $file) { if ($file->getExtension() === 'php') { $files[] = $file->getPathname(); diff --git a/src/Architecture/ArchitectureConfig.php b/src/Architecture/ArchitectureConfig.php index 7e40ad7..f265219 100644 --- a/src/Architecture/ArchitectureConfig.php +++ b/src/Architecture/ArchitectureConfig.php @@ -46,10 +46,10 @@ public static function fromFile(string $file): self } return new self( - layers: self::listMap($config['layers'] ?? [], 'layers'), - rules: self::listMap($config['rules'] ?? [], 'rules'), - paths: array_values((array) ($config['paths'] ?? [])), - exclude: isset($config['exclude']) ? array_values((array) $config['exclude']) : null, + layers: ConfigValue::listMap($config['layers'] ?? [], 'layers'), + rules: ConfigValue::listMap($config['rules'] ?? [], 'rules'), + paths: ConfigValue::stringList($config['paths'] ?? [], 'paths'), + exclude: isset($config['exclude']) ? ConfigValue::stringList($config['exclude'], 'exclude') : null, baseline: is_string($baseline) ? $baseline : null, cycles: self::cycleRule($config['cycles'] ?? null), ); @@ -69,21 +69,4 @@ private static function cycleRule(mixed $value): ?CycleRule default => throw new InvalidConfigException('"cycles" must be a boolean or an array'), }; } - - /** - * @return array - */ - private static function listMap(mixed $value, string $key): array - { - if (!is_array($value)) { - throw new InvalidConfigException("\"{$key}\" must be an array of lists"); - } - - $map = []; - foreach ($value as $name => $items) { - $map[(string) $name] = array_values(array_map('strval', (array) $items)); - } - - return $map; - } } diff --git a/src/Architecture/Baseline.php b/src/Architecture/Baseline.php index cfc9b16..a8c2c7c 100644 --- a/src/Architecture/Baseline.php +++ b/src/Architecture/Baseline.php @@ -32,7 +32,7 @@ public static function load(string $file): self throw new InvalidConfigException("Invalid baseline file: {$file}"); } - return new self(array_fill_keys(array_map('strval', $keys), true)); + return new self(array_fill_keys(ConfigValue::stringList($keys, 'baseline'), true)); } /** diff --git a/src/Architecture/ConfigValue.php b/src/Architecture/ConfigValue.php new file mode 100644 index 0000000..5ab4a3a --- /dev/null +++ b/src/Architecture/ConfigValue.php @@ -0,0 +1,62 @@ + + */ + public static function stringList(mixed $value, string $key): array + { + $values = is_array($value) ? array_values($value) : [$value]; + foreach ($values as $item) { + if (!is_string($item)) { + throw new InvalidConfigException("\"{$key}\" must be a string or a list of strings"); + } + } + + /** @var list $values */ + return $values; + } + + /** + * @return array> + */ + public static function listMap(mixed $value, string $key): array + { + if (!is_array($value)) { + throw new InvalidConfigException("\"{$key}\" must be an array of lists"); + } + + $map = []; + foreach ($value as $name => $items) { + $map[(string) $name] = self::stringList($items, "{$key}.{$name}"); + } + + return $map; + } + + public static function string(mixed $value, string $key): string + { + if (!is_string($value)) { + throw new InvalidConfigException("\"{$key}\" must be a string"); + } + + return $value; + } + + public static function int(mixed $value, string $key): int + { + if (!is_int($value)) { + throw new InvalidConfigException("\"{$key}\" must be an integer"); + } + + return $value; + } +} diff --git a/src/Architecture/CycleRule.php b/src/Architecture/CycleRule.php index 8844eb5..78f9cfb 100644 --- a/src/Architecture/CycleRule.php +++ b/src/Architecture/CycleRule.php @@ -29,15 +29,15 @@ public function __construct( } /** - * @param array $config + * @param array $config */ public static function fromArray(array $config): self { return new self( - level: (string) ($config['level'] ?? self::LEVEL_NAMESPACE), - depth: (int) ($config['depth'] ?? 0), - scope: array_values(array_map('strval', (array) ($config['scope'] ?? ['**']))), - ignore: array_values(array_map('strval', (array) ($config['ignore'] ?? []))), + level: ConfigValue::string($config['level'] ?? self::LEVEL_NAMESPACE, 'cycles.level'), + depth: ConfigValue::int($config['depth'] ?? 0, 'cycles.depth'), + scope: ConfigValue::stringList($config['scope'] ?? ['**'], 'cycles.scope'), + ignore: ConfigValue::stringList($config['ignore'] ?? [], 'cycles.ignore'), ); } diff --git a/src/Architecture/CycleRuleChecker.php b/src/Architecture/CycleRuleChecker.php index bff9cc5..4849f2b 100644 --- a/src/Architecture/CycleRuleChecker.php +++ b/src/Architecture/CycleRuleChecker.php @@ -7,6 +7,9 @@ use DepsGraph\Metrics\CycleDetector; use DepsGraph\Metrics\DependencyMap; +/** + * @phpstan-import-type MinimalGraph from DependencyMap + */ final class CycleRuleChecker { public function __construct( @@ -16,7 +19,7 @@ public function __construct( } /** - * @param array{nodes: array>, edges: array} $graph + * @param MinimalGraph $graph * @return CycleViolation[] */ public function check(array $graph): array @@ -32,7 +35,7 @@ public function check(array $graph): array $files = []; foreach ($graph['nodes'] as $node) { if ($node['type'] === 'class') { - $files[$node['id']] = (string) ($node['file'] ?? ''); + $files[$node['id']] = $node['file'] ?? ''; } } diff --git a/src/Architecture/DependencyRuleChecker.php b/src/Architecture/DependencyRuleChecker.php index bcdf3bc..7ef911b 100644 --- a/src/Architecture/DependencyRuleChecker.php +++ b/src/Architecture/DependencyRuleChecker.php @@ -4,9 +4,13 @@ namespace DepsGraph\Architecture; +use DepsGraph\Metrics\DependencyMap; + /** * Checks graph edges against the allowed layer dependencies. * Classes outside every layer are not checked, nor are layers without rules. + * + * @phpstan-import-type MinimalGraph from DependencyMap */ final class DependencyRuleChecker { @@ -15,7 +19,7 @@ public function __construct(private readonly ArchitectureConfig $config) } /** - * @param array{nodes: array>, edges: array} $graph + * @param MinimalGraph $graph * @return Violation[] one per class pair, at its first occurrence */ public function check(array $graph): array @@ -24,7 +28,7 @@ public function check(array $graph): array $files = []; foreach ($graph['nodes'] as $node) { if ($node['type'] === 'class') { - $files[$node['id']] = (string) ($node['file'] ?? ''); + $files[$node['id']] = $node['file'] ?? ''; } } diff --git a/src/CallVisitor.php b/src/CallVisitor.php index ad4080d..5138bd1 100644 --- a/src/CallVisitor.php +++ b/src/CallVisitor.php @@ -13,6 +13,8 @@ * Expects PhpParser's NameResolver to run before it in the same traverser. * * Edges are raw: targets may be external or inherited members, GraphBuilder sorts them out. + * + * @phpstan-import-type Edge from GraphBuilder */ final class CallVisitor extends NodeVisitorAbstract { @@ -20,7 +22,7 @@ final class CallVisitor extends NodeVisitorAbstract private const CONTAINER_METHODS = ['make', 'makewith']; private const CONTAINER_CLASSES = ['Illuminate\Support\Facades\App', 'App', 'Illuminate\Container\Container']; - /** @var array */ + /** @var list */ private array $edges = []; /** @var ClassScope[] */ @@ -36,7 +38,7 @@ public function __construct( } /** - * @return array + * @return list */ public function getEdges(): array { @@ -160,6 +162,9 @@ private function collectLocalTypes(Node $node): void private function collectEdges(Node $node): void { $frame = end($this->frames); + if ($frame === false) { + return; + } $from = $frame['scope']->class . '::' . $frame['method']; $line = $node->getStartLine(); diff --git a/src/ClassHierarchy.php b/src/ClassHierarchy.php index 39738b5..80ca99d 100644 --- a/src/ClassHierarchy.php +++ b/src/ClassHierarchy.php @@ -7,6 +7,8 @@ /** * Member lookups that follow inheritance: own members first, then traits, * parent classes and interfaces. Method names are case-insensitive, as in PHP. + * + * @phpstan-import-type ClassData from ClassVisitor */ final class ClassHierarchy { @@ -17,13 +19,13 @@ final class ClassHierarchy private array $declaringCache = []; /** - * @param array> $classMap + * @param array $classMap */ public function __construct(private readonly array $classMap) { foreach ($classMap as $fqcn => $data) { foreach (array_keys($data['methods']) as $method) { - $this->methodIndex[$fqcn][strtolower((string) $method)] = (string) $method; + $this->methodIndex[$fqcn][strtolower($method)] = $method; } } } diff --git a/src/ClassVisitor.php b/src/ClassVisitor.php index 271035d..786d9a0 100644 --- a/src/ClassVisitor.php +++ b/src/ClassVisitor.php @@ -10,10 +10,13 @@ /** * First pass: extracts class metadata, methods, properties and DI. * Expects PhpParser's NameResolver to run before it in the same traverser. + * + * @phpstan-type MethodData array{visibility: string, params: array, returnType: string|null, static: bool, line: int} + * @phpstan-type ClassData array{fqcn: string, name: string, namespace: string, type: string, layer: string, line: int, methods: array, properties: array, extends: list, implements: list, traits: list, constructorParams: array, file?: string} */ final class ClassVisitor extends NodeVisitorAbstract { - /** @var array> keyed by FQCN */ + /** @var array keyed by FQCN */ private array $classes = []; /** @var ClassScope[] */ @@ -24,7 +27,7 @@ public function __construct(private readonly TypeResolver $typeResolver) } /** - * @return array> + * @return array */ public function getClasses(): array { @@ -47,14 +50,15 @@ public function enterNode(Node $node): ?int } $scope = end($this->scopes); - if ($scope === false || $scope->isAnonymous()) { + $class = $scope === false ? null : $scope->class; + if ($scope === false || $class === null) { return null; } match (true) { - $node instanceof Node\Stmt\TraitUse => $this->addTraits($node, $scope), - $node instanceof Node\Stmt\Property => $this->addProperty($node, $scope), - $node instanceof Node\Stmt\ClassMethod => $this->addMethod($node, $scope), + $node instanceof Node\Stmt\TraitUse => $this->addTraits($node, $class), + $node instanceof Node\Stmt\Property => $this->addProperty($node, $class, $scope), + $node instanceof Node\Stmt\ClassMethod => $this->addMethod($node, $class, $scope), default => null, }; @@ -80,11 +84,11 @@ private function enterClassLike(Node\Stmt\ClassLike $node): void $fqcn = $node->namespacedName->toString(); $parents = match (true) { $node instanceof Node\Stmt\Class_ && $node->extends !== null => [$node->extends->toString()], - $node instanceof Node\Stmt\Interface_ => array_map(fn(Node\Name $n) => $n->toString(), $node->extends), + $node instanceof Node\Stmt\Interface_ => array_values(array_map(fn(Node\Name $n) => $n->toString(), $node->extends)), default => [], }; $implements = $node instanceof Node\Stmt\Class_ || $node instanceof Node\Stmt\Enum_ - ? array_map(fn(Node\Name $n) => $n->toString(), $node->implements) + ? array_values(array_map(fn(Node\Name $n) => $n->toString(), $node->implements)) : []; $this->scopes[] = new ClassScope($fqcn, $node instanceof Node\Stmt\Class_ ? ($parents[0] ?? null) : null); @@ -113,26 +117,26 @@ private function enterClassLike(Node\Stmt\ClassLike $node): void ]; } - private function addTraits(Node\Stmt\TraitUse $node, ClassScope $scope): void + private function addTraits(Node\Stmt\TraitUse $node, string $class): void { foreach ($node->traits as $trait) { - $this->classes[$scope->class]['traits'][] = $trait->toString(); + $this->classes[$class]['traits'][] = $trait->toString(); } } - private function addProperty(Node\Stmt\Property $node, ClassScope $scope): void + private function addProperty(Node\Stmt\Property $node, string $class, ClassScope $scope): void { foreach ($node->props as $prop) { $type = $this->typeResolver->resolveTypeNode($node->type, $scope) ?? $this->typeResolver->resolveDocTag($node->getDocComment(), 'var', null, $scope); if ($type !== null) { - $this->classes[$scope->class]['properties'][$prop->name->toString()] = $type; + $this->classes[$class]['properties'][$prop->name->toString()] = $type; } } } - private function addMethod(Node\Stmt\ClassMethod $node, ClassScope $scope): void + private function addMethod(Node\Stmt\ClassMethod $node, string $class, ClassScope $scope): void { $methodName = $node->name->toString(); $doc = $node->getDocComment(); @@ -149,15 +153,15 @@ private function addMethod(Node\Stmt\ClassMethod $node, ClassScope $scope): void $params[$paramName] = $paramType; if ($param->isPromoted() && $paramType !== null) { - $this->classes[$scope->class]['properties'][$paramName] = $paramType; + $this->classes[$class]['properties'][$paramName] = $paramType; } } if (strtolower($methodName) === '__construct') { - $this->classes[$scope->class]['constructorParams'] = $params; + $this->classes[$class]['constructorParams'] = $params; } - $this->classes[$scope->class]['methods'][$methodName] = [ + $this->classes[$class]['methods'][$methodName] = [ 'visibility' => $node->isPrivate() ? 'private' : ($node->isProtected() ? 'protected' : 'public'), 'params' => $params, 'returnType' => $this->typeResolver->resolveTypeNode($node->returnType, $scope) diff --git a/src/Console/AnalyzeCommand.php b/src/Console/AnalyzeCommand.php index 03e7fde..eb214c9 100644 --- a/src/Console/AnalyzeCommand.php +++ b/src/Console/AnalyzeCommand.php @@ -5,6 +5,7 @@ namespace DepsGraph\Console; use DepsGraph\Analyzer; +use DepsGraph\GraphBuilder; use DepsGraph\Architecture\ArchitectureConfig; use DepsGraph\Architecture\Baseline; use DepsGraph\Architecture\CheckResult; @@ -18,6 +19,9 @@ use DepsGraph\Metrics\GraphMetrics; use DepsGraph\Metrics\MetricsReport; +/** + * @phpstan-import-type Graph from GraphBuilder + */ final class AnalyzeCommand { public const SUCCESS = 0; @@ -161,7 +165,7 @@ public function run(array $arguments, string $cwd): int /** * @param array $options - * @return array{nodes: array, edges: array, stats: array} + * @return Graph */ private function analyze(array $options, ArchitectureConfig $config, string $root): array { @@ -195,7 +199,7 @@ private function analyze(array $options, ArchitectureConfig $config, string $roo } /** - * @param array $graph + * @param Graph $graph */ private function writeGraph(array $graph, string $outputFile, bool $isPretty): void { diff --git a/src/GraphBuilder.php b/src/GraphBuilder.php index f4c495c..61f715a 100644 --- a/src/GraphBuilder.php +++ b/src/GraphBuilder.php @@ -7,6 +7,17 @@ /** * Builds the graph JSON structure. Only classes found in the scanned * directories become nodes: edges towards anything else are dropped. + * + * @phpstan-import-type ClassData from ClassVisitor + * @phpstan-import-type MethodData from ClassVisitor + * @phpstan-type Edge array{source: string, target: string, type: string, line: int} + * @phpstan-type ClassMetrics array{ca: int, ce: int, instability: float} + * @phpstan-type ClassNode array{id: string, type: 'class', label: string, namespace: string, layer: string, classType: string, file: string|null, line: int, methods: list, methodCount: int, metrics?: ClassMetrics, cycle?: int|null} + * @phpstan-type MethodNode array{id: string, type: 'method', label: string, class: string, visibility: string, static: bool, line: int, layer: string} + * @phpstan-type Cycle array{members: list, path: list} + * @phpstan-type NamespaceMetrics array{id: string, size: int, ca: int, ce: int, instability: float, abstractness: float, distance: float} + * @phpstan-type Stats array{totalClasses: int, totalMethods: int, totalEdges: int, callsCollapsedToClass: int, layerCounts: array, edgeTypeCounts: array, classTypeCounts: array, classCycles?: int, namespaceCycles?: int} + * @phpstan-type Graph array{root?: string, nodes: list, edges: list, stats: Stats, cycles?: list, namespaces?: list, namespaceCycles?: list} */ final class GraphBuilder { @@ -28,13 +39,13 @@ final class GraphBuilder /** @var array "SourceClass|TargetClass" */ private array $linkedClasses = []; - /** @var array */ + /** @var list */ private array $graphEdges = []; /** - * @param array> $classes keyed by FQCN - * @param array $rawEdges - * @return array{nodes: array, edges: array, stats: array} + * @param array $classes keyed by FQCN + * @param list $rawEdges + * @return Graph */ public function build(array $classes, array $rawEdges, ClassHierarchy $hierarchy): array { @@ -46,7 +57,7 @@ public function build(array $classes, array $rawEdges, ClassHierarchy $hierarchy foreach ($classes as $fqcn => $data) { $nodes[] = $this->classNode($fqcn, $data); foreach ($data['methods'] as $methodName => $methodData) { - $nodes[] = $this->methodNode($fqcn, (string) $methodName, $methodData, $data['layer']); + $nodes[] = $this->methodNode($fqcn, $methodName, $methodData, $data['layer']); } $this->addClassLevelEdges($fqcn, $data, $hierarchy); } @@ -81,7 +92,7 @@ public function build(array $classes, array $rawEdges, ClassHierarchy $hierarchy } /** - * @param array $data + * @param ClassData $data */ private function addClassLevelEdges(string $fqcn, array $data, ClassHierarchy $hierarchy): void { @@ -104,8 +115,8 @@ private function addClassLevelEdges(string $fqcn, array $data, ClassHierarchy $h /** * Types used in method signatures and property declarations. * - * @param array> $classes - * @return array + * @param array $classes + * @return list */ private function signatureEdges(array $classes): array { @@ -132,7 +143,8 @@ private function signatureEdges(array $classes): array */ private function resolveTarget(string $target, ClassHierarchy $hierarchy, int &$collapsedCalls): ?string { - [$targetClass, $targetMethod] = array_pad(explode('::', $target, 2), 2, null); + $targetClass = self::classOf($target); + $targetMethod = str_contains($target, '::') ? explode('::', $target, 2)[1] : null; if (!$hierarchy->has($targetClass)) { return null; } @@ -156,8 +168,8 @@ private static function classOf(string $nodeId): string } /** - * @param array $data - * @return array + * @param ClassData $data + * @return ClassNode */ private function classNode(string $fqcn, array $data): array { @@ -176,8 +188,8 @@ private function classNode(string $fqcn, array $data): array } /** - * @param array $methodData - * @return array + * @param MethodData $methodData + * @return MethodNode */ private function methodNode(string $fqcn, string $methodName, array $methodData, string $layer): array { @@ -206,9 +218,9 @@ private function addEdge(string $source, string $target, string $type, int $line } /** - * @param array> $classes - * @param array> $nodes - * @return array + * @param array $classes + * @param list $nodes + * @return Stats */ private function stats(array $classes, array $nodes, int $collapsedCalls): array { diff --git a/src/Metrics/CycleDetector.php b/src/Metrics/CycleDetector.php index c0680b5..4918ac5 100644 --- a/src/Metrics/CycleDetector.php +++ b/src/Metrics/CycleDetector.php @@ -10,7 +10,7 @@ final class CycleDetector { /** - * @return array components of 2+ nodes, largest first, members sorted + * @return list> components of 2+ nodes, largest first, members sorted */ public function components(DependencyMap $map): array { @@ -53,7 +53,7 @@ public function components(DependencyMap $map): array if ($lowLink[$node] === $index[$node]) { $component = []; do { - $member = array_pop($stack); + $member = array_pop($stack) ?? throw new \LogicException('Tarjan stack exhausted before reaching the component root'); unset($onStack[$member]); $component[] = $member; } while ($member !== $node); @@ -79,8 +79,8 @@ public function components(DependencyMap $map): array /** * Shortest cycle going through $start inside its component: a concrete path to break. * - * @param string[] $component - * @return string[] nodes in order, the last one depends on the first + * @param list $component + * @return list nodes in order, the last one depends on the first */ public function shortestCycle(DependencyMap $map, array $component, string $start): array { diff --git a/src/Metrics/DependencyMap.php b/src/Metrics/DependencyMap.php index 51f304e..98ecf1c 100644 --- a/src/Metrics/DependencyMap.php +++ b/src/Metrics/DependencyMap.php @@ -4,11 +4,16 @@ namespace DepsGraph\Metrics; +use DepsGraph\GraphBuilder; + /** * Class-to-class (or namespace-to-namespace) dependencies, whatever the edge type. * * Traits are inlined: their dependencies count for every class using them, and `uses_trait` * is not a dependency, otherwise a trait referencing its host class would always form a cycle. + * + * @phpstan-import-type Edge from GraphBuilder + * @phpstan-type MinimalGraph array{nodes: list, edges: list} */ final class DependencyMap { @@ -16,7 +21,7 @@ final class DependencyMap * @param array> $dependencies node => [dependency => true] * @param array $sizes node => number of classes it groups * @param array $abstractSizes node => number of interfaces and abstract classes - * @param array> $evidence + * @param array> $evidence * source => [target => first edge behind that dependency] */ private function __construct( @@ -28,7 +33,7 @@ private function __construct( } /** - * @return array{source: string, target: string, type: string, line: int}|null + * @return Edge|null */ public function evidence(string $source, string $target): ?array { @@ -36,7 +41,7 @@ public function evidence(string $source, string $target): ?array } /** - * @param array{nodes: array>, edges: array} $graph + * @param MinimalGraph $graph * @param int|null $namespaceDepth null for class level, 0 for full namespaces, N for the first N segments * @param (\Closure(string): bool)|null $classFilter classes rejected are left out entirely */ @@ -56,11 +61,12 @@ public static function fromGraph(array $graph, ?int $namespaceDepth = null, ?\Cl $group = $namespaceDepth === null ? $node['id'] : self::namespaceOf($node['id'], $namespaceDepth); $groupOf[$node['id']] = $group; - $isTrait[$node['id']] = $node['classType'] === 'trait'; + $classType = $node['classType'] ?? 'class'; + $isTrait[$node['id']] = $classType === 'trait'; $dependencies[$group] ??= []; $sizes[$group] = ($sizes[$group] ?? 0) + 1; $abstractSizes[$group] = ($abstractSizes[$group] ?? 0) - + (in_array($node['classType'], ['interface', 'abstract'], true) ? 1 : 0); + + (in_array($classType, ['interface', 'abstract'], true) ? 1 : 0); } $traitUsers = []; @@ -94,7 +100,7 @@ public static function fromGraph(array $graph, ?int $namespaceDepth = null, ?\Cl 'source' => $edge['source'], 'target' => $edge['target'], 'type' => $edge['type'], - 'line' => (int) ($edge['line'] ?? 0), + 'line' => $edge['line'] ?? 0, ]; } } diff --git a/src/Metrics/GraphMetrics.php b/src/Metrics/GraphMetrics.php index d51e063..0dc4da4 100644 --- a/src/Metrics/GraphMetrics.php +++ b/src/Metrics/GraphMetrics.php @@ -4,11 +4,16 @@ namespace DepsGraph\Metrics; +use DepsGraph\GraphBuilder; + /** * Adds coupling metrics and cycles to a built graph: * - class nodes get `metrics` {ca, ce, instability} and `cycle`, the index of their class cycle or null * - `cycles` lists class cycles, largest first, with a shortest path through each * - `namespaces` lists namespace metrics and namespace cycles + * + * @phpstan-import-type Graph from GraphBuilder + * @phpstan-import-type Cycle from GraphBuilder */ final class GraphMetrics { @@ -19,8 +24,8 @@ public function __construct( } /** - * @param array{nodes: array>, edges: array, stats: array} $graph - * @return array{nodes: array>, edges: array, stats: array, cycles: array, namespaces: array, namespaceCycles: array} + * @param Graph $graph + * @return Graph */ public function enrich(array $graph, int $namespaceDepth = 0): array { @@ -35,15 +40,16 @@ public function enrich(array $graph, int $namespaceDepth = 0): array } } - foreach ($graph['nodes'] as &$node) { - if ($node['type'] !== 'class') { - continue; + $nodes = []; + foreach ($graph['nodes'] as $node) { + if ($node['type'] === 'class') { + $metrics = $classMetrics[$node['id']]; + $node['metrics'] = ['ca' => $metrics['ca'], 'ce' => $metrics['ce'], 'instability' => $metrics['instability']]; + $node['cycle'] = $cycleOf[$node['id']] ?? null; } - $metrics = $classMetrics[$node['id']]; - $node['metrics'] = ['ca' => $metrics['ca'], 'ce' => $metrics['ce'], 'instability' => $metrics['instability']]; - $node['cycle'] = $cycleOf[$node['id']] ?? null; + $nodes[] = $node; } - unset($node); + $graph['nodes'] = $nodes; $namespaceMap = DependencyMap::fromGraph($graph, $namespaceDepth); $namespaces = []; @@ -62,7 +68,7 @@ public function enrich(array $graph, int $namespaceDepth = 0): array } /** - * @return array + * @return list */ private function cycles(DependencyMap $map): array { diff --git a/src/Metrics/MetricsReport.php b/src/Metrics/MetricsReport.php index 51d168c..103d639 100644 --- a/src/Metrics/MetricsReport.php +++ b/src/Metrics/MetricsReport.php @@ -4,8 +4,15 @@ namespace DepsGraph\Metrics; +use DepsGraph\GraphBuilder; + /** * Text summary of the metrics added by GraphMetrics. + * + * @phpstan-import-type Graph from GraphBuilder + * @phpstan-import-type ClassNode from GraphBuilder + * @phpstan-import-type Cycle from GraphBuilder + * @phpstan-import-type NamespaceMetrics from GraphBuilder */ final class MetricsReport { @@ -13,21 +20,26 @@ final class MetricsReport private const MIN_NAMESPACE_SIZE = 5; /** - * @param array $graph enriched by GraphMetrics + * @param Graph $graph enriched by GraphMetrics */ public function render(array $graph): string { - $classes = array_values(array_filter($graph['nodes'], static fn(array $node) => $node['type'] === 'class')); + $classes = []; + foreach ($graph['nodes'] as $node) { + if ($node['type'] === 'class') { + $classes[] = $node; + } + } - return $this->cycles('Class cycles', $graph['cycles'], 'classes') - . $this->cycles('Namespace cycles', $graph['namespaceCycles'], 'namespaces') + return $this->cycles('Class cycles', $graph['cycles'] ?? [], 'classes') + . $this->cycles('Namespace cycles', $graph['namespaceCycles'] ?? [], 'namespaces') . $this->ranking('Most depended upon classes (afferent coupling)', $classes, 'ca') . $this->ranking('Classes depending on the most classes (efferent coupling)', $classes, 'ce') - . $this->namespaceDistances($graph['namespaces']); + . $this->namespaceDistances($graph['namespaces'] ?? []); } /** - * @param array $cycles + * @param list $cycles */ private function cycles(string $title, array $cycles, string $unit): string { @@ -40,25 +52,35 @@ private function cycles(string $title, array $cycles, string $unit): string } /** - * @param array> $classes + * @param list $classes + * @param 'ca'|'ce' $metric */ private function ranking(string $title, array $classes, string $metric): string { - usort($classes, static fn(array $a, array $b) => $b['metrics'][$metric] <=> $a['metrics'][$metric]); + usort($classes, static fn(array $a, array $b) => self::metric($b, $metric) <=> self::metric($a, $metric)); $output = "\n{$title}\n"; foreach (array_slice($classes, 0, self::TOP) as $class) { - $output .= sprintf(" %5d %s\n", $class['metrics'][$metric], $class['id']); + $output .= sprintf(" %5d %s\n", self::metric($class, $metric), $class['id']); } return $output; } + /** + * @param ClassNode $class + * @param 'ca'|'ce' $metric + */ + private static function metric(array $class, string $metric): int + { + return $class['metrics'][$metric] ?? 0; + } + /** * Concrete and stable (hard to change, everything depends on it) or abstract and unstable (abstractions nobody uses). * Namespaces depending on nothing (enums, DTOs, exceptions) are concrete and stable by nature, they would fill the list. * - * @param array> $namespaces + * @param list $namespaces */ private function namespaceDistances(array $namespaces): string { diff --git a/tests/AnalyzerTest.php b/tests/AnalyzerTest.php index 7a6ab4f..247c81d 100644 --- a/tests/AnalyzerTest.php +++ b/tests/AnalyzerTest.php @@ -5,13 +5,18 @@ namespace DepsGraph\Tests; use DepsGraph\Analyzer; +use DepsGraph\GraphBuilder; use PHPUnit\Framework\TestCase; +/** + * @phpstan-import-type Graph from GraphBuilder + * @phpstan-import-type ClassNode from GraphBuilder + */ final class AnalyzerTest extends TestCase { private const SERVICE = 'Acme\Services\UserService'; - /** @var array{nodes: array, edges: array, stats: array} */ + /** @var Graph */ private static array $graph; public static function setUpBeforeClass(): void @@ -74,7 +79,7 @@ public function testDetectsContainerResolutions(): void public function testKeepsAnonymousClassesOutOfTheEnclosingClass(): void { - $methods = $this->node(self::SERVICE)['methods']; + $methods = $this->classNode(self::SERVICE)['methods']; $this->assertSame(['__construct', 'handle', 'documented', 'afterAnonymousClass'], $methods); $this->assertEdge(self::SERVICE . '::handle', self::SERVICE . '::afterAnonymousClass', 'calls'); @@ -152,16 +157,16 @@ private function assertNodeMissing(string $id): void } /** - * @return array + * @return ClassNode */ - private function node(string $id): array + private function classNode(string $id): array { foreach (self::$graph['nodes'] as $node) { - if ($node['id'] === $id) { + if ($node['type'] === 'class' && $node['id'] === $id) { return $node; } } - $this->fail("Node {$id} not found"); + $this->fail("Class node {$id} not found"); } } diff --git a/tests/Architecture/DependencyRuleCheckerTest.php b/tests/Architecture/DependencyRuleCheckerTest.php index c1746f0..bbd87d7 100644 --- a/tests/Architecture/DependencyRuleCheckerTest.php +++ b/tests/Architecture/DependencyRuleCheckerTest.php @@ -63,7 +63,7 @@ public function testRejectsRulesOnUndefinedLayers(): void } /** - * @param array $edges + * @param list $edges * @return Violation[] */ private function check(array $edges): array diff --git a/tests/Console/AnalyzeCommandTest.php b/tests/Console/AnalyzeCommandTest.php index 53c735d..cdc6bd7 100644 --- a/tests/Console/AnalyzeCommandTest.php +++ b/tests/Console/AnalyzeCommandTest.php @@ -5,8 +5,12 @@ namespace DepsGraph\Tests\Console; use DepsGraph\Console\AnalyzeCommand; +use DepsGraph\GraphBuilder; use PHPUnit\Framework\TestCase; +/** + * @phpstan-import-type Graph from GraphBuilder + */ final class AnalyzeCommandTest extends TestCase { private const FIXTURE = __DIR__ . '/../Fixtures/Architecture'; @@ -26,8 +30,8 @@ protected function setUp(): void foreach (['php-deps.php', 'src'] as $entry) { exec('cp -R ' . escapeshellarg(self::FIXTURE . '/' . $entry) . ' ' . escapeshellarg($this->workDir)); } - $this->stdout = fopen('php://memory', 'w+'); - $this->stderr = fopen('php://memory', 'w+'); + $this->stdout = fopen('php://memory', 'w+') ?: throw new \RuntimeException('Cannot open stdout stream'); + $this->stderr = fopen('php://memory', 'w+') ?: throw new \RuntimeException('Cannot open stderr stream'); } protected function tearDown(): void @@ -72,6 +76,7 @@ public function testWritesAGitlabCodeQualityReport(): void { $this->execute('--check', '--quiet', '--format=gitlab'); + /** @var list $report */ $report = json_decode($this->stdout(), true); $this->assertSame('src/Billing/Domain/Invoice.php', $report[0]['location']['path']); $this->assertSame(14, $report[0]['location']['lines']['begin']); @@ -121,12 +126,21 @@ public function testPrintsMetrics(): void $this->assertStringContainsString('Class cycles: 1', $this->stdout()); $this->assertStringContainsString('Most depended upon classes', $this->stdout()); + /** @var Graph $graph */ $graph = json_decode((string) file_get_contents($this->workDir . '/graph.json'), true); $cycles = array_column(array_filter($graph['nodes'], fn(array $node) => $node['type'] === 'class'), 'cycle', 'id'); $this->assertSame(0, $cycles['Shop\Billing\Domain\Invoice']); $this->assertNull($cycles['Shop\Legacy\Helpers']); } + public function testRejectsInvalidConfigValues(): void + { + $this->useConfig(['cycles' => ['level' => 'class', 'depth' => '3']]); + + $this->assertSame(AnalyzeCommand::INVALID_USAGE, $this->execute('--check', '--quiet')); + $this->assertStringContainsString('"cycles.depth" must be an integer', $this->stderr()); + } + public function testRejectsUnknownOptions(): void { $this->assertSame(AnalyzeCommand::INVALID_USAGE, $this->execute('--chek')); @@ -144,6 +158,7 @@ public function testWritesTheGraphWhenNotChecking(): void $exitCode = $this->execute('--quiet', '--output=graph.json'); $this->assertSame(AnalyzeCommand::SUCCESS, $exitCode); + /** @var Graph $graph */ $graph = json_decode((string) file_get_contents($this->workDir . '/graph.json'), true); $this->assertSame(4, $graph['stats']['totalClasses']); } @@ -164,6 +179,13 @@ private function useConfig(array $config): void file_put_contents($this->workDir . '/php-deps.php', 'stderr); + + return (string) stream_get_contents($this->stderr); + } + private function stdout(): string { rewind($this->stdout); diff --git a/tests/Metrics/GraphFactory.php b/tests/Metrics/GraphFactory.php index 916b42d..2a4c2b8 100644 --- a/tests/Metrics/GraphFactory.php +++ b/tests/Metrics/GraphFactory.php @@ -4,12 +4,17 @@ namespace DepsGraph\Tests\Metrics; +use DepsGraph\Metrics\DependencyMap; + +/** + * @phpstan-import-type MinimalGraph from DependencyMap + */ final class GraphFactory { /** * @param array $classes FQCN => classType - * @param array $edges [source, target, type = calls] - * @return array{nodes: array>, edges: array} + * @param list $edges [source, target, type = calls] + * @return MinimalGraph */ public static function make(array $classes, array $edges): array {