Skip string-with-integer-offset decomposition in NonexistentOffsetInArrayDimFetchCheck "might not exist" report#5703
Conversation
…ArrayDimFetchCheck` "might not exist" report
- In the `reportMaybes` decomposition loop of `NonexistentOffsetInArrayDimFetchCheck`,
when flattened union members are checked individually, skip reporting for string types
with integer offsets where a specific member returns `no` for `hasOffsetValueType`
- The union-level `hasOffsetValueType` already correctly returns `maybe` for these cases;
the decomposition adds no new information for strings and produces false positives
when one union member (e.g. empty string) is shorter than the offset
- Example: `(''|':')[0]` — union-level returns `maybe`, decomposition found `''` returns
`no` for offset 0 and incorrectly escalated to "might not exist" report
- Definitive `no` results (single string, non-integer offset) are unaffected
- Regression test covers the original issue pattern and string union variants
| if ( | ||
| $innerType->hasOffsetValueType($innerDimType)->no() | ||
| ) { | ||
| if ($innerType->isString()->yes() && $innerDimType->isInteger()->yes()) { |
There was a problem hiding this comment.
this should not be fixed on the rule level. revert this fix.
instead we should keep track of a conditional type for $input based on $inputLen to infer non-empty-string when inputLen > 0 for:
$inputLen = \strlen($input);
$hasTrailingColon = $inputLen > 0 && $input[$inputLen-1] === ':';
There was a problem hiding this comment.
Done. Here's a summary of the changes:
What was reverted: The rule-level fix in NonexistentOffsetInArrayDimFetchCheck that blanket-skipped string+integer offset checks was removed.
What was added instead: In TypeSpecifier::specifyTypesInCondition(), inside the existing FuncCall handler for named functions (line 583), when strlen() or mb_strlen() is used in a truthy/falsey context, the argument is now narrowed:
- Truthy context: argument is narrowed to
non-empty-string - Falsey context:
non-empty-stringis removed from the argument type (leaving''for plainstring)
How this fixes the bug: When $len = strlen($input) is assigned, the AssignHandler calls specifyTypesInCondition on the RHS in truthy context. The new narrowing produces a SpecifiedTypes with $input => non-empty-string, which gets turned into a ConditionalExpressionHolder: "when $len is truthy (positive), then $input is non-empty-string". Later, when $len > 0 is evaluated, $len is narrowed to positive-int, the conditional expression fires, and $input gets narrowed to non-empty-string.
Test updates:
- Rule test data simplified to just the original reproducer
- Added nsrt test verifying indirect narrowing through variable assignment
- Updated
narrow-cast.phpexpectations to reflect that(bool) strlen($x)now correctly narrows$x
…uthy context Revert the rule-level fix that skipped string-with-integer-offset checks in NonexistentOffsetInArrayDimFetchCheck. Instead, teach the TypeSpecifier to narrow strlen()/mb_strlen() arguments to non-empty-string when the result is used in a truthy context. This enables the existing ConditionalExpressionHolder machinery to propagate the narrowing through variable assignments like `$len = strlen($x); if ($len > 0)`. Co-Authored-By: Claude Opus 4.6 <[email protected]>
2 participants
Fixes phpstan/phpstan#13688