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

The "--set" option does not exist. #7162

Closed
Dragon321123 opened this issue May 12, 2022 · 18 comments
Closed

The "--set" option does not exist. #7162

Dragon321123 opened this issue May 12, 2022 · 18 comments

Comments

@Dragon321123
Copy link

I'm trying to move from the library to phpexcel on phpspreadsheet.
vendor/bin/rector process /src/ --set phpexcel-to-phpspreadsheet
Gives an error message 'The "--set" option does not exist.'
I understood that you turned off the option "--set".
But I did not find which option should be added in
$rectorConfig->sets([ SetList::CODE_QUALITY ]);

@samsonasik
Copy link
Member

You can define:

use Rector\PHPOffice\Set\PHPOfficeSetList;
use Rector\Config\RectorConfig;

return static function (RectorConfig $rectorConfig): void {
    $rectorConfig->sets([
        PHPOfficeSetList::PHPEXCEL_TO_PHPSPREADSHEET
    ]);
};

ref https://github.com/rectorphp/rector-phpoffice#use-sets

@TomasVotruba
Copy link
Member

Hi, try to type hint SetList in the rector.php. It will show you all available classes.

Peek 2022-05-12 19-38

@linaori
Copy link

linaori commented May 31, 2022

I'm running into the same issue. I've got that specific config in rector.php:

<?php

declare(strict_types=1);

use Rector\Config\RectorConfig;
use Rector\PHPOffice\Set\PHPOfficeSetList;

return static function (RectorConfig $rectorConfig): void {
    $rectorConfig->sets([
        PHPOfficeSetList::PHPEXCEL_TO_PHPSPREADSHEET
    ]);
};

When I then run vendor/rector/rector/bin/rector process src --set phpexcel-to-phpspreadsheet I get: The "--set" option does not exist. This is a fresh rector install, and I've done the init.

In addition to that it seems to process rules I don't want it to process. I only want it to process the phpexcel -> spreadsheet changes, but it's touching things like unused arguments. Yes I need to fix them, but another time.

@TomasVotruba
Copy link
Member

The "--set" option does not exist.

This is expected. This option does now exist anymore. It was removed ~1,5 year ago, when we moved to config sets() method.

@samsonasik
Copy link
Member

Unused argument removal is depends on version you're using, which on purpose due to different version usage, see #7072

@linaori
Copy link

linaori commented May 31, 2022

Different guides saying different things, joy :(

Any idea how to limit it to just that one list? It's making thousands of changes I did not configure.

Example

Applied rules:
 * AddRemovedDefaultValuesRector (https://github.com/PHPOffice/PhpSpreadsheet/commit/033a4bdad56340795a5bf7ec3c8a2fde005cda24 https://github.com/PHPOffice/PhpSpreadsheet/blob/master/docs/topics/migration-from-PHPExcel.md#removed-default-values)
 * ChangePdfWriterRector (https://github.com/PHPOffice/PhpSpreadsheet/blob/master/docs/topics/migration-from-PHPExcel.md#writing-pdf)
 * RemoveExtraParametersRector (https://www.reddit.com/r/PHP/comments/a1ie7g/is_there_a_linter_for_argumentcounterror_for_php/)

@TomasVotruba
Copy link
Member

TomasVotruba commented May 31, 2022

Different guides saying different things, joy :(

Where is the --set option mentioned? We should update it.

@linaori
Copy link

linaori commented May 31, 2022

I came here following directions from the spreadsheet repository, I don't think I've found any guides that reference it incorrectly, just differently:
PHPOffice/PhpSpreadsheet#1445
https://phpspreadsheet.readthedocs.io/en/latest/topics/migration-from-PHPExcel/

@TomasVotruba
Copy link
Member

@linaori Thanks!
I've send update PR and updated the issue: https://github.com/PHPOffice/PhpSpreadsheet/pull/2861/files

@linaori
Copy link

linaori commented May 31, 2022

Awesome, thanks!

I've been trying to figure out how to set the configuration to only run the phpspreadsheet rules, and found this: https://github.com/rectorphp/rector/blob/main/docs/how_to_ignore_rule_or_paths.md

But I can't find the inverse of this. Is there no way to define "only PHPOfficeSetList::PHPEXCEL_TO_PHPSPREADSHEET"?

@TomasVotruba
Copy link
Member

TomasVotruba commented May 31, 2022

Thank you 👍

@linaori You've done correct way already: #7162 (comment)
Only the sets that you include in the config are run. I can see just 1 set and it's correct one.

The set is group of rules. If you want to run only 1 rule from the set, you can cherry pick them manually from the set:
https://github.com/rectorphp/rector-phpoffice/blob/e00fa9187605c28af79a0ac6c4b13080363ed24e/config/sets/phpexcel-to-phpspreadsheet.php#L30-L306

@linaori
Copy link

linaori commented May 31, 2022

Oh, a bit odd that this specific set is doing more than just what you'd expect to be fixing. Would it make sense to reduce this to the bare minimum required for the migration? Not arguing that these changes aren't good, but if I run this over my code-base I get thousands of changes which will break things 😅

Thanks for the pointers, I'll be cherry picking the rules and will try again!

edit:
Okay running into some hickups:

 [ERROR] Could not process                                                                                              
         "/var/www/html/vendor/rector/rector/vendor/symplify/easy-parallel/src/ValueObject/ParallelProcess
         .php" file, due to:                                                                                            
         "Child process timed out after 120 seconds". On line: 103   

@TomasVotruba
Copy link
Member

TomasVotruba commented May 31, 2022

Could you be more specific about "a bit odd that this specific set is doing more than just what you'd expect to be fixing"?
Reproducible repository would be the best.

It should handle the sheet/excel upgrade only, so we're on the same page :)

@samsonasik
Copy link
Member

You can increase timeout seconds with something like this:

$rectorConfig->parallel(200);

for very large codebase.

@linaori
Copy link

linaori commented May 31, 2022

Here's an example where I run in a directory where I don't have a single reference to PHPExcel for example:

1) src/Domain/Stock/Reservation/RevokeStockReservationHandler.php:118

    ---------- begin diff ----------
@@ @@

     private static function logInfo($message, $context = []): void
     {
-        Logger::getLogger(static::class)->getMonolog()->info($message, $context);
+        Logger::getLogger()->getMonolog()->info($message, $context);
     }
 }
    ----------- end diff -----------

Applied rules:
 * AddRemovedDefaultValuesRector (https://github.com/PHPOffice/PhpSpreadsheet/commit/033a4bdad56340795a5bf7ec3c8a2fde005cda24 https://github.com/PHPOffice/PhpSpreadsheet/blob/master/docs/topics/migration-from-PHPExcel.md#removed-default-values)
 * ChangePdfWriterRector (https://github.com/PHPOffice/PhpSpreadsheet/blob/master/docs/topics/migration-from-PHPExcel.md#writing-pdf)
 * RemoveExtraParametersRector (https://www.reddit.com/r/PHP/comments/a1ie7g/is_there_a_linter_for_argumentcounterror_for_php/)

I'm also experiencing some serious memory issues, every time I run the script, it crashes my chrome and intellij 😂

@TomasVotruba
Copy link
Member

That's probably job of RemoveExtraParametersRector. The only generic rule. It was meant to be to remove unneded parameters in a smart way, but I see it's making more problems.

Skip it in ->skip() option.

I'm also experiencing some serious memory issues, every time I run the script, it crashes my chrome and intellij joy

That could be caused by something like mpdf. Try to narrow refactoring to narrow scope.

@linaori
Copy link

linaori commented May 31, 2022

Yeah I think I'm going to set up a limit to only process the files that contain a reference to PHPExcel, because this codebase is an absolute mess with embedded dependencies, thanks again for the pointers!

@samsonasik
Copy link
Member

I created PR rectorphp/rector-phpoffice#18 to ensure show "Applied rules" only when changed for

  • AddRemovedDefaultValuesRector
  • ChangePdfWriterRecto

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants