Skip to content

fix: Undefined array input type#819

Open
michael-georgiadis wants to merge 3 commits into
thecodingmachine:masterfrom
michael-georgiadis:fix/undefined-array-input-type
Open

fix: Undefined array input type#819
michael-georgiadis wants to merge 3 commits into
thecodingmachine:masterfrom
michael-georgiadis:fix/undefined-array-input-type

Conversation

@michael-georgiadis

@michael-georgiadis michael-georgiadis commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Description

Optional-via-Undefined works for scalars but explodes on lists:

/** @param list<string>|null $ids */
public readonly array|Undefined|null $ids = Undefined::VALUE,
// CannotMapTypeException: type-hinted to "array|list<string>" ... forbidden

Fix: drop Undefined and take the non-null core before the container check, reusing existing helpers (UndefinedTypeMapper + a now-public static getNonNullableType).

Covers array/list. Native iterable isn't handled (PHP expands it to array|Traversable) and since it's rare for an input, I left it alone

@oojacoboo

Copy link
Copy Markdown
Collaborator

@michael-georgiadis I'm not quite sure I understand the issue here. Where are things breaking?

@michael-georgiadis

Copy link
Copy Markdown
Contributor Author

@oojacoboo So, up until now the library handled the undefined compounds on the non-doc-block based type hints.

An int|Undefined|null for example would work fine.

But for this case

/** @param list<int> $array */
private array|Undefined|null $array;

if the code block below does not exist:

  if ($innerType instanceof Compound && UndefinedTypeMapper::containsUndefined($innerType)) {                                                                                                                                                                                                                               
      $innerType = NullableTypeMapperAdapter::getNonNullableType(                                                                                                                                                                                                                                                           
          UndefinedTypeMapper::replaceUndefinedWith($innerType),                                                                                                                                                                                                                                                            
      ) ?? $innerType;                                                                                                                                                                                                                                                                                                      
  }   

The type coming in in this if statement

        if (
            $innerType instanceof Array_
            || $innerType instanceof Iterable_
            || $innerType instanceof Mixed_
            || $innerType instanceof Callable_
            || ($innerType instanceof Object_
                && $docBlockType instanceof Collection
                && (string) $innerType->getFqsen() === (string) $docBlockType->getFqsen()
            )
        )

was a Compound(array, Undefined, null). That would make it go to the else statement and blow up during schema generation.

So the fix here is basically unwrapping the compound with previously written code and making sure that the innerType is handled as an Array instead and prints the type correctly.

Scalars also work by passing the else statement, but there are no conflicting docblocks so they survive

@michael-georgiadis

Copy link
Copy Markdown
Contributor Author

This is not me being passive aggressive btw, but you can checkout the branch and try the integration tests with the fixtures but reverting the actual code-change and you will see that it blows up

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