rectorphp / rector-symfony

Rector upgrade rules for Symfony
http://getrector.com
MIT License
180 stars 86 forks source link

The rector rule for TranslatorInterface does not take into account possible null parameter #592

Closed julienfastre closed 5 months ago

julienfastre commented 5 months ago

The signature of Symfony\Contracts\Translation\TranslatorInterface::trans is

interface TranslatorInterface
{
    // ...

    public function trans(string $id, array $parameters = [], ?string $domain = null, ?string $locale = null): string;

   // ...
}

But the configured rules does not add the ? before the ?string $domain = null:

    ---------- begin diff ----------
@@ @@
     public function getFilter()
     {
         $translator = new class () implements TranslatorInterface {
-            public function trans(string $id, array $parameters = [], ?string $domain = null, string $locale = null)
+            public function trans(string $id, array $parameters = [], string $domain = null, string $locale = null)
             {
                 return $id;
             }
    ----------- end diff -----------

Applied rules:
 * AddParamTypeDeclarationRector

As it is syntactically correct to remove this ?, it leads to conflicts between rector rules and other tools, like php-cs-fixer.

Can I suggest a PR to add this question mark ?

julienfastre commented 5 months ago

As the PR is quite straightforward, I submitted it.