diff --git a/src/CodeBlockAnalyser.php b/src/CodeBlockAnalyser.php index 9cdd790..cc9082d 100644 --- a/src/CodeBlockAnalyser.php +++ b/src/CodeBlockAnalyser.php @@ -4,15 +4,45 @@ use LogicException; use PhpParser\Node; +use PhpParser\Node\Expr\ArrowFunction; +use PhpParser\Node\Expr\Closure; +use PhpParser\Node\Expr\Match_; use PhpParser\Node\Stmt\ClassLike; use PhpParser\Node\Stmt\ClassMethod; +use PhpParser\Node\Stmt\Do_; +use PhpParser\Node\Stmt\For_; +use PhpParser\Node\Stmt\Foreach_; +use PhpParser\Node\Stmt\Function_; +use PhpParser\Node\Stmt\If_; +use PhpParser\Node\Stmt\Switch_; +use PhpParser\Node\Stmt\TryCatch; +use PhpParser\Node\Stmt\While_; use PhpParser\NodeVisitorAbstract; +use ShipMonk\CoverageGuard\Hierarchy\ArrowFunctionBlock; +use ShipMonk\CoverageGuard\Hierarchy\CaseBlock; +use ShipMonk\CoverageGuard\Hierarchy\CatchBlock; use ShipMonk\CoverageGuard\Hierarchy\ClassMethodBlock; +use ShipMonk\CoverageGuard\Hierarchy\ClosureBlock; +use ShipMonk\CoverageGuard\Hierarchy\CodeBlock; +use ShipMonk\CoverageGuard\Hierarchy\DoWhileBlock; +use ShipMonk\CoverageGuard\Hierarchy\ElseBlock; +use ShipMonk\CoverageGuard\Hierarchy\ElseIfBlock; +use ShipMonk\CoverageGuard\Hierarchy\FinallyBlock; +use ShipMonk\CoverageGuard\Hierarchy\ForBlock; +use ShipMonk\CoverageGuard\Hierarchy\ForeachBlock; +use ShipMonk\CoverageGuard\Hierarchy\FunctionBlock; +use ShipMonk\CoverageGuard\Hierarchy\IfBlock; use ShipMonk\CoverageGuard\Hierarchy\LineOfCode; +use ShipMonk\CoverageGuard\Hierarchy\MatchBlock; +use ShipMonk\CoverageGuard\Hierarchy\SwitchBlock; +use ShipMonk\CoverageGuard\Hierarchy\TryBlock; +use ShipMonk\CoverageGuard\Hierarchy\WhileBlock; use ShipMonk\CoverageGuard\Report\ReportedError; use ShipMonk\CoverageGuard\Rule\CoverageRule; use function assert; +use function end; use function range; +use function spl_object_id; final class CodeBlockAnalyser extends NodeVisitorAbstract { @@ -24,6 +54,11 @@ final class CodeBlockAnalyser extends NodeVisitorAbstract */ private array $reportedErrors = []; + /** + * @var array Stack of parent blocks (node id => block) + */ + private array $parentStack = []; + /** * @param array $linesChanged line => line * @param array $linesCoverage executable_line => hits @@ -61,24 +96,171 @@ public function enterNode(Node $node): ?int return null; } + $parent = $this->getCurrentParent(); $block = new ClassMethodBlock( $this->currentClass, $node->name->toString(), $this->filePath, $lines, + $parent, ); - if ($this->patchMode && $block->getChangedLinesCount() === 0) { - return null; // unchanged methods not passed to rules in patch mode + $this->trackBlock($node, $block); + $this->processBlock($block); + return null; + } + + if ($node instanceof Foreach_ && $node->stmts !== []) { + $block = $this->createBlock($node, ForeachBlock::class); + if ($block !== null) { + $this->processBlock($block); + } + return null; + } + + if ($node instanceof For_ && $node->stmts !== []) { + $block = $this->createBlock($node, ForBlock::class); + if ($block !== null) { + $this->processBlock($block); + } + return null; + } + + if ($node instanceof While_ && $node->stmts !== []) { + $block = $this->createBlock($node, WhileBlock::class); + if ($block !== null) { + $this->processBlock($block); + } + return null; + } + + if ($node instanceof If_ && $node->stmts !== []) { + $block = $this->createBlock($node, IfBlock::class); + if ($block !== null) { + $this->processBlock($block); + } + + // Process elseif blocks + foreach ($node->elseifs as $elseif) { + if ($elseif->stmts !== []) { + $elseifBlock = $this->createChildBlock($elseif, ElseIfBlock::class); + if ($elseifBlock !== null) { + $this->processBlock($elseifBlock); + } + } + } + + // Process else block + if ($node->else !== null && $node->else->stmts !== []) { + $elseBlock = $this->createChildBlock($node->else, ElseBlock::class); + if ($elseBlock !== null) { + $this->processBlock($elseBlock); + } + } + + return null; + } + + if ($node instanceof Do_ && $node->stmts !== []) { + $block = $this->createBlock($node, DoWhileBlock::class); + if ($block !== null) { + $this->processBlock($block); + } + return null; + } + + if ($node instanceof Switch_ && $node->cases !== []) { + $block = $this->createBlock($node, SwitchBlock::class); + if ($block !== null) { + $this->processBlock($block); + } + + // Process individual case blocks + foreach ($node->cases as $case) { + if ($case->stmts !== []) { + $caseBlock = $this->createChildBlock($case, CaseBlock::class); + if ($caseBlock !== null) { + $this->processBlock($caseBlock); + } + } + } + + return null; + } + + if ($node instanceof TryCatch && $node->stmts !== []) { + $block = $this->createBlock($node, TryBlock::class); + if ($block !== null) { + $this->processBlock($block); + } + + // Process catch blocks + foreach ($node->catches as $catch) { + if ($catch->stmts !== []) { + $catchBlock = $this->createChildBlock($catch, CatchBlock::class); + if ($catchBlock !== null) { + $this->processBlock($catchBlock); + } + } } - foreach ($this->inspectCodeBlock($block) as $reportedError) { - $this->reportedErrors[] = $reportedError; + // Process finally block + if ($node->finally !== null && $node->finally->stmts !== []) { + $finallyBlock = $this->createChildBlock($node->finally, FinallyBlock::class); + if ($finallyBlock !== null) { + $this->processBlock($finallyBlock); + } } return null; } + if ($node instanceof Function_ && $node->stmts !== []) { + $startLine = $node->name->getStartLine(); + $endLine = $node->getEndLine(); + + $lines = $this->getLines($startLine, $endLine); + if ($lines === []) { + return null; + } + + $parent = $this->getCurrentParent(); + $block = new FunctionBlock( + $node->name->toString(), + $this->filePath, + $lines, + $parent, + ); + + $this->trackBlock($node, $block); + $this->processBlock($block); + return null; + } + + if ($node instanceof Closure && $node->stmts !== []) { + $block = $this->createBlock($node, ClosureBlock::class); + if ($block !== null) { + $this->processBlock($block); + } + return null; + } + + if ($node instanceof ArrowFunction) { + $block = $this->createBlock($node, ArrowFunctionBlock::class); + if ($block !== null) { + $this->processBlock($block); + } + return null; + } + + if ($node instanceof Match_) { + $block = $this->createBlock($node, MatchBlock::class); + if ($block !== null) { + $this->processBlock($block); + } + return null; + } + return null; } @@ -88,9 +270,44 @@ public function leaveNode(Node $node): mixed $this->currentClass = null; } + // Pop from parent stack when leaving nodes that create blocks + if ( + $node instanceof ClassMethod || + $node instanceof Function_ || + $node instanceof Foreach_ || + $node instanceof For_ || + $node instanceof While_ || + $node instanceof Do_ || + $node instanceof If_ || + $node instanceof Switch_ || + $node instanceof TryCatch || + $node instanceof Closure || + $node instanceof ArrowFunction || + $node instanceof Match_ + ) { + unset($this->parentStack[spl_object_id($node)]); + } + return null; } + private function getCurrentParent(): ?CodeBlock + { + if ($this->parentStack === []) { + return null; + } + + return end($this->parentStack); + } + + private function trackBlock( + Node $node, + CodeBlock $block, + ): void + { + $this->parentStack[spl_object_id($node)] = $block; + } + /** * @return list */ @@ -116,10 +333,71 @@ private function getLines( return $executableLines; } + /** + * @param class-string $blockClass + * @return T|null + * + * @template T of CodeBlock + */ + private function createBlock( + Node $node, + string $blockClass, + ): ?CodeBlock + { + $startLine = $node->getStartLine(); + $endLine = $node->getEndLine(); + + $lines = $this->getLines($startLine, $endLine); + if ($lines === []) { + return null; + } + + $parent = $this->getCurrentParent(); + $block = new $blockClass($this->filePath, $lines, $parent); + $this->trackBlock($node, $block); + return $block; + } + + /** + * Creates a block without tracking it as a parent (for child blocks like ElseIf, Else, Case, Catch, Finally) + * + * @param class-string $blockClass + * @return T|null + * + * @template T of CodeBlock + */ + private function createChildBlock( + Node $node, + string $blockClass, + ): ?CodeBlock + { + $startLine = $node->getStartLine(); + $endLine = $node->getEndLine(); + + $lines = $this->getLines($startLine, $endLine); + if ($lines === []) { + return null; + } + + $parent = $this->getCurrentParent(); + return new $blockClass($this->filePath, $lines, $parent); + } + + private function processBlock(CodeBlock $block): void + { + if ($this->patchMode && $block->getChangedLinesCount() === 0) { + return; // unchanged blocks not passed to rules in patch mode + } + + foreach ($this->inspectCodeBlock($block) as $reportedError) { + $this->reportedErrors[] = $reportedError; + } + } + /** * @return list */ - private function inspectCodeBlock(ClassMethodBlock $block): array + private function inspectCodeBlock(CodeBlock $block): array { $reportedErrors = []; foreach ($this->rules as $rule) { diff --git a/src/Hierarchy/ArrowFunctionBlock.php b/src/Hierarchy/ArrowFunctionBlock.php new file mode 100644 index 0000000..9962958 --- /dev/null +++ b/src/Hierarchy/ArrowFunctionBlock.php @@ -0,0 +1,13 @@ +methodName = $methodName; $this->className = $className; } diff --git a/src/Hierarchy/ClosureBlock.php b/src/Hierarchy/ClosureBlock.php new file mode 100644 index 0000000..16ecd2e --- /dev/null +++ b/src/Hierarchy/ClosureBlock.php @@ -0,0 +1,13 @@ +lines; } + public function getParent(): ?CodeBlock + { + return $this->parent; + } + public function getExecutableLinesCount(): int { return count($this->getExecutableLines()); diff --git a/src/Hierarchy/DoWhileBlock.php b/src/Hierarchy/DoWhileBlock.php new file mode 100644 index 0000000..53d36f3 --- /dev/null +++ b/src/Hierarchy/DoWhileBlock.php @@ -0,0 +1,13 @@ + $lines + */ + public function __construct( + string $functionName, + string $filePath, + array $lines, + ?CodeBlock $parent = null, + ) + { + parent::__construct($filePath, $lines, $parent); + $this->functionName = $functionName; + } + + public function getFunctionName(): string + { + return $this->functionName; + } + +} diff --git a/src/Hierarchy/IfBlock.php b/src/Hierarchy/IfBlock.php new file mode 100644 index 0000000..905b22b --- /dev/null +++ b/src/Hierarchy/IfBlock.php @@ -0,0 +1,13 @@ + $lineContent) { + $actualLineNumber = $lineNumber + 1; // Line numbers start at 1 + $linesContents[$actualLineNumber] = $lineContent; + // Mark non-empty lines as executable and covered for this test + if (trim($lineContent) !== '' && !str_contains($lineContent, ' + */ + private array $blocks = []; + + public function inspect( + CodeBlock $codeBlock, + bool $patchMode, + ): ?CoverageError + { + $this->blocks[] = $codeBlock; + return null; + } + + /** + * @return list + */ + public function getBlocks(): array + { + return $this->blocks; + } + + }; + + // Parse and analyze the file + $parser = (new ParserFactory())->createForHostVersion(); + $stmts = $parser->parse($fileContents); + self::assertNotNull($stmts); + + $traverser = new NodeTraverser(); + $traverser->addVisitor(new NameResolver()); + $analyser = new CodeBlockAnalyser( + patchMode: false, + filePath: $filePath, + linesChanged: $linesChanged, + linesCoverage: $linesCoverage, + linesContents: $linesContents, + rules: [$collectingRule], + ); + $traverser->addVisitor($analyser); + $traverser->traverse($stmts); + + // Verify we collected the expected block types + $blockTypes = array_map( + static fn (CodeBlock $block) => $block::class, + $collectingRule->getBlocks(), + ); + + // We expect to find these block types in ConditionalBlocks.php + self::assertContains(ForeachBlock::class, $blockTypes, 'ForeachBlock should be detected'); + self::assertContains(ForBlock::class, $blockTypes, 'ForBlock should be detected'); + self::assertContains(WhileBlock::class, $blockTypes, 'WhileBlock should be detected'); + self::assertContains(DoWhileBlock::class, $blockTypes, 'DoWhileBlock should be detected'); + self::assertContains(IfBlock::class, $blockTypes, 'IfBlock should be detected'); + self::assertContains(ElseIfBlock::class, $blockTypes, 'ElseIfBlock should be detected'); + self::assertContains(ElseBlock::class, $blockTypes, 'ElseBlock should be detected'); + self::assertContains(SwitchBlock::class, $blockTypes, 'SwitchBlock should be detected'); + self::assertContains(CaseBlock::class, $blockTypes, 'CaseBlock should be detected'); + self::assertContains(TryBlock::class, $blockTypes, 'TryBlock should be detected'); + self::assertContains(CatchBlock::class, $blockTypes, 'CatchBlock should be detected'); + self::assertContains(FinallyBlock::class, $blockTypes, 'FinallyBlock should be detected'); + self::assertContains(FunctionBlock::class, $blockTypes, 'FunctionBlock should be detected'); + self::assertContains(ClosureBlock::class, $blockTypes, 'ClosureBlock should be detected'); + self::assertContains(ArrowFunctionBlock::class, $blockTypes, 'ArrowFunctionBlock should be detected'); + self::assertContains(MatchBlock::class, $blockTypes, 'MatchBlock should be detected'); + + // Verify FunctionBlock has the correct function name + $functionBlock = null; + foreach ($collectingRule->getBlocks() as $block) { + if ($block instanceof FunctionBlock) { + $functionBlock = $block; + break; + } + } + self::assertNotNull($functionBlock, 'FunctionBlock should be found'); + self::assertSame('standaloneFunction', $functionBlock->getFunctionName()); + + // Verify parent relationships: find blocks inside methods + $ifBlockInsideMethod = null; + $foreachBlockInsideMethod = null; + + foreach ($collectingRule->getBlocks() as $block) { + if ($block instanceof IfBlock && $block->getParent() instanceof ClassMethodBlock) { + $ifBlockInsideMethod = $block; + } + + if ($block instanceof ForeachBlock && $block->getParent() instanceof ClassMethodBlock) { + $foreachBlockInsideMethod = $block; + } + } + + self::assertNotNull($ifBlockInsideMethod, 'IfBlock inside method should be found'); + self::assertInstanceOf(ClassMethodBlock::class, $ifBlockInsideMethod->getParent()); + self::assertSame('testIf', $ifBlockInsideMethod->getParent()->getMethodName()); + + self::assertNotNull($foreachBlockInsideMethod, 'ForeachBlock inside method should be found'); + self::assertInstanceOf(ClassMethodBlock::class, $foreachBlockInsideMethod->getParent()); + self::assertSame('testForeach', $foreachBlockInsideMethod->getParent()->getMethodName()); + + // Verify standalone function has no parent + self::assertNull($functionBlock->getParent(), 'Standalone function should have no parent'); + } + +} diff --git a/tests/_fixtures/ConditionalBlocks.php b/tests/_fixtures/ConditionalBlocks.php new file mode 100644 index 0000000..cce5091 --- /dev/null +++ b/tests/_fixtures/ConditionalBlocks.php @@ -0,0 +1,118 @@ + 0) { + return 'positive'; + } elseif ($value < 0) { + return 'negative'; + } else { + return 'zero'; + } + } + + public function testSwitch(int $value): string + { + switch ($value) { + case 1: + return 'one'; + case 2: + return 'two'; + default: + return 'other'; + } + } + + public function testTryCatch(): mixed + { + try { + return $this->riskyOperation(); + } catch (\Exception $e) { + return 'error'; + } finally { + // cleanup + $this->cleanup(); + } + } + + public function testClosure(): callable + { + return function (int $x): int { + return $x * 2; + }; + } + + public function testArrowFunction(): callable + { + return fn(int $x): int => $x * 2; + } + + public function testMatch(int $value): string + { + return match ($value) { + 1 => 'one', + 2 => 'two', + default => 'other', + }; + } + + private function riskyOperation(): mixed + { + return 'success'; + } + + private function cleanup(): void + { + } + +} + +function standaloneFunction(): string +{ + return 'standalone'; +}