Skip to content

Commit

Permalink
Reworked @var support so that view files can be analysed successfully
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Jan 14, 2021
1 parent d41f065 commit 27481c3
Show file tree
Hide file tree
Showing 9 changed files with 74 additions and 32 deletions.
6 changes: 5 additions & 1 deletion src/Analyser/MutatingScope.php
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,11 @@ public function enterDeclareStrictTypes(): self
{
return $this->scopeFactory->create(
$this->context,
true
true,
[],
null,
null,
$this->variableTypes
);
}

Expand Down
20 changes: 9 additions & 11 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -336,21 +336,19 @@ private function processStmtNode(
): StatementResult
{
if (
$stmt instanceof Echo_
|| (
$stmt instanceof Node\Stmt\Expression
&& !$stmt->expr instanceof Assign && !$stmt->expr instanceof AssignRef
)
|| $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, $stmt->expr);
} elseif (
!$stmt instanceof Static_
&& !$stmt instanceof Foreach_
&& (
!$stmt instanceof Node\Stmt\Expression
|| !$stmt->expr instanceof Assign && !$stmt->expr instanceof AssignRef
)
) {
$scope = $this->processStmtVarAnnotation($scope, $stmt, null);
}

if ($stmt instanceof Node\Stmt\ClassMethod) {
Expand Down
26 changes: 6 additions & 20 deletions src/Type/FileTypeMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ private function shouldPhpDocNodeBeCachedToDisk(PhpDocNode $phpDocNode): bool
private function getResolvedPhpDocMap(string $fileName): array
{
if (!isset($this->memoryCache[$fileName])) {
$cacheKey = sprintf('%s-phpdocstring-v2', $fileName);
$cacheKey = sprintf('%s-phpdocstring-v3', $fileName);
$variableCacheKey = implode(',', array_map(static function (array $file): string {
return sprintf('%s-%d', $file['filename'], $file['modifiedTime']);
}, $this->getCachedDependentFilesWithTimestamps($fileName)));
Expand Down Expand Up @@ -353,12 +353,10 @@ function (\PhpParser\Node $node) use ($fileName, $lookForTrait, $traitMethodAlia
return null;
} elseif ($node instanceof \PhpParser\Node\Stmt\Namespace_) {
$namespace = (string) $node->name;
return null;
} elseif ($node instanceof \PhpParser\Node\Stmt\Use_ && $node->type === \PhpParser\Node\Stmt\Use_::TYPE_NORMAL) {
foreach ($node->uses as $use) {
$uses[strtolower($use->getAlias()->name)] = (string) $use->name;
}
return null;
} elseif ($node instanceof \PhpParser\Node\Stmt\GroupUse) {
$prefix = (string) $node->prefix;
foreach ($node->uses as $use) {
Expand All @@ -368,7 +366,6 @@ function (\PhpParser\Node $node) use ($fileName, $lookForTrait, $traitMethodAlia

$uses[strtolower($use->getAlias()->name)] = sprintf('%s\\%s', $prefix, (string) $use->name);
}
return null;
} elseif ($node instanceof Node\Stmt\ClassMethod) {
$functionName = $node->name->name;
if (array_key_exists($functionName, $traitMethodAliases)) {
Expand All @@ -385,22 +382,11 @@ function (\PhpParser\Node $node) use ($fileName, $lookForTrait, $traitMethodAlia
$resolvableTemplateTypes = true;
} elseif ($node instanceof Node\Stmt\Property) {
$resolvableTemplateTypes = true;
} elseif (!in_array(get_class($node), [
Node\Stmt\Foreach_::class,
Node\Expr\Assign::class,
Node\Expr\AssignRef::class,
Node\Stmt\Class_::class,
Node\Stmt\ClassConst::class,
Node\Stmt\Static_::class,
Node\Stmt\Echo_::class,
Node\Stmt\Return_::class,
Node\Stmt\Expression::class,
Node\Stmt\Throw_::class,
Node\Stmt\If_::class,
Node\Stmt\While_::class,
Node\Stmt\Switch_::class,
Node\Stmt\Nop::class,
], true)) {
} elseif (
!$node instanceof Node\Stmt
&& !$node instanceof Node\Expr\Assign
&& !$node instanceof Node\Expr\AssignRef
) {
return null;
}

Expand Down
12 changes: 12 additions & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10710,6 +10710,16 @@ public function dataBug4351(): array
return $this->gatherAssertTypes(__DIR__ . '/data/bug-4351.php');
}

public function dataVarAboveUse(): array
{
return $this->gatherAssertTypes(__DIR__ . '/data/var-above-use.php');
}

public function dataVarAboveDeclare(): array
{
return $this->gatherAssertTypes(__DIR__ . '/data/var-above-declare.php');
}

/**
* @param string $file
* @return array<string, mixed[]>
Expand Down Expand Up @@ -10917,6 +10927,8 @@ private function gatherAssertTypes(string $file): array
* @dataProvider dataBug4343
* @dataProvider dataImpureMethod
* @dataProvider dataBug4351
* @dataProvider dataVarAboveUse
* @dataProvider dataVarAboveDeclare
* @param string $assertType
* @param string $file
* @param mixed ...$args
Expand Down
10 changes: 10 additions & 0 deletions tests/PHPStan/Analyser/data/var-above-declare.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

/**
* @var string $foo
*/

declare(strict_types = 1);

\PHPStan\Analyser\assertVariableCertainty(\PHPStan\TrinaryLogic::createYes(), $foo);
\PHPStan\Analyser\assertType('string', $foo);
10 changes: 10 additions & 0 deletions tests/PHPStan/Analyser/data/var-above-use.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

/**
* @var string $foo
*/

use Foo\Bar\Baz;

\PHPStan\Analyser\assertVariableCertainty(\PHPStan\TrinaryLogic::createYes(), $foo);
\PHPStan\Analyser\assertType('string', $foo);
10 changes: 10 additions & 0 deletions tests/PHPStan/Rules/PhpDoc/WrongVariableNameInVarTagRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,4 +90,14 @@ public function testEmptyFileWithVarThis(): void
$this->analyse([__DIR__ . '/data/wrong-variable-name-var-empty-this.php'], []);
}

public function testAboveUse(): void
{
$this->analyse([__DIR__ . '/data/var-above-use.php'], []);
}

public function testAboveDeclare(): void
{
$this->analyse([__DIR__ . '/data/var-above-declare.php'], []);
}

}
7 changes: 7 additions & 0 deletions tests/PHPStan/Rules/PhpDoc/data/var-above-declare.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

/**
* @var string $foo
*/

declare(strict_types = 1);
5 changes: 5 additions & 0 deletions tests/PHPStan/Rules/PhpDoc/data/var-above-use.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?php

/** @var string $foo */

use Foo\Bar\Baz;

1 comment on commit 27481c3

@b1rdex
Copy link
Contributor

@b1rdex b1rdex commented on 27481c3 Jan 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more uncovered scenario:

<?php

/**
 * @var LogicException $this
 */

use \Some\Namespaced\ClassName;
use \Symfony\Component\Console\Exception\LogicException;

\PHPStan\dumpType($this); // yields \LogicException (not a FQCN, error)

https://phpstan.org/r/a4c648ae-ab38-455f-b69f-aa588a4025a7

Should I open a separate issue for this?

FYI, this works:

<?php

/**
 * @var LogicException $this
 */

use \Symfony\Component\Console\Exception\LogicException;

\PHPStan\dumpType($this); // \Symfony\Component\Console\Exception\LogicException

Please sign in to comment.