Skip to content

Commit b93e3a0

Browse files
SanderMullerclaude
SanderMuller
and
claude
committed
Report unset() of an offset on a type that only maybe has offsets
unset($x['k']) deprecates when $x is false and throws an Error when $x is true, an int or a float. UnsetRule reported this only when the offset can never be accessed, so array|false and array|int went unreported. Report the maybe case as well, with reportMaybes, the way NonexistentOffsetInArrayDimFetchRule reports offset reads. A type whose offset access is not even legal (an object without ArrayAccess) is left to NonexistentOffsetInArrayDimFetchRule, which already reports it inside unset(). The type goes through RuleLevelHelper::findTypeToCheck(), which keeps mixed out of this check. A string is offset-accessible for reads, so array|string is still not reported. Gated behind the unsetOffsetOnMaybeAccessible bleeding edge toggle. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 184d34e commit b93e3a0

6 files changed

Lines changed: 200 additions & 1 deletion

File tree

‎conf/bleedingEdge.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,4 @@ parameters:
3232
staticVariablesFromUsages: true
3333
unusedVariable: true
3434
unusedParameters: true
35+
unsetOffsetOnMaybeAccessible: true

‎conf/config.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ parameters:
6464
staticVariablesFromUsages: false
6565
unusedVariable: false
6666
unusedParameters: false
67+
unsetOffsetOnMaybeAccessible: false
6768
fileExtensions:
6869
- php
6970
checkAdvancedIsset: false

‎conf/parametersSchema.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ parametersSchema:
6161
staticVariablesFromUsages: bool()
6262
unusedVariable: bool()
6363
unusedParameters: bool()
64+
unsetOffsetOnMaybeAccessible: bool()
6465
])
6566
fileExtensions: listOf(string())
6667
checkAdvancedIsset: bool()

‎src/Rules/Variables/UnsetRule.php‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,16 @@
44

55
use PhpParser\Node;
66
use PHPStan\Analyser\Scope;
7+
use PHPStan\DependencyInjection\AutowiredParameter;
78
use PHPStan\DependencyInjection\RegisteredRule;
89
use PHPStan\Php\PhpVersion;
910
use PHPStan\Rules\IdentifierRuleError;
1011
use PHPStan\Rules\Properties\PropertyReflectionFinder;
1112
use PHPStan\Rules\Rule;
1213
use PHPStan\Rules\RuleErrorBuilder;
14+
use PHPStan\Rules\RuleLevelHelper;
15+
use PHPStan\Type\ErrorType;
16+
use PHPStan\Type\Type;
1317
use PHPStan\Type\VerbosityLevel;
1418
use function is_string;
1519
use function sprintf;
@@ -24,6 +28,11 @@ final class UnsetRule implements Rule
2428
public function __construct(
2529
private PropertyReflectionFinder $propertyReflectionFinder,
2630
private PhpVersion $phpVersion,
31+
private RuleLevelHelper $ruleLevelHelper,
32+
#[AutowiredParameter]
33+
private bool $reportMaybes,
34+
#[AutowiredParameter(ref: '%featureToggles.unsetOffsetOnMaybeAccessible%')]
35+
private bool $unsetOffsetOnMaybeAccessible,
2736
)
2837
{
2938
}
@@ -127,7 +136,11 @@ private function canBeUnset(Node $node, Scope $scope): ?IdentifierRuleError
127136
$type = $scope->getType($node->var);
128137
$dimType = $scope->getType($node->dim);
129138

130-
if ($type->isOffsetAccessible()->no() || $type->hasOffsetValueType($dimType)->no()) {
139+
if (
140+
$type->isOffsetAccessible()->no()
141+
|| $type->hasOffsetValueType($dimType)->no()
142+
|| $this->isOffsetMaybeNotUnsettable($scope, $node->var)
143+
) {
131144
return RuleErrorBuilder::message(
132145
sprintf(
133146
'Cannot unset offset %s on %s.',
@@ -146,4 +159,27 @@ private function canBeUnset(Node $node, Scope $scope): ?IdentifierRuleError
146159
return null;
147160
}
148161

162+
/**
163+
* A union like array|false or array|int: unset() of an offset deprecates (false) or throws on the other members.
164+
* Types whose offset access is not even legal (an object without ArrayAccess) are reported by NonexistentOffsetInArrayDimFetchRule.
165+
*/
166+
private function isOffsetMaybeNotUnsettable(Scope $scope, Node\Expr $var): bool
167+
{
168+
if (!$this->unsetOffsetOnMaybeAccessible || !$this->reportMaybes) {
169+
return false;
170+
}
171+
172+
$type = $this->ruleLevelHelper->findTypeToCheck(
173+
$scope,
174+
$var,
175+
'',
176+
static fn (Type $type): bool => $type->isOffsetAccessible()->yes(),
177+
)->getType();
178+
if ($type instanceof ErrorType) {
179+
return false;
180+
}
181+
182+
return $type->isOffsetAccessible()->maybe() && $type->isOffsetAccessLegal()->yes();
183+
}
184+
149185
}

‎tests/PHPStan/Rules/Variables/UnsetRuleTest.php‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use PHPStan\Php\PhpVersion;
66
use PHPStan\Rules\Properties\PropertyReflectionFinder;
77
use PHPStan\Rules\Rule;
8+
use PHPStan\Rules\RuleLevelHelper;
89
use PHPStan\Testing\RuleTestCase;
910
use PHPUnit\Framework\Attributes\RequiresPhp;
1011
use function array_merge;
@@ -16,12 +17,19 @@
1617
class UnsetRuleTest extends RuleTestCase
1718
{
1819

20+
private bool $reportMaybes = false;
21+
22+
private bool $unsetOffsetOnMaybeAccessible = false;
23+
1924
protected function getRule(): Rule
2025
{
2126
$container = self::getContainer();
2227
return new UnsetRule(
2328
$container->getByType(PropertyReflectionFinder::class),
2429
$container->getByType(PhpVersion::class),
30+
$container->getByType(RuleLevelHelper::class),
31+
$this->reportMaybes,
32+
$this->unsetOffsetOnMaybeAccessible,
2533
);
2634
}
2735

@@ -229,4 +237,52 @@ public function testNullCoalesceAssignRightSideScope(): void
229237
$this->analyse([__DIR__ . '/data/null-coalesce-assign-right-side-scope.php'], []);
230238
}
231239

240+
public function testUnsetOffsetOnMaybeAccessible(): void
241+
{
242+
$this->reportMaybes = true;
243+
$this->unsetOffsetOnMaybeAccessible = true;
244+
$this->analyse([__DIR__ . '/data/unset-offset-maybe-accessible.php'], [
245+
[
246+
'Cannot unset offset \'port\' on array{port?: int, path?: string}|false.',
247+
13,
248+
],
249+
[
250+
'Cannot unset offset \'port\' on array<string, int>|int.',
251+
21,
252+
],
253+
[
254+
'Cannot unset offset \'port\' on array{scheme?: string, host?: string, port?: int<0, 65535>, user?: string, pass?: string, path?: string, query?: string, fragment?: string}|false.',
255+
56,
256+
],
257+
[
258+
'Cannot unset offset \'port\' on array<string, int>|bool.',
259+
91,
260+
],
261+
[
262+
'Cannot unset offset \'port\' on array<string, int>|true.',
263+
92,
264+
],
265+
[
266+
'Cannot unset offset \'port\' on array<string, int>|float.',
267+
93,
268+
],
269+
[
270+
'Cannot unset offset \'port\' on array<string, int>|false.',
271+
101,
272+
],
273+
]);
274+
}
275+
276+
public function testUnsetOffsetOnMaybeAccessibleWithoutReportMaybes(): void
277+
{
278+
$this->unsetOffsetOnMaybeAccessible = true;
279+
$this->analyse([__DIR__ . '/data/unset-offset-maybe-accessible.php'], []);
280+
}
281+
282+
public function testUnsetOffsetOnMaybeAccessibleWithoutFeatureToggle(): void
283+
{
284+
$this->reportMaybes = true;
285+
$this->analyse([__DIR__ . '/data/unset-offset-maybe-accessible.php'], []);
286+
}
287+
232288
}

‎tests/PHPStan/Rules/Variables/data/unset-offset-maybe-accessible.php‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
<?php // lint >= 8.0
2+
3+
namespace UnsetOffsetMaybeAccessible;
4+
5+
class Foo
6+
{
7+
8+
/**
9+
* @param array{port?: int, path?: string}|false $parsed
10+
*/
11+
public function arrayOrFalse($parsed): void
12+
{
13+
unset($parsed['port']);
14+
}
15+
16+
/**
17+
* @param array<string, int>|int $value
18+
*/
19+
public function arrayOrInt($value): void
20+
{
21+
unset($value['port']);
22+
}
23+
24+
/**
25+
* @param array<string, int>|null $value
26+
*/
27+
public function arrayOrNull($value): void
28+
{
29+
unset($value['port']);
30+
}
31+
32+
/**
33+
* @param array<string, int>|\ArrayAccess<string, int> $value
34+
*/
35+
public function arrayOrArrayAccess($value): void
36+
{
37+
unset($value['port']);
38+
}
39+
40+
/**
41+
* @param array<string, int> $value
42+
*/
43+
public function array($value): void
44+
{
45+
unset($value['port']);
46+
}
47+
48+
public function object(\stdClass $value): void
49+
{
50+
unset($value['port']);
51+
}
52+
53+
public function parseUrl(string $url): void
54+
{
55+
$parsed = parse_url($url);
56+
unset($parsed['port']);
57+
}
58+
59+
public function explicitMixed(mixed $value): void
60+
{
61+
unset($value['port']);
62+
}
63+
64+
public function implicitMixed($value): void
65+
{
66+
unset($value['port']);
67+
}
68+
69+
public function mixedWithoutFalse(mixed $value): void
70+
{
71+
if ($value !== false) {
72+
unset($value['port']);
73+
}
74+
}
75+
76+
public function jsonDecodedAfterIsset(string $json): void
77+
{
78+
$body = json_decode($json, true);
79+
if (isset($body['ttl'])) {
80+
unset($body['ttl']);
81+
}
82+
}
83+
84+
/**
85+
* @param array<string, int>|bool $bool
86+
* @param array<string, int>|true $true
87+
* @param array<string, int>|float $float
88+
*/
89+
public function scalars($bool, $true, $float): void
90+
{
91+
unset($bool['port']);
92+
unset($true['port']);
93+
unset($float['port']);
94+
}
95+
96+
/**
97+
* @param array{x: array<string, int>|false} $value
98+
*/
99+
public function nested(array $value): void
100+
{
101+
unset($value['x']['port']);
102+
}
103+
104+
}

0 commit comments

Comments
 (0)