StaticMap documents that NULL is a supported map value but causes an error

I confirm this issue exists, as I needed to map a value when one field was empty (NULL), but doing so causes a YAML parse error (as described in the ticket).

This is now even more complex because NULL array keys are deprecated in PHP 8.5 - see #3546535: [meta] Deal with NULL as array key/offset. So we need to consider the whole approach here. I guess one thing that this bug's existence means is that the feature is not going to be used so maybe that helps us...

Not sure what to do here.

You can use the default_value plugin to handle the case where the source value is NULL, so perhaps we just mention this in the static_map docs? Or just remove this from the docs given that it doesn't work anyway.

@longwave the problem is that we have tests using NULL as an array key though - see \Drupal\Tests\migrate\Unit\process\StaticMapTest::testWithNullSource - so someone one might be using this programatically.

But I think this is the way forward... we deprecate support for NULL in the static map and tell people to use the default value plugin instead. For PHP 8.5 we allow the deprecation to be triggered because it is a legacy code path.

One interesting thing here is that potentially people have worked around this by using an empty string.

 process:
   bar:
     plugin: static_map
     source: foo
     map:
       '': to

Would map to a NULL value because PHP has been converting NULLs to empty strings for us in the background...

Drush Site-Install (Drupal 11.3-dev)
> $a = [NULL => 'test'];
= [ …1]

> $a;
= [
    "" => "test",
  ]

Added test coverage to prove the behaviour in #6

I think what we should do is deprecate NULL maps in StaticMap - you can use the default_value functionality and it is dangerous because NULL and empty string are the same.

The real problem here is that using array keys in a map is problematic because there are quite a few data types they can not be. We also see this with the instructions for booleans...

 * If your source data contains booleans, the boolean is treated as a numeric 0
 * or 1. If the value of the source property 'foo' is TRUE then the value of the
 * destination property bar will be 'bar'. And if the value of the source
 * property 'foo' is FALSE then the value of the destination property bar will
 * be 'bar'.
 *
 * @code
 * process:
 *   bar:
 *     plugin: static_map
 *     source: foo
 *     map:
 *       0: foo
 *       1: bar
 * @endcode

I think what we should do is deprecate NULL maps in StaticMap - you can use the default_value functionality and it is dangerous because NULL and empty string are the same.

This sounds fine and we can open a follow-up for anything else.

Yeah I think this is the way to go - but we also need to deprecate empty string in static maps ... as we can't differentiate between the two.

That also seems OK, if the only use case is covered by the default_value case we're just reducing the number of ways to do the same thing.

I've got a new idea here... let's leave the capability to map empty strings alone.

We can deprecate getting NULL values into a static map without a default_value setting and tell people to add a default value process plugin before a static map. That way people can have pipelines that can differentiate between NULLs and empty strings and we have way less complex deprecation work to do.

awesome to see 8.5 passing! Congratulations.

Only moving to NW as there appears to be failing functional and kernel tests.

Should this be a 11.3 priority to get 8.5 passing going forward?

Green and I've updated the issue summary and CR.

Quick response! I’ll get to it in the morning if no one beats me to it

Was the gitlab change intended for this issue?

Yes - because we don't want to regress on PHPUnit tests on PHP 8.5 now that they are green.

okay, then I believe this one is ready to go.

+1 RTBC, nice trick with bypass!

A fair bit of work went into adding examples to all the process plugins. This removes the example without adding a replacement. This needs a followup to add the api documentation.

@quietone there is no replacement. The example never worked and there are examples for bypass and default_value already. Which is what you'll need to use to handle NULLs without skipping.

Removing tag due to #25

Committed/pushed to 11.x and cherry-picked to 11.3.x, thanks!