Skip to content

Preserve reflector-derived column types when AST narrowing yields ErrorType#777

Open
ArtemGoutsoul wants to merge 1 commit into
staabm:mainfrom
ArtemGoutsoul:fix/parser-inference-alias-qualified-error-type
Open

Preserve reflector-derived column types when AST narrowing yields ErrorType#777
ArtemGoutsoul wants to merge 1 commit into
staabm:mainfrom
ArtemGoutsoul:fix/parser-inference-alias-qualified-error-type

Conversation

@ArtemGoutsoul
Copy link
Copy Markdown

Preserve reflector-derived column types when AST narrowing yields ErrorType

Issue

With utilizeSqlAst(true) + defaultFetchMode(FETCH_TYPE_ASSOC) on PHPStan 2.2+, any SELECT that references columns through a table alias collapses every column's value type to *ERROR*:

// service_group(service_group_id int unsigned, name varchar(45))

SELECT service_group_id, name FROM service_group
//  array{service_group_id: int<0, 4294967295>, name: string}        ✅

SELECT sg.service_group_id, sg.name FROM service_group sg
//  array{service_group_id: *ERROR*, name: *ERROR*}                  ❌

Worked on PHPStan 2.1; regressed in 2.2.

Why

In ParserInference::narrowResultType(), per column:

  1. $valueType = $resultType->getOffsetValueType(ConstantIntegerType($i)) — but the FETCH_TYPE_ASSOC row shape from the reflector is string-keyed only. PHPStan 2.2 changed missing-offset reads to return ErrorType (previously benign).
  2. $queryScope->getType($expression) is supposed to refine it. For unqualified columns it returns the real column type and overwrites the ErrorType. For alias-qualified columns (t.col) resolveExpression() has no branch for qualified identifiers and falls through to MixedType, so the guard ! $type instanceof MixedType skips the assignment.
  3. $valueType (still ErrorType) is then written over the correct reflector-derived type under the column-name key.

Fix

After the QueryScope refinement step, skip the column when $valueType is still ErrorType — there is no information to add, so leave the reflector's type untouched.

$type = $queryScope->getType($expression);
if (! $type instanceof MixedType) {
    $valueType = $type;
}

if ($valueType instanceof ErrorType) {
    continue;
}

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 drives ParserInference::narrowResultType() directly with a synthetic assoc-only ConstantArrayType, covering both unqualified and t.col-qualified queries. Fails without the patch (*ERROR* in the alias-qualified case), passes with it.

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.
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;
}
Copy link
Copy Markdown
Owner

@staabm staabm May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we check $resultType->hasOffsetValueType() before we call $resultType->getOffsetValueType instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants