Cleanup CLAUDE.md

Author: ondrejmirtes

.github/workflows/claude-fix-issue.yml

@@ -123,7 +123,6 @@ jobs:
           4. Fix any failures that come up
           5. Run make cs-fix to fix any coding standard violations
           6. Run make name-collision and fix violations - add different tests in unique namespaces. If the function and class declarations are exactly the same, you can reuse them across files instead of duplicating them.
-          7. Update CLAUDE.md with a few concise points about what new you learnt about the codebase
 
           Do not create a branch, push, or create a PR - this will be handled automatically.
 

CLAUDE.md

@@ -240,122 +240,6 @@ Historical analysis of `Type.php` via `git blame` shows that new methods are add
 
 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.
 
-### Arrow function vs closure parameter handling parity
-
-`MutatingScope` has separate methods for entering arrow functions (`enterArrowFunctionWithoutReflection`) and closures (`enterAnonymousFunctionWithoutReflection`). Both iterate over parameters and assign types, but they must use the same logic for computing parameter types. In particular, both must call `getFunctionType($parameter->type, $isNullable, $parameter->variadic)` to properly handle variadic parameters (wrapping the inner type in `ArrayType`). Shortcuts like `new MixedType()` for untyped parameters skip the variadic wrapping and cause variadic args to be typed as `mixed` instead of `array<int|string, mixed>`. When fixing bugs in one method, check the other for the same issue.
-
-### MutatingScope: expression invalidation during scope merging
-
-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.
-
-### MutatingScope: expression invalidation after method calls and private property visibility
-
-When a method with side effects is called, `invalidateExpression()` invalidates tracked expression types that depend on the call target. When `$this` is invalidated, `shouldInvalidateExpression()` also matches `self`, `static`, `parent`, and class name references — so `self::$prop`, `$this->prop`, etc. all get invalidated. However, private properties of the current class cannot be modified by methods declared in a different class (parent/other). The `invalidatingClass` parameter on `invalidateExpression()` and `shouldInvalidateExpression()` enables skipping invalidation for private properties whose declaring class differs from the method's declaring class. This is checked via `isPrivatePropertyOfDifferentClass()`. The pattern mirrors the existing readonly property protection (`isReadonlyPropertyFetch`). Both `NodeScopeResolver` call sites (instance method calls at ~line 3188, static method calls at ~line 3398) pass `$methodReflection->getDeclaringClass()` as the invalidating class.
-
-### Closure::bind() scope leaking into argument evaluation
-
-`NodeScopeResolver::processArgs()` has special handling for `Closure::bind()` and `Closure::bindTo()` calls. When the first argument is a closure/arrow function literal, a `$closureBindScope` is created with `$this` rebound to the second argument's type, and this scope is used to process the closure body. However, this `$closureBindScope` must ONLY be applied when the first argument is actually an `Expr\Closure` or `Expr\ArrowFunction`. If the first argument is a general expression that returns a closure (e.g. `$this->hydrate()`), the expression itself must be evaluated in the original scope — otherwise `$this` in the expression gets incorrectly resolved as the bound object type instead of the current class. The condition at the `$scopeToPass` assignment must check the argument node type.
-
-### Array type tracking: SetExistingOffsetValueTypeExpr vs SetOffsetValueTypeExpr
-
-When assigning to an array offset, NodeScopeResolver must distinguish:
-- `SetExistingOffsetValueTypeExpr` - modifying a key known to exist (preserves list type, doesn't widen the array)
-- `SetOffsetValueTypeExpr` - adding a potentially new key (may break list type, widens the array)
-
-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.
-
-This distinction also applies in `MutatingScope::enterForeach()`. When a foreach loop iterates by reference (`foreach ($list as &$value)`), modifying `$value` changes an existing offset, not a new one. The `IntertwinedVariableByReferenceWithExpr` created for the no-key by-reference case must use `SetExistingOffsetValueTypeExpr` (not `SetOffsetValueTypeExpr`) so that `AccessoryArrayListType::setExistingOffsetValueType()` preserves the list type. Using `SetOffsetValueTypeExpr` causes `AccessoryArrayListType::setOffsetValueType()` to return `ErrorType` for non-null/non-zero offsets, destroying the list type in the intersection.
-
-### ConstantArrayType and offset tracking
-
-Many bugs involve `ConstantArrayType` (array shapes with known keys). Common issues:
-- `hasOffsetValueType()` returning wrong results for expression-based offsets
-- Offset types not being unioned with empty array when the offset might not exist
-- `array_key_exists()` not properly narrowing to `non-empty-array`
-- `OversizedArrayType` (array shapes that grew too large to track precisely) needing correct `isSuperTypeOf()` and truthiness behavior
-
-Fixes typically involve `ConstantArrayType`, `TypeSpecifier` (for narrowing after `array_key_exists`/`isset`), and `MutatingScope` (for tracking assignments).
-
-### Array literal spread operator and ConstantArrayTypeBuilder degradation
-
-`InitializerExprTypeResolver::getArrayType()` computes the type of array literals like `[...$a, ...$b]`. It uses `ConstantArrayTypeBuilder` to build the result type. When a spread item is a single constant array (`getConstantArrays()` returns exactly one), its key/value pairs are added individually. When it's not (e.g., `array<string, mixed>`), the builder is degraded via `degradeToGeneralArray()`, and all subsequent items are merged into a general `ArrayType` with unioned keys and values.
-
-The degradation loses specific key information. To preserve it, `getArrayType()` tracks `HasOffsetValueType` accessories for non-optional keys from constant array spreads with string keys. After building, these are intersected with the degraded result. When a non-constant spread appears later that could overwrite tracked keys (its key type is a supertype of the tracked offsets), those entries are invalidated. This ensures correct handling of PHP's spread ordering semantics where later spreads override earlier ones for same-named string keys.
-
-### Nullsafe operator and ensureShallowNonNullability / revertNonNullability
-
-`NodeScopeResolver` handles `NullsafeMethodCall` and `NullsafePropertyFetch` by temporarily removing null from the variable's type (`ensureShallowNonNullability`), processing the inner expression, then restoring the original nullable type (`revertNonNullability`). When the expression is an `ArrayDimFetch` (e.g. `$arr['key']?->method()`), `specifyExpressionType` recursively narrows the parent array type via `TypeCombinator::intersect` with `HasOffsetValueType`. This intersection only narrows and cannot widen, so `revertNonNullability` fails to restore the parent array's offset type. The fix is to also save and restore the parent expression's type in `ensureShallowNonNullability`. Without this, subsequent uses of the same nullsafe call are falsely reported as "Using nullsafe method call on non-nullable type" because the parent array retains the narrowed (non-null) offset type.
-
-### Loop analysis: foreach, for, while
-
-Loops are a frequent source of false positives because PHPStan must reason about types across iterations:
-- **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.
-- **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.
-- **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.
-
-### Match expression scope merging
-
-Match expressions in `NodeScopeResolver` (around line 4154) process each arm and merge the resulting scopes. The critical pattern for variable certainty is: when a match is exhaustive (has a `default` arm or an always-true condition), arm body scopes should be merged only with each other (not with the original pre-match scope). This mirrors how if/else merging works — `$finalScope` starts as `null`, and each branch's scope is merged via `$branchScope->mergeWith($finalScope)`. When the match is NOT exhaustive, the original scope must also participate in the merge (via `$scope->mergeWith($armBodyFinalScope)`) because execution may skip all arms and throw `UnhandledMatchError`. The `mergeVariableHolders()` method in `MutatingScope` uses `ExpressionTypeHolder::createMaybe()` for variables present in only one scope, so merging an arm scope that defines `$x` with the original scope that lacks `$x` degrades certainty to "maybe" — this is the root cause of false "might not be defined" reports for exhaustive match expressions.
-
-### GenericClassStringType narrowing and tryRemove
-
-`GenericClassStringType` represents `class-string<T>` where `T` is the generic object type. When the generic type is a union (e.g., `class-string<Car|Bike|Boat>`), it's a single `GenericClassStringType` with an inner `UnionType`. This is distinct from `class-string<Car>|class-string<Bike>|class-string<Boat>` which is a `UnionType` of individual `GenericClassStringType`s.
-
-The `tryRemove()` method handles removing a `ConstantStringType` (e.g., `'Car'`) from the class-string type. It must check whether the class is final — only for final classes can exact class-string removal be performed, since non-final classes could have subclasses whose class-strings would still be valid values. When the inner generic type is a union, `TypeCombinator::remove()` is used to remove the corresponding `ObjectType` from the inner union.
-
-This affects match expression exhaustiveness: `class-string<FinalA|FinalB>` matched against `FinalA::class` and `FinalB::class` is exhaustive only because both classes are final.
-
-### StaticType::transformStaticType and ThisType downgrading
-
-`StaticType::transformStaticType()` is used when resolving method return types on a `StaticType` caller. It traverses the return type and transforms `StaticType`/`ThisType` instances via `changeBaseClass()`. Since `ThisType extends StaticType`, both are caught by the `$type instanceof StaticType` check. The critical invariant: when the **caller** is a `StaticType` (not `ThisType`) and the method's return type contains `ThisType`, the `ThisType` must be downgraded to a plain `StaticType`. This is because `$this` (the exact instance) cannot be guaranteed when calling on a `static` type (which could be any subclass instance). `ThisType::changeBaseClass()` returns a new `ThisType`, which preserves the `$this` semantics — so the downgrade must happen explicitly after `changeBaseClass()`. The `CallbackUnresolvedMethodPrototypeReflection` at line 91 also has special handling for `ThisType` return types intersected with `selfOutType`.
-
-### PHPDoc inheritance
-
-PHPDoc types (`@return`, `@param`, `@throws`, `@property`) are inherited through class hierarchies. Bugs arise when:
-- Child methods with narrower native return types don't inherit parent PHPDoc return types
-- `@property` tags on parent classes don't consider native property types on children
-- Trait PHPDoc resolution uses wrong file context
-
-The `PhpDocInheritanceResolver` and `PhpDocBlock` classes handle this. Recent optimization: resolve through reflection instead of re-walking the hierarchy manually.
-
-### Dynamic return type extensions for built-in functions
-
-Many built-in PHP functions need `DynamicFunctionReturnTypeExtension` implementations because their return types depend on arguments:
-- `array_rand()`, `array_count_values()`, `array_first()`/`array_last()`, `filter_var()`, `curl_setopt()`, DOM methods, etc.
-- Extensions live in `src/Type/Php/` and are registered in `conf/services.neon`
-- Each reads the argument types from `Scope::getType()` and returns a more precise `Type`
-
-### Function signature corrections (`src/Reflection/SignatureMap/`)
-
-PHPStan maintains its own signature map for built-in PHP functions in `functionMap.php` and delta files. Fixes involve:
-- Correcting return types (e.g. `DOMNode::C14N` returning `string|false`)
-- Adding `@param-out` for reference parameters (e.g. `stream_socket_client`)
-- Marking functions as impure (e.g. `time()`, Redis methods)
-- PHP-version-specific signatures (e.g. `bcround` only in PHP 8.4+)
-
-### PHP-parser name resolution and `originalName` attribute
-
-PHP-parser's `NameResolver` resolves names through `use` statements. When `preserveOriginalNames: true` is configured (as PHPStan does in `conf/services.neon`), the original unresolved Name node is preserved as an `originalName` attribute on the resolved `FullyQualified` node. This matters for case-sensitivity checking: when `use DateTimeImmutable;` is followed by `dateTimeImmutable` in a typehint, the resolved node has the case from the `use` statement (`DateTimeImmutable`), losing the wrong case from the source. The `originalName` attribute preserves the source-code case (`dateTimeImmutable`). Rules that check class name case (like `class.nameCase` via `ClassCaseSensitivityCheck`) must use this attribute rather than relying on `Type::getReferencedClasses()` which returns already-resolved names. The fix pattern is in `FunctionDefinitionCheck::getOriginalClassNamePairsFromTypeNode()` which extracts original-case class names from AST type nodes.
-
-### Impure points and side effects
-
-PHPStan tracks whether expressions/statements have side effects ("impure points"). This enables:
-- Reporting useless calls to pure methods (`expr.resultUnused`)
-- Detecting void methods with no side effects
-- `@phpstan-pure` enforcement
-
-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.
-
-### FunctionCallParametersCheck: by-reference argument validation
-
-`FunctionCallParametersCheck` (`src/Rules/FunctionCallParametersCheck.php`) validates arguments passed to functions/methods. For by-reference parameters, it checks whether the argument is a valid lvalue (variable, array dim fetch, property fetch). It also allows function/method calls that return by reference (`&getString()`, `&staticGetString()`, `&refFunction()`), using `returnsByReference()` on the resolved reflection. The class is manually instantiated in ~20 test files, so adding a constructor parameter requires updating all of them. The `Scope` interface provides `getMethodReflection()` for method calls, while `ReflectionProvider` (injected into the class) is needed for resolving function reflections.
-
-### FunctionCallParametersCheck: spread argument expansion with optional named keys
-
-When spreading a constant array into a function/method call (`foo(...$array)`), `FunctionCallParametersCheck::check()` (lines 139-213) expands each array position into an individual argument. For each position, it checks whether the key is optional (`getOptionalKeys()`), extracts the value type, and determines the key name. Optional keys (array positions that might not exist) are normally skipped to avoid asserting they're always present.
-
-However, when the optional key has a string name (named argument), skipping it causes a fallback path (lines 195-203) that loses the key-to-type correspondence. The fallback uses `getIterableValueType()` which unions ALL value types, then passes this as a single generic unpacked argument. This causes false positives when different keys have different value types — e.g., `non-empty-array{width?: int, bgColor?: string}` spread into `Foo(int|null $width, string|null $bgColor)` reports "int|string given" for `$width` because the fallback unions `int` and `string`. The fix: only skip optional keys when they don't have a named key (`$keyArgumentName === null`), so named optional keys are still expanded as individual named arguments with their correct per-key types.
-
 ### Testing patterns
 
 - **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/`.
@@ -411,7 +295,3 @@ When adding or editing PHPDoc comments in this codebase, follow these guidelines
 - `react/child-process`, `react/async` - Parallel analysis
 - `symfony/console` - CLI interface
 - `hoa/compiler` - Used for regex type parsing
-
-### Ternary expression type narrowing in TypeSpecifier
-
-`TypeSpecifier::specifyTypesInCondition()` handles ternary expressions (`$cond ? $a : $b`) for type narrowing. In a truthy context (e.g., inside `assert()`), the ternary is semantically equivalent to `($cond && $a) || (!$cond && $b)` — meaning if the condition is true, the "if" branch must be truthy, and if false, the "else" branch must be truthy. The fix converts ternary expressions to this `BooleanOr(BooleanAnd(...), BooleanAnd(...))` form so the existing OR/AND narrowing logic handles both branches correctly. This enables `assert($cond ? $x instanceof A : $x instanceof B)` to narrow `$x` to `A|B`. The `AssertFunctionTypeSpecifyingExtension` calls `specifyTypesInCondition` with `TypeSpecifierContext::createTruthy()` context for the assert argument.