From 0e69f0df88c7d6cf636264fcd33b189cb38c3b8a Mon Sep 17 00:00:00 2001 From: Jan Nedbal Date: Tue, 2 Apr 2024 13:18:49 +0200 Subject: [PATCH] Report proper lines --- src/Rule/ForbidNotNormalizedTypeRule.php | 45 ++++++++++++------- .../data/ForbidNotNormalizedTypeRule/code.php | 12 ++--- 2 files changed, 36 insertions(+), 21 deletions(-) diff --git a/src/Rule/ForbidNotNormalizedTypeRule.php b/src/Rule/ForbidNotNormalizedTypeRule.php index 869318b..5b7b430 100644 --- a/src/Rule/ForbidNotNormalizedTypeRule.php +++ b/src/Rule/ForbidNotNormalizedTypeRule.php @@ -317,12 +317,13 @@ public function processParamTags( $errors = []; foreach ($paramTagValues as $paramTagValue) { - foreach ($this->extractUnionAndIntersectionPhpDocTypeNodes($paramTagValue->type) as $multiTypeNode) { + $line = $this->getPhpDocLine($sourceNode, $paramTagValue); + + foreach ($this->extractUnionAndIntersectionPhpDocTypeNodes($paramTagValue->type, $line) as $multiTypeNode) { $newErrors = $this->processMultiTypePhpDocNode( $multiTypeNode, $nameSpace, "parameter {$paramTagValue->parameterName}", - $this->getPhpDocLine($sourceNode, $paramTagValue), ); $errors = array_merge($errors, $newErrors); } @@ -344,7 +345,9 @@ public function processVarTags( $errors = []; foreach ($varTagValues as $varTagValue) { - foreach ($this->extractUnionAndIntersectionPhpDocTypeNodes($varTagValue->type) as $multiTypeNode) { + $line = $this->getPhpDocLine($originalNode, $varTagValue); + + foreach ($this->extractUnionAndIntersectionPhpDocTypeNodes($varTagValue->type, $line) as $multiTypeNode) { $identification = $varTagValue->variableName !== '' ? "variable {$varTagValue->variableName}" : null; @@ -353,7 +356,6 @@ public function processVarTags( $multiTypeNode, $nameSpace, $identification, - $this->getPhpDocLine($originalNode, $varTagValue), ); $errors = array_merge($errors, $newErrors); } @@ -375,8 +377,10 @@ public function processReturnTags( $errors = []; foreach ($returnTagValues as $returnTagValue) { - foreach ($this->extractUnionAndIntersectionPhpDocTypeNodes($returnTagValue->type) as $multiTypeNode) { - $newErrors = $this->processMultiTypePhpDocNode($multiTypeNode, $nameSpace, 'return', $this->getPhpDocLine($originalNode, $returnTagValue)); + $line = $this->getPhpDocLine($originalNode, $returnTagValue); + + foreach ($this->extractUnionAndIntersectionPhpDocTypeNodes($returnTagValue->type, $line) as $multiTypeNode) { + $newErrors = $this->processMultiTypePhpDocNode($multiTypeNode, $nameSpace, 'return'); $errors = array_merge($errors, $newErrors); } } @@ -397,10 +401,14 @@ public function processThrowsTags( $thrownTypes = []; foreach ($throwsTagValues as $throwsTagValue) { - $multiTypeNodes = $this->extractUnionAndIntersectionPhpDocTypeNodes($throwsTagValue->type); + $line = $this->getPhpDocLine($originalNode, $throwsTagValue); + $multiTypeNodes = $this->extractUnionAndIntersectionPhpDocTypeNodes($throwsTagValue->type, $line); if ($multiTypeNodes === []) { - $thrownTypes[] = $throwsTagValue->type; + $innerType = $throwsTagValue->type; + $innerType->setAttribute('line', $line); + + $thrownTypes[] = $innerType; } else { foreach ($multiTypeNodes as $multiTypeNode) { foreach ($multiTypeNode->types as $typeNode) { @@ -411,14 +419,15 @@ public function processThrowsTags( } $unionNode = new UnionTypeNode($thrownTypes); - return $this->processMultiTypePhpDocNode($unionNode, $nameSpace, 'throws', $originalNode->getLine()); + return $this->processMultiTypePhpDocNode($unionNode, $nameSpace, 'throws'); } /** * @return list */ - private function extractUnionAndIntersectionPhpDocTypeNodes(TypeNode $typeNode): array + private function extractUnionAndIntersectionPhpDocTypeNodes(TypeNode $typeNode, int $line): array { + /** @var list $nodes */ $nodes = []; $this->traversePhpDocTypeNode($typeNode, static function (TypeNode $typeNode) use (&$nodes): void { if ($typeNode instanceof UnionTypeNode || $typeNode instanceof IntersectionTypeNode) { @@ -429,6 +438,13 @@ private function extractUnionAndIntersectionPhpDocTypeNodes(TypeNode $typeNode): $nodes[] = new UnionTypeNode([$typeNode->type, new IdentifierTypeNode('null')]); } }); + + foreach ($nodes as $node) { + foreach ($node->types as $innerType) { + $innerType->setAttribute('line', $line); + } + } + return $nodes; } @@ -545,8 +561,7 @@ private function printPhpParserNode(PhpParserNode $node): string private function processMultiTypePhpDocNode( TypeNode $multiTypeNode, NameScope $nameSpace, - ?string $identification, - int $line + ?string $identification ): array { $errors = []; @@ -559,7 +574,7 @@ private function processMultiTypePhpDocNode( $dnf = $this->typeNodeResolver->resolve($multiTypeNode, $nameSpace)->describe(VerbosityLevel::typeOnly()); $errors[] = RuleErrorBuilder::message("Found non-normalized type {$multiTypeNode}{$forWhat}: this is not disjunctive normal form, use {$dnf}") - ->line($line) + ->line($type->getAttribute('line')) ->identifier('shipmonk.nonNormalizedType') ->build(); } @@ -578,7 +593,7 @@ private function processMultiTypePhpDocNode( if ($typeA->isSuperTypeOf($typeB)->yes()) { $errors[] = RuleErrorBuilder::message("Found non-normalized type {$multiTypeNode}{$forWhat}: {$typeNodeB} is a subtype of {$typeNodeA}.") - ->line($line) + ->line($typeNodeB->getAttribute('line')) ->identifier('shipmonk.nonNormalizedType') ->build(); continue; @@ -586,7 +601,7 @@ private function processMultiTypePhpDocNode( if ($typeB->isSuperTypeOf($typeA)->yes()) { $errors[] = RuleErrorBuilder::message("Found non-normalized type {$multiTypeNode}{$forWhat}: {$typeNodeA} is a subtype of {$typeNodeB}.") - ->line($line) + ->line($typeNodeA->getAttribute('line')) ->identifier('shipmonk.nonNormalizedType') ->build(); } diff --git a/tests/Rule/data/ForbidNotNormalizedTypeRule/code.php b/tests/Rule/data/ForbidNotNormalizedTypeRule/code.php index 4395507..96ecd45 100644 --- a/tests/Rule/data/ForbidNotNormalizedTypeRule/code.php +++ b/tests/Rule/data/ForbidNotNormalizedTypeRule/code.php @@ -170,25 +170,25 @@ public function testCatch() } /** - * @throws InterfaceImplementor + * @throws InterfaceImplementor // error: Found non-normalized type (InterfaceImplementor | MyInterface) for throws: InterfaceImplementor is a subtype of MyInterface. * @throws MyInterface */ - public function testThrows() // error: Found non-normalized type (InterfaceImplementor | MyInterface) for throws: InterfaceImplementor is a subtype of MyInterface. + public function testThrows() { } /** - * @throws InterfaceImplementor|MyInterface + * @throws InterfaceImplementor|MyInterface // error: Found non-normalized type (InterfaceImplementor | MyInterface) for throws: InterfaceImplementor is a subtype of MyInterface. */ - public function testThrowsUnion() // error: Found non-normalized type (InterfaceImplementor | MyInterface) for throws: InterfaceImplementor is a subtype of MyInterface. + public function testThrowsUnion() { } /** * @throws MyInterface|ChildOne - * @throws InterfaceImplementor + * @throws InterfaceImplementor // error: Found non-normalized type (MyInterface | ChildOne | InterfaceImplementor) for throws: InterfaceImplementor is a subtype of MyInterface. */ - public function testThrowsUnionCombined() // error: Found non-normalized type (MyInterface | ChildOne | InterfaceImplementor) for throws: InterfaceImplementor is a subtype of MyInterface. + public function testThrowsUnionCombined() { }