Skip to content

Commit 698b9aa

Browse files
committed
More CLAUDE.md
1 parent d81f413 commit 698b9aa

1 file changed

Lines changed: 89 additions & 0 deletions

File tree

CLAUDE.md

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,95 @@ PHPStan is highly extensible. Key extension interfaces:
202202
- **Allowed subtypes** - define sealed class hierarchies
203203
- **Always-read/written properties, always-used constants/methods** - suppress false positives for dead code detection
204204

205+
## Common bug fix patterns and development guidance
206+
207+
Based on analysis of recent releases (2.1.30-2.1.38), these are the recurring patterns for how bugs are found and fixed:
208+
209+
### Type system: never use `instanceof` to check types
210+
211+
A recurring cleanup theme: never use `$type instanceof StringType` or similar. This misses union types, intersection types with accessory types, and other composite forms. Always use `$type->isString()->yes()` or `(new StringType())->isSuperTypeOf($type)`. Multiple PRs have systematically replaced `instanceof *Type` checks throughout the codebase.
212+
213+
### MutatingScope: expression invalidation during scope merging
214+
215+
When two scopes are merged (e.g. after if/else branches), `MutatingScope::generalizeWith()` must invalidate dependent expressions. If variable `$i` changes, then `$locations[$i]` must be invalidated too. Bugs arise when stale `ExpressionTypeHolder` entries survive scope merges. Fix pattern: in `MutatingScope`, when a root expression changes, skip/invalidate all deep expressions that depend on it.
216+
217+
### Array type tracking: SetExistingOffsetValueTypeExpr vs SetOffsetValueTypeExpr
218+
219+
When assigning to an array offset, NodeScopeResolver must distinguish:
220+
- `SetExistingOffsetValueTypeExpr` - modifying a key known to exist (preserves list type, doesn't widen the array)
221+
- `SetOffsetValueTypeExpr` - adding a potentially new key (may break list type, widens the array)
222+
223+
Misusing these leads to false positives like "might not be a list" or incorrect offset-exists checks. The fix is in `NodeScopeResolver` where property/variable assignments are processed.
224+
225+
### ConstantArrayType and offset tracking
226+
227+
Many bugs involve `ConstantArrayType` (array shapes with known keys). Common issues:
228+
- `hasOffsetValueType()` returning wrong results for expression-based offsets
229+
- Offset types not being unioned with empty array when the offset might not exist
230+
- `array_key_exists()` not properly narrowing to `non-empty-array`
231+
- `OversizedArrayType` (array shapes that grew too large to track precisely) needing correct `isSuperTypeOf()` and truthiness behavior
232+
233+
Fixes typically involve `ConstantArrayType`, `TypeSpecifier` (for narrowing after `array_key_exists`/`isset`), and `MutatingScope` (for tracking assignments).
234+
235+
### Loop analysis: foreach, for, while
236+
237+
Loops are a frequent source of false positives because PHPStan must reason about types across iterations:
238+
- **List type preservation in for loops**: When appending to a list inside a `for` loop, the list type must be preserved if operations maintain sequential integer keys.
239+
- **Always-overwritten arrays in foreach**: NodeScopeResolver examines `$a[$k]` at loop body end and `continue` statements. If no `break` exists, the entire array type can be rewritten based on the observed value types.
240+
- **Variable types across iterations**: PHP Fibers are used (PHP 8.1+) for more precise analysis of repeated variable assignments in loops, by running the loop body analysis multiple times to reach a fixpoint.
241+
242+
### PHPDoc inheritance
243+
244+
PHPDoc types (`@return`, `@param`, `@throws`, `@property`) are inherited through class hierarchies. Bugs arise when:
245+
- Child methods with narrower native return types don't inherit parent PHPDoc return types
246+
- `@property` tags on parent classes don't consider native property types on children
247+
- Trait PHPDoc resolution uses wrong file context
248+
249+
The `PhpDocInheritanceResolver` and `PhpDocBlock` classes handle this. Recent optimization: resolve through reflection instead of re-walking the hierarchy manually.
250+
251+
### Dynamic return type extensions for built-in functions
252+
253+
Many built-in PHP functions need `DynamicFunctionReturnTypeExtension` implementations because their return types depend on arguments:
254+
- `array_rand()`, `array_count_values()`, `array_first()`/`array_last()`, `filter_var()`, `curl_setopt()`, DOM methods, etc.
255+
- Extensions live in `src/Type/Php/` and are registered in `conf/services.neon`
256+
- Each reads the argument types from `Scope::getType()` and returns a more precise `Type`
257+
258+
### Function signature corrections (`src/Reflection/SignatureMap/`)
259+
260+
PHPStan maintains its own signature map for built-in PHP functions in `functionMap.php` and delta files. Fixes involve:
261+
- Correcting return types (e.g. `DOMNode::C14N` returning `string|false`)
262+
- Adding `@param-out` for reference parameters (e.g. `stream_socket_client`)
263+
- Marking functions as impure (e.g. `time()`, Redis methods)
264+
- PHP-version-specific signatures (e.g. `bcround` only in PHP 8.4+)
265+
266+
### Impure points and side effects
267+
268+
PHPStan tracks whether expressions/statements have side effects ("impure points"). This enables:
269+
- Reporting useless calls to pure methods (`expr.resultUnused`)
270+
- Detecting void methods with no side effects
271+
- `@phpstan-pure` enforcement
272+
273+
Bugs occur when impure points are missed (e.g. inherited constructors of anonymous classes) or when `clearstatcache()` calls don't invalidate filesystem function return types.
274+
275+
### Testing patterns
276+
277+
- **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/`.
278+
- **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.
279+
- **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`).
280+
- **Integration tests**: `AnalyserIntegrationTest` runs full analysis on test files and checks error output.
281+
282+
### Adding support for new PHP versions
283+
284+
Recent work on PHP 8.5 support shows the pattern:
285+
- **Parser support**: Update nikic/php-parser dependency, handle new AST node types
286+
- **NodeScopeResolver**: Handle new syntax (pipe operator, clone-with, void cast)
287+
- **Type system**: New type representations if needed
288+
- **Rules**: Version-gated rules (e.g. deprecated casts only reported on PHP 8.5+, `#[NoDiscard]` only on PHP 8.5+)
289+
- **InitializerExprTypeResolver**: Support new constant expression forms (casts, first-class callables, static closures in initializers)
290+
- **Reflection**: Support new attributes, property features (asymmetric visibility on static properties, `#[Override]` on properties)
291+
- **PhpVersion**: Add detection methods like `supportsPropertyHooks()`, `supportsPipeOperator()`, etc.
292+
- **Stubs**: Update function/class stubs for new built-in functions and changed signatures
293+
205294
## Important dependencies
206295

207296
- `nikic/php-parser` ^5.7.0 - PHP AST parsing

0 commit comments

Comments
 (0)