Skip to content

Commit

Permalink
Rework processing inline @var
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Feb 7, 2021
1 parent 1a843eb commit e501091
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 28 deletions.
74 changes: 46 additions & 28 deletions src/Analyser/NodeScopeResolver.php
Expand Up @@ -1168,13 +1168,20 @@ private function processStmtNode(
}
} elseif ($stmt instanceof Static_) {
$hasYield = false;
$comment = CommentHelper::getDocComment($stmt);

$vars = [];
foreach ($stmt->vars as $var) {
$scope = $this->processStmtNode($var, $scope, $nodeCallback)->getScope();
if ($comment === null || !is_string($var->var->name)) {
if (!is_string($var->var->name)) {
continue;
}
$scope = $this->processVarAnnotation($scope, $var->var->name, $comment, false);

$vars[] = $var->var->name;
}

$comment = CommentHelper::getDocComment($stmt);
if ($comment !== null) {
$scope = $this->processVarAnnotation($scope, $vars, $comment);
}
} elseif ($stmt instanceof StaticVar) {
$hasYield = false;
Expand Down Expand Up @@ -1523,7 +1530,7 @@ function (MutatingScope $scope) use ($expr, $nodeCallback, $context): Expression
if ($expr->var instanceof Variable && is_string($expr->var->name)) {
$comment = CommentHelper::getDocComment($expr);
if ($comment !== null) {
$scope = $this->processVarAnnotation($scope, $expr->var->name, $comment, false, $varChangedScope);
$scope = $this->processVarAnnotation($scope, [$expr->var->name], $comment, $varChangedScope);
}
}

Expand Down Expand Up @@ -1552,6 +1559,7 @@ function (MutatingScope $scope) use ($expr, $nodeCallback, $context): Expression
$scope = $this->lookForArrayDestructuringArray($scope, $expr->var, $scope->getType($expr->expr));
$comment = CommentHelper::getDocComment($expr);
if ($comment !== null) {
$vars = [];
foreach ($expr->var->items as $arrayItem) {
if ($arrayItem === null) {
continue;
Expand All @@ -1560,15 +1568,17 @@ function (MutatingScope $scope) use ($expr, $nodeCallback, $context): Expression
continue;
}

$vars[] = $arrayItem->value->name;
}

if (count($vars) > 0) {
$varChangedScope = false;
$scope = $this->processVarAnnotation($scope, $arrayItem->value->name, $comment, true, $varChangedScope);
if ($varChangedScope) {
continue;
$scope = $this->processVarAnnotation($scope, $vars, $comment, $varChangedScope);
if (!$varChangedScope) {
$scope = $this->processStmtVarAnnotation($scope, new Node\Stmt\Expression($expr, [
'comments' => $expr->getAttribute('comments'),
]), null);
}

$scope = $this->processStmtVarAnnotation($scope, new Node\Stmt\Expression($expr, [
'comments' => $expr->getAttribute('comments'),
]), null);
}
}
}
Expand Down Expand Up @@ -2820,7 +2830,14 @@ private function processStmtVarAnnotation(MutatingScope $scope, Node\Stmt $stmt,
return $scope;
}

private function processVarAnnotation(MutatingScope $scope, string $variableName, string $comment, bool $strict, bool &$changed = false): MutatingScope
/**
* @param MutatingScope $scope
* @param array<int, string> $variableNames
* @param string $comment
* @param bool $changed
* @return MutatingScope
*/
private function processVarAnnotation(MutatingScope $scope, array $variableNames, string $comment, bool &$changed = false): MutatingScope
{
$function = $scope->getFunction();
$resolvedPhpDoc = $this->fileTypeMapper->getResolvedPhpDoc(
Expand All @@ -2832,27 +2849,29 @@ private function processVarAnnotation(MutatingScope $scope, string $variableName
);
$varTags = $resolvedPhpDoc->getVarTags();

if (isset($varTags[$variableName])) {
foreach ($variableNames as $variableName) {
if (!isset($varTags[$variableName])) {
continue;
}

$variableType = $varTags[$variableName]->getType();
$changed = true;
return $scope->assignVariable($variableName, $variableType);

$scope = $scope->assignVariable($variableName, $variableType);
}

if (!$strict && count($varTags) === 1 && isset($varTags[0])) {
if (count($variableNames) === 1 && count($varTags) === 1 && isset($varTags[0])) {
$variableType = $varTags[0]->getType();
$changed = true;
return $scope->assignVariable($variableName, $variableType);

$scope = $scope->assignVariable($variableNames[0], $variableType);
}

return $scope;
}

private function enterForeach(MutatingScope $scope, Foreach_ $stmt): MutatingScope
{
$comment = CommentHelper::getDocComment($stmt);
$iterateeType = $scope->getType($stmt->expr);
$vars = [];
if ($stmt->valueVar instanceof Variable && is_string($stmt->valueVar->name)) {
$scope = $scope->enterForeach(
$stmt->expr,
Expand All @@ -2863,21 +2882,17 @@ private function enterForeach(MutatingScope $scope, Foreach_ $stmt): MutatingSco
? $stmt->keyVar->name
: null
);
if ($comment !== null) {
$scope = $this->processVarAnnotation($scope, $stmt->valueVar->name, $comment, true);
}
$vars[] = $stmt->valueVar->name;
}

if (
$stmt->keyVar instanceof Variable && is_string($stmt->keyVar->name)
) {
$scope = $scope->enterForeachKey($stmt->expr, $stmt->keyVar->name);

if ($comment !== null) {
$scope = $this->processVarAnnotation($scope, $stmt->keyVar->name, $comment, true);
}
$vars[] = $stmt->keyVar->name;
}

$comment = CommentHelper::getDocComment($stmt);
if (
$comment === null
&& $iterateeType instanceof ConstantArrayType
Expand All @@ -2902,7 +2917,6 @@ private function enterForeach(MutatingScope $scope, Foreach_ $stmt): MutatingSco
$exprType = $scope->getType($stmt->expr);
$itemType = $exprType->getIterableValueType();
$scope = $this->lookForArrayDestructuringArray($scope, $stmt->valueVar, $itemType);
$comment = CommentHelper::getDocComment($stmt);
if ($comment !== null) {
foreach ($stmt->valueVar->items as $arrayItem) {
if ($arrayItem === null) {
Expand All @@ -2912,11 +2926,15 @@ private function enterForeach(MutatingScope $scope, Foreach_ $stmt): MutatingSco
continue;
}

$scope = $this->processVarAnnotation($scope, $arrayItem->value->name, $comment, true);
$vars[] = $arrayItem->value->name;
}
}
}

if ($comment !== null) {
$scope = $this->processVarAnnotation($scope, $vars, $comment);
}

return $scope;
}

Expand Down
3 changes: 3 additions & 0 deletions src/Rules/PhpDoc/WrongVariableNameInVarTagRule.php
Expand Up @@ -138,6 +138,9 @@ private function processForeach(?Node\Expr $keyVar, Node\Expr $valueVar, array $
$errors = [];
foreach (array_keys($varTags) as $name) {
if (is_int($name)) {
if (count($variableNames) === 1) {
continue;
}
$errors[] = RuleErrorBuilder::message(
'PHPDoc tag @var above foreach loop does not specify variable name.'
)->build();
Expand Down
5 changes: 5 additions & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
Expand Up @@ -4771,6 +4771,11 @@ public function dataForeachArrayType(): array
],
[
__DIR__ . '/data/foreach/type-in-comment-no-variable.php',
'bool',
'$value',
],
[
__DIR__ . '/data/foreach/type-in-comment-no-variable-2.php',
'mixed',
'$value',
],
Expand Down
@@ -0,0 +1,11 @@
<?php

namespace TypeInCommentOnForeach;

/** @var mixed[] $values */
$values = [];

/** @var bool */
foreach ($values as $key => $value) {
die;
}
13 changes: 13 additions & 0 deletions tests/PHPStan/Rules/PhpDoc/data/wrong-variable-name-var.php
Expand Up @@ -224,3 +224,16 @@ class Bar
const TEST = 'str';

}

class ForeachJustValueVar
{

public function doBar(array $list)
{
/** @var int */
foreach ($list as $val) {

}
}

}

0 comments on commit e501091

Please sign in to comment.