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

Feedback from using rector for the first time #1195

Closed
enumag opened this issue Mar 13, 2019 · 4 comments
Closed

Feedback from using rector for the first time #1195

enumag opened this issue Mar 13, 2019 · 4 comments

Comments

@enumag
Copy link
Contributor

enumag commented Mar 13, 2019

I have an application with some outdated dependencies and found out I need to update Twig to namespaces because Roave/SecurityAdvisories won't allow anything below Twig 2.7 (because of this vulnerability) which throws deprections when using Twig_ classes. So I thought I'd solve it easily with Rector.

Since you're generally trying to make your packages as easy to use as possible I think you might be interested in the problems I ran into. For Rector in particular ease of use with outdated application is high priority in my opinion. If you're not interested in this feedback, feel free to close the issue.


Looking at the installation section in readme there seem to be 3 ways:

  • composer require rector/rector --dev
  • composer require rector/rector-prefixed --dev
  • docker run -v $(pwd):/project rector/rector:latest ...

Long story short, none worked for me. It seems Rector is not so easy to use as I expected after all.

Normal installation

$ composer require rector/rector --dev
Using version ^0.4.4 for rector/rector
./composer.json has been updated
Gathering patches for root package.
Loading composer repositories with package information
Updating dependencies (including require-dev)
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Installation request for rector/rector ^0.4.4 -> satisfiable by rector/rector[v0.4.4].
    - rector/rector v0.4.4 requires phpstan/phpstan ^0.11 -> satisfiable by phpstan/phpstan[0.11, 0.11.1, 0.11.2, 0.11.3, 0.11.x-dev] but these conflict with your requirements or minimum-stability.


Installation failed, reverting ./composer.json to its original content.

I fully expected a conflict with the normal version of course since some of my dependencies are outdated.

Prefixed version

$ composer require rector/rector-prefixed --dev
Using version ^0.3.4 for rector/rector-prefixed
./composer.json has been updated
Gathering patches for root package.
Loading composer repositories with package information
Updating dependencies (including require-dev)
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Installation request for rector/rector-prefixed ^0.3.4 -> satisfiable by rector/rector-prefixed[v0.3.4].
    - rector/rector-prefixed v0.3.4 requires phpstan/phpstan ^0.10.2 -> satisfiable by phpstan/phpstan[0.10.2, 0.10.3, 0.10.4, 0.10.5, 0.10.6, 0.10.7, 0.10.8, 0.10.x-dev] but these conflict with your requirements or minimum-stability.


Installation failed, reverting ./composer.json to its original content.

I don't really understand why the prefixed version requires a non-prefixed PHPStan. For the record my application is using PHPStan 0.8. I've updated it on the dev branch of the project already, however I need to use Rector on the stable branch where it is not updated (and won't be).

Docker image

Running this caused a lot of errors to appear which I have 0 idea what they mean or how I can fix them. Maybe it's again caused by some incompatibilities? Here is only a small subset of the errors.

$ docker run -v $(pwd):/project rector/rector:latest process /project --level underscore-to-namespace --dry-run | head -n 50
Rector No version set (parsed as 1.0.0)@

     0/13764 [░░░░░░░░░░░░░░░░░░░░░░░░░░░░]   0%
  1376/13764 [▓▓░░░░░░░░░░░░░░░░░░░░░░░░░░]   9%
  2752/13764 [▓▓▓▓▓░░░░░░░░░░░░░░░░░░░░░░░]  19%
  4128/13764 [▓▓▓▓▓▓▓▓░░░░░░░░░░░░░░░░░░░░]  29%
  5504/13764 [▓▓▓▓▓▓▓▓▓▓▓░░░░░░░░░░░░░░░░░]  39%
  6880/13764 [▓▓▓▓▓▓▓▓▓▓▓▓▓░░░░░░░░░░░░░░░]  49%
  8256/13764 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓░░░░░░░░░░░░]  59%
  9632/13764 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓░░░░░░░░░]  69%
 11008/13764 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓░░░░░░]  79%
 12384/13764 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓░░░]  89%
 13760/13764 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓░]  99%
 13764/13764 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 [ERROR] Could not process "/project/app/AppCache.php" file, due to:            
         "Call to a member function isSpecialClassName() on string".            

 [ERROR] Could not process "/project/app/AppKernel.php" file, due to:           
         "Call to a member function isSpecialClassName() on string".            

 [ERROR] Could not process "/project/app/SymfonyRequirements.php" file, due to: 
         "Call to undefined method PhpParser\Node\Name::isSpecialClassName()".  

 [ERROR] Could not process "/project/app/autoload.php" file, due to:            
         "Call to a member function isSpecialClassName() on string".            

 [ERROR] Could not process                                                      
         "/project/app/cache/dev/ContainerDz5k0e4/AuthenticatedTokenHandler_e367
         7d3.php" file, due to:                                                 
         "Call to undefined method PhpParser\Node\Name::toLowerString()".
@TomasVotruba
Copy link
Member

TomasVotruba commented Mar 13, 2019

Thank you for your feedback. I'm interested!

This is basically shim issue. E.g. your code uses php-parser 3.0, but Rector uses php-parser 4.0. It can have 2 versions of 1 class, e.g. with extra typehint → error.

See identical issues: https://github.com/rectorphp/rector/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc+label%3Awaits-on-prefixed

This can be solved only by prefixed Rector. There is repository rector/compiler that will handle at, it's just WIP for lack of time. Check this PR deprecated-packages/compiler#5 if you want to help.

@enumag
Copy link
Contributor Author

enumag commented Mar 13, 2019

Thanks for explaining!

It is not quite clear what needs to be done in rectorphp/compiler to fix it. But I don't have time to work on it right now anyway.

One thing I'm unsure about: Is the docker image using prefixed or non-prefixed version? In my opinion the ideal way to use rector would be dockerized prefixed version.

Anyway prefixed version of rector seems like a necessity to me. Most applications that could benefit from using rector are very likely to have conflicts with the normal version in my opinion.

@TomasVotruba
Copy link
Member

TomasVotruba commented Mar 13, 2019

It is not quite clear what needs to be done in rectorphp/compiler to fix it.

Me neither, it's not an easy task.

One thing I'm unsure about: Is the docker image using prefixed or non-prefixed version? In my opinion the ideal way to use rector would be dockerized prefixed version.

Prefixed version of course, when it's ready.

Anyway prefixed version of rector seems like a necessity to me. Most applications that could benefit from using rector are very likely to have conflicts with the normal version in my opinion.

Agreed 👍

@TomasVotruba
Copy link
Member

Closing as answered and duplicate of #985

Any PR to improve docs and compiler are welcomed 👍

TomasVotruba added a commit that referenced this issue Nov 9, 2021
rectorphp/rector-src@3747b9e [CI] Add generate changelog helper command (#1195)
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

2 participants