Skip to content

Commit 7e813e2

Browse files
jack-wormanjack-worman
committed
Report redundant inline @var tags with reportRedundantVarTag
1 parent 6ebd2d7 commit 7e813e2

22 files changed

Lines changed: 1697 additions & 421 deletions

‎conf/config.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ parameters:
104104
reportStaticMethodSignatures: false
105105
reportWrongPhpDocTypeInVarTag: false
106106
reportAnyTypeWideningInVarTag: false
107+
reportRedundantVarTag: false
107108
reportNonIntStringArrayKey: false
108109
reportUnsafeArrayStringKeyCasting: null
109110
reportPossiblyNonexistentGeneralArrayOffset: false

‎conf/parametersSchema.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ parametersSchema:
108108
reportStaticMethodSignatures: bool()
109109
reportWrongPhpDocTypeInVarTag: bool()
110110
reportAnyTypeWideningInVarTag: bool()
111+
reportRedundantVarTag: bool()
111112
reportNonIntStringArrayKey: bool()
112113
reportUnsafeArrayStringKeyCasting: schema(string(), pattern('detect|prevent'), nullable())
113114
reportPossiblyNonexistentGeneralArrayOffset: bool()

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,7 @@ public function processStmtNode(
451451
if ($stmtHandler !== null) {
452452
$stmtResult = $stmtHandler->processStmt($this, $stmt, $scope, $storage, $nodeCallback, $context);
453453
if ($varTagDependencies !== null) {
454+
$this->statementsHandler->emitRedundantVarTagNodes($this, $scope, $storage, $stmt, $nodeCallback);
454455
$stmtResult = $stmtResult->withDependencies(Dependencies::merge($stmtResult->getDependencies(), $varTagDependencies));
455456
}
456457
if ($overridingThrowPoints !== null) {

‎src/Analyser/RedundantVarTagDetector.php‎

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Analyser;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Expr;
7+
use PHPStan\DependencyInjection\AutowiredService;
8+
use PHPStan\Node\RedundantVarTagNode;
9+
use PHPStan\PhpDoc\Tag\VarTag;
10+
use PHPStan\Type\Constant\ConstantArrayType;
11+
use PHPStan\Type\ErrorType;
12+
use PHPStan\Type\MixedType;
13+
use PHPStan\Type\NeverType;
14+
use PHPStan\Type\Type;
15+
use PHPStan\Type\TypeCombinator;
16+
use PHPStan\Type\TypeTraverser;
17+
use function is_string;
18+
use function spl_object_id;
19+
20+
/**
21+
* Finds the inline @var tags that change nothing about the variable or
22+
* expression they re-type, for the opt-in reportRedundantVarTag.
23+
*
24+
* The walk applies a tag every time it walks the tag's statement: in every
25+
* pass over a loop body, in the observation pass of template arguments, in
26+
* every iteration over a closure with by-reference uses. Its own type can flow
27+
* back into what it re-types in a later pass, so a tag is redundant only when
28+
* none of these applications changed anything. The engine records each one,
29+
* and emits the redundant tags once it has processed the statement.
30+
*
31+
* Only the type and the certainty are compared: an assignment's tag also makes
32+
* the native type mixed and forgets what the assigned value narrowed, and
33+
* without the tag both would only be more precise.
34+
*/
35+
#[AutowiredService]
36+
final class RedundantVarTagDetector implements PerFileAnalysisResettable
37+
{
38+
39+
private const EXPLICIT_MIXED = 1;
40+
private const IMPLICIT_MIXED = 2;
41+
private const EXPLICIT_NEVER = 4;
42+
private const IMPLICIT_NEVER = 8;
43+
44+
/** @var array<int, array{Node\Stmt, array<int|string, array{VarTag, bool}>}> statement id => [statement, tag key => [tag, changed nothing so far]] */
45+
private array $applications = [];
46+
47+
public function resetFileAnalysisState(): void
48+
{
49+
$this->applications = [];
50+
}
51+
52+
/**
53+
* Records the application of $varTag to a variable, which gets the tag's
54+
* type, and becomes certainly defined when $defines is true. $narrowedByForeach
55+
* is true for the tag of a foreach's iteratee applied again on entering the
56+
* loop body, where the loop may have narrowed the variable to a non-empty
57+
* array the tag widens again.
58+
*
59+
* @param int|string $key the tag's key in ResolvedPhpDocBlock::getVarTags()
60+
*/
61+
public function recordVariable(Node\Stmt $stmt, $key, VarTag $varTag, MutatingScope $scope, string $variableName, bool $defines, bool $narrowedByForeach = false): void
62+
{
63+
$certainty = $scope->hasVariableType($variableName);
64+
if ($certainty->no() || ($defines && !$certainty->yes())) {
65+
$this->record($stmt, $key, $varTag, false);
66+
return;
67+
}
68+
69+
$originalType = $scope->getVariableType($variableName);
70+
$tagType = $varTag->getType();
71+
$this->record(
72+
$stmt,
73+
$key,
74+
$varTag,
75+
$this->isSameType($originalType, $tagType)
76+
|| ($narrowedByForeach && $this->isSameType($originalType, TypeCombinator::remove($tagType, new ConstantArrayType([], [])))),
77+
);
78+
}
79+
80+
/**
81+
* Records the application of the variable-less $varTag to $expr, $scope
82+
* being the scope before the tag re-typed it.
83+
*/
84+
public function recordExpression(NodeScopeResolver $nodeScopeResolver, Node\Stmt $stmt, VarTag $varTag, Expr $expr, MutatingScope $scope): void
85+
{
86+
// priced again from scratch: the walk's results of the expression and
87+
// of its parts saw the tag's type, re-typing $a['k'] narrows $a as well
88+
$originalType = $nodeScopeResolver->processExprOnDemand($expr, $scope, new ExpressionResultStorage())->getType();
89+
$this->record($stmt, 0, $varTag, $this->isSameType($originalType, $varTag->getType()));
90+
}
91+
92+
/**
93+
* Emits a RedundantVarTagNode for each tag above $stmt that changed
94+
* nothing in any of its applications so far.
95+
*
96+
* @param callable(Node $node, Scope $scope): void $nodeCallback
97+
*/
98+
public function emitNodes(NodeScopeResolver $nodeScopeResolver, MutatingScope $scope, ExpressionResultStorage $storage, Node\Stmt $stmt, callable $nodeCallback): void
99+
{
100+
$id = spl_object_id($stmt);
101+
if (!isset($this->applications[$id]) || $this->applications[$id][0] !== $stmt) {
102+
return;
103+
}
104+
105+
foreach ($this->applications[$id][1] as $key => [$varTag, $changedNothing]) {
106+
if (!$changedNothing) {
107+
continue;
108+
}
109+
110+
$nodeScopeResolver->callNodeCallback($nodeCallback, new RedundantVarTagNode($stmt, $varTag, is_string($key) ? $key : null), $scope, $storage);
111+
}
112+
}
113+
114+
/**
115+
* @param int|string $key
116+
*/
117+
private function record(Node\Stmt $stmt, $key, VarTag $varTag, bool $changedNothing): void
118+
{
119+
$id = spl_object_id($stmt);
120+
if (!isset($this->applications[$id]) || $this->applications[$id][0] !== $stmt) {
121+
// the id of a statement freed since is reused
122+
$this->applications[$id] = [$stmt, []];
123+
}
124+
125+
$changedNothingBefore = $this->applications[$id][1][$key][1] ?? true;
126+
$this->applications[$id][1][$key] = [$varTag, $changedNothingBefore && $changedNothing];
127+
}
128+
129+
private function isSameType(Type $originalType, Type $tagType): bool
130+
{
131+
if (
132+
// equals() is one-way for a type extending another one, like a
133+
// template type or a benevolent union, and does not compare
134+
// whether a constant array is a list
135+
!$originalType->equals($tagType)
136+
|| !$tagType->equals($originalType)
137+
|| !$originalType->isList()->equals($tagType->isList())
138+
) {
139+
return false;
140+
}
141+
142+
// equals() also ignores whether mixed and never are explicit, which
143+
// decides what is reported about them
144+
$kinds = $this->getMixedAndNeverKinds($originalType);
145+
146+
return $kinds !== null && $kinds === $this->getMixedAndNeverKinds($tagType);
147+
}
148+
149+
/**
150+
* Which kinds of mixed and never the type contains; null when it contains an
151+
* error type, or both kinds of mixed or of never, whose positions are not compared.
152+
*/
153+
private function getMixedAndNeverKinds(Type $type): ?int
154+
{
155+
$kinds = 0;
156+
$containsError = false;
157+
TypeTraverser::map($type, static function (Type $type, callable $traverse) use (&$kinds, &$containsError): Type {
158+
if ($type instanceof ErrorType) {
159+
$containsError = true;
160+
return $type;
161+
}
162+
163+
if ($type instanceof MixedType) {
164+
$kinds |= $type->isExplicitMixed() ? self::EXPLICIT_MIXED : self::IMPLICIT_MIXED;
165+
} elseif ($type instanceof NeverType) {
166+
$kinds |= $type->isExplicit() ? self::EXPLICIT_NEVER : self::IMPLICIT_NEVER;
167+
}
168+
169+
return $traverse($type);
170+
});
171+
172+
if (
173+
$containsError
174+
|| ($kinds & (self::EXPLICIT_MIXED | self::IMPLICIT_MIXED)) === (self::EXPLICIT_MIXED | self::IMPLICIT_MIXED)
175+
|| ($kinds & (self::EXPLICIT_NEVER | self::IMPLICIT_NEVER)) === (self::EXPLICIT_NEVER | self::IMPLICIT_NEVER)
176+
) {
177+
return null;
178+
}
179+
180+
return $kinds;
181+
}
182+
183+
}

‎src/Analyser/StatementsHandler.php‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,11 @@ public function __construct(
8989
private TemplateArgumentResolver $templateArgumentResolver,
9090
private Container $container,
9191
private StaticVariableInference $staticVariableInference,
92+
private RedundantVarTagDetector $redundantVarTagDetector,
9293
#[AutowiredParameter(ref: '%featureToggles.unresolvedTemplateArguments%')]
9394
private bool $unresolvedTemplateArguments,
95+
#[AutowiredParameter(ref: '%reportRedundantVarTag%')]
96+
private bool $reportRedundantVarTag,
9497
)
9598
{
9699
$this->debugTemplateArguments = getenv('PHPSTAN_TEMPLATE_ARGUMENTS_DEBUG') === '1';
@@ -1977,6 +1980,10 @@ public function processStmtVarAnnotation(NodeScopeResolver $nodeScopeResolver, M
19771980
$scope = $scope->addTemplateArgumentConstraints($this->templateArgumentObserver->collectSend($varTag->getType(), $originalType));
19781981
}
19791982

1983+
if ($this->reportRedundantVarTag) {
1984+
$this->redundantVarTagDetector->recordVariable($stmt, $name, $varTag, $scope, $name, $certainty->yes());
1985+
}
1986+
19801987
$nativeScope = $scope->doNotTreatPhpDocTypesAsCertain();
19811988
$scope = $scope->assignVariable(
19821989
$name,
@@ -2047,11 +2054,30 @@ public function emitVarTagChangedNode(NodeScopeResolver $nodeScopeResolver, Muta
20472054
}
20482055

20492056
$nodeScopeResolver->callNodeCallback($nodeCallback, new VarTagChangedExpressionTypeNode($varTag, $defaultExpr), $scope, $storage);
2057+
if ($this->reportRedundantVarTag) {
2058+
$this->redundantVarTagDetector->recordExpression($nodeScopeResolver, $stmt, $varTag, $defaultExpr, $scope);
2059+
}
20502060
$defaultExprResult = $storage->findExpressionResult($defaultExpr);
20512061
if ($defaultExprResult === null || $nodeScopeResolver->observingTemplateArgumentFrame($defaultExprResult->getScope()) === null) {
20522062
return TemplateArgumentConstraints::createEmpty();
20532063
}
20542064
return $this->templateArgumentObserver->collectSend($varTag->getType(), $defaultExprResult->getType());
20552065
}
20562066

2067+
/**
2068+
* Emits the nodes the redundant @var tag rule listens to, for the tags above
2069+
* $stmt that changed nothing in any application so far - called once the
2070+
* statement has been processed (see RedundantVarTagDetector).
2071+
*
2072+
* @param callable(Node $node, Scope $scope): void $nodeCallback
2073+
*/
2074+
public function emitRedundantVarTagNodes(NodeScopeResolver $nodeScopeResolver, MutatingScope $scope, ExpressionResultStorage $storage, Node\Stmt $stmt, callable $nodeCallback): void
2075+
{
2076+
if (!$this->reportRedundantVarTag) {
2077+
return;
2078+
}
2079+
2080+
$this->redundantVarTagDetector->emitNodes($nodeScopeResolver, $scope, $storage, $stmt, $nodeCallback);
2081+
}
2082+
20572083
}

‎src/Analyser/StmtHandler/ForeachHandler.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,7 @@ static function () use ($condResult, $emptyArrayType): Type {
537537
private function enterForeach(NodeScopeResolver $nodeScopeResolver, MutatingScope $scope, ExpressionResultStorage $storage, MutatingScope $originalScope, Foreach_ $stmt, Type $iterateeType, Type $nativeIterateeType, callable $nodeCallback): MutatingScope
538538
{
539539
if ($stmt->expr instanceof Variable && is_string($stmt->expr->name)) {
540-
$scope = $this->varAnnotationProcessor->processVarAnnotation($scope, [$stmt->expr->name], $stmt);
540+
$scope = $this->varAnnotationProcessor->processForeachIterateeVarAnnotation($scope, $stmt->expr->name, $stmt);
541541
}
542542

543543
if (

‎src/Analyser/VarAnnotationProcessor.php‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use PhpParser\Comment\Doc;
66
use PhpParser\Node;
7+
use PHPStan\DependencyInjection\AutowiredParameter;
78
use PHPStan\DependencyInjection\AutowiredService;
89
use PHPStan\TrinaryLogic;
910
use PHPStan\Turbo\ShadowedByTurboExtension;
@@ -18,6 +19,9 @@ final class VarAnnotationProcessor
1819

1920
public function __construct(
2021
private FileTypeMapper $fileTypeMapper,
22+
private RedundantVarTagDetector $redundantVarTagDetector,
23+
#[AutowiredParameter(ref: '%reportRedundantVarTag%')]
24+
private bool $reportRedundantVarTag,
2125
)
2226
{
2327
}
@@ -26,6 +30,25 @@ public function __construct(
2630
* @param array<int, string> $variableNames
2731
*/
2832
public function processVarAnnotation(MutatingScope $scope, array $variableNames, Node\Stmt $node, bool &$changed = false): MutatingScope
33+
{
34+
return $this->applyVarTags($scope, $variableNames, $node, $changed, false);
35+
}
36+
37+
/**
38+
* processVarAnnotation() for the variable a foreach iterates, applied again on
39+
* entering the loop body, where the loop may have narrowed it to a non-empty array.
40+
*/
41+
public function processForeachIterateeVarAnnotation(MutatingScope $scope, string $variableName, Node\Stmt\Foreach_ $node): MutatingScope
42+
{
43+
$changed = false;
44+
45+
return $this->applyVarTags($scope, [$variableName], $node, $changed, true);
46+
}
47+
48+
/**
49+
* @param array<int, string> $variableNames
50+
*/
51+
private function applyVarTags(MutatingScope $scope, array $variableNames, Node\Stmt $node, bool &$changed, bool $narrowedByForeach): MutatingScope
2952
{
3053
$function = $scope->getFunction();
3154
$varTags = [];
@@ -57,12 +80,18 @@ public function processVarAnnotation(MutatingScope $scope, array $variableNames,
5780

5881
$variableType = $varTags[$variableName]->getType();
5982
$changed = true;
83+
if ($this->reportRedundantVarTag) {
84+
$this->redundantVarTagDetector->recordVariable($node, $variableName, $varTags[$variableName], $scope, $variableName, true, $narrowedByForeach);
85+
}
6086
$scope = $scope->assignVariable($variableName, $variableType, new MixedType(), TrinaryLogic::createYes());
6187
}
6288

6389
if (count($variableNames) === 1 && count($varTags) === 1 && isset($varTags[0])) {
6490
$variableType = $varTags[0]->getType();
6591
$changed = true;
92+
if ($this->reportRedundantVarTag) {
93+
$this->redundantVarTagDetector->recordVariable($node, 0, $varTags[0], $scope, $variableNames[0], true, $narrowedByForeach);
94+
}
6695
$scope = $scope->assignVariable($variableNames[0], $variableType, new MixedType(), TrinaryLogic::createYes());
6796
}
6897

‎src/Node/RedundantVarTagNode.php‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Node;
4+
5+
use Override;
6+
use PhpParser\Node\Stmt;
7+
use PhpParser\NodeAbstract;
8+
use PHPStan\PhpDoc\Tag\VarTag;
9+
10+
/**
11+
* An inline @var tag above the statement that changed nothing about what it
12+
* re-typed, emitted with reportRedundantVarTag once the statement is processed.
13+
*/
14+
final class RedundantVarTagNode extends NodeAbstract implements VirtualNode
15+
{
16+
17+
public function __construct(private Stmt $stmt, private VarTag $varTag, private ?string $variableName)
18+
{
19+
parent::__construct($stmt->getAttributes());
20+
}
21+
22+
public function getStmt(): Stmt
23+
{
24+
return $this->stmt;
25+
}
26+
27+
public function getVarTag(): VarTag
28+
{
29+
return $this->varTag;
30+
}
31+
32+
/** Null for a tag without a variable name. */
33+
public function getVariableName(): ?string
34+
{
35+
return $this->variableName;
36+
}
37+
38+
#[Override]
39+
public function getType(): string
40+
{
41+
return 'PHPStan_Node_RedundantVarTag';
42+
}
43+
44+
/**
45+
* @return string[]
46+
*/
47+
#[Override]
48+
public function getSubNodeNames(): array
49+
{
50+
return [];
51+
}
52+
53+
}

0 commit comments

Comments
 (0)