ExecutableLineExcluder: ability not to count certain lines as executable (#67)

Author: janedbal

README.md

@@ -69,6 +69,7 @@ vendor/bin/coverage-guard check clover.xml --patch changes.patch
 <?php
 
 use ShipMonk\CoverageGuard\Config;
+use ShipMonk\CoverageGuard\Excluder\IgnoreThrowNewExceptionLineExcluder;
 use ShipMonk\CoverageGuard\Rule\EnforceCoverageForMethodsRule;
 
 $config = new Config();
@@ -80,6 +81,9 @@ $config->addRule(new EnforceCoverageForMethodsRule(
     minExecutableLines: 5, // only check methods with at least 5 executable lines
 ));
 
+// Lines you don't want to enforce coverage for (they stop being treated as executable)
+$config->addExecutableLineExcluder(new IgnoreThrowNewExceptionLineExcluder([LogicException::class]));
+
 // Replace prefix of absolute paths in coverage files
 // Handy if you want to reuse clover.xml generated in CI
 $config->addCoveragePathMapping('/absolute/ci/prefix', __DIR__);
@@ -102,10 +106,14 @@ return $config;
      - [own coverage config](./coverage-guard.php)
      - [dead-code-detector](https://github.com/shipmonk-rnd/dead-code-detector/blob/master/coverage-guard.php)
      - [phpstan-rules](https://github.com/shipmonk-rnd/phpstan-rules/blob/master/coverage-guard.php)
+- For custom line exclusion, implement `ExecutableLineExcluder` and pass it to `Config::addExecutableLineExcluder()` method:
+  - It receives each AST node together with `ExclusionContext` (file path, line contents) and may return an `ExcludedLineRange`
+  - Excluded lines are honored by both `check` and `patch-coverage` commands and rendered with gray background
+  - Inspire by prepared [`IgnoreThrowNewExceptionLineExcluder`](src/Excluder/IgnoreThrowNewExceptionLineExcluder.php)
 
 
 ### What can you enforce:
-The `CodeBlock` class passed to `CoverageRule` is aware **which line is executable, changed and covered**.
+The `CodeBlock` class passed to `CoverageRule` is aware **which line is executable, excluded, changed and covered**.
 Also, you can use **reflection** to pinpoint your rules.
 This allows you to setup huge variety of rules, examples:
 

bin/coverage-guard

@@ -65,7 +65,7 @@ try {
     $registry->register(new CheckCommand($configResolver, $coverageGuard, $errorFormatter));
     $registry->register(new MergeCommand($coverageProvider, new CoverageMerger(), $coverageFormatDetector, $configResolver, $stdoutPrinter));
     $registry->register(new ConvertCommand($coverageProvider, $configResolver, $stdoutPrinter));
-    $registry->register(new PatchCoverageCommand($stdoutPrinter, $patchParser, $configResolver, $coverageProvider));
+    $registry->register(new PatchCoverageCommand($stdoutPrinter, $patchParser, $configResolver, $coverageProvider, $fileTraverser));
     $registry->register(new InitCommand($cwd, $stdoutPrinter));
 
     $parameterResolver = new ParameterResolver();

composer-dependency-analyser.php

@@ -4,6 +4,7 @@
 use ShipMonk\ComposerDependencyAnalyser\Config\ErrorType;
 
 $config = new Configuration();
+$config->ignoreUnknownClasses(['PhpParser\Node\Stmt\Throw_']); // old version of php-parser
 $config->ignoreErrorsOnPackage('phpunit/php-code-coverage', [ErrorType::DEV_DEPENDENCY_IN_PROD]); // optional dependency to load .cov files
 $config->ignoreErrorsOnPackage('sebastian/diff', [ErrorType::DEV_DEPENDENCY_IN_PROD]); // optional dependency to parse patch files
 $config->ignoreErrorsOnExtension('ext-tokenizer', [ErrorType::SHADOW_DEPENDENCY]); // optional dependency to have syntax highlighting

coverage-guard.php

@@ -1,6 +1,7 @@
 <?php declare(strict_types = 1);
 
 use ShipMonk\CoverageGuard\Config;
+use ShipMonk\CoverageGuard\Excluder\IgnoreThrowNewExceptionLineExcluder;
 use ShipMonk\CoverageGuard\Hierarchy\ClassMethodBlock;
 use ShipMonk\CoverageGuard\Hierarchy\CodeBlock;
 use ShipMonk\CoverageGuard\Rule\CoverageError;
@@ -45,6 +46,8 @@ private function isPublicApiClass(ReflectionClass $classReflection): bool
 
 });
 
+$config->addExecutableLineExcluder(new IgnoreThrowNewExceptionLineExcluder([LogicException::class]));
+
 $localConfig = __DIR__ . '/coverage-guard.local.php';
 if (is_file($localConfig)) {
     require $localConfig; // handy for $config->setEditorUrl()

phpstan.neon.dist

@@ -62,6 +62,7 @@ parameters:
                 ShipMonk\CoverageGuard\Rule\CoverageRule: Rule
                 ShipMonk\CoverageGuard\Writer\CoverageWriter: CoverageWriter
                 ShipMonk\CoverageGuard\Extractor\CoverageExtractor: Extractor
+                ShipMonk\CoverageGuard\Excluder\ExecutableLineExcluder: LineExcluder
 
     ignoreErrors:
         # allow calling internal methods from SebastianBergmann\CodeCoverage
@@ -77,6 +78,13 @@ parameters:
             paths:
                 - src/Utils/PatchParser.php
 
+        # support even old nikic/php-parser
+        -
+            message: '#PhpParser\\Node\\Stmt\\Throw_#'
+            identifier: class.notFound
+            paths:
+                - src/Excluder/IgnoreThrowNewExceptionLineExcluder.php
+
         # do not track checked exceptions in tests
         -
             identifiers:

src/Ast/FileTraverser.php

@@ -5,9 +5,9 @@
 use LogicException;
 use PhpParser\Error as ParseError;
 use PhpParser\NodeTraverser;
+use PhpParser\NodeVisitor;
 use PhpParser\NodeVisitor\NameResolver;
 use PhpParser\Parser as PhpParser;
-use ShipMonk\CoverageGuard\CodeBlockAnalyser;
 use ShipMonk\CoverageGuard\Exception\ErrorException;
 use function implode;
 use const PHP_EOL;
@@ -29,17 +29,14 @@ public function __construct(
     public function traverse(
         string $file,
         array $fileLines,
-        CodeBlockAnalyser $analyser,
+        NodeVisitor ...$visitors,
     ): void
     {
         $nameResolver = new NameResolver();
 
         $nameResolvingTraverser = new NodeTraverser();
         $nameResolvingTraverser->addVisitor($nameResolver);
 
-        $analyserTraverser = new NodeTraverser();
-        $analyserTraverser->addVisitor($analyser);
-
         try {
             /** @throws ParseError */
             $ast = $this->phpParser->parse(implode(PHP_EOL, $fileLines));
@@ -51,7 +48,14 @@ public function traverse(
             throw new LogicException("Failed to parse PHP code in file {$file}. Should never happen as Throwing error handler is used.");
         }
 
-        $analyserTraverser->traverse($nameResolvingTraverser->traverse($ast));
+        $resolvedAst = $nameResolvingTraverser->traverse($ast);
+
+        // each visitor gets its own traversal so that later visitors can rely on complete state of earlier ones
+        foreach ($visitors as $visitor) {
+            $visitorTraverser = new NodeTraverser();
+            $visitorTraverser->addVisitor($visitor);
+            $visitorTraverser->traverse($resolvedAst);
+        }
     }
 
 }

src/CodeBlockAnalyser.php

@@ -7,6 +7,7 @@
 use PhpParser\Node\Stmt\ClassLike;
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\NodeVisitorAbstract;
+use ShipMonk\CoverageGuard\Excluder\ExcluderVisitor;
 use ShipMonk\CoverageGuard\Hierarchy\ClassMethodBlock;
 use ShipMonk\CoverageGuard\Hierarchy\LineOfCode;
 use ShipMonk\CoverageGuard\Report\ReportedError;
@@ -36,6 +37,7 @@ final class CodeBlockAnalyser extends NodeVisitorAbstract
      * @param array<int, int> $linesCoverage executable_line => hits
      * @param array<int, string> $linesContents
      * @param list<CoverageRule> $rules
+     * @param ExcluderVisitor $excluderVisitor must be traversed before this visitor
      */
     public function __construct(
         private readonly bool $patchMode,
@@ -44,6 +46,7 @@ public function __construct(
         private readonly array $linesCoverage,
         private readonly array $linesContents,
         private readonly array $rules,
+        private readonly ExcluderVisitor $excluderVisitor,
     )
     {
         $this->updateContext();
@@ -130,6 +133,7 @@ private function getLines(
             $executableLines[] = new LineOfCode(
                 number: $lineNumber,
                 executable: isset($this->linesCoverage[$lineNumber]),
+                excluded: $this->excluderVisitor->isLineExcluded($lineNumber),
                 covered: isset($this->linesCoverage[$lineNumber]) && $this->linesCoverage[$lineNumber] > 0,
                 changed: isset($this->linesChanged[$lineNumber]),
                 contents: $this->linesContents[$lineNumber],

src/Command/PatchCoverageCommand.php

@@ -2,15 +2,24 @@
 
 namespace ShipMonk\CoverageGuard\Command;
 
+use ShipMonk\CoverageGuard\Ast\FileTraverser;
 use ShipMonk\CoverageGuard\Cli\Arguments\CoverageFileCliArgument;
 use ShipMonk\CoverageGuard\Cli\Options\ConfigCliOption;
 use ShipMonk\CoverageGuard\Cli\Options\PatchCliOption;
+use ShipMonk\CoverageGuard\Coverage\FileCoverage;
 use ShipMonk\CoverageGuard\CoverageProvider;
 use ShipMonk\CoverageGuard\Exception\ErrorException;
+use ShipMonk\CoverageGuard\Excluder\ExcluderVisitor;
+use ShipMonk\CoverageGuard\Excluder\ExclusionContext;
+use ShipMonk\CoverageGuard\Excluder\ExecutableLineExcluder;
 use ShipMonk\CoverageGuard\Printer;
 use ShipMonk\CoverageGuard\Utils\ConfigResolver;
+use ShipMonk\CoverageGuard\Utils\FileUtils;
 use ShipMonk\CoverageGuard\Utils\PatchParser;
+use function array_combine;
+use function count;
 use function number_format;
+use function range;
 
 final class PatchCoverageCommand implements Command
 {
@@ -20,6 +29,7 @@ public function __construct(
         private readonly PatchParser $patchParser,
         private readonly ConfigResolver $configResolver,
         private readonly CoverageProvider $coverageProvider,
+        private readonly FileTraverser $fileTraverser,
     )
     {
     }
@@ -42,6 +52,7 @@ public function __invoke(
 
         $coveragePerFile = $this->coverageProvider->getCoverage($config, $coverageFile);
         $changesPerFile = $this->patchParser->getPatchChangedLines($patchPath, $config);
+        $excluders = $config->getExecutableLineExcluders();
 
         // Calculate coverage for changed lines
         $totalChangedLines = 0;
@@ -52,23 +63,76 @@ public function __invoke(
                 continue; // File not in coverage report
             }
 
-            $fileCoverage = $coveragePerFile[$file];
-            $executableLinesMap = [];
+            $changedExecutableLines = $this->getChangedExecutableLines($coveragePerFile[$file], $changedLines);
 
-            foreach ($fileCoverage->executableLines as $line) {
-                $executableLinesMap[$line->lineNumber] = $line->hits > 0;
-            }
+            $excluderVisitor = $excluders !== [] && $changedExecutableLines !== []
+                ? $this->createExcluderVisitor($excluders, $file)
+                : null;
 
-            foreach ($changedLines as $lineNumber) {
-                if (isset($executableLinesMap[$lineNumber])) {
-                    $totalChangedLines++;
-                    if ($executableLinesMap[$lineNumber]) {
-                        $totalCoveredLines++;
-                    }
+            foreach ($changedExecutableLines as $lineNumber => $isCovered) {
+                if ($excluderVisitor?->isLineExcluded($lineNumber) === true) {
+                    continue;
+                }
+                $totalChangedLines++;
+                if ($isCovered) {
+                    $totalCoveredLines++;
                 }
             }
         }
 
+        $this->printStatistics($totalChangedLines, $totalCoveredLines);
+
+        return 0;
+    }
+
+    /**
+     * @param list<int> $changedLines
+     * @return array<int, bool> line number => is covered
+     */
+    private function getChangedExecutableLines(
+        FileCoverage $fileCoverage,
+        array $changedLines,
+    ): array
+    {
+        $executableLinesMap = [];
+        foreach ($fileCoverage->executableLines as $line) {
+            $executableLinesMap[$line->lineNumber] = $line->hits > 0;
+        }
+
+        $changedExecutableLines = [];
+        foreach ($changedLines as $lineNumber) {
+            if (isset($executableLinesMap[$lineNumber])) {
+                $changedExecutableLines[$lineNumber] = $executableLinesMap[$lineNumber];
+            }
+        }
+
+        return $changedExecutableLines;
+    }
+
+    /**
+     * @param non-empty-list<ExecutableLineExcluder> $excluders
+     *
+     * @throws ErrorException
+     */
+    private function createExcluderVisitor(
+        array $excluders,
+        string $file,
+    ): ExcluderVisitor
+    {
+        $fileLines = FileUtils::readFileLines($file);
+        $linesContents = array_combine(range(1, count($fileLines)), $fileLines);
+
+        $excluderVisitor = new ExcluderVisitor($excluders, new ExclusionContext($file, $linesContents));
+        $this->fileTraverser->traverse($file, $fileLines, $excluderVisitor);
+
+        return $excluderVisitor;
+    }
+
+    private function printStatistics(
+        int $totalChangedLines,
+        int $totalCoveredLines,
+    ): void
+    {
         if ($totalChangedLines === 0) {
             $percentage = 0;
         } else {
@@ -84,8 +148,6 @@ public function __invoke(
         $this->stdoutPrinter->printLine('  Uncovered lines:          <orange>' . ($totalChangedLines - $totalCoveredLines) . '</orange>');
         $this->stdoutPrinter->printLine("  Coverage:                 {$percentageFormatted}%");
         $this->stdoutPrinter->printLine('');
-
-        return 0;
     }
 
     public function getName(): string

src/Config.php

@@ -3,6 +3,7 @@
 namespace ShipMonk\CoverageGuard;
 
 use ShipMonk\CoverageGuard\Exception\ErrorException;
+use ShipMonk\CoverageGuard\Excluder\ExecutableLineExcluder;
 use ShipMonk\CoverageGuard\Rule\CoverageRule;
 use ShipMonk\CoverageGuard\Utils\FileUtils;
 use function file_exists;
@@ -31,6 +32,11 @@ final class Config
      */
     private array $rules = [];
 
+    /**
+     * @var list<ExecutableLineExcluder>
+     */
+    private array $excluders = [];
+
     private ?string $editorUrl = null;
 
     public function __construct()
@@ -86,6 +92,15 @@ public function addRule(CoverageRule $rule): self
         return $this;
     }
 
+    /**
+     * Allows you to ignore/exclude certain executable lines from coverage calculations.
+     */
+    public function addExecutableLineExcluder(ExecutableLineExcluder $excluder): self
+    {
+        $this->excluders[] = $excluder;
+        return $this;
+    }
+
     /**
      * Set the editor URL pattern to make filepaths clickable in CLI output via OSC 8 hyperlink
      *
@@ -118,6 +133,14 @@ public function getCoveragePathMapping(): array
         return $this->coveragePathMapping;
     }
 
+    /**
+     * @return list<ExecutableLineExcluder>
+     */
+    public function getExecutableLineExcluders(): array
+    {
+        return $this->excluders;
+    }
+
     /**
      * @return list<CoverageRule>
      */