Skip to content

Conversation

@DjordyKoert
Copy link
Contributor

Q A
Branch? 7.3
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #...
License MIT

#62388 broke type detection for template keys for array:

/**
 * @template TKey of array-key
 * @template TValue
 */
class Collection {
}

Above example currently throws an exception Symfony\Component\TypeInfo\Exception\InvalidArgumentException: "TKey" is not a valid array key type. even though TKey is a type of array-key (int|string).

This PR fixes this by getting the wrapped type when dealing with a template type when checking if a valid array key type is provided.

Copilot AI review requested due to automatic review settings December 11, 2025 13:39
@carsonbot carsonbot added this to the 7.3 milestone Dec 11, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug where template keys constrained to array-key (which is int|string) incorrectly threw an InvalidArgumentException when used as array key types. The fix unwraps template types to access their bounds before validation.

Key Changes:

  • Added unwrapping logic for template types in array key validation to check the bound type rather than the template wrapper
  • Added comprehensive test cases covering template keys with array-key bounds
  • Updated the DummyCollection fixture with template annotations for TKey and TValue

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/Symfony/Component/TypeInfo/Type/CollectionType.php Added logic to unwrap TemplateType and validate its bound when checking array key types
src/Symfony/Component/TypeInfo/Tests/TypeResolver/StringTypeResolverTest.php Added test cases for arrays and lists with template types, plus a negative test for invalid template key bounds
src/Symfony/Component/TypeInfo/Tests/Fixtures/DummyCollection.php Added template annotations for TKey (constrained to array-key) and TValue to support new test cases

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 53 to 65
$keyType = $this->getCollectionKeyType();

if ($keyType instanceof TemplateType) {
$keyType = $keyType->getWrappedType();
}

$keyTypes = $keyType instanceof UnionType ? $keyType->getTypes() : [$keyType];

foreach ($keyTypes as $type) {
if (!$type instanceof BuiltinType || !\in_array($type->getTypeIdentifier(), [TypeIdentifier::INT, TypeIdentifier::STRING], true)) {
throw new InvalidArgumentException(\sprintf('"%s" is not a valid array key type.', (string) $keyType));
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We must be able to handle

  • Union of scalars
  • Union of template types, such as TFoo|TBar.
  • Template type that is a union
  • Template type that is scalar

To handle every case, maybe we can do the following:

$keyType = $this->getCollectionKeyType();

foreach ($keyType->traverse(false) as $t) { // to traverse composite but not wrapped (because nullable, collection, etc are not valid)
    if ($t instanceof UnionType) {
        continue;
    }

    if (($t instanceof BuiltinType || $t instanceof TemplateType) && $t->isIdentifiedBy(TypeIdentifier::INT, TypeIdentifier::STRING) {
        continue;
    }

    // others types, such as intersection, nullable types, etc are not valid
    throw new InvalidArgumentException(\sprintf('"%s" is not a valid array key type.', (string) $keyType));
}

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants