Skip to content

Commit

Permalink
Do not require variable name in @var above Return_ and Throw_
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Dec 12, 2019
1 parent 8e1ae67 commit f73a2f4
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 11 deletions.
22 changes: 16 additions & 6 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -307,13 +307,16 @@ private function processStmtNode(
$stmt instanceof Node\Stmt\Expression
&& !$stmt->expr instanceof Assign && !$stmt->expr instanceof AssignRef
)
|| $stmt instanceof Throw_
|| $stmt instanceof If_
|| $stmt instanceof While_
|| $stmt instanceof Switch_
) {
$scope = $this->processStmtVarAnnotation($scope, $stmt, null);
} elseif (
$stmt instanceof Throw_
|| $stmt instanceof Return_
) {
$scope = $this->processStmtVarAnnotation($scope, $stmt);
$scope = $this->processStmtVarAnnotation($scope, $stmt, $stmt->expr);
}

$nodeCallback($stmt, $scope);
Expand Down Expand Up @@ -1023,7 +1026,7 @@ private function processStmtNode(
$scope = $scope->specifyExpressionType(new ConstFetch(new Name\FullyQualified($const->name->toString())), $scope->getType($const->value));
}
} elseif ($stmt instanceof Node\Stmt\Nop) {
$scope = $this->processStmtVarAnnotation($scope, $stmt);
$scope = $this->processStmtVarAnnotation($scope, $stmt, null);
$hasYield = false;
} else {
$hasYield = false;
Expand Down Expand Up @@ -1281,7 +1284,7 @@ function (MutatingScope $scope) use ($expr, $nodeCallback, $context): Expression
if (!$varChangedScope) {
$scope = $this->processStmtVarAnnotation($scope, new Node\Stmt\Expression($expr, [
'comments' => $expr->getAttribute('comments'),
]));
]), null);
}
} else {
$result = $this->processExprNode($expr->expr, $scope, $nodeCallback, $context->enterDeep());
Expand Down Expand Up @@ -1319,7 +1322,7 @@ function (MutatingScope $scope) use ($expr, $nodeCallback, $context): Expression

$scope = $this->processStmtVarAnnotation($scope, new Node\Stmt\Expression($expr, [
'comments' => $expr->getAttribute('comments'),
]));
]), null);
}
}
}
Expand Down Expand Up @@ -2332,7 +2335,7 @@ private function processAssignVar(
return new ExpressionResult($scope, $hasYield);
}

private function processStmtVarAnnotation(MutatingScope $scope, Node\Stmt $stmt): MutatingScope
private function processStmtVarAnnotation(MutatingScope $scope, Node\Stmt $stmt, ?Expr $defaultExpr): MutatingScope
{
$comment = CommentHelper::getDocComment($stmt);
if ($comment === null) {
Expand All @@ -2348,8 +2351,11 @@ private function processStmtVarAnnotation(MutatingScope $scope, Node\Stmt $stmt)
$function !== null ? $function->getName() : null,
$comment
);

$variableLessTags = [];
foreach ($resolvedPhpDoc->getVarTags() as $name => $varTag) {
if (is_int($name)) {
$variableLessTags[] = $varTag;
continue;
}

Expand All @@ -2361,6 +2367,10 @@ private function processStmtVarAnnotation(MutatingScope $scope, Node\Stmt $stmt)
$scope = $scope->assignVariable($name, $varTag->getType(), $certainty);
}

if (count($variableLessTags) === 1 && $defaultExpr !== null) {
$scope = $scope->specifyExpressionType($defaultExpr, $variableLessTags[0]->getType());
}

return $scope;
}

Expand Down
23 changes: 18 additions & 5 deletions src/Rules/PhpDoc/WrongVariableNameInVarTagRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,11 @@ public function processNode(Node $node, Scope $scope): array
return $this->processExpression($scope, $node->expr, $varTags);
}

return $this->processStmt($scope, $varTags);
if ($node instanceof Node\Stmt\Throw_ || $node instanceof Node\Stmt\Return_) {
return $this->processStmt($scope, $varTags, $node->expr);
}

return $this->processStmt($scope, $varTags, null);
}

/**
Expand Down Expand Up @@ -254,20 +258,23 @@ private function processExpression(Scope $scope, Expr $expr, array $varTags): ar
return [];
}

return $this->processStmt($scope, $varTags);
return $this->processStmt($scope, $varTags, null);
}

/**
* @param \PHPStan\Analyser\Scope $scope
* @param \PHPStan\PhpDoc\Tag\VarTag[] $varTags
* @param Expr|null $defaultExpr
* @return \PHPStan\Rules\RuleError[]
*/
private function processStmt(Scope $scope, array $varTags): array
private function processStmt(Scope $scope, array $varTags, ?Expr $defaultExpr): array
{
$errors = [];
foreach (array_keys($varTags) as $name) {

$variableLessVarTags = [];
foreach ($varTags as $name => $varTag) {
if (is_int($name)) {
$errors[] = RuleErrorBuilder::message('PHPDoc tag @var does not specify variable name.')->build();
$variableLessVarTags[] = $varTag;
continue;
}

Expand All @@ -278,6 +285,12 @@ private function processStmt(Scope $scope, array $varTags): array
$errors[] = RuleErrorBuilder::message(sprintf('Variable $%s in PHPDoc tag @var does not exist.', $name))->build();
}

if (count($variableLessVarTags) !== 1 || $defaultExpr === null) {
if (count($variableLessVarTags) > 0) {
$errors[] = RuleErrorBuilder::message('PHPDoc tag @var does not specify variable name.')->build();
}
}

return $errors;
}

Expand Down
6 changes: 6 additions & 0 deletions tests/PHPStan/Rules/Methods/data/returnTypes.php
Original file line number Diff line number Diff line change
Expand Up @@ -1175,4 +1175,10 @@ public function doFoo(\DateTimeInterface $date): \DateTimeImmutable
return $date;
}

public function doBar(\DateTimeInterface $date): \DateTimeImmutable
{
/** @var \DateTimeImmutable */
return $date;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,14 @@ public function testRule(): void
'Variable $b in PHPDoc tag @var does not exist.',
134,
],
[
'PHPDoc tag @var does not specify variable name.',
155,
],
[
'PHPDoc tag @var does not specify variable name.',
176,
],
]);
}

Expand Down
42 changes: 42 additions & 0 deletions tests/PHPStan/Rules/PhpDoc/data/wrong-variable-name-var.php
Original file line number Diff line number Diff line change
Expand Up @@ -134,4 +134,46 @@ public function testEcho($a)
echo $a;
}

public function throwVar($a)
{
/** @var \Exception $a */
throw $a;
}

public function throwVar2($a)
{
/** @var \Exception */
throw $a;
}

public function throwVar3($a)
{
/**
* @var \Exception
* @var \InvalidArgumentException
*/
throw $a;
}

public function returnVar($a)
{
/** @var \stdClass $a */
return $a;
}

public function returnVar2($a)
{
/** @var \stdClass */
return $a;
}

public function returnVar3($a)
{
/**
* @var \stdClass
* @var \DateTime
*/
return $a;
}

}
5 changes: 5 additions & 0 deletions tests/PHPStan/Rules/Variables/data/throw-values.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,8 @@ function (\stdClass $foo) {
/** @var \Exception $foo */
throw $foo;
};

function (\stdClass $foo) {
/** @var \Exception */
throw $foo;
};

0 comments on commit f73a2f4

Please sign in to comment.