diff --git a/src/Data/RawCodeCoverageData.php b/src/Data/RawCodeCoverageData.php index 56512f94d..ec7761d0c 100644 --- a/src/Data/RawCodeCoverageData.php +++ b/src/Data/RawCodeCoverageData.php @@ -108,11 +108,17 @@ public static function fromLineAndBranchCoverage(array $lineCoverage, array $fun */ public static function fromUncoveredFile(string $filename, FileAnalyser $analyser): self { + $analysisResult = $analyser->analyse($filename); + $lineCoverage = array_map( static fn (): int => Driver::LINE_NOT_EXECUTED, - $analyser->analyse($filename)->executableLines(), + $analysisResult->executableLines(), ); + foreach ($analysisResult->deadLines() as $line => $_) { + $lineCoverage[$line] = Driver::LINE_NOT_EXECUTABLE; + } + return new self([$filename => $lineCoverage], []); } @@ -184,6 +190,21 @@ public function addMissingExecutableLines(string $filename, array $lines): void } } + /** + * @param non-empty-string $filename + * @param array $lines + */ + public function markLinesAsNotExecutable(string $filename, array $lines): void + { + if (!isset($this->lineCoverage[$filename])) { + return; + } + + foreach ($lines as $line => $_) { + $this->lineCoverage[$filename][$line] = Driver::LINE_NOT_EXECUTABLE; + } + } + /** * @param non-empty-string $filename * @param array $linesToBranchMap diff --git a/src/Driver/XdebugDriver.php b/src/Driver/XdebugDriver.php index e19c817a7..5e82f0aef 100644 --- a/src/Driver/XdebugDriver.php +++ b/src/Driver/XdebugDriver.php @@ -10,7 +10,6 @@ namespace SebastianBergmann\CodeCoverage\Driver; use const XDEBUG_CC_BRANCH_CHECK; -use const XDEBUG_CC_DEAD_CODE; use const XDEBUG_CC_UNUSED; use const XDEBUG_FILTER_CODE_COVERAGE; use const XDEBUG_PATH_INCLUDE; @@ -81,7 +80,7 @@ public function __construct(Filter $filter) public function start(): void { - $flags = XDEBUG_CC_UNUSED | XDEBUG_CC_DEAD_CODE; + $flags = XDEBUG_CC_UNUSED; if ($this->granularity() === Granularity::LineBranchAndPath) { $flags |= XDEBUG_CC_BRANCH_CHECK; diff --git a/src/FilterProcessor.php b/src/FilterProcessor.php index 75b122f03..ec133564d 100644 --- a/src/FilterProcessor.php +++ b/src/FilterProcessor.php @@ -112,6 +112,11 @@ public function applyExecutableLinesFilter(RawCodeCoverageData $data, Filter $fi $filename, $linesToBranchMap, ); + + $data->markLinesAsNotExecutable( + $filename, + $analysisResult->deadLines(), + ); } } diff --git a/src/StaticAnalysis/ParsingSourceAnalyser.php b/src/StaticAnalysis/ParsingSourceAnalyser.php index 034bd1887..23c26b685 100644 --- a/src/StaticAnalysis/ParsingSourceAnalyser.php +++ b/src/StaticAnalysis/ParsingSourceAnalyser.php @@ -11,6 +11,7 @@ use const T_COMMENT; use const T_DOC_COMMENT; +use function array_intersect_key; use function array_keys; use function array_replace; use function assert; @@ -62,6 +63,7 @@ public function analyse(string $sourceCodeFile, string $sourceCode, bool $useAnn $lineCountingVisitor = new LineCountingVisitor($linesOfCode); $ignoredLinesFindingVisitor = new IgnoredLinesFindingVisitor($useAnnotationsForIgnoringCode, $ignoreDeprecatedCode); $executableLinesFindingVisitor = new ExecutableLinesFindingVisitor($sourceCode); + $deadCodeFindingVisitor = new DeadCodeFindingVisitor; $traverser->addVisitor(new NameResolver); $traverser->addVisitor(new AttributeParentConnectingVisitor); @@ -69,6 +71,7 @@ public function analyse(string $sourceCodeFile, string $sourceCode, bool $useAnn $traverser->addVisitor($lineCountingVisitor); $traverser->addVisitor($ignoredLinesFindingVisitor); $traverser->addVisitor($executableLinesFindingVisitor); + $traverser->addVisitor($deadCodeFindingVisitor); /* @noinspection UnusedFunctionResultInspection */ $traverser->traverse($nodes); @@ -99,6 +102,13 @@ public function analyse(string $sourceCodeFile, string $sourceCode, bool $useAnn $ignoredLines = array_keys($ignoredLines); + $executableLines = $executableLinesFindingVisitor->executableLinesGroupedByBranch(); + + $deadLines = array_intersect_key( + $deadCodeFindingVisitor->deadLines(), + $executableLines, + ); + return new AnalysisResult( $codeUnitFindingVisitor->interfaces(), $codeUnitFindingVisitor->classes(), @@ -109,8 +119,9 @@ public function analyse(string $sourceCodeFile, string $sourceCode, bool $useAnn $lineCountingVisitor->result()->commentLinesOfCode(), $lineCountingVisitor->result()->nonCommentLinesOfCode(), ), - $executableLinesFindingVisitor->executableLinesGroupedByBranch(), + $executableLines, $executableLinesFindingVisitor->branchOperatorLines(), + $deadLines, $ignoredLines, ); } diff --git a/src/StaticAnalysis/Value/AnalysisResult.php b/src/StaticAnalysis/Value/AnalysisResult.php index 4fc862888..0fb0270e2 100644 --- a/src/StaticAnalysis/Value/AnalysisResult.php +++ b/src/StaticAnalysis/Value/AnalysisResult.php @@ -49,6 +49,11 @@ */ private array $branchOperatorLines; + /** + * @var array + */ + private array $deadLines; + /** * @var array */ @@ -61,9 +66,10 @@ * @param array $functions * @param LinesType $executableLines * @param array $branchOperatorLines + * @param array $deadLines * @param array $ignoredLines */ - public function __construct(array $interfaces, array $classes, array $traits, array $functions, LinesOfCode $linesOfCode, array $executableLines, array $branchOperatorLines, array $ignoredLines) + public function __construct(array $interfaces, array $classes, array $traits, array $functions, LinesOfCode $linesOfCode, array $executableLines, array $branchOperatorLines, array $deadLines, array $ignoredLines) { $this->interfaces = $interfaces; $this->classes = $classes; @@ -72,6 +78,7 @@ public function __construct(array $interfaces, array $classes, array $traits, ar $this->linesOfCode = $linesOfCode; $this->executableLines = $executableLines; $this->branchOperatorLines = $branchOperatorLines; + $this->deadLines = $deadLines; $this->ignoredLines = $ignoredLines; } @@ -128,6 +135,14 @@ public function branchOperatorLines(): array return $this->branchOperatorLines; } + /** + * @return array + */ + public function deadLines(): array + { + return $this->deadLines; + } + /** * @return array */ diff --git a/src/StaticAnalysis/Visitor/DeadCodeFindingVisitor.php b/src/StaticAnalysis/Visitor/DeadCodeFindingVisitor.php new file mode 100644 index 000000000..1e8902543 --- /dev/null +++ b/src/StaticAnalysis/Visitor/DeadCodeFindingVisitor.php @@ -0,0 +1,294 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ +namespace SebastianBergmann\CodeCoverage\StaticAnalysis; + +use function ksort; +use function strtolower; +use PhpParser\Node; +use PhpParser\NodeFinder; +use PhpParser\NodeVisitorAbstract; + +/** + * Identifies lines that the PHP-Parser AST shows to be statically unreachable. + * + * The visitor recognizes three structural patterns: + * + * - Statements that follow an unconditional control-flow transfer within the + * same `stmts` array (`return`, `throw`, exit/die, `break`, `continue`, `goto`). + * - Bodies of branches with literal-constant conditions: `if (false) { ... }`, + * `elseif (false) { ... }`, `while (false) { ... }`, `for (...; false; ...) { ... }`, + * the `elseif`/`else` tail after an `if (true)`, and the unreachable arm of + * a ternary with a literal-constant condition. + * + * A label makes the code that follows it reachable again via `goto`. Since + * `goto` may also jump into a conditional block (only loops and switches are + * forbidden jump targets), a statement or block that contains a label anywhere + * in its subtree is never reported as dead. + * + * Whole-program reasoning (never-called functions, opcode-level optimization) + * is out of scope; the visitor reports only what is locally derivable from the + * AST. + * + * @internal This class is not covered by the backward compatibility promise for phpunit/php-code-coverage + * + * @no-named-arguments Parameter names are not covered by the backward compatibility promise for phpunit/php-code-coverage + */ +final class DeadCodeFindingVisitor extends NodeVisitorAbstract +{ + /** + * @var array + */ + private array $deadLines = []; + + public function enterNode(Node $node): null + { + $stmts = $this->statementsOf($node); + + if ($stmts !== []) { + $this->markStatementsAfterTerminator($stmts); + } + + if ($node instanceof Node\Stmt\If_) { + $this->handleIf($node); + + return null; + } + + if ($node instanceof Node\Stmt\ElseIf_ && $this->isLiteralFalse($node->cond)) { + $this->markBlock($node->stmts, $node); + + return null; + } + + if ($node instanceof Node\Stmt\While_ && $this->isLiteralFalse($node->cond)) { + $this->markBlock($node->stmts, $node); + + return null; + } + + if ($node instanceof Node\Stmt\For_ && $this->forBodyIsUnreachable($node)) { + $this->markBlock($node->stmts, $node); + + return null; + } + + if ($node instanceof Node\Expr\Ternary) { + $this->handleTernary($node); + + return null; + } + + return null; + } + + /** + * @return array + */ + public function deadLines(): array + { + ksort($this->deadLines); + + return $this->deadLines; + } + + private function handleIf(Node\Stmt\If_ $node): void + { + if ($this->isLiteralFalse($node->cond)) { + $this->markBlock($node->stmts, $node); + + return; + } + + if (!$this->isLiteralTrue($node->cond)) { + return; + } + + foreach ($node->elseifs as $elseif) { + if ($this->containsLabel($elseif->stmts)) { + continue; + } + + $this->markRange($elseif->getStartLine(), $elseif->getEndLine()); + } + + if ($node->else !== null && !$this->containsLabel($node->else->stmts)) { + $this->markRange($node->else->getStartLine(), $node->else->getEndLine()); + } + } + + private function handleTernary(Node\Expr\Ternary $node): void + { + if ($node->getStartLine() === $node->getEndLine()) { + return; + } + + if ($this->isLiteralTrue($node->cond)) { + $this->markRange($node->else->getStartLine(), $node->else->getEndLine()); + + return; + } + + if ($this->isLiteralFalse($node->cond) && $node->if !== null) { + $this->markRange($node->if->getStartLine(), $node->if->getEndLine()); + } + } + + /** + * @param array $stmts + */ + private function markStatementsAfterTerminator(array $stmts): void + { + $terminated = false; + + foreach ($stmts as $stmt) { + if (!$terminated) { + if ($this->isTerminator($stmt)) { + $terminated = true; + } + + continue; + } + + if ($this->containsLabel([$stmt])) { + $terminated = false; + + continue; + } + + if (!$stmt instanceof Node\Stmt\Nop) { + $this->markRange($stmt->getStartLine(), $stmt->getEndLine()); + } + } + } + + /** + * @param array $stmts + */ + private function markBlock(array $stmts, Node $wrapper): void + { + if ($this->containsLabel($stmts)) { + return; + } + + $wrapperStart = $wrapper->getStartLine(); + $wrapperEnd = $wrapper->getEndLine(); + + foreach ($stmts as $stmt) { + if ($stmt instanceof Node\Stmt\Nop) { + continue; + } + + for ($line = $stmt->getStartLine(); $line <= $stmt->getEndLine(); $line++) { + if ($line === $wrapperStart || $line === $wrapperEnd) { + continue; + } + + if ($line < 1) { + continue; + } + + $this->deadLines[$line] = true; + } + } + } + + private function markRange(int $start, int $end): void + { + for ($line = $start; $line <= $end; $line++) { + if ($line < 1) { + continue; + } + + $this->deadLines[$line] = true; + } + } + + private function isTerminator(Node\Stmt $stmt): bool + { + if ($stmt instanceof Node\Stmt\Return_ || + $stmt instanceof Node\Stmt\Break_ || + $stmt instanceof Node\Stmt\Continue_ || + $stmt instanceof Node\Stmt\Goto_) { + return true; + } + + if ($stmt instanceof Node\Stmt\Expression) { + return $stmt->expr instanceof Node\Expr\Throw_ || + $stmt->expr instanceof Node\Expr\Exit_; + } + + return false; + } + + /** + * @param array $stmts + */ + private function containsLabel(array $stmts): bool + { + return (new NodeFinder)->findFirstInstanceOf($stmts, Node\Stmt\Label::class) !== null; + } + + /** + * @return array + */ + private function statementsOf(Node $node): array + { + if ($node instanceof Node\Stmt\Function_ || + $node instanceof Node\Stmt\If_ || + $node instanceof Node\Stmt\Else_ || + $node instanceof Node\Stmt\ElseIf_ || + $node instanceof Node\Stmt\While_ || + $node instanceof Node\Stmt\Do_ || + $node instanceof Node\Stmt\For_ || + $node instanceof Node\Stmt\Foreach_ || + $node instanceof Node\Stmt\Case_ || + $node instanceof Node\Stmt\Catch_ || + $node instanceof Node\Stmt\Finally_ || + $node instanceof Node\Stmt\TryCatch || + $node instanceof Node\Expr\Closure) { + return $node->stmts; + } + + if ($node instanceof Node\Stmt\ClassMethod || + $node instanceof Node\Stmt\Namespace_ || + $node instanceof Node\Stmt\Declare_) { + return $node->stmts ?? []; + } + + return []; + } + + private function forBodyIsUnreachable(Node\Stmt\For_ $node): bool + { + if ($node->cond === []) { + return false; + } + + foreach ($node->cond as $condition) { + if ($this->isLiteralFalse($condition)) { + return true; + } + } + + return false; + } + + private function isLiteralTrue(?Node $node): bool + { + return $node instanceof Node\Expr\ConstFetch && + strtolower($node->name->toString()) === 'true'; + } + + private function isLiteralFalse(?Node $node): bool + { + return $node instanceof Node\Expr\ConstFetch && + strtolower($node->name->toString()) === 'false'; + } +} diff --git a/tests/_files/source_with_dead_code.php b/tests/_files/source_with_dead_code.php new file mode 100644 index 000000000..9f545a660 --- /dev/null +++ b/tests/_files/source_with_dead_code.php @@ -0,0 +1,211 @@ + 0) { + return 2; + } + + return 3; + } + + public function deadElseifFalse(int $n): int + { + if ($n > 0) { + return $n; + } elseif (false) { + return -$n; + } + + return 0; + } + + public function deadWhileFalse(): void + { + while (false) { + $x = 1; + } + } + + public function deadForFalse(): void + { + for ($i = 0; false; $i++) { + $x = 1; + } + } + + public function deadTernaryArm(int $n): int + { + return false + ? $n + : $n + 1; + } + + public function liveCode(int $n): int + { + if ($n > 0) { + return $n; + } + + return -$n; + } + + public function deadTernaryArmAfterTrue(int $n): int + { + return true + ? $n + : $n + 1; + } + + public function singleLineTernary(int $n): int + { + return $n > 0 ? $n : -$n; + } + + public function deadIfFalseSingleLine(): void + { + if (false) { $x = 1; } + } + + public function deadIfFalseWithNop(): void + { + if (false) { + $x = 1; + /** trailing docblock parses to a Nop and must be skipped */ + } + } + + public function codeAfterLabelIsReachable(bool $x): int + { + if ($x) { + goto retry; + } + + return 1; + + retry: + + return 2; + } + + public function deadAfterGoto(): int + { + goto end; + + $x = 1; + + end: + + return 2; + } + + public function labelledBlockInIfFalseIsReachable(bool $x): int + { + if ($x) { + goto inside; + } + + if (false) { + inside: + + return 1; + } + + return 2; + } + + public function labelledElseAfterIfTrueIsReachable(bool $x): int + { + if ($x) { + goto here; + } + + if (true) { + return 1; + } else { + here: + + return 2; + } + } + + public function labelledElseifAfterIfTrueIsReachable(bool $x): int + { + if ($x) { + goto there; + } + + if (true) { + return 1; + } elseif ($x) { + there: + + return 2; + } + + return 3; + } +} diff --git a/tests/_files/source_with_dead_code_in_various_contexts.php b/tests/_files/source_with_dead_code_in_various_contexts.php new file mode 100644 index 000000000..38942d5af --- /dev/null +++ b/tests/_files/source_with_dead_code_in_various_contexts.php @@ -0,0 +1,108 @@ + 0) { + return 1; + } elseif ($n < 0) { + return 2; + $x = 3; + } + + return 0; +} + +function deadCodeInWhileBody(int $n): void +{ + while ($n > 0) { + break; + $x = 1; + } +} + +function deadCodeInDoWhileBody(int $n): void +{ + do { + break; + $x = 1; + } while ($n > 0); +} + +function deadCodeInForeachBody(array $items): void +{ + foreach ($items as $item) { + continue; + $x = 1; + } +} + +function deadCodeInSwitchCase(int $n): int +{ + switch ($n) { + case 1: + return 1; + $x = 2; + + default: + return 0; + } +} + +function deadCodeInTryCatchFinally(): int +{ + try { + throw new \RuntimeException; + $x = 1; + } catch (\RuntimeException $e) { + return 1; + $x = 2; + } finally { + return 2; + $x = 3; + } +} + +function multiLineShortTernaryWithLiteralFalseCondition(): int +{ + return false + ?: 1; +} + +return 1; + +$x = 2; diff --git a/tests/_files/source_without_dead_code.php b/tests/_files/source_without_dead_code.php new file mode 100644 index 000000000..f8ad473d4 --- /dev/null +++ b/tests/_files/source_without_dead_code.php @@ -0,0 +1,41 @@ + 0) { + return 'positive'; + } + + if ($n < 0) { + return 'negative'; + } + + return 'zero'; + } + + public function loop(int $limit): int + { + $sum = 0; + + for ($i = 0; $i < $limit; $i++) { + $sum += $i; + } + + return $sum; + } + + public function infiniteLoop(): int + { + for (;;) { + return 0; + } + } +} diff --git a/tests/src/SourceAnalyserTestCase.php b/tests/src/SourceAnalyserTestCase.php index beebe7f27..8bf74a45a 100644 --- a/tests/src/SourceAnalyserTestCase.php +++ b/tests/src/SourceAnalyserTestCase.php @@ -239,6 +239,40 @@ public function testIgnoreEndWithoutStartIsHandledGracefully(): void $this->assertContains(5, $ignoredLines); } + public function testDeadCodeDetectionReportsUnreachableLines(): void + { + $result = $this->analyser()->analyse( + TEST_FILES_PATH . 'source_with_dead_code.php', + file_get_contents(TEST_FILES_PATH . 'source_with_dead_code.php'), + false, + false, + ); + + $deadLines = $result->deadLines(); + $executableLines = $result->executableLines(); + + foreach ([9, 10, 16, 22, 28, 35, 43, 50, 51, 60, 68, 69, 80, 89, 96, 103, 158] as $line) { + $this->assertArrayHasKey($line, $deadLines, "Line {$line} should be reported as dead"); + $this->assertArrayHasKey($line, $executableLines, 'Dead lines must be a subset of executable lines'); + } + } + + public function testDeadCodeDetectionLeavesLiveLinesAlone(): void + { + $result = $this->analyser()->analyse( + TEST_FILES_PATH . 'source_with_dead_code.php', + file_get_contents(TEST_FILES_PATH . 'source_with_dead_code.php'), + false, + false, + ); + + $deadLines = $result->deadLines(); + + foreach ([8, 15, 21, 27, 34, 42, 49, 58, 67, 72, 78, 83, 88, 95, 104, 110, 113, 151, 162, 174, 191] as $line) { + $this->assertArrayNotHasKey($line, $deadLines, "Line {$line} should be live"); + } + } + public function testCodeUnitsAreFound(): void { $analyser = new ParsingSourceAnalyser; diff --git a/tests/tests/CodeCoverageTest.php b/tests/tests/CodeCoverageTest.php index c34330842..88045552f 100644 --- a/tests/tests/CodeCoverageTest.php +++ b/tests/tests/CodeCoverageTest.php @@ -110,7 +110,7 @@ public function testExcludeNonExecutableLines(): void 24 => [], 29 => [], 31 => [], - 32 => [], + 32 => null, ], ]; diff --git a/tests/tests/Data/RawCodeCoverageDataTest.php b/tests/tests/Data/RawCodeCoverageDataTest.php index 0e718966c..b48ad2f9e 100644 --- a/tests/tests/Data/RawCodeCoverageDataTest.php +++ b/tests/tests/Data/RawCodeCoverageDataTest.php @@ -668,13 +668,44 @@ public function testFromUncoveredFile(): void $this->assertArrayHasKey($filename, $lineCoverage); $this->assertNotEmpty($lineCoverage[$filename]); - foreach ($lineCoverage[$filename] as $status) { + foreach ($lineCoverage[$filename] as $line => $status) { + if ($line === 32) { + // the second return statement in withdrawMoney() is dead code + $this->assertSame(-2, $status); + + continue; + } + $this->assertSame(-1, $status); } $this->assertEmpty($dataObject->functionCoverage()); } + public function testFromUncoveredFileMarksDeadLinesAsNotExecutable(): void + { + $filename = TEST_FILES_PATH . 'source_with_dead_code.php'; + $analyser = new FileAnalyser(new ParsingSourceAnalyser, false, false); + + $dataObject = RawCodeCoverageData::fromUncoveredFile($filename, $analyser); + + $lineCoverage = $dataObject->lineCoverage(); + + $this->assertArrayHasKey($filename, $lineCoverage); + + $fileCoverage = $lineCoverage[$filename]; + + foreach ([9, 10, 16, 22, 28, 35, 43, 50, 51, 60, 68, 69, 80, 89, 96, 103, 158] as $deadLine) { + $this->assertArrayHasKey($deadLine, $fileCoverage, "Line {$deadLine} should be present in the coverage map"); + $this->assertSame(-2, $fileCoverage[$deadLine], "Line {$deadLine} should be marked as not executable"); + } + + foreach ([8, 15, 21, 27, 34, 42, 49, 58, 67, 72, 78, 83, 88, 95, 104, 110, 113, 151, 162, 174, 191] as $liveLine) { + $this->assertArrayHasKey($liveLine, $fileCoverage, "Line {$liveLine} should be present in the coverage map"); + $this->assertSame(-1, $fileCoverage[$liveLine], "Line {$liveLine} should be marked as not executed"); + } + } + public function testKeepLineCoverageDataOnlyForLinesWithUnknownFile(): void { $lineDataFromDriver = [ @@ -810,6 +841,49 @@ public function testAddMissingExecutableLinesWithUnknownFile(): void $this->assertEquals($lineDataFromDriver, $dataObject->lineCoverage()); } + public function testMarkLinesAsNotExecutableDemotesLinesToTheDeadCodeStatus(): void + { + $lineDataFromDriver = [ + '/some/path/SomeClass.php' => [ + 8 => 1, + 9 => -1, + 10 => -1, + ], + ]; + + $dataObject = RawCodeCoverageData::fromXdebugWithoutPathCoverage($lineDataFromDriver); + + $dataObject->markLinesAsNotExecutable('/some/path/SomeClass.php', [9 => true, 10 => true]); + + $lineCoverage = $dataObject->lineCoverage(); + + $this->assertArrayHasKey('/some/path/SomeClass.php', $lineCoverage); + + $fileCoverage = $lineCoverage['/some/path/SomeClass.php']; + + $this->assertArrayHasKey(8, $fileCoverage); + $this->assertArrayHasKey(9, $fileCoverage); + $this->assertArrayHasKey(10, $fileCoverage); + $this->assertSame(1, $fileCoverage[8]); + $this->assertSame(-2, $fileCoverage[9]); + $this->assertSame(-2, $fileCoverage[10]); + } + + public function testMarkLinesAsNotExecutableWithUnknownFileIsNoOp(): void + { + $lineDataFromDriver = [ + '/some/path/SomeClass.php' => [ + 8 => 1, + ], + ]; + + $dataObject = RawCodeCoverageData::fromXdebugWithoutPathCoverage($lineDataFromDriver); + + $dataObject->markLinesAsNotExecutable('/some/path/UnknownFile.php', [8 => true]); + + $this->assertEquals($lineDataFromDriver, $dataObject->lineCoverage()); + } + public function testMarkExecutableLineByBranchSkipsBranchAlreadyMarkedExecuted(): void { $lineDataFromDriver = [ diff --git a/tests/tests/FilterProcessorTest.php b/tests/tests/FilterProcessorTest.php index 620ca25e1..7183b1571 100644 --- a/tests/tests/FilterProcessorTest.php +++ b/tests/tests/FilterProcessorTest.php @@ -97,7 +97,7 @@ public function testApplyExecutableLinesFilterKeepsOnlyExecutableLines(): void $filter->includeFile($file); $analyser = $this->createFileAnalyser( - new AnalysisResult([], [], [], [], new LinesOfCode(0, 0, 0), [2 => 2], [], []), + new AnalysisResult([], [], [], [], new LinesOfCode(0, 0, 0), [2 => 2], [], [], []), ); $this->processor->applyExecutableLinesFilter($data, $filter, $analyser); @@ -141,7 +141,7 @@ public function testApplyIgnoredLinesFilterRemovesIgnoredLines(): void $filter->includeFile($file); $analyser = $this->createFileAnalyser( - new AnalysisResult([], [], [], [], new LinesOfCode(0, 0, 0), [], [], [2 => 2]), + new AnalysisResult([], [], [], [], new LinesOfCode(0, 0, 0), [], [], [], [2 => 2]), ); $this->processor->applyIgnoredLinesFilter($data, $filter, $analyser); diff --git a/tests/tests/StaticAnalysis/FileAnalyserTest.php b/tests/tests/StaticAnalysis/FileAnalyserTest.php index ef08dcfa2..8a65a1f02 100644 --- a/tests/tests/StaticAnalysis/FileAnalyserTest.php +++ b/tests/tests/StaticAnalysis/FileAnalyserTest.php @@ -66,6 +66,7 @@ private function analysisResult(): AnalysisResult [], [], [], + [], ); } } diff --git a/tests/tests/StaticAnalysis/Value/AnalysisResultTest.php b/tests/tests/StaticAnalysis/Value/AnalysisResultTest.php index 1e7a2b9c1..1673b5491 100644 --- a/tests/tests/StaticAnalysis/Value/AnalysisResultTest.php +++ b/tests/tests/StaticAnalysis/Value/AnalysisResultTest.php @@ -128,6 +128,18 @@ public function testHasIgnoredLines(): void $this->assertSame($ignoredLines, $this->analysisResult(ignoredLines: $ignoredLines)->ignoredLines()); } + public function testHasDeadLines(): void + { + $deadLines = [9 => true]; + + $this->assertSame($deadLines, $this->analysisResult(deadLines: $deadLines)->deadLines()); + } + + public function testHasNoDeadLinesByDefault(): void + { + $this->assertSame([], $this->analysisResult()->deadLines()); + } + /** * @param array $interfaces * @param array $classes @@ -135,9 +147,10 @@ public function testHasIgnoredLines(): void * @param array $functions * @param array $executableLines * @param array $branchOperatorLines + * @param array $deadLines * @param array $ignoredLines */ - private function analysisResult(array $interfaces = [], array $classes = [], array $traits = [], array $functions = [], ?LinesOfCode $linesOfCode = null, array $executableLines = [], array $branchOperatorLines = [], array $ignoredLines = []): AnalysisResult + private function analysisResult(array $interfaces = [], array $classes = [], array $traits = [], array $functions = [], ?LinesOfCode $linesOfCode = null, array $executableLines = [], array $branchOperatorLines = [], array $deadLines = [], array $ignoredLines = []): AnalysisResult { return new AnalysisResult( $interfaces, @@ -147,6 +160,7 @@ private function analysisResult(array $interfaces = [], array $classes = [], arr $linesOfCode ?? new LinesOfCode(0, 0, 0), $executableLines, $branchOperatorLines, + $deadLines, $ignoredLines, ); } diff --git a/tests/tests/StaticAnalysis/Visitor/DeadCodeFindingVisitorTest.php b/tests/tests/StaticAnalysis/Visitor/DeadCodeFindingVisitorTest.php new file mode 100644 index 000000000..7ecb9024f --- /dev/null +++ b/tests/tests/StaticAnalysis/Visitor/DeadCodeFindingVisitorTest.php @@ -0,0 +1,187 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ +namespace SebastianBergmann\CodeCoverage\StaticAnalysis; + +use function array_keys; +use function assert; +use function file_get_contents; +use PhpParser\Node\Expr\Assign; +use PhpParser\Node\Expr\ConstFetch; +use PhpParser\Node\Expr\Variable; +use PhpParser\Node\Name; +use PhpParser\Node\Scalar\Int_; +use PhpParser\Node\Stmt\Else_; +use PhpParser\Node\Stmt\Expression; +use PhpParser\Node\Stmt\If_; +use PhpParser\NodeTraverser; +use PhpParser\ParserFactory; +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\Group; +use PHPUnit\Framework\Attributes\Small; +use PHPUnit\Framework\TestCase; + +#[CoversClass(DeadCodeFindingVisitor::class)] +#[Small] +#[Group('static-analysis')] +final class DeadCodeFindingVisitorTest extends TestCase +{ + public function testIdentifiesLinesUnreachableFromTheStaticControlFlow(): void + { + $deadLines = $this->deadLines(TEST_FILES_PATH . 'source_with_dead_code.php'); + + // The raw set includes wrapper-boundary lines (e.g. `} else {` and the + // closing `}` of the dead block) that are not executable on their own. + // ParsingSourceAnalyser intersects this set with the executable-lines + // map before publishing it, which drops those boundaries. + $this->assertSame( + [ + 9, // after return + 10, // after return + 16, // after throw statement + 22, // after throw statement (second occurrence) + 28, // after exit + 35, // after break + 43, // after continue + 50, // body of if (false) + 51, // body of if (false) + 59, // `} else {` wrapper line, dropped by the intersection + 60, // dead body of else after if (true) + 61, // closing `}` wrapper line, dropped by the intersection + 68, // elseif after if (true) — condition line itself + 69, // dead body of elseif after if (true) + 70, // closing `}` wrapper line, dropped by the intersection + 80, // body of elseif (false) + 89, // body of while (false) + 96, // body of for (...; false; ...) + 103, // dead arm of ternary with literal-false condition + 120, // dead arm of ternary with literal-true condition + 136, // body of if (false), with a trailing Nop that must be skipped + 158, // after goto + ], + array_keys($deadLines), + ); + } + + public function testReportsLiveCodeAsNotDead(): void + { + $deadLines = $this->deadLines(TEST_FILES_PATH . 'source_with_dead_code.php'); + + foreach ([8, 15, 21, 27, 34, 42, 49, 58, 67, 72, 78, 83, 88, 95, 104, 110, 113, 125, 130] as $liveLine) { + $this->assertArrayNotHasKey($liveLine, $deadLines); + } + } + + public function testCodeReachableOnlyThroughGotoIsNotDead(): void + { + $deadLines = $this->deadLines(TEST_FILES_PATH . 'source_with_dead_code.php'); + + foreach ([ + 147, // return before a label; the label resets reachability for what follows + 149, // label after a terminator + 151, // return reachable only through goto + 160, // label after dead code following a goto + 162, // return reachable by falling through the label + 172, // label inside if (false) + 174, // return inside if (false), reachable only through goto + 189, // label inside else after if (true) + 191, // return inside else after if (true), reachable only through goto + 204, // label inside elseif after if (true) + 206, // return inside elseif after if (true), reachable only through goto + ] as $liveLine) { + $this->assertArrayNotHasKey($liveLine, $deadLines, "Line {$liveLine} should be live"); + } + } + + public function testIdentifiesDeadCodeAfterTerminatorInAllStatementContexts(): void + { + $deadLines = $this->deadLines(TEST_FILES_PATH . 'source_with_dead_code_in_various_contexts.php'); + + $this->assertSame( + [ + 7, // after return in a function body + 14, // after return in a closure body + 22, // after return in an if body + 34, // after return in an else body + 44, // after return in an elseif body + 54, // after break in a while body + 62, // after break in a do-while body + 70, // after continue in a foreach body + 79, // after return in a switch case + 90, // after throw in a try body + 93, // after return in a catch body + 96, // after return in a finally body + 108, // after return at namespace level + ], + array_keys($deadLines), + ); + } + + public function testLiveArmOfMultiLineShortTernaryWithLiteralFalseConditionIsNotDead(): void + { + $deadLines = $this->deadLines(TEST_FILES_PATH . 'source_with_dead_code_in_various_contexts.php'); + + // `false ?: 1` has no "if" arm to mark dead; its result is the + // (live) else arm + foreach ([102, 103] as $liveLine) { + $this->assertArrayNotHasKey($liveLine, $deadLines, "Line {$liveLine} should be live"); + } + } + + public function testReportsNoDeadLinesForSourceWithoutAny(): void + { + $deadLines = $this->deadLines(TEST_FILES_PATH . 'source_without_dead_code.php'); + + $this->assertSame([], $deadLines); + } + + public function testIgnoresNodesWithoutLineInformation(): void + { + // Synthetic AST nodes default to a -1 line (per PHP-Parser's + // phpstan-return contract for getStartLine()/getEndLine()). The + // visitor must not record those as dead lines. + // + // The wrapper If_ carries positive line attributes so the + // wrapper-boundary skip in markBlock cannot mask the < 1 guard. + $deadBody = new Expression(new Assign(new Variable('x'), new Int_(1))); + $ifFalse = new If_( + new ConstFetch(new Name('false')), + ['stmts' => [$deadBody]], + ['startLine' => 5, 'endLine' => 10], + ); + $ifTrue = new If_(new ConstFetch(new Name('true')), ['stmts' => [], 'else' => new Else_([])]); + + $visitor = new DeadCodeFindingVisitor; + $visitor->enterNode($ifFalse); + $visitor->enterNode($ifTrue); + + $this->assertSame([], $visitor->deadLines()); + } + + /** + * @return array + */ + private function deadLines(string $file): array + { + $source = file_get_contents($file); + assert($source !== false); + + $parser = (new ParserFactory)->createForHostVersion(); + $nodes = $parser->parse($source); + assert($nodes !== null); + + $visitor = new DeadCodeFindingVisitor; + + $traverser = new NodeTraverser; + $traverser->addVisitor($visitor); + $traverser->traverse($nodes); + + return $visitor->deadLines(); + } +}