Skip to content

Commit d8e66db

Browse files
committed
Merge branch 2.1.x into 2.2.x
2 parents d3e43d4 + 5e793f7 commit d8e66db

9 files changed

Lines changed: 222 additions & 46 deletions

File tree

CLAUDE.md

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,11 +240,36 @@ Historical analysis of `Type.php` via `git blame` shows that new methods are add
240240

241241
When considering a bug fix that involves checking "is this type a Foo?", first check whether an appropriate method already exists on `Type`. If not, consider whether adding one would be the right fix — especially if the check is needed in more than one place or involves logic that varies by type class.
242242

243+
### Type system: never sort, compare, or transform types via `describe()`
244+
245+
`describe()` is for human-readable error messages, not for ordering or matching. To sort the keys of a `ConstantArrayType`, sort the underlying key `Type` objects (introduce a new method on `ConstantArrayType` if needed) — never `usort()` on `describe()` output. To compare types, use `equals()`, `isSuperTypeOf()`, or `accepts()`, not `describe() === describe()`. String-manipulating type descriptions is a recurring red flag.
246+
247+
### Performance fixes: solve the algorithm, not the limit
248+
249+
When PHPStan is slow on a case, do not "fix" it by lowering or introducing a hard cap on iterations, recursion depth, fan-out, or array size. Find the algorithmic root cause first. It is fine to *temporarily* raise an existing limit to make the slowness visible during investigation — but restore the original limit once the algorithmic fix lands. Limits are last-resort safety nets, not fixes. Dumbing down analysis to make a benchmark pass is not acceptable.
250+
251+
### Don't refactor code under analysis — make PHPStan smarter
252+
253+
When `make phpstan` flags PHPStan's own source, or when an issue's playground sample reproduces a bug, the default fix is to teach inference to handle the original code, not to rewrite the code. The same applies to regression test data: copy the reproducer from the issue's playground link as close to verbatim as practical, and once the test fails for the right reason, do not simplify or rephrase it.
254+
255+
### After fixing one bug, look for the same bug in adjacent code
256+
257+
Bugs in PHPStan's type system rarely live alone. A fix in one accessory type usually has counterparts in the other accessory types. A fix in property handling often has analogues in methods and class constants. A fix at one call site of a `Type` API often repeats at other call sites. After landing a fix, actively scan the parallel code paths (sibling `*Type` classes, the same trait, the same dispatch table, all callers of the changed API) and try to reproduce and fix the pattern there too — each with its own failing test.
258+
259+
### Magic numbers belong in named `_LIMIT` constants
260+
261+
Every threshold, fan-out cap, recursion bound, or collection-size limit must be a class constant whose name ends in `_LIMIT`, declared at the top of the owning class. Inline numerics in conditionals (`if (count($this->optionalKeys) <= 10)`) are not acceptable — extract to `self::OPTIONAL_KEYS_LIMIT` (or similar) so the threshold is named, discoverable, and tunable in one place.
262+
243263
### Testing patterns
244264

245265
- **Rule tests**: Extend `RuleTestCase`, implement `getRule()`, call `$this->analyse([__DIR__ . '/data/my-test.php'], [...expected errors...])`. Expected errors are `[message, line]` pairs. Test data files live in `tests/PHPStan/Rules/*/data/`.
246266
- **Type inference tests**: Use `assertType()` and `assertNativeType()` helper functions in test data files. The test runner verifies PHPStan infers the declared types at each `assertType()` call.
247-
- **Regression tests**: For each bug fix, add a test data file reproducing the issue (e.g. `tests/PHPStan/Rules/*/data/bug-12345.php` or `tests/PHPStan/Analyser/nsrt/bug-12345.php`).
267+
- **Choosing the test home for a regression**: pick by what the bug is about.
268+
- *Wrong inferred type / missing narrowing*`tests/PHPStan/Analyser/nsrt/bug-<n>.php` with `assertType()` calls.
269+
- *Rule reports a wrong/missing error*`tests/PHPStan/Rules/<Category>/data/bug-<n>.php` plus expectations in the rule's test class.
270+
- If the bug touches both, add both.
271+
- **Always verify the test fails before the fix.** Stating "tests pass" is not enough — that only proves the fix doesn't break things. `git stash` the source change, run the test, see it fail for the right reason; `git stash pop`, run again, see it pass. Only then is the fix confirmed. The `regression-test` skill bakes this in for bug-issue workflows.
272+
- **Reproducers come from the OP's playground link** in the issue body, not from later comments. Copy the snippet as close to verbatim as practical.
248273
- **Integration tests**: `AnalyserIntegrationTest` runs full analysis on test files and checks error output.
249274

250275
### Adding support for new PHP versions
@@ -259,6 +284,14 @@ Recent work on PHP 8.5 support shows the pattern:
259284
- **PhpVersion**: Add detection methods like `supportsPropertyHooks()`, `supportsPipeOperator()`, etc.
260285
- **Stubs**: Update function/class stubs for new built-in functions and changed signatures
261286

287+
## Workflow for bug fixes
288+
289+
- **One fix, one commit (or one PR)**. Bench scripts and tests for a fix go in the same commit as the source change, not separately.
290+
- **Commit messages and PR titles describe the change, not the bug they close.** Prefer "Do not subtract TemplateType from TemplateType" over "Fix #14459: type subtraction". Issue closure goes in the PR body via `Closes https://github.com/phpstan/phpstan/issues/<n>`.
291+
- **Standard reproducer command**: `bin/phpstan analyse -l 8 <file> --debug` (add `-vvv` for hangs and infinite loops). Files under `tests/bench/data` are not directly runnable as PHPStan inputs — copy the reproducing snippet into a `test.php` at the repo root first, then analyse that.
292+
- **Debug helpers in analysed code**: `\PHPStan\dumpType($expr)` prints the inferred type of an expression at that point; `\PHPStan\debugScope()` prints the current scope. Inside `NodeScopeResolver` itself, `var_dump($scope->debug())` is the canonical inspection point. Reach for these before guessing what PHPStan is doing.
293+
- **When the user says "I don't understand why X is needed"** about existing code, treat it as a request to investigate whether X is actually needed — not a request to defend the status quo. The answer is often that two paths can be unified or one block deleted.
294+
262295
## Writing PHPDocs
263296

264297
When adding or editing PHPDoc comments in this codebase, follow these guidelines:

src/Type/Constant/ConstantArrayType.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ class ConstantArrayType implements Type
101101
* @api
102102
* @param list<ConstantIntegerType|ConstantStringType> $keyTypes
103103
* @param array<int, Type> $valueTypes
104-
* @param non-empty-list<int> $nextAutoIndexes
104+
* @param list<int> $nextAutoIndexes
105105
* @param int[] $optionalKeys
106106
*/
107107
public function __construct(
@@ -128,7 +128,7 @@ public function __construct(
128128
/**
129129
* @param list<ConstantIntegerType|ConstantStringType> $keyTypes
130130
* @param array<int, Type> $valueTypes
131-
* @param non-empty-list<int> $nextAutoIndexes
131+
* @param list<int> $nextAutoIndexes
132132
* @param int[] $optionalKeys
133133
*/
134134
protected function recreate(
@@ -208,7 +208,7 @@ public function isConstantValue(): TrinaryLogic
208208
}
209209

210210
/**
211-
* @return non-empty-list<int>
211+
* @return list<int>
212212
*/
213213
public function getNextAutoIndexes(): array
214214
{
@@ -745,6 +745,10 @@ public function getOffsetValueType(Type $offsetType): Type
745745

746746
public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $unionValues = true): Type
747747
{
748+
if ($offsetType === null && count($this->nextAutoIndexes) === 0) {
749+
return new ErrorType();
750+
}
751+
748752
$builder = ConstantArrayTypeBuilder::createFromConstantArray($this);
749753
$builder->setOffsetValueType($offsetType, $valueType);
750754

src/Type/Constant/ConstantArrayTypeBuilder.php

Lines changed: 45 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ final class ConstantArrayTypeBuilder
4444
/**
4545
* @param list<Type> $keyTypes
4646
* @param array<int, Type> $valueTypes
47-
* @param non-empty-list<int> $nextAutoIndexes
47+
* @param list<int> $nextAutoIndexes
4848
* @param array<int> $optionalKeys
4949
*/
5050
private function __construct(
@@ -85,6 +85,10 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $opt
8585
$offsetType = $offsetType->toArrayKey();
8686
}
8787

88+
if ($offsetType === null && count($this->nextAutoIndexes) === 0) {
89+
return;
90+
}
91+
8892
if (!$this->degradeToGeneralArray) {
8993
if (
9094
$valueType instanceof ClosureType
@@ -108,6 +112,10 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $opt
108112
}
109113

110114
if ($offsetType === null) {
115+
if (count($this->nextAutoIndexes) === 0) {
116+
return;
117+
}
118+
111119
$newAutoIndexes = $optional ? $this->nextAutoIndexes : [];
112120
$hasOptional = false;
113121
foreach ($this->keyTypes as $i => $keyType) {
@@ -127,11 +135,10 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $opt
127135

128136
/** @var int|float $newAutoIndex */
129137
$newAutoIndex = $keyType->getValue() + 1;
130-
if (is_float($newAutoIndex)) {
131-
$newAutoIndex = $keyType->getValue();
138+
if (!is_float($newAutoIndex)) {
139+
$newAutoIndexes[] = $newAutoIndex;
132140
}
133141

134-
$newAutoIndexes[] = $newAutoIndex;
135142
$hasOptional = true;
136143
}
137144

@@ -142,11 +149,10 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $opt
142149

143150
/** @var int|float $newAutoIndex */
144151
$newAutoIndex = $max + 1;
145-
if (is_float($newAutoIndex)) {
146-
$newAutoIndex = $max;
152+
if (!is_float($newAutoIndex)) {
153+
$newAutoIndexes[] = $newAutoIndex;
147154
}
148155

149-
$newAutoIndexes[] = $newAutoIndex;
150156
$this->nextAutoIndexes = array_values(array_unique($newAutoIndexes));
151157

152158
if ($optional || $hasOptional) {
@@ -180,11 +186,7 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $opt
180186
if (!$optional) {
181187
$this->optionalKeys = array_values(array_filter($this->optionalKeys, static fn (int $index): bool => $index !== $i));
182188
if ($keyType instanceof ConstantIntegerType) {
183-
$nextAutoIndexes = array_values(array_filter($this->nextAutoIndexes, static fn (int $index) => $index > $keyType->getValue()));
184-
if (count($nextAutoIndexes) === 0) {
185-
throw new ShouldNotHappenException();
186-
}
187-
$this->nextAutoIndexes = $nextAutoIndexes;
189+
$this->nextAutoIndexes = array_values(array_filter($this->nextAutoIndexes, static fn (int $index) => $index > $keyType->getValue()));
188190
}
189191
}
190192
return;
@@ -194,33 +196,38 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $opt
194196
$this->valueTypes[] = $valueType;
195197

196198
if ($offsetType instanceof ConstantIntegerType) {
197-
$min = min($this->nextAutoIndexes);
198-
$max = max($this->nextAutoIndexes);
199-
$offsetValue = $offsetType->getValue();
200-
if ($offsetValue >= 0) {
201-
if ($offsetValue > $min) {
202-
if ($offsetValue <= $max) {
203-
$this->isList = $this->isList->and(TrinaryLogic::createMaybe());
199+
if (count($this->nextAutoIndexes) > 0) {
200+
$min = min($this->nextAutoIndexes);
201+
$max = max($this->nextAutoIndexes);
202+
$offsetValue = $offsetType->getValue();
203+
if ($offsetValue >= 0) {
204+
if ($offsetValue > $min) {
205+
if ($offsetValue <= $max) {
206+
$this->isList = $this->isList->and(TrinaryLogic::createMaybe());
207+
} else {
208+
$this->isList = TrinaryLogic::createNo();
209+
}
210+
}
211+
} else {
212+
$this->isList = TrinaryLogic::createNo();
213+
}
214+
215+
if ($offsetValue >= $max) {
216+
/** @var int|float $newAutoIndex */
217+
$newAutoIndex = $offsetValue + 1;
218+
if (is_float($newAutoIndex)) {
219+
if (!$optional) {
220+
$this->nextAutoIndexes = [];
221+
}
222+
} elseif (!$optional) {
223+
$this->nextAutoIndexes = [$newAutoIndex];
204224
} else {
205-
$this->isList = TrinaryLogic::createNo();
225+
$this->nextAutoIndexes[] = $newAutoIndex;
206226
}
207227
}
208228
} else {
209229
$this->isList = TrinaryLogic::createNo();
210230
}
211-
212-
if ($offsetValue >= $max) {
213-
/** @var int|float $newAutoIndex */
214-
$newAutoIndex = $offsetValue + 1;
215-
if (is_float($newAutoIndex)) {
216-
$newAutoIndex = $max;
217-
}
218-
if (!$optional) {
219-
$this->nextAutoIndexes = [$newAutoIndex];
220-
} else {
221-
$this->nextAutoIndexes[] = $newAutoIndex;
222-
}
223-
}
224231
} else {
225232
$this->isList = TrinaryLogic::createNo();
226233
}
@@ -300,6 +307,10 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $opt
300307
continue;
301308
}
302309

310+
if (count($this->nextAutoIndexes) === 0) {
311+
continue;
312+
}
313+
303314
$max = max($this->nextAutoIndexes);
304315
$offsetValue = $scalarType->getValue();
305316
if ($offsetValue < $max) {
@@ -309,7 +320,7 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $opt
309320
/** @var int|float $newAutoIndex */
310321
$newAutoIndex = $offsetValue + 1;
311322
if (is_float($newAutoIndex)) {
312-
$newAutoIndex = $max;
323+
continue;
313324
}
314325
$this->nextAutoIndexes[] = $newAutoIndex;
315326
}

src/Type/Generic/TemplateConstantArrayType.php

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

55
use PHPStan\TrinaryLogic;
66
use PHPStan\Type\Constant\ConstantArrayType;
7-
use PHPStan\Type\Constant\ConstantIntegerType;
8-
use PHPStan\Type\Constant\ConstantStringType;
97
use PHPStan\Type\Traits\UndecidedComparisonCompoundTypeTrait;
108
use PHPStan\Type\Type;
119

@@ -38,12 +36,6 @@ public function __construct(
3836
$this->default = $default;
3937
}
4038

41-
/**
42-
* @param list<ConstantIntegerType|ConstantStringType> $keyTypes
43-
* @param array<int, Type> $valueTypes
44-
* @param non-empty-list<int> $nextAutoIndexes
45-
* @param int[] $optionalKeys
46-
*/
4739
protected function recreate(
4840
array $keyTypes,
4941
array $valueTypes,

tests/PHPStan/Analyser/AnalyserIntegrationTest.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,12 @@ public function testExtendsPdoStatementCrash(): void
164164
$this->assertNoErrors($errors);
165165
}
166166

167+
public function testBug14548(): void
168+
{
169+
$errors = $this->runAnalyse(__DIR__ . '/data/bug-14548.php');
170+
$this->assertNoErrors($errors);
171+
}
172+
167173
public function testBug12803(): void
168174
{
169175
// crash
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php
2+
3+
namespace Bug14548;
4+
5+
class TProcessHelper
6+
{
7+
8+
public static function setProcessPriority(int $priority): void
9+
{
10+
$priorityValues = [ // The priority cap to windows text priority.
11+
-15 => 24,
12+
-10 => 13,
13+
-5 => 10,
14+
4 => 8,
15+
9 => 6,
16+
PHP_INT_MAX => 4,
17+
];
18+
foreach ($priorityValues as $keyPriority => $priorityName) {
19+
if ($priority <= $keyPriority) {
20+
break;
21+
}
22+
}
23+
}
24+
}

tests/PHPStan/Rules/Arrays/OffsetAccessAssignmentRuleTest.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,4 +207,19 @@ public function testBug8648(): void
207207
$this->analyse([__DIR__ . '/data/bug-8648.php'], []);
208208
}
209209

210+
public function testAppendToArrayWithPhpIntMaxKey(): void
211+
{
212+
$this->checkUnionTypes = true;
213+
$this->analyse([__DIR__ . '/data/offset-access-assignment-php-int-max.php'], [
214+
[
215+
'Cannot assign new offset to array<int, int>.',
216+
9,
217+
],
218+
[
219+
'Cannot assign new offset to array<int, int>.',
220+
15,
221+
],
222+
]);
223+
}
224+
210225
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace OffsetAccessAssignmentPhpIntMax;
4+
5+
function (): void {
6+
$a = [
7+
9223372036854775807 => 4,
8+
];
9+
$a[] = 5;
10+
};
11+
12+
function (): void {
13+
$a = [];
14+
$a[9223372036854775807] = 4;
15+
$a[] = 5;
16+
};
17+
18+
function (): void {
19+
$a = [
20+
9223372036854775807 => 4,
21+
];
22+
$a[10] = 5;
23+
};

0 commit comments

Comments
 (0)