diff --git a/src/CodeCoverage.php b/src/CodeCoverage.php index ea86162d4..595c9e806 100644 --- a/src/CodeCoverage.php +++ b/src/CodeCoverage.php @@ -49,6 +49,7 @@ final class CodeCoverage private bool $includeUncoveredFiles = true; private bool $ignoreDeprecatedCode = false; private bool $useAnnotationsForIgnoringCode = true; + private bool $staticallyDetectDeadCode = false; /** * @var list @@ -330,6 +331,21 @@ public function doNotIgnoreDeprecatedCode(): void $this->ignoreDeprecatedCode = false; } + public function enableStaticDeadCodeDetection(): void + { + $this->staticallyDetectDeadCode = true; + } + + public function disableStaticDeadCodeDetection(): void + { + $this->staticallyDetectDeadCode = false; + } + + public function staticallyDetectsDeadCode(): bool + { + return $this->staticallyDetectDeadCode; + } + /** * @phpstan-assert-if-true !null $this->cacheDirectory */ @@ -455,6 +471,7 @@ private function analyser(): FileAnalyser $this->cacheDirectory, $this->useAnnotationsForIgnoringCode, $this->ignoreDeprecatedCode, + $this->staticallyDetectDeadCode, ); } } 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/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/CacheWarmer.php b/src/StaticAnalysis/CacheWarmer.php index d02fef3c7..33a27c66b 100644 --- a/src/StaticAnalysis/CacheWarmer.php +++ b/src/StaticAnalysis/CacheWarmer.php @@ -24,11 +24,12 @@ * * @return array{cacheHits: non-negative-int, cacheMisses: non-negative-int} */ - public function warmCache(string $cacheDirectory, bool $useAnnotationsForIgnoringCode, bool $ignoreDeprecatedCode, Filter $filter): array + public function warmCache(string $cacheDirectory, bool $useAnnotationsForIgnoringCode, bool $ignoreDeprecatedCode, Filter $filter, bool $detectDeadCode = false): array { $analyser = new CachingSourceAnalyser( $cacheDirectory, - new ParsingSourceAnalyser, + new ParsingSourceAnalyser($detectDeadCode), + $detectDeadCode, ); foreach ($filter->files() as $file) { diff --git a/src/StaticAnalysis/CachingSourceAnalyser.php b/src/StaticAnalysis/CachingSourceAnalyser.php index 09714e4a7..469cff19b 100644 --- a/src/StaticAnalysis/CachingSourceAnalyser.php +++ b/src/StaticAnalysis/CachingSourceAnalyser.php @@ -32,6 +32,7 @@ final class CachingSourceAnalyser implements SourceAnalyser */ private readonly string $directory; private readonly SourceAnalyser $sourceAnalyser; + private readonly bool $detectDeadCode; /** * @var non-negative-int @@ -46,12 +47,13 @@ final class CachingSourceAnalyser implements SourceAnalyser /** * @param non-empty-string $directory */ - public function __construct(string $directory, SourceAnalyser $sourceAnalyser) + public function __construct(string $directory, SourceAnalyser $sourceAnalyser, bool $detectDeadCode = false) { Filesystem::createDirectory($directory); $this->directory = $directory; $this->sourceAnalyser = $sourceAnalyser; + $this->detectDeadCode = $detectDeadCode; } /** @@ -165,6 +167,7 @@ private function cacheFile(string $source, bool $useAnnotationsForIgnoringCode, Version::id(), $useAnnotationsForIgnoringCode, $ignoreDeprecatedCode, + $this->detectDeadCode, ], ), ); diff --git a/src/StaticAnalysis/ParsingSourceAnalyser.php b/src/StaticAnalysis/ParsingSourceAnalyser.php index 034bd1887..92f0746ac 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; @@ -37,10 +38,12 @@ final readonly class ParsingSourceAnalyser implements SourceAnalyser { private Parser $parser; + private bool $detectDeadCode; - public function __construct() + public function __construct(bool $detectDeadCode = false) { - $this->parser = (new ParserFactory)->createForHostVersion(); + $this->parser = (new ParserFactory)->createForHostVersion(); + $this->detectDeadCode = $detectDeadCode; } /** @@ -62,6 +65,7 @@ public function analyse(string $sourceCodeFile, string $sourceCode, bool $useAnn $lineCountingVisitor = new LineCountingVisitor($linesOfCode); $ignoredLinesFindingVisitor = new IgnoredLinesFindingVisitor($useAnnotationsForIgnoringCode, $ignoreDeprecatedCode); $executableLinesFindingVisitor = new ExecutableLinesFindingVisitor($sourceCode); + $deadCodeFindingVisitor = null; $traverser->addVisitor(new NameResolver); $traverser->addVisitor(new AttributeParentConnectingVisitor); @@ -70,6 +74,11 @@ public function analyse(string $sourceCodeFile, string $sourceCode, bool $useAnn $traverser->addVisitor($ignoredLinesFindingVisitor); $traverser->addVisitor($executableLinesFindingVisitor); + if ($this->detectDeadCode) { + $deadCodeFindingVisitor = new DeadCodeFindingVisitor; + $traverser->addVisitor($deadCodeFindingVisitor); + } + /* @noinspection UnusedFunctionResultInspection */ $traverser->traverse($nodes); // @codeCoverageIgnoreStart @@ -99,6 +108,16 @@ public function analyse(string $sourceCodeFile, string $sourceCode, bool $useAnn $ignoredLines = array_keys($ignoredLines); + $executableLines = $executableLinesFindingVisitor->executableLinesGroupedByBranch(); + $deadLines = []; + + if ($deadCodeFindingVisitor !== null) { + $deadLines = array_intersect_key( + $deadCodeFindingVisitor->deadLines(), + $executableLines, + ); + } + return new AnalysisResult( $codeUnitFindingVisitor->interfaces(), $codeUnitFindingVisitor->classes(), @@ -109,8 +128,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/Registry.php b/src/StaticAnalysis/Registry.php index 3ea34ad24..7e65935bd 100644 --- a/src/StaticAnalysis/Registry.php +++ b/src/StaticAnalysis/Registry.php @@ -21,18 +21,19 @@ final class Registry /** * @param ?non-empty-string $cacheDirectory */ - public static function analyser(?string $cacheDirectory, bool $useAnnotationsForIgnoringCode, bool $ignoreDeprecatedCode): FileAnalyser + public static function analyser(?string $cacheDirectory, bool $useAnnotationsForIgnoringCode, bool $ignoreDeprecatedCode, bool $detectDeadCode = false): FileAnalyser { if (self::$analyser !== null) { return self::$analyser; } - $sourceAnalyser = new ParsingSourceAnalyser; + $sourceAnalyser = new ParsingSourceAnalyser($detectDeadCode); if ($cacheDirectory !== null) { $sourceAnalyser = new CachingSourceAnalyser( $cacheDirectory, $sourceAnalyser, + $detectDeadCode, ); } 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..5c93a664e --- /dev/null +++ b/src/StaticAnalysis/Visitor/DeadCodeFindingVisitor.php @@ -0,0 +1,261 @@ + + * + * 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\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`). + * - 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. + * + * 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) { + $this->markRange($elseif->getStartLine(), $elseif->getEndLine()); + } + + if ($node->else !== null) { + $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 && !$stmt instanceof Node\Stmt\Nop) { + $this->markRange($stmt->getStartLine(), $stmt->getEndLine()); + } + + if (!$terminated && $this->isTerminator($stmt)) { + $terminated = true; + } + } + } + + /** + * @param array $stmts + */ + private function markBlock(array $stmts, Node $wrapper): void + { + $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_) { + return true; + } + + if ($stmt instanceof Node\Stmt\Expression) { + return $stmt->expr instanceof Node\Expr\Throw_ || + $stmt->expr instanceof Node\Expr\Exit_; + } + + return false; + } + + /** + * @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..8a71ba101 --- /dev/null +++ b/tests/_files/source_with_dead_code.php @@ -0,0 +1,140 @@ + 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 */ + } + } +} 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..f6eca7780 100644 --- a/tests/src/SourceAnalyserTestCase.php +++ b/tests/src/SourceAnalyserTestCase.php @@ -239,6 +239,52 @@ public function testIgnoreEndWithoutStartIsHandledGracefully(): void $this->assertContains(5, $ignoredLines); } + public function testDeadCodeDetectionIsDisabledByDefault(): 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, + ); + + $this->assertSame([], $result->deadLines()); + } + + public function testDeadCodeDetectionReportsUnreachableLinesWhenEnabled(): void + { + $result = $this->analyserWithDeadCodeDetection()->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] 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->analyserWithDeadCodeDetection()->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] as $line) { + $this->assertArrayNotHasKey($line, $deadLines, "Line {$line} should be live"); + } + } + public function testCodeUnitsAreFound(): void { $analyser = new ParsingSourceAnalyser; @@ -267,4 +313,6 @@ public function testCodeUnitsAreFound(): void } abstract protected function analyser(): SourceAnalyser; + + abstract protected function analyserWithDeadCodeDetection(): SourceAnalyser; } diff --git a/tests/tests/CodeCoverageTest.php b/tests/tests/CodeCoverageTest.php index 654d4f574..8baf54966 100644 --- a/tests/tests/CodeCoverageTest.php +++ b/tests/tests/CodeCoverageTest.php @@ -272,6 +272,30 @@ public function testIgnoreAndDoNotIgnoreDeprecatedCode(): void $this->assertNotNull($coverage->getData(true)); } + public function testDeadCodeDetectionIsDisabledByDefault(): void + { + $coverage = new CodeCoverage( + $this->createStub(Driver::class), + new Filter, + ); + + $this->assertFalse($coverage->staticallyDetectsDeadCode()); + } + + public function testEnableAndDisableDeadCodeDetection(): void + { + $coverage = new CodeCoverage( + $this->createStub(Driver::class), + new Filter, + ); + + $coverage->enableStaticDeadCodeDetection(); + $this->assertTrue($coverage->staticallyDetectsDeadCode()); + + $coverage->disableStaticDeadCodeDetection(); + $this->assertFalse($coverage->staticallyDetectsDeadCode()); + } + public function testCacheStaticAnalysisAndDoNotCacheStaticAnalysis(): void { $coverage = new CodeCoverage( diff --git a/tests/tests/Data/RawCodeCoverageDataTest.php b/tests/tests/Data/RawCodeCoverageDataTest.php index 0e718966c..d72cb59b5 100644 --- a/tests/tests/Data/RawCodeCoverageDataTest.php +++ b/tests/tests/Data/RawCodeCoverageDataTest.php @@ -675,6 +675,30 @@ public function testFromUncoveredFile(): void $this->assertEmpty($dataObject->functionCoverage()); } + public function testFromUncoveredFileMarksDeadLinesAsNotExecutable(): void + { + $filename = TEST_FILES_PATH . 'source_with_dead_code.php'; + $analyser = new FileAnalyser(new ParsingSourceAnalyser(true), 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] 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] 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 +834,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/CachingSourceAnalyserTest.php b/tests/tests/StaticAnalysis/CachingSourceAnalyserTest.php index 0799f95a4..cc87b0ffb 100644 --- a/tests/tests/StaticAnalysis/CachingSourceAnalyserTest.php +++ b/tests/tests/StaticAnalysis/CachingSourceAnalyserTest.php @@ -31,4 +31,13 @@ protected function analyser(): SourceAnalyser new ParsingSourceAnalyser, ); } + + protected function analyserWithDeadCodeDetection(): SourceAnalyser + { + return new CachingSourceAnalyser( + sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'php-code-coverage-tests-for-static-analysis-dead-code', + new ParsingSourceAnalyser(true), + true, + ); + } } 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/ParsingSourceAnalyserTest.php b/tests/tests/StaticAnalysis/ParsingSourceAnalyserTest.php index 3a49d499e..2d8cc573b 100644 --- a/tests/tests/StaticAnalysis/ParsingSourceAnalyserTest.php +++ b/tests/tests/StaticAnalysis/ParsingSourceAnalyserTest.php @@ -24,4 +24,9 @@ protected function analyser(): SourceAnalyser { return new ParsingSourceAnalyser; } + + protected function analyserWithDeadCodeDetection(): SourceAnalyser + { + return new ParsingSourceAnalyser(true); + } } 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..8df779bab --- /dev/null +++ b/tests/tests/StaticAnalysis/Visitor/DeadCodeFindingVisitorTest.php @@ -0,0 +1,130 @@ + + * + * 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 + ], + 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 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(); + } +}