-
-
Notifications
You must be signed in to change notification settings - Fork 699
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
Default error handler is not helping #6418
Comments
Even with -vvv it's not able to get the real errors sometimes.
|
Thanks for reporting 👍 We'll need a minimal reproducible repository. |
I could provide something. Had the same issues some days ago. |
It´s all about clear communication. Nothing wrong but just the message could be clearer here. |
Not sure what has changed but I'm only getting Tried both |
Could you provide minimal PHP file this fails on? |
It's surprisingly hard to minimize this, I encountered it while following the guide: https://getrector.org/blog/2020/04/16/how-to-migrate-from-phpexcel-to-phpspreadsheet-with-rector-in-30-minutes Basically an exception gets thrown from https://github.com/PHPOffice/PHPExcel/blob/1.8/Classes/PHPExcel/Writer/PDF/DomPDF.php#L1-L10 Even tho I'm not using PDF outputs (but that's another issue). To recreate: php --version
PHP 7.4.3 (cli) (built: Jul 5 2021 15:13:35) ( NTS )
Copyright (c) The PHP Group
Zend Engine v3.4.0, Copyright (c) Zend Technologies
with Zend OPcache v7.4.3, Copyright (c), by Zend Technologies
mkdir test && cd test
composer init
composer require PHPOffice/PHPExcel
composer require rector/rector rector.php <?php
use Rector\Set\ValueObject\SetList;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
return static function (ContainerConfigurator $containerConfigurator): void {
$containerConfigurator->import(SetList::PHPEXCEL_TO_PHPSPREADSHEET);
}; test.php <?php
include 'vendor/autoload.php';
$obj = new \PHPExcel_Writer_Excel2007(new \PHPExcel());
$obj->setPreCalculateFormulas(true); output $ vendor/bin/rector process test.php -vvv --debug
0/1 [░░░░░░░░░░░░░░░░░░░░░░░░░░░░] 0% < 1 sec/< 1 sec 48.5 MiB[parsing] test.php
PHP Deprecated: Array and string offset access syntax with curly braces is deprecated in /mnt/c/Users/user/Desktop/a/vendor/phpoffice/phpexcel/Classes/PHPExcel/Shared/String.php on line 529
[...]
Deprecated: Array and string offset access syntax with curly braces is deprecated in /mnt/c/Users/user/Desktop/a/vendor/phpoffice/phpexcel/Classes/PHPExcel/Shared/String.php on line 537
[refactoring] test.php
[applying] Rector\Renaming\Rector\Name\RenameClassRector
[applying] Rector\PHPOffice\Rector\StaticCall\AddRemovedDefaultValuesRector
PHP Deprecated: Array and string offset access syntax with curly braces is deprecated in /mnt/c/Users/user/Desktop/a/vendor/phpoffice/phpexcel/Classes/PHPExcel/Calculation.php on line 2189
[...]
Deprecated: Array and string offset access syntax with curly braces is deprecated in /mnt/c/Users/user/Desktop/a/vendor/phpoffice/phpexcel/Classes/PHPExcel/Writer/Excel5/Worksheet.php on line 907
[post rectors] test.php
[printing skipped due error] test.php
[OK] Rector is done!
^ lots of deprecation warnings but the error isn't shown (full output). I'm not sure what or how exactly includes the file. I patched back in rectorphp/rector-src@276dd96 to see the error. Commenting out the |
Hi, this is actually something Rector cannot handle, as it's syntax issue that breaks php-parser.
This is a way to fix it, see https://gist.github.com/theodorejb/763b83a43522b0fc1755a537663b1863#gistcomment-3254515 |
I don't see how this is connected to the warnings, removing the exceptions fixed it. It runs fine with the deprecation warnings still there? vendor/bin/rector process test.php -vvv --debug
0/1 [░░░░░░░░░░░░░░░░░░░░░░░░░░░░] 0% < 1 sec/< 1 sec 48.5 MiB[parsing] test.php
PHP Deprecated: Array and string offset access syntax with curly braces is deprecated in /mnt/c/Users/user/Desktop/a/vendor/phpoffice/phpexcel/Classes/PHPExcel/Shared/String.php on line 529
[...]
Deprecated: Array and string offset access syntax with curly braces is deprecated in /mnt/c/Users/user/Desktop/a/vendor/phpoffice/phpexcel/Classes/PHPExcel/Shared/String.php on line 537
[refactoring] test.php
[applying] Rector\Renaming\Rector\Name\RenameClassRector
[applying] Rector\PHPOffice\Rector\StaticCall\AddRemovedDefaultValuesRector
PHP Deprecated: Array and string offset access syntax with curly braces is deprecated in /mnt/c/Users/user/Desktop/a/vendor/phpoffice/phpexcel/Classes/PHPExcel/Calculation.php on line 2189
[...]
Deprecated: Array and string offset access syntax with curly braces is deprecated in /mnt/c/Users/user/Desktop/a/vendor/phpoffice/phpexcel/Classes/PHPExcel/Writer/Excel5/Worksheet.php on line 907
[applying] Rector\PHPOffice\Rector\MethodCall\ChangeConditionalReturnedCellRector
[applying] Rector\PHPOffice\Rector\MethodCall\ChangeConditionalGetConditionRector
[applying] Rector\PHPOffice\Rector\MethodCall\ChangeConditionalSetConditionRector
[applying] Rector\PHPOffice\Rector\MethodCall\RemoveSetTempDirOnExcelWriterRector
[applying] Rector\PHPOffice\Rector\MethodCall\ChangeDuplicateStyleArrayToApplyFromArrayRector
[applying] Rector\PHPOffice\Rector\MethodCall\GetDefaultStyleToGetParentRector
[applying] Rector\PHPOffice\Rector\MethodCall\IncreaseColumnIndexRector
[applying] Rector\Renaming\Rector\MethodCall\RenameMethodRector
[post rectors] test.php
[print] test.php
1 file with changes
===================
1) test.php
---------- begin diff ----------
--- Original
+++ New
@@ @@
include 'vendor/autoload.php';
-$obj = new \PHPExcel_Writer_Excel2007(new \PHPExcel());
+$obj = new \PhpOffice\PhpSpreadsheet\Writer\Xlsx(new \PhpOffice\PhpSpreadsheet\Spreadsheet());
$obj->setPreCalculateFormulas(true);
----------- end diff -----------
Applied rules:
* RenameClassRector
[OK] 1 file has been changed by Rector
Lines removed: https://github.com/PHPOffice/PHPExcel/blob/1.8/Classes/PHPExcel/Writer/PDF/DomPDF.php#L8 I'm only complaining that the exceptions were thrown but were not shown on screen - makes it confusing and hard to debug. Not a major issue ofc. |
I see, you're right. Rector should be able to report this better and recover if not fatal error. Let's fix it then. Could you come up with solution that would accept PHP deprecations from code and let Rector run anyway? So it would work on your code-base without removing lines manually. |
I'm still not sure we're on the same page - sorry I couldn't provide a simpler example.
The deprecation warnings seem to be irrelevant in this case. |
I understand. It was only moved here: https://github.com/rectorphp/rector-src/blob/48ccb539d49aede106e02c5a855b51f8548f3c07/packages/ChangesReporting/Output/ConsoleOutputFormatter.php#L41 Could you verify if that line is runned in your code? It's possible this deprecation is not collected as error, while previous errors you've seen were actually thrown as exceptions. |
The line is ran but there's no errors collected. I added: /**
* @param \Rector\Core\ValueObject\ProcessResult $processResult
* @param \Rector\Core\ValueObject\Configuration $configuration
*/
public function report($processResult, $configuration) : void
{
if ($configuration->shouldShowDiffs()) {
$this->reportFileDiffs($processResult->getFileDiffs());
}
var_dump($processResult->getErrors()); // returns empty array
$this->reportErrors($processResult->getErrors());
$this->reportRemovedFilesAndNodes($processResult); Output: [...]
Deprecated: Array and string offset access syntax with curly braces is deprecated in /mnt/c/Users/user/Desktop/a/vendor/phpoffice/phpexcel/Classes/PHPExcel/Writer/Excel5/Worksheet.php on line 907
[post rectors] test.php
[printing skipped due error] test.php
array(0) {
}
[OK] Rector is done!
However adding: foreach ($file->getErrors() as $error) {
$this->symfonyStyle->error($error->getMessage());
} clearly prints the error Deprecated: Array and string offset access syntax with curly braces is deprecated in /mnt/c/Users/user/Desktop/a/vendor/phpoffice/phpexcel/Classes/PHPExcel/Writer/Excel5/Worksheet.php on line 907
[post rectors] test.php
[ERROR] Analyze error: "PHPExcel_Writer_Exception (Unable to load PDF Rendering library) thrown while looking for class
PHPExcel_Writer_PDF_DomPDF.". Include your files in "$parameters->set(Option::AUTOLOAD_PATHS, [...]);" in
"rector.php" config.
See https://github.com/rectorphp/rector#configuration
[printing skipped due error] test.php
[OK] Rector is done!
|
I see, thanks for the investigation. |
I think I can see it now: Errors were only displayed, if there was some change in file content. So if there was a fatal error → no diff → no error displayed. My bad. Could you check if this change work for you? |
Resolved by rectorphp/rector-src#489 |
rectorphp/rector-src@d2db35e [Php80] Returns null on no change on ClassPropertyAssignToConstructorPromotionRector (#6418)
carlos-granados/rector-src@f7ea3db [PHP8.3] add new rectors for get_class()/get_parent_class() without arguments (rectorphp#6405) carlos-granados/rector-src@1afc647 Rectify (rectorphp#6408) carlos-granados/rector-src@afe7c46 [DeadCode] Skip nullable @ template on RemoveUselessReturnTagRector (rectorphp#6409) carlos-granados/rector-src@293eb97 [Php81] Skip Doctrine Embeddable on ReadOnlyPropertyRector (rectorphp#6411) carlos-granados/rector-src@1b1807b [Privatization] Skip with parameter on PrivatizeLocalGetterToPropertyRector (rectorphp#6412) carlos-granados/rector-src@2c67908 Fix ClassDependencyManipulator to add dependency on right position (rectorphp#6413) carlos-granados/rector-src@29e8932 [automated] Apply Coding Standard (rectorphp#6414) carlos-granados/rector-src@3949fc2 Move instanceof PHPStan Type to ->is*() take 1 (rectorphp#6416) carlos-granados/rector-src@0cac71a [CodeQuality] Add fixture test for skip nullable array from property with default value (rectorphp#6417) carlos-granados/rector-src@d2db35e [Php80] Returns null on no change on ClassPropertyAssignToConstructorPromotionRector (rectorphp#6418) carlos-granados/rector-src@06adce3 use ->isInteger()->yes() on ReturnTypeFromStrictNewArrayRector (rectorphp#6419) carlos-granados/rector-src@bcfb598 Add ClassNameFromObjectTypeResolver to cover TypeWithClassName instance checks (rectorphp#6420) carlos-granados/rector-src@84bb596 [StaticTypeMapper] Reduce instanceof TypeWithClassName usage via ClassNameFromObjectTypeResolver (rectorphp#6421) carlos-granados/rector-src@26b8b54 [Naming] Reduce instanceof ObjectType on ExpectedNameResolver (take 5) (rectorphp#6423) carlos-granados/rector-src@5baf487 Require-dev phpstan/phpstan-deprecation-rules to show list of deprecated (rectorphp#6415) carlos-granados/rector-src@c23ba82 [deprecation] Remove deprecated PublicConstantVisibilityRector, cleanup deprecated SetListInterface (rectorphp#6424) carlos-granados/rector-src@c2053c7 [deprecation] Deprecate AbstractScopeAwareRector in favor of single AbstractRector (rectorphp#6425) carlos-granados/rector-src@2c5cd97 [deprecated] Remove deprecated AbstractTestCase (rectorphp#6426) carlos-granados/rector-src@fb3c33a Merge commit '2c5cd97a3a98ab5af9efb846f6f78738e4a195a6'
Bug Report
Minimal PHP Code Causing Issue
Expected Behaviour
See the error and not a green OK. Maybe change it to warning with an help text to use -vvv
The text was updated successfully, but these errors were encountered: