Skip to content

Commit 6a09253

Browse files
ondrejmirtesclaude
ondrejmirtes
and
claude
committed
Treat possible Error throws as an effect of an expression statement again
Reverts the src/ changes of c7301f6 and d3a36a9. Dropping explicit Error throw points made calls like `FooEnum::from($v)` in a loop, used purely for validation, report "on a separate line has no effect". The tests added by both commits are kept, with expectations updated. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LQS31c8jwYW7xZwaA37tVR
1 parent d88544a commit 6a09253

13 files changed

Lines changed: 98 additions & 99 deletions

‎src/Analyser/ExprHandler/FuncCallHandler.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
133133
) {
134134
// processed later
135135
} elseif ($parametersAcceptor instanceof CallableParametersAcceptor) {
136-
$callableThrowPoints = array_map(static fn (SimpleThrowPoint $throwPoint) => $throwPoint->isExplicit() ? InternalThrowPoint::createExplicit($scope, $throwPoint->getType(), $expr, $throwPoint->canContainAnyThrowable(), $throwPoint->isFromThrowExpr()) : InternalThrowPoint::createImplicit($scope, $expr), $parametersAcceptor->getThrowPoints());
136+
$callableThrowPoints = array_map(static fn (SimpleThrowPoint $throwPoint) => $throwPoint->isExplicit() ? InternalThrowPoint::createExplicit($scope, $throwPoint->getType(), $expr, $throwPoint->canContainAnyThrowable()) : InternalThrowPoint::createImplicit($scope, $expr), $parametersAcceptor->getThrowPoints());
137137
if (!$this->implicitThrows) {
138138
$callableThrowPoints = array_values(array_filter($callableThrowPoints, static fn (InternalThrowPoint $throwPoint) => $throwPoint->isExplicit()));
139139
}

‎src/Analyser/ExprHandler/Helper/ClosureTypeResolver.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -908,7 +908,7 @@ private function assembleClosureType(
908908
);
909909
}
910910

911-
$throwPointsForClosureType = array_map(static fn (ThrowPoint $throwPoint) => $throwPoint->isExplicit() ? SimpleThrowPoint::createExplicit($throwPoint->getType(), $throwPoint->canContainAnyThrowable(), $throwPoint->isFromThrowExpr()) : SimpleThrowPoint::createImplicit(), $throwPoints);
911+
$throwPointsForClosureType = array_map(static fn (ThrowPoint $throwPoint) => $throwPoint->isExplicit() ? SimpleThrowPoint::createExplicit($throwPoint->getType(), $throwPoint->canContainAnyThrowable()) : SimpleThrowPoint::createImplicit(), $throwPoints);
912912
$impurePointsForClosureType = array_map(static fn (ImpurePoint $impurePoint) => new SimpleImpurePoint($impurePoint->getIdentifier(), $impurePoint->getDescription(), $impurePoint->isCertain()), $impurePoints);
913913

914914
if ($writeCache) {

‎src/Analyser/ExprHandler/ThrowHandler.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
4848
expr: $expr,
4949
hasYield: false,
5050
isAlwaysTerminating: true,
51-
throwPoints: array_merge($exprResult->getThrowPoints(), [InternalThrowPoint::createExplicit($scope, $exprResult->getType(), $expr, false, fromThrowExpr: true)]),
51+
throwPoints: array_merge($exprResult->getThrowPoints(), [InternalThrowPoint::createExplicit($scope, $exprResult->getType(), $expr, false)]),
5252
impurePoints: $exprResult->getImpurePoints(),
5353
);
5454
}

‎src/Analyser/InternalThrowPoint.php‎

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,14 @@ private function __construct(
2020
private Node $node,
2121
private bool $explicit,
2222
private bool $canContainAnyThrowable,
23-
private bool $fromThrowExpr = false,
2423
)
2524
{
2625
}
2726

2827
public function toPublic(): ThrowPoint
2928
{
3029
if ($this->explicit) {
31-
return ThrowPoint::createExplicit($this->scope, $this->type, $this->node, $this->canContainAnyThrowable, $this->fromThrowExpr);
30+
return ThrowPoint::createExplicit($this->scope, $this->type, $this->node, $this->canContainAnyThrowable);
3231
}
3332

3433
return ThrowPoint::createImplicit($this->scope, $this->node);
@@ -37,9 +36,9 @@ public function toPublic(): ThrowPoint
3736
/**
3837
* @param Node\Expr|Node\Stmt $node
3938
*/
40-
public static function createExplicit(MutatingScope $scope, Type $type, Node $node, bool $canContainAnyThrowable, bool $fromThrowExpr = false): self
39+
public static function createExplicit(MutatingScope $scope, Type $type, Node $node, bool $canContainAnyThrowable): self
4140
{
42-
return new self($scope, $type, $node, true, $canContainAnyThrowable, $fromThrowExpr);
41+
return new self($scope, $type, $node, true, $canContainAnyThrowable);
4342
}
4443

4544
/**
@@ -52,7 +51,7 @@ public static function createImplicit(MutatingScope $scope, Node $node): self
5251

5352
public static function createFromPublic(ThrowPoint $throwPoint, MutatingScope $scope): self
5453
{
55-
return new self($scope, $throwPoint->getType(), $throwPoint->getNode(), $throwPoint->isExplicit(), $throwPoint->canContainAnyThrowable(), $throwPoint->isFromThrowExpr());
54+
return new self($scope, $throwPoint->getType(), $throwPoint->getNode(), $throwPoint->isExplicit(), $throwPoint->canContainAnyThrowable());
5655
}
5756

5857
public function getScope(): MutatingScope
@@ -83,19 +82,9 @@ public function canContainAnyThrowable(): bool
8382
return $this->canContainAnyThrowable;
8483
}
8584

86-
/**
87-
* Whether the throw point comes from a `throw` written in the analysed code,
88-
* as opposed to a throw inferred from what a called function or an operation
89-
* can throw.
90-
*/
91-
public function isFromThrowExpr(): bool
92-
{
93-
return $this->fromThrowExpr;
94-
}
95-
9685
public function subtractCatchType(Type $catchType): self
9786
{
98-
return new self($this->scope, TypeCombinator::remove($this->type, $catchType), $this->node, $this->explicit, $this->canContainAnyThrowable, $this->fromThrowExpr);
87+
return new self($this->scope, TypeCombinator::remove($this->type, $catchType), $this->node, $this->explicit, $this->canContainAnyThrowable);
9988
}
10089

10190
}

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2135,7 +2135,7 @@ public function processArgs(
21352135
// still complete the gathered data - then it keeps re-walking)
21362136
$this->container->getByType(ClosureTypeResolver::class)->seedCacheFromClosureWalk($scopeToPass, $arg->value, $closureResult);
21372137
if ($this->callCallbackImmediately($parameter, $parameterType, $calleeReflection)) {
2138-
$throwPoints = array_merge($throwPoints, array_map(static fn (InternalThrowPoint $throwPoint) => $throwPoint->isExplicit() ? InternalThrowPoint::createExplicit($scope, $throwPoint->getType(), $arg->value, $throwPoint->canContainAnyThrowable(), $throwPoint->isFromThrowExpr()) : InternalThrowPoint::createImplicit($scope, $arg->value), $closureResult->getThrowPoints()));
2138+
$throwPoints = array_merge($throwPoints, array_map(static fn (InternalThrowPoint $throwPoint) => $throwPoint->isExplicit() ? InternalThrowPoint::createExplicit($scope, $throwPoint->getType(), $arg->value, $throwPoint->canContainAnyThrowable()) : InternalThrowPoint::createImplicit($scope, $arg->value), $closureResult->getThrowPoints()));
21392139
$impurePoints = array_merge($impurePoints, $closureResult->getImpurePoints());
21402140
}
21412141

@@ -2227,7 +2227,7 @@ public function processArgs(
22272227
$this->container->getByType(ClosureTypeResolver::class)->seedCacheFromArrowFunctionWalk($scopeToPass, $arg->value, $processArrowFunctionResult);
22282228
$arrowFunctionResult = $processArrowFunctionResult->getExpressionResult();
22292229
if ($this->callCallbackImmediately($parameter, $parameterType, $calleeReflection)) {
2230-
$throwPoints = array_merge($throwPoints, array_map(static fn (InternalThrowPoint $throwPoint) => $throwPoint->isExplicit() ? InternalThrowPoint::createExplicit($scope, $throwPoint->getType(), $arg->value, $throwPoint->canContainAnyThrowable(), $throwPoint->isFromThrowExpr()) : InternalThrowPoint::createImplicit($scope, $arg->value), $arrowFunctionResult->getThrowPoints()));
2230+
$throwPoints = array_merge($throwPoints, array_map(static fn (InternalThrowPoint $throwPoint) => $throwPoint->isExplicit() ? InternalThrowPoint::createExplicit($scope, $throwPoint->getType(), $arg->value, $throwPoint->canContainAnyThrowable()) : InternalThrowPoint::createImplicit($scope, $arg->value), $arrowFunctionResult->getThrowPoints()));
22312231
$impurePoints = array_merge($impurePoints, $arrowFunctionResult->getImpurePoints());
22322232
}
22332233
if ($this->shouldInvalidateCallbackExpressions($parameter)) {
@@ -2264,7 +2264,7 @@ public function processArgs(
22642264
$deferredInvalidateExpressions[] = [$acceptors[0]->getInvalidateExpressions(), $acceptors[0]->getUsedVariables()];
22652265
}
22662266
if ($this->callCallbackImmediately($parameter, $parameterType, $calleeReflection)) {
2267-
$callableThrowPoints = array_map(static fn (SimpleThrowPoint $throwPoint) => $throwPoint->isExplicit() ? InternalThrowPoint::createExplicit($scope, $throwPoint->getType(), $arg->value, $throwPoint->canContainAnyThrowable(), $throwPoint->isFromThrowExpr()) : InternalThrowPoint::createImplicit($scope, $arg->value), $acceptors[0]->getThrowPoints());
2267+
$callableThrowPoints = array_map(static fn (SimpleThrowPoint $throwPoint) => $throwPoint->isExplicit() ? InternalThrowPoint::createExplicit($scope, $throwPoint->getType(), $arg->value, $throwPoint->canContainAnyThrowable()) : InternalThrowPoint::createImplicit($scope, $arg->value), $acceptors[0]->getThrowPoints());
22682268
if (!$this->implicitThrows) {
22692269
$callableThrowPoints = array_values(array_filter($callableThrowPoints, static fn (InternalThrowPoint $throwPoint) => $throwPoint->isExplicit()));
22702270
}

‎src/Analyser/StmtHandler/ExpressionHandler.php‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
namespace PHPStan\Analyser\StmtHandler;
44

5-
use Error;
65
use PhpParser\Node;
76
use PhpParser\Node\Expr;
87
use PhpParser\Node\Stmt;
@@ -11,7 +10,6 @@
1110
use PHPStan\Analyser\ExpressionResultStorage;
1211
use PHPStan\Analyser\InternalStatementExitPoint;
1312
use PHPStan\Analyser\InternalStatementResult;
14-
use PHPStan\Analyser\InternalThrowPoint;
1513
use PHPStan\Analyser\MutatingScope;
1614
use PHPStan\Analyser\NodeScopeResolver;
1715
use PHPStan\Analyser\Scope;
@@ -25,7 +23,6 @@
2523
use PHPStan\Node\VariableAssignNode;
2624
use PHPStan\Type\Constant\ConstantBooleanType;
2725
use PHPStan\Type\NeverType;
28-
use PHPStan\Type\ObjectType;
2926
use function array_filter;
3027
use function count;
3128

@@ -77,12 +74,7 @@ public function processStmt(
7774
}
7875

7976
$nodeScopeResolver->callNodeCallback($nodeCallback, $stmt, $entryScope, $storage);
80-
// Errors signal programmer mistakes (ValueError, TypeError, DivisionByZeroError...),
81-
// nobody calls an otherwise pure expression just to have them thrown, so they
82-
// do not make the expression statement meaningful. A `throw` written in the
83-
// statement is a different story - throwing is the whole point of it.
84-
$errorType = new ObjectType(Error::class);
85-
$throwPoints = array_filter($result->getThrowPoints(), static fn (InternalThrowPoint $throwPoint) => $throwPoint->isExplicit() && ($throwPoint->isFromThrowExpr() || !$errorType->isSuperTypeOf($throwPoint->getType())->yes()));
77+
$throwPoints = array_filter($result->getThrowPoints(), static fn ($throwPoint) => $throwPoint->isExplicit());
8678
if (
8779
count($result->getImpurePoints()) === 0
8880
&& count($throwPoints) === 0

‎src/Analyser/ThrowPoint.php‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,16 @@ private function __construct(
2323
private Node $node,
2424
private bool $explicit,
2525
private bool $canContainAnyThrowable,
26-
private bool $fromThrowExpr = false,
2726
)
2827
{
2928
}
3029

3130
/**
3231
* @param Node\Expr|Node\Stmt $node
3332
*/
34-
public static function createExplicit(Scope $scope, Type $type, Node $node, bool $canContainAnyThrowable, bool $fromThrowExpr = false): self
33+
public static function createExplicit(Scope $scope, Type $type, Node $node, bool $canContainAnyThrowable): self
3534
{
36-
return new self($scope, $type, $node, true, $canContainAnyThrowable, $fromThrowExpr);
35+
return new self($scope, $type, $node, true, $canContainAnyThrowable);
3736
}
3837

3938
/**
@@ -72,19 +71,9 @@ public function canContainAnyThrowable(): bool
7271
return $this->canContainAnyThrowable;
7372
}
7473

75-
/**
76-
* Whether the throw point comes from a `throw` written in the analysed code,
77-
* as opposed to a throw inferred from what a called function or an operation
78-
* can throw.
79-
*/
80-
public function isFromThrowExpr(): bool
81-
{
82-
return $this->fromThrowExpr;
83-
}
84-
8574
public function subtractCatchType(Type $catchType): self
8675
{
87-
return new self($this->scope, TypeCombinator::remove($this->type, $catchType), $this->node, $this->explicit, $this->canContainAnyThrowable, $this->fromThrowExpr);
76+
return new self($this->scope, TypeCombinator::remove($this->type, $catchType), $this->node, $this->explicit, $this->canContainAnyThrowable);
8877
}
8978

9079
}

‎src/Reflection/Callables/SimpleThrowPoint.php‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,13 @@ private function __construct(
2424
private Type $type,
2525
private bool $explicit,
2626
private bool $canContainAnyThrowable,
27-
private bool $fromThrowExpr = false,
2827
)
2928
{
3029
}
3130

32-
public static function createExplicit(Type $type, bool $canContainAnyThrowable, bool $fromThrowExpr = false): self
31+
public static function createExplicit(Type $type, bool $canContainAnyThrowable): self
3332
{
34-
return new self($type, true, $canContainAnyThrowable, $fromThrowExpr);
33+
return new self($type, true, $canContainAnyThrowable);
3534
}
3635

3736
public static function createImplicit(): self
@@ -54,13 +53,4 @@ public function canContainAnyThrowable(): bool
5453
return $this->canContainAnyThrowable;
5554
}
5655

57-
/**
58-
* Whether the throw point comes from a `throw` written in the callable's body,
59-
* as opposed to a throw only declared in PHPDoc.
60-
*/
61-
public function isFromThrowExpr(): bool
62-
{
63-
return $this->fromThrowExpr;
64-
}
65-
6656
}

‎tests/PHPStan/Rules/DeadCode/NoopRuleTest.php‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -143,20 +143,7 @@ public function testRule(): void
143143
#[RequiresPhp('>= 8.0.0')]
144144
public function testErrorThrows(): void
145145
{
146-
$this->analyse([__DIR__ . '/data/noop-error-throws.php'], [
147-
[
148-
'Expression "$a / $b" on a separate line does not do anything.',
149-
6,
150-
],
151-
[
152-
'Expression "$a % $b" on a separate line does not do anything.',
153-
7,
154-
],
155-
[
156-
'Expression "match ($a) {…" on a separate line does not do anything.',
157-
8,
158-
],
159-
]);
146+
$this->analyse([__DIR__ . '/data/noop-error-throws.php'], []);
160147
}
161148

162149
#[RequiresPhp('>= 8.0.0')]

‎tests/PHPStan/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRuleTest.php‎

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -113,30 +113,10 @@ public function testBug11101(): void
113113
]);
114114
}
115115

116+
#[RequiresPhp('>= 8.0.0')]
116117
public function testErrorThrows(): void
117118
{
118-
$this->analyse([__DIR__ . '/data/function-call-statement-no-side-effects-error-throws.php'], [
119-
[
120-
'Call to function sprintf() on a separate line has no effect.',
121-
22,
122-
],
123-
[
124-
'Call to function strpos() on a separate line has no effect.',
125-
23,
126-
],
127-
[
128-
'Call to function intdiv() on a separate line has no effect.',
129-
24,
130-
],
131-
[
132-
'Call to function array_combine() on a separate line has no effect.',
133-
25,
134-
],
135-
[
136-
'Call to function FunctionCallStatementNoSideEffectsErrorThrows\\pureAndThrowsError() on a separate line has no effect.',
137-
27,
138-
],
139-
]);
119+
$this->analyse([__DIR__ . '/data/function-call-statement-no-side-effects-error-throws.php'], []);
140120
}
141121

142122
#[RequiresPhp('>= 8.0.0')]

0 commit comments

Comments
 (0)