Skip to content

Commit dfe79f7

Browse files
Address review comments for possibly-impure tip
- Carry declared return type as expression type in assignExpression() instead of as a PossiblyImpureCallExpr constructor parameter - Remove unnecessary hasTips() check on new error builder in MatchExpressionRule - Remove hasTips condition in PossiblyImpureTipHelper and remove hasTips() method from RuleErrorBuilder Co-authored-by: Ondřej Mirtes <ondrejmirtes@users.noreply.github.com>
1 parent 6820560 commit dfe79f7

File tree

6 files changed

+9
-26
lines changed

6 files changed

+9
-26
lines changed

src/Analyser/MutatingScope.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -815,7 +815,7 @@ public function findPossiblyImpureCallDescriptions(Expr $expr): array
815815
// narrowing affected the type (the cached value was narrowed).
816816
assert($found instanceof Expr);
817817
$scopeType = $this->getType($found);
818-
$declaredReturnType = $holderExpr->getDeclaredReturnType();
818+
$declaredReturnType = $holder->getType();
819819
if ($declaredReturnType->isSuperTypeOf($scopeType)->yes() && $scopeType->isSuperTypeOf($declaredReturnType)->yes()) {
820820
continue;
821821
}

src/Analyser/NodeScopeResolver.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2929,8 +2929,8 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto
29292929
&& $parametersAcceptor !== null
29302930
) {
29312931
$scope = $scope->assignExpression(
2932-
new PossiblyImpureCallExpr($normalizedExpr, $normalizedExpr, sprintf('%s()', $functionReflection->getName()), $parametersAcceptor->getReturnType()),
2933-
new MixedType(),
2932+
new PossiblyImpureCallExpr($normalizedExpr, $normalizedExpr, sprintf('%s()', $functionReflection->getName())),
2933+
$parametersAcceptor->getReturnType(),
29342934
new MixedType(),
29352935
);
29362936
}
@@ -3235,8 +3235,8 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto
32353235
$scope = $scope->invalidateExpression($normalizedExpr->var, true, $methodReflection->getDeclaringClass());
32363236
} elseif ($this->rememberPossiblyImpureFunctionValues && $methodReflection->hasSideEffects()->maybe() && !$methodReflection->getDeclaringClass()->isBuiltin() && $parametersAcceptor !== null) {
32373237
$scope = $scope->assignExpression(
3238-
new PossiblyImpureCallExpr($normalizedExpr, $normalizedExpr->var, sprintf('%s::%s()', $methodReflection->getDeclaringClass()->getDisplayName(), $methodReflection->getName()), $parametersAcceptor->getReturnType()),
3239-
new MixedType(),
3238+
new PossiblyImpureCallExpr($normalizedExpr, $normalizedExpr->var, sprintf('%s::%s()', $methodReflection->getDeclaringClass()->getDisplayName(), $methodReflection->getName())),
3239+
$parametersAcceptor->getReturnType(),
32403240
new MixedType(),
32413241
);
32423242
}
@@ -3459,8 +3459,8 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto
34593459
&& $scope->getClassReflection()->is($methodReflection->getDeclaringClass()->getName())
34603460
) {
34613461
$scope = $scope->assignExpression(
3462-
new PossiblyImpureCallExpr($normalizedExpr, new Variable('this'), sprintf('%s::%s()', $methodReflection->getDeclaringClass()->getDisplayName(), $methodReflection->getName()), $parametersAcceptor->getReturnType()),
3463-
new MixedType(),
3462+
new PossiblyImpureCallExpr($normalizedExpr, new Variable('this'), sprintf('%s::%s()', $methodReflection->getDeclaringClass()->getDisplayName(), $methodReflection->getName())),
3463+
$parametersAcceptor->getReturnType(),
34643464
new MixedType(),
34653465
);
34663466
}

src/Node/Expr/PossiblyImpureCallExpr.php

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
use Override;
66
use PhpParser\Node\Expr;
77
use PHPStan\Node\VirtualNode;
8-
use PHPStan\Type\Type;
98

109
final class PossiblyImpureCallExpr extends Expr implements VirtualNode
1110
{
@@ -14,7 +13,6 @@ public function __construct(
1413
public Expr $callExpr,
1514
public Expr $impactedExpr,
1615
private string $callDescription,
17-
private Type $declaredReturnType,
1816
)
1917
{
2018
parent::__construct([]);
@@ -25,11 +23,6 @@ public function getCallDescription(): string
2523
return $this->callDescription;
2624
}
2725

28-
public function getDeclaredReturnType(): Type
29-
{
30-
return $this->declaredReturnType;
31-
}
32-
3326
#[Override]
3427
public function getType(): string
3528
{

src/Rules/Comparison/MatchExpressionRule.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,8 @@ public function processNode(Node $node, Scope $scope): array
117117
);
118118
$errorBuilder = RuleErrorBuilder::message($message)
119119
->line($armLine)
120-
->identifier('match.alwaysTrue');
121-
if (!$errorBuilder->hasTips()) {
122-
$errorBuilder->tip('Remove remaining cases below this one and this error will disappear too.');
123-
}
120+
->identifier('match.alwaysTrue')
121+
->tip('Remove remaining cases below this one and this error will disappear too.');
124122
$this->possiblyImpureTipHelper->addTip($armConditionScope, $armConditionExpr, $errorBuilder);
125123
$errors[] = $errorBuilder->build();
126124
}

src/Rules/Comparison/PossiblyImpureTipHelper.php

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,6 @@ public function addTip(
3030
if (!$this->possiblyImpureTip) {
3131
return $ruleErrorBuilder;
3232
}
33-
if ($ruleErrorBuilder->hasTips()) {
34-
return $ruleErrorBuilder;
35-
}
3633
if (!$scope instanceof MutatingScope) {
3734
return $ruleErrorBuilder;
3835
}

src/Rules/RuleErrorBuilder.php

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -241,11 +241,6 @@ public function possiblyImpureTip(array $callDescriptions): self
241241
return $this;
242242
}
243243

244-
public function hasTips(): bool
245-
{
246-
return count($this->tips) > 0;
247-
}
248-
249244
/**
250245
* Sets an error identifier.
251246
*

0 commit comments

Comments
 (0)