Preserve reflector-derived column types when AST narrowing yields ErrorType#777
Open
ArtemGoutsoul wants to merge 1 commit into
Open
Conversation
With FETCH_TYPE_ASSOC + utilizeSqlAst(true) on PHPStan >= 2.2, a SELECT that references columns through a table alias (e.g. `SELECT sg.col FROM service_group sg`) collapses every column's value type to *ERROR*. In ParserInference::narrowResultType() the per-column seed reads the row shape at an integer offset, but the FETCH_TYPE_ASSOC reflector row is string-keyed only. PHPStan 2.2 changed missing-offset reads to return ErrorType (previously benign). For unqualified columns QueryScope refines this back to the real column type; for alias- qualified columns resolveExpression() has no branch and falls through to MixedType, so the ErrorType ends up overwriting the correct reflector-derived type under the column-name key. Fix: after the QueryScope refinement step, skip the column when the value type is still ErrorType - there is no information to add, so leave the reflector's type untouched.
staabm
reviewed
May 29, 2026
Comment on lines
+183
to
+190
| // PHPStan 2.2+ returns ErrorType when reading a missing offset (here | ||
| // the integer offset is missing on a string-keyed assoc row shape). | ||
| // If QueryScope couldn't refine the column either (e.g. alias-qualified | ||
| // references like `t.col`, which resolveExpression() returns MixedType | ||
| // for), there is nothing to narrow — keep the reflector-derived type. | ||
| if ($valueType instanceof ErrorType) { | ||
| continue; | ||
| } |
Owner
There was a problem hiding this comment.
could we check $resultType->hasOffsetValueType() before we call $resultType->getOffsetValueType instead?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Preserve reflector-derived column types when AST narrowing yields
ErrorTypeIssue
With
utilizeSqlAst(true)+defaultFetchMode(FETCH_TYPE_ASSOC)on PHPStan 2.2+, anySELECTthat references columns through a table alias collapses every column's value type to*ERROR*:Worked on PHPStan 2.1; regressed in 2.2.
Why
In
ParserInference::narrowResultType(), per column:$valueType = $resultType->getOffsetValueType(ConstantIntegerType($i))— but theFETCH_TYPE_ASSOCrow shape from the reflector is string-keyed only. PHPStan 2.2 changed missing-offset reads to returnErrorType(previously benign).$queryScope->getType($expression)is supposed to refine it. For unqualified columns it returns the real column type and overwrites theErrorType. For alias-qualified columns (t.col)resolveExpression()has no branch for qualified identifiers and falls through toMixedType, so the guard! $type instanceof MixedTypeskips the assignment.$valueType(stillErrorType) is then written over the correct reflector-derived type under the column-name key.Fix
After the QueryScope refinement step, skip the column when
$valueTypeis stillErrorType— there is no information to add, so leave the reflector's type untouched.Minimal, targeted: only changes behavior on the previously-broken path. A follow-up could teach
QueryScope::resolveExpression()to resolve qualified identifiers and restore full AST narrowing (JOIN/WHERE nullability) for aliased columns, but that's a larger change.Test
Added
tests/default/ParserInferenceTest.php— a plain PHPUnit test that drivesParserInference::narrowResultType()directly with a synthetic assoc-onlyConstantArrayType, covering both unqualified andt.col-qualified queries. Fails without the patch (*ERROR*in the alias-qualified case), passes with it.