[Downgrade Php 7.2] Add opt-out parameter for unsafe types to avoid piling list of safe types

[TomasVotruba]

Seeing the list of safe types in downgrade-config.php, it looks like we only have to maintain all the interfaces and parent types.

On the other hand, there are just 2 methods that are currently unsafe:

• in Symfony Loader interface
• in PSR Container interface

Instead of a "removing all types bug X", we should use "remove types on these known cases", to safe work and make list of weak methods easy to see.

[samsonasik]

@TomasVotruba it seems make error in rector scoped at https://github.com/rectorphp/rector

[samsonasik]

@TomasVotruba error scoped bare run shown too at https://github.com/rectorphp/rector-src/runs/4481808266?check_suite_focus=true

[TomasVotruba]

I'm currently working on it...

[samsonasik]

@TomasVotruba now seems error in php 7.1:

Run php bin/rector --version --ansi
PHP Warning:  Declaration of RectorPrefix20211210\Symfony\Component\DependencyInjection\Loader\FileLoader::import($resource, ?string $type = NULL, $ignoreErrors = false, ?string $sourceResource = NULL, $exclude = NULL) should be compatible with RectorPrefix20211210\Symfony\Component\Config\Loader\FileLoader::import($resource, ?string $type = NULL, bool $ignoreErrors = false, ?string $sourceResource = NULL, $exclude = NULL) in /home/runner/work/rector/rector/vendor/symfony/dependency-injection/Loader/FileLoader.php on line 213
Warning: Declaration of RectorPrefix20211210\Symfony\Component\DependencyInjection\Loader\FileLoader::import($resource, ?string $type = NULL, $ignoreErrors = false, ?string $sourceResource = NULL, $exclude = NULL) should be compatible with RectorPrefix20211210\Symfony\Component\Config\Loader\FileLoader::import($resource, ?string $type = NULL, bool $ignoreErrors = false, ?string $sourceResource = NULL, $exclude = NULL) in /home/runner/work/rector/rector/vendor/symfony/dependency-injection/Loader/FileLoader.php on line 213
Rector f409d3d5286aedbc4b580515d451b655ac0df774

https://github.com/rectorphp/rector/runs/4482567098?check_suite_focus=true#step:4:1

[samsonasik]

@TomasVotruba I tried locally, by add import in after load in LoaderInterface unsafe type methods, it seems make Syntax error:

                 LoaderInterface::class => [
-                    'load'
+                    'load',
+                    'import',

It happen only in php 7.1 and 7.2:

[samsonasik]

@TomasVotruba looking for existing error at php 7.1 at https://github.com/rectorphp/rector-src/runs/4482411634?check_suite_focus=true#step:23:6

PHP Warning:  Declaration of 

RectorPrefix20211210\Symfony\Component\DependencyInjection\Loader\FileLoader::import($resource, ?string $type = NULL, $ignoreErrors = false, ?string $sourceResource = NULL, $exclude = NULL) 

should be compatible with 

RectorPrefix20211210\Symfony\Component\Config\Loader\FileLoader::import($resource, ?string $type = NULL, bool $ignoreErrors = false, ?string $sourceResource = NULL, $exclude = NULL) in /home/runner/work/rector/rector/vendor/symfony/dependency-injection/Loader/FileLoader.php on line 213

The diff seems on bool type param:

-import($resource, ?string $type = NULL, $ignoreErrors = false, ?string $sourceResource = NULL, $exclude = NULL) 
+import($resource, ?string $type = NULL, bool $ignoreErrors = false, ?string $sourceResource = NULL, $exclude = NULL) 

[samsonasik]

@TomasVotruba here the reproduced error https://3v4l.org/ZQBp3#v7.1.33

The issue seems possibly due to changed of ?string $type = null which extends $type = null

[samsonasik]

@TomasVotruba it seems because there are 2 FileLoader::import(), there are in:

• symfony/dependency-injection
• symfony/loader

which in symfony/dependency-injection, the $ignoreErrors = false is not has type.

[samsonasik]

@TomasVotruba it seems php 7.1 error with the following code:

class A
{
public function import($resource, string $type = null, bool $ignoreErrors = false, string $sourceResource = null, $exclude = null){}
}

class B extends A
{
public function import($resource, string $type = null, $ignoreErrors = false, string $sourceResource = null, $exclude = null){}    
}

while it working on php >7.2, ref https://3v4l.org/CLL3e#v7.1.33

[TomasVotruba]

Could you add the import method to list of unsafe methods?

[samsonasik]

I tried that, got syntax error, it seems the existing DowngradeParameterTypeWideningRectorTest actually got errors:

PHPUnit 9.5.10 by Sebastian Bergmann and contributors.

......F.F.FFFFFFFF                                                18 / 18 (100%)

Time: 00:00.467, Memory: 78.50 MB

There were 10 failures:

1) Rector\Tests\DowngradePhp72\Rector\ClassMethod\DowngradeParameterTypeWideningRector\DowngradeParameterTypeWideningRectorTest::test with data set #1 (Symplify\SmartFileSystem\SmartFileInfo Object (...))
rules-tests/DowngradePhp72/Rector/ClassMethod/DowngradeParameterTypeWideningRector/Fixture/indirect_implements_native_interface.php.inc
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 
 abstract class ParentClass implements \SplObserver
 {
-    /**
-     * @param \SplSubject $subject
-     */
-    public function update($subject): void
+    public function update(\SplSubject $subject): void
     {
     }
 }

/Users/samsonasik/www/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:154
/Users/samsonasik/www/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:115
/Users/samsonasik/www/rector-src/rules-tests/DowngradePhp72/Rector/ClassMethod/DowngradeParameterTypeWideningRector/DowngradeParameterTypeWideningRectorTest.php:19

2) Rector\Tests\DowngradePhp72\Rector\ClassMethod\DowngradeParameterTypeWideningRector\DowngradeParameterTypeWideningRectorTest::test with data set #8 (Symplify\SmartFileSystem\SmartFileInfo Object (...))
rules-tests/DowngradePhp72/Rector/ClassMethod/DowngradeParameterTypeWideningRector/Fixture/interface_on_parent_class.php.inc
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 
 interface WhateverInterface
 {
-    /**
-     * @param string $input
-     */
-    public function test($input);
+    public function test(string $input);
 }
 
 abstract class AbstractSomeAncestorClass implements WhateverInterface

/Users/samsonasik/www/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:154
/Users/samsonasik/www/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:115
/Users/samsonasik/www/rector-src/rules-tests/DowngradePhp72/Rector/ClassMethod/DowngradeParameterTypeWideningRector/DowngradeParameterTypeWideningRectorTest.php:19

3) Rector\Tests\DowngradePhp72\Rector\ClassMethod\DowngradeParameterTypeWideningRector\DowngradeParameterTypeWideningRectorTest::test with data set #6 (Symplify\SmartFileSystem\SmartFileInfo Object (...))
rules-tests/DowngradePhp72/Rector/ClassMethod/DowngradeParameterTypeWideningRector/Fixture/fixture.php.inc
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 
 interface A
 {
-    /**
-     * @param mixed[] $input
-     */
-    public function test($input);
+    public function test(array $input);
 }
 
 class B implements A
@@ @@
 
 final class MostChild implements A
 {
-    /**
-     * @param mixed[] $input
-     */
-    public function test($input)
+    public function test(array $input)
     {
     }
 }

/Users/samsonasik/www/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:154
/Users/samsonasik/www/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:115
/Users/samsonasik/www/rector-src/rules-tests/DowngradePhp72/Rector/ClassMethod/DowngradeParameterTypeWideningRector/DowngradeParameterTypeWideningRectorTest.php:19

4) Rector\Tests\DowngradePhp72\Rector\ClassMethod\DowngradeParameterTypeWideningRector\DowngradeParameterTypeWideningRectorTest::test with data set #13 (Symplify\SmartFileSystem\SmartFileInfo Object (...))
rules-tests/DowngradePhp72/Rector/ClassMethod/DowngradeParameterTypeWideningRector/Fixture/nullable_callable_value.php.inc
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 
 interface SomeAskingInterfaceWithNullable
 {
-    /**
-     * @param callable|null $callable
-     */
-    public function ask($callable = null);
+    public function ask(?callable $callable = null);
 }
 
 final class AskForMore implements SomeAskingInterfaceWithNullable

/Users/samsonasik/www/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:154
/Users/samsonasik/www/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:115
/Users/samsonasik/www/rector-src/rules-tests/DowngradePhp72/Rector/ClassMethod/DowngradeParameterTypeWideningRector/DowngradeParameterTypeWideningRectorTest.php:19

5) Rector\Tests\DowngradePhp72\Rector\ClassMethod\DowngradeParameterTypeWideningRector\DowngradeParameterTypeWideningRectorTest::test with data set #15 (Symplify\SmartFileSystem\SmartFileInfo Object (...))
rules-tests/DowngradePhp72/Rector/ClassMethod/DowngradeParameterTypeWideningRector/Fixture/include_used_trait.php.inc
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 
 trait AnotherServiceLocatorTrait
 {
-    /**
-     * @param string $name
-     */
-    public function get($name)
+    public function get(string $name)
     {
     }
 }

/Users/samsonasik/www/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:154
/Users/samsonasik/www/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:115
/Users/samsonasik/www/rector-src/rules-tests/DowngradePhp72/Rector/ClassMethod/DowngradeParameterTypeWideningRector/DowngradeParameterTypeWideningRectorTest.php:19

6) Rector\Tests\DowngradePhp72\Rector\ClassMethod\DowngradeParameterTypeWideningRector\DowngradeParameterTypeWideningRectorTest::test with data set #16 (Symplify\SmartFileSystem\SmartFileInfo Object (...))
rules-tests/DowngradePhp72/Rector/ClassMethod/DowngradeParameterTypeWideningRector/Fixture/container_has_interface.php.inc
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 
 interface SomeContainerInterface
 {
-    /**
-     * @param string $id
-     */
-    public function has($id);
+    public function has(string $id);
 }
 
 final class SomeContainerBuilder extends SomeUniqueContainer

/Users/samsonasik/www/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:154
/Users/samsonasik/www/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:115
/Users/samsonasik/www/rector-src/rules-tests/DowngradePhp72/Rector/ClassMethod/DowngradeParameterTypeWideningRector/DowngradeParameterTypeWideningRectorTest.php:19

7) Rector\Tests\DowngradePhp72\Rector\ClassMethod\DowngradeParameterTypeWideningRector\DowngradeParameterTypeWideningRectorTest::test with data set #14 (Symplify\SmartFileSystem\SmartFileInfo Object (...))
rules-tests/DowngradePhp72/Rector/ClassMethod/DowngradeParameterTypeWideningRector/Fixture/anonymous_class.php.inc
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 
 class SomeClass
 {
-    /**
-     * @param string $world
-     */
-    public function hello($world = 'world') {
+    public function hello(string $world = 'world') {
         printf('Hello %s', $world);
     }
 }
@@ @@
     public function doSomething(): void
     {
         $class = new class () extends SomeClass {
-            /**
-             * @param string $world
-             */
-            public function hello($world = 'world') {
+            public function hello(string $world = 'world') {
                 printf('Hi %s', $world);
             }
         };

/Users/samsonasik/www/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:154
/Users/samsonasik/www/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:115
/Users/samsonasik/www/rector-src/rules-tests/DowngradePhp72/Rector/ClassMethod/DowngradeParameterTypeWideningRector/DowngradeParameterTypeWideningRectorTest.php:19

8) Rector\Tests\DowngradePhp72\Rector\ClassMethod\DowngradeParameterTypeWideningRector\DowngradeParameterTypeWideningRectorTest::test with data set #12 (Symplify\SmartFileSystem\SmartFileInfo Object (...))
rules-tests/DowngradePhp72/Rector/ClassMethod/DowngradeParameterTypeWideningRector/Fixture/anonymous_return_abstract_interface.php.inc
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 
 interface SomeInterface2
 {
-    /**
-     * @param \SplFileInfo $file
-     */
-    public function test($file);
+    public function test(\SplFileInfo $file);
 }
 
 abstract class SomeAbstractClass implements SomeInterface2
 {
-    /**
-     * @param \SplFileInfo $file
-     * @param string $world
-     */
-    public function test2($file, $world = 'world') {
+    public function test2(\SplFileInfo $file, string $world = 'world') {
         printf('Hello %s', $world);
     }
 }
 
 return new class extends SomeAbstractClass {
-    /**
-     * @param \SplFileInfo $file
-     */
-    public function test($file) {
+    public function test(\SplFileInfo $file) {
         $this->test2($file);
     }
 };

/Users/samsonasik/www/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:154
/Users/samsonasik/www/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:115
/Users/samsonasik/www/rector-src/rules-tests/DowngradePhp72/Rector/ClassMethod/DowngradeParameterTypeWideningRector/DowngradeParameterTypeWideningRectorTest.php:19

9) Rector\Tests\DowngradePhp72\Rector\ClassMethod\DowngradeParameterTypeWideningRector\DowngradeParameterTypeWideningRectorTest::test with data set #17 (Symplify\SmartFileSystem\SmartFileInfo Object (...))
rules-tests/DowngradePhp72/Rector/ClassMethod/DowngradeParameterTypeWideningRector/Fixture/anonymous_class_alternative.php.inc
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 
 interface SomeInterface
 {
-    /**
-     * @param string $world
-     */
-    public function hello($world = 'world');
+    public function hello(string $world = 'world');
 }
 
 class SomeClass2
@@ @@
         $class = new class (function () {
             return $this->doSomethingElse();
         }) implements SomeInterface {
-            /**
-             * @param string $world
-             */
-            public function hello($world = 'world') {
+            public function hello(string $world = 'world') {
                 printf('Hi %s', $world);
             }
         };

/Users/samsonasik/www/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:154
/Users/samsonasik/www/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:115
/Users/samsonasik/www/rector-src/rules-tests/DowngradePhp72/Rector/ClassMethod/DowngradeParameterTypeWideningRector/DowngradeParameterTypeWideningRectorTest.php:19

10) Rector\Tests\DowngradePhp72\Rector\ClassMethod\DowngradeParameterTypeWideningRector\DowngradeParameterTypeWideningRectorTest::test with data set #2 (Symplify\SmartFileSystem\SmartFileInfo Object (...))
rules-tests/DowngradePhp72/Rector/ClassMethod/DowngradeParameterTypeWideningRector/Fixture/skip_container_parameters.php.inc
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 
 final class SkipContainerParameters implements ContainerInterface
 {
-    /**
-     * @param string $id
-     */
-    public function set($id, $service)
+    public function set(string $id, $service)
     {
     }
 
-    /**
-     * @param string $id
-     * @param int $invalidBehavior
-     */
-    public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE)
+    public function get(string $id, int $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE)
     {
     }
 
-    /**
-     * @param string $id
-     */
-    public function has($id)
+    public function has(string $id)
     {
     }
 
-    /**
-     * @param string $id
-     */
-    public function initialized($id)
+    public function initialized(string $id)
     {
         // TODO: Implement initialized() method.
     }

/Users/samsonasik/www/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:154
/Users/samsonasik/www/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:115
/Users/samsonasik/www/rector-src/rules-tests/DowngradePhp72/Rector/ClassMethod/DowngradeParameterTypeWideningRector/DowngradeParameterTypeWideningRectorTest.php:19

FAILURES!
Tests: 18, Assertions: 46, Failures: 10.