Skip to content

Commit

Permalink
Report proper lines
Browse files Browse the repository at this point in the history
  • Loading branch information
janedbal committed Apr 2, 2024
1 parent 3f6bedd commit 0e69f0d
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 21 deletions.
45 changes: 30 additions & 15 deletions src/Rule/ForbidNotNormalizedTypeRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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;
Expand All @@ -353,7 +356,6 @@ public function processVarTags(
$multiTypeNode,
$nameSpace,
$identification,
$this->getPhpDocLine($originalNode, $varTagValue),
);
$errors = array_merge($errors, $newErrors);
}
Expand All @@ -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);
}
}
Expand All @@ -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) {
Expand All @@ -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<UnionTypeNode|IntersectionTypeNode>
*/
private function extractUnionAndIntersectionPhpDocTypeNodes(TypeNode $typeNode): array
private function extractUnionAndIntersectionPhpDocTypeNodes(TypeNode $typeNode, int $line): array
{
/** @var list<UnionTypeNode|IntersectionTypeNode> $nodes */
$nodes = [];
$this->traversePhpDocTypeNode($typeNode, static function (TypeNode $typeNode) use (&$nodes): void {
if ($typeNode instanceof UnionTypeNode || $typeNode instanceof IntersectionTypeNode) {
Expand All @@ -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;
}

Expand Down Expand Up @@ -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 = [];
Expand All @@ -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();
}
Expand All @@ -578,15 +593,15 @@ 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;
}

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();
}
Expand Down
12 changes: 6 additions & 6 deletions tests/Rule/data/ForbidNotNormalizedTypeRule/code.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
}

Expand Down

0 comments on commit 0e69f0d

Please sign in to comment.