ArrayPropertyDefaultValueRector should take into account docblock for allowed null value

[Aerendir]

    ---------- begin diff ----------
--- Original
+++ New
@@ -30,25 +30,25 @@
 class SocialProfiles
 {
     /** @var string[]|null */
-    private $anArrayVariable;
+    private $anArrayVariable = [];

    ----------- end diff -----------

Applied rectors:

 * Rector\CodingStyle\Rector\Property\ArrayPropertyDefaultValueRector

This is the class in which this property is set:

<?php

declare(strict_types=1);

namespace App\TrustAnalyzer\Collectors;

class AClassWithNullableArrayProperty
{
    /** @var string[]|null */
    private $anArrayVariable;

    /**
     * @param array|null $data
     */
    public function __construct(?array $data)
    {
        if (null === $data) {
            return;
        }

        $this->setAnArrayVariable($data['anArrayVariable']);
    }

    /**
     * @return string[]|null
     */
    public function getAnArrayVariable(): ? array
    {
        return $this->anArrayVariable;
    }

    /**
     * @param string[]|null $withdrawal
     *
     * @return AClassWithNullableArrayProperty
     */
    private function setAnArrayVariable($anArrayVariable): self
    {
        $this->anArrayVariable = $this->ensureAreStrings($anArrayVariable);

        return $this;
    }

    /**
     * @param string[]|null $urls
     *
     * @return string[]|null
     */
    private function ensureAreString($anArrayVariable): ?array
    {
        if (null === $anArrayVariable) {
            return $anArrayVariable;
        }

        $transformed = [];
        foreach ($anArrayVariable as $arrayVariable) {
            if ($arrayVariable instanceof ClassWithToString) {
                $arrayVariable = $arrayVariable->toString();
            }

            $transformed[] = $arrayVariable;
        }

        return $transformed;
    }
}

As you can see, the getAnArrayVariable() method allows null values in return and the property $anArrayVariable is annotated as string[]|null.

This means that if this code is changed like Rector would, a lot of breaks will happen, both executing the code and analyzing it statically.

More, I'm not sure adding a default value is what is always wanted.

I think that the rector should add a default value only when it should have been set but wasn't, not always.

WDYT? If you agree with me, I'll submit a failing test case.

[TomasVotruba]

Agreed, nullable should be skipped 👍

Thanks for full example!