fix: Undefined array input type

[michael-georgiadis]

Description

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

/** @param list<string>|Undefined|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]

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

[michael-georgiadis]

@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]

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

[oojacoboo]

So, I guess I'm uncertain why you'd want to docblock type differently from the actual type signature.

[michael-georgiadis]

@oojacoboo I am unsure what you mean by that. I will try my best to clear the uncertainty tho. The docblock and the actualy type signature for arrays carry different information (as ofc you know).

So, the only thing that array gives you is basically that it's a list. But for GQLite to know which scalar or type is in that list you'd need the docblock type hint.

[oojacoboo]

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

I'm saying this is an incorrect phpdoc type, as it doesn't include Undefined or null. Instead, it should be:

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

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.72%. Comparing base (53f9d49) to head (d18bf4f).
⚠️ Report is 163 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #819      +/-   ##
============================================
- Coverage     95.72%   91.72%   -4.01%     
- Complexity     1773     2025     +252     
============================================
  Files           154      198      +44     
  Lines          4586     5436     +850     
============================================
+ Hits           4390     4986     +596     
- Misses          196      450     +254     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[michael-georgiadis]

@oojacoboo fair point. Tried it with the proper docblock typing (as you can see in the latest commit on the fixtures) and the issue still remains if I revert my code.

[oojacoboo]

Can you add a test here that checks that a passed null value into the input type is respected as an update? So, the default value would be something other than null, and it's updated to be nullified.

[michael-georgiadis]

@oojacoboo, Sure thing. I added the test case

[oojacoboo]

@michael-georgiadis sorry to be pedantic about this. But can you please assert the value before the mutation and after, so we can be sure that the change is respected. I don't actually recall if the previous mutations maintain state, or if each mutation within a test is stateless.

[michael-georgiadis]

@oojacoboo Made it more explicit 🫡

[oojacoboo]

@michael-georgiadis thanks

[oojacoboo]

@michael-georgiadis Thanks for the PR and working through it with me!

[michael-georgiadis]

@oojacoboo, no worries. Do we know when the release is coming up?