Identify dead code using static analysis#1231
Open
sebastianbergmann wants to merge 3 commits into
Open
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1231 +/- ##
============================================
+ Coverage 98.04% 98.09% +0.05%
- Complexity 1618 1696 +78
============================================
Files 114 115 +1
Lines 5419 5563 +144
============================================
+ Hits 5313 5457 +144
Misses 106 106 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
API Surface ChangesIf any of the additions below are not intended as public API, mark them with New API SurfaceMethods
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Xdebug populates a third line status, not executable (
-2), for source lines that the PHP compiler/optimiser has identified as dead code.PCOV does not do this: it only distinguishes executed from not executed, so suites running under PCOV report unreachable lines as ordinary coverage gaps and surface false negatives in the report.
This is an attempt to close that gap: it adds an AST-based dead-code analysis pass to the static analyser so dead lines can be surfaced as
LINE_NOT_EXECUTABLEregardless of which collector produced the raw coverage data, without depending on bytecode-level introspection that PCOV does not expose.What "dead" means here
The new analysis is structural and intra-procedural. It works entirely from the PHP-Parser AST, so it can only report what is locally derivable from the source:
stmtsblock:return,throw,exit/die,break,continueif (false) { ... }/elseif (false) { ... }elseifandelsetails afterif (true)while (false) { ... }andfor (...; false; ...)Whole-program reasoning (never-called functions, opcode-folded branches under user-defined constants, optimiser decisions) is explicitly out of scope.
The AST analyser will always be a strict subset of what bytecode-level dead-code detection can see, but it covers the structural cases that drive most user-visible "phantom uncovered line" complaints under PCOV.
Architecture
The analysis is layered the same way the existing line classification is, visitor → source analyser → file analyser → consumer, so it composes cleanly with the existing pipeline and the static-analysis cache.
DeadCodeFindingVisitorA
NodeVisitorAbstractthat walks every parsed file once and accumulates a setarray<positive-int, true>of line numbers that the patterns above mark as unreachable.It marks line ranges rather than line-by-line: e.g. for
if (true) { ... } else { ... }it marks the entire range of theelsenode, including wrapper-boundary lines like} else {and the closing}. Those boundary lines are not classified as executable in the first place, so the downstream intersection drops them; this keeps the visitor simple and avoids depending on the executable-lines classifier from inside the visitor.ParsingSourceAnalyserOptionally constructs the new visitor (controlled by a constructor flag) and intersects its raw output with
ExecutableLinesFindingVisitor::executableLinesGroupedByBranch()before publishing the dead-line set on theAnalysisResult. This gives the invariantdeadLines() ⊆ executableLines()by construction and the consumer never has to defend against a "dead but not classified executable" line.When the flag is off (the default) the visitor is not constructed, the traversal runs exactly as before, and
AnalysisResult::deadLines()returns[]. The traversal cost when the flag is on is one extra visitor on the same node walk, no second parse or second pass over the AST.AnalysisResultGains a
deadLines(): array<positive-int, true>accessor alongside the existingexecutableLines()/branchOperatorLines()/ignoredLines(). The constructor parameter is required (no defaulting) so it cannot be silently omitted when the result is constructed by hand in tests.CachingSourceAnalyserThe dead-code-detection flag participates in the cache key. Flipping the flag invalidates cached entries automatically, so a project that toggles the option on does not need to nuke its analysis cache.
FilterProcessorandRawCodeCoverageDataTwo integration points so dead lines surface in both covered and uncovered files:
FilterProcessor::applyExecutableLinesFilter()calls a newRawCodeCoverageData::markLinesAsNotExecutable()after the existing executable-line normalization. This demotes dead lines fromLINE_NOT_EXECUTED(or whatever propagation left behind) toLINE_NOT_EXECUTABLE.RawCodeCoverageData::fromUncoveredFile()seeds dead lines asLINE_NOT_EXECUTABLEdirectly, so files that no test ever touched still report dead code correctly.The ordering inside
applyExecutableLinesFilter()matters: dead-line marking runs aftermarkExecutableLineByBranch()so the override beats any branch-propagation that may have crossed a dead line.Configuration surface
A single boolean toggle on
CodeCoverage:Off by default to preserve existing behaviour and existing reports across upgrades. The flag is threaded through
Registry::analyser()into the source analyser; no other public API moves.