Unify ignoring lines in traits with parser tokens

phpstan · Jul 24, 2023 · 01294d8 · 01294d8
1 parent 40cadaa
commit 01294d8
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 70 deletions.
diff --git a/src/Analyser/FileAnalyser.php b/src/Analyser/FileAnalyser.php
@@ -2,7 +2,6 @@
 
 namespace PHPStan\Analyser;
 
-use PhpParser\Comment;
 use PhpParser\Node;
 use PHPStan\AnalysedCodeException;
 use PHPStan\BetterReflection\NodeCompiler\Exception\UnableToCompileNode;
@@ -12,6 +11,7 @@
 use PHPStan\Collectors\Registry as CollectorRegistry;
 use PHPStan\Dependency\DependencyResolver;
 use PHPStan\Node\FileNode;
+use PHPStan\Node\InTraitNode;
 use PHPStan\Parser\Parser;
 use PHPStan\Parser\ParserErrorsException;
 use PHPStan\Rules\Registry as RuleRegistry;
@@ -27,7 +27,6 @@
 use function restore_error_handler;
 use function set_error_handler;
 use function sprintf;
-use function strpos;
 use const E_DEPRECATED;
 
 class FileAnalyser
@@ -71,18 +70,22 @@ public function analyseFile(
 			try {
 				$this->collectErrors($analysedFiles);
 				$parserNodes = $this->parser->parseFile($file);
-				$linesToIgnore = $this->getLinesToIgnoreFromTokens($file, $parserNodes);
+				$linesToIgnore = $unmatchedLineIgnores = [$file => $this->getLinesToIgnoreFromTokens($parserNodes)];
 				$temporaryFileErrors = [];
-				$nodeCallback = function (Node $node, Scope $scope) use (&$fileErrors, &$fileCollectedData, &$fileDependencies, &$exportedNodes, $file, $ruleRegistry, $collectorRegistry, $outerNodeCallback, $analysedFiles, &$linesToIgnore, &$temporaryFileErrors): void {
+				$nodeCallback = function (Node $node, Scope $scope) use (&$fileErrors, &$fileCollectedData, &$fileDependencies, &$exportedNodes, $file, $ruleRegistry, $collectorRegistry, $outerNodeCallback, $analysedFiles, &$linesToIgnore, &$unmatchedLineIgnores, &$temporaryFileErrors): void {
 					if ($node instanceof Node\Stmt\Trait_) {
 						foreach (array_keys($linesToIgnore[$file] ?? []) as $lineToIgnore) {
 							if ($lineToIgnore < $node->getStartLine() || $lineToIgnore > $node->getEndLine()) {
 								continue;
 							}
 
-							unset($linesToIgnore[$file][$lineToIgnore]);
+							unset($unmatchedLineIgnores[$file][$lineToIgnore]);
 						}
 					}
+					if ($node instanceof InTraitNode) {
+						$traitNode = $node->getOriginalNode();
+						$linesToIgnore[$scope->getFileDescription()] = $this->getLinesToIgnoreFromTokens([$traitNode]);
+					}
 					if ($outerNodeCallback !== null) {
 						$outerNodeCallback($node, $scope);
 					}
@@ -112,18 +115,6 @@ public function analyseFile(
 						}
 					}
 
-					if ($scope->isInTrait()) {
-						$sameTraitFile = $file === $scope->getTraitReflection()->getFileName();
-						foreach ($this->getLinesToIgnore($node) as $lineToIgnore) {
-							$linesToIgnore[$scope->getFileDescription()][$lineToIgnore] = true;
-							if (!$sameTraitFile) {
-								continue;
-							}
-
-							unset($linesToIgnore[$file][$lineToIgnore]);
-						}
-					}
-
 					foreach ($collectorRegistry->getCollectors($nodeType) as $collector) {
 						try {
 							$collectedData = $collector->processNode($node, $scope);
@@ -178,7 +169,6 @@ public function analyseFile(
 					$scope,
 					$nodeCallback,
 				);
-				$unmatchedLineIgnores = $linesToIgnore;
 				foreach ($temporaryFileErrors as $tmpFileError) {
 					$line = $tmpFileError->getLine();
 					if (
@@ -242,36 +232,11 @@ public function analyseFile(
 		return new FileAnalyserResult($fileErrors, $fileCollectedData, array_values(array_unique($fileDependencies)), $exportedNodes);
 	}
 
-	/**
-	 * @return int[]
-	 */
-	private function getLinesToIgnore(Node $node): array
-	{
-		$lines = [];
-		if ($node->getDocComment() !== null) {
-			$line = $this->findLineToIgnoreComment($node->getDocComment());
-			if ($line !== null) {
-				$lines[] = $line;
-			}
-		}
-
-		foreach ($node->getComments() as $comment) {
-			$line = $this->findLineToIgnoreComment($comment);
-			if ($line === null) {
-				continue;
-			}
-
-			$lines[] = $line;
-		}
-
-		return $lines;
-	}
-
 	/**
 	 * @param Node[] $nodes
-	 * @return array<string, array<int, true>>
+	 * @return array<int, true>
 	 */
-	private function getLinesToIgnoreFromTokens(string $file, array $nodes): array
+	private function getLinesToIgnoreFromTokens(array $nodes): array
 	{
 		if (!isset($nodes[0])) {
 			return [];
@@ -281,35 +246,12 @@ private function getLinesToIgnoreFromTokens(string $file, array $nodes): array
 		$tokenLines = $nodes[0]->getAttribute('linesToIgnore', []);
 		$lines = [];
 		foreach ($tokenLines as $tokenLine) {
-			$lines[$file][$tokenLine] = true;
+			$lines[$tokenLine] = true;
 		}
 
 		return $lines;
 	}
 
-	private function findLineToIgnoreComment(Comment $comment): ?int
-	{
-		$text = $comment->getText();
-		if ($comment instanceof Comment\Doc) {
-			$line = $comment->getEndLine();
-		} else {
-			if (strpos($text, "\n") === false || strpos($text, '//') === 0) {
-				$line = $comment->getStartLine();
-			} else {
-				$line = $comment->getEndLine();
-			}
-		}
-		if (strpos($text, '@phpstan-ignore-next-line') !== false) {
-			return $line + 1;
-		}
-
-		if (strpos($text, '@phpstan-ignore-line') !== false) {
-			return $line;
-		}
-
-		return null;
-	}
-
 	/**
 	 * @param array<string, true> $analysedFiles
 	 */

diff --git a/src/Parser/RichParser.php b/src/Parser/RichParser.php
@@ -10,6 +10,8 @@
 use PHPStan\DependencyInjection\Container;
 use PHPStan\File\FileReader;
 use PHPStan\ShouldNotHappenException;
+use function array_filter;
+use function array_values;
 use function is_string;
 use function strpos;
 use function substr_count;
@@ -61,14 +63,22 @@ public function parseString(string $sourceCode): array
 		$nodeTraverser = new NodeTraverser();
 		$nodeTraverser->addVisitor($this->nameResolver);
 
+		$traitCollectingVisitor = new TraitCollectingVisitor();
+		$nodeTraverser->addVisitor($traitCollectingVisitor);
+
 		foreach ($this->container->getServicesByTag(self::VISITOR_SERVICE_TAG) as $visitor) {
 			$nodeTraverser->addVisitor($visitor);
 		}
 
 		/** @var array<Node\Stmt> */
 		$nodes = $nodeTraverser->traverse($nodes);
+		$linesToIgnore = $this->getLinesToIgnore($tokens);
 		if (isset($nodes[0])) {
-			$nodes[0]->setAttribute('linesToIgnore', $this->getLinesToIgnore($tokens));
+			$nodes[0]->setAttribute('linesToIgnore', $linesToIgnore);
+		}
+
+		foreach ($traitCollectingVisitor->traits as $trait) {
+			$trait->setAttribute('linesToIgnore', array_values(array_filter($linesToIgnore, static fn (int $line): bool => $line >= $trait->getStartLine() && $line <= $trait->getEndLine())));
 		}
 
 		return $nodes;

diff --git a/src/Parser/TraitCollectingVisitor.php b/src/Parser/TraitCollectingVisitor.php
@@ -0,0 +1,25 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Parser;
+
+use PhpParser\Node;
+use PhpParser\NodeVisitorAbstract;
+
+final class TraitCollectingVisitor extends NodeVisitorAbstract
+{
+
+	/** @var list<Node\Stmt\Trait_> */
+	public array $traits = [];
+
+	public function enterNode(Node $node): ?Node
+	{
+		if (!$node instanceof Node\Stmt\Trait_) {
+			return null;
+		}
+
+		$this->traits[] = $node;
+
+		return null;
+	}
+
+}