Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Core] Fix undefined array key 0 on FormatPerservingPrinter #1619

Merged
merged 18 commits into from
Jan 3, 2022

Conversation

samsonasik
Copy link
Member

@samsonasik samsonasik commented Jan 3, 2022

@samsonasik
Copy link
Member Author

Checking current stmts seems remove use statement:

1) Rector\Core\Tests\Issues\PrintStmtsIndex\PrintStmtsIndexTest::test with data set #0 (Symplify\SmartFileSystem\SmartFileInfo Object (...))
tests/Issues/PrintStmtsIndex/Fixture/fixture.php.inc
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 <?php
 
-use Illuminate\Http\Request;
-

which should not.

@samsonasik
Copy link
Member Author

Fixed 🎉

@samsonasik
Copy link
Member Author

@gander I updated the fixture to show the changed Request to \Illuminate\Http\Request 0b9a682

@samsonasik
Copy link
Member Author

The configured config :

   $services->set(RenameClassRector::class)
        ->configure([
            'Request' => 'Illuminate\Http\Request',
        ]);

applied succesfully, but it still removes the use statement, which should not.

@samsonasik
Copy link
Member Author

@gander I updated fixture config to show it change Route to \Illuminate\Support\Facades\Route as well 29905df

@gander
Copy link
Contributor

gander commented Jan 3, 2022

The code originally had unused use:

use Illuminate\Http\Request;

and it was removed during the process, so there was no element with index 0. I don't know if adding a use of this class will break the initialization code for this error?

@samsonasik
Copy link
Member Author

I added fixtures for auto import enabled d6b6266 seems will make issue :

1) Rector\Core\Tests\Issues\PrintStmtsIndexAutoImport\PrintStmtsIndexAutoImportTest::test with data set #0 (Symplify\SmartFileSystem\SmartFileInfo Object (...))
tests/Issues/PrintStmtsIndexAutoImport/Fixture/fixture_request_call.php.inc
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 <?php
 
 use Illuminate\Support\Facades\Route;
-use Illuminate\Http\Request;

@samsonasik
Copy link
Member Author

rolling back FormatPerservingPrinter, it possibly the change should be in RenameClassRector

@samsonasik samsonasik marked this pull request as draft January 3, 2022 20:44
@samsonasik samsonasik marked this pull request as ready for review January 3, 2022 22:33
@samsonasik
Copy link
Member Author

Fixed 🎉 with ensure use name has \ for short name use compare check 6fd270e

@samsonasik
Copy link
Member Author

All checks have passed 🎉 @TomasVotruba it is ready for review.

@TomasVotruba
Copy link
Member

Thank you, let's ship it

@TomasVotruba TomasVotruba merged commit 37e26d5 into main Jan 3, 2022
@TomasVotruba TomasVotruba deleted the crash-index-zero-print branch January 3, 2022 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants