-
-
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
Document current limitations #5678
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
# Current Limitations and Installation Alternatives | ||
|
||
At least until [#3490](https://github.com/rectorphp/rector/issues/3490) is resolved, Rector relies on _runtime PHP reflection_ to analyze your source code. That means the code Rector works on cannot merely be treated as _data_, but has to be loaded (through the autoloader) into memory to be analyzed by the PHP interpreter. | ||
|
||
> Read the [PHPStan 0.12.26 release announcement](https://phpstan.org/blog/zero-config-analysis-with-static-reflection) to learn more about runtime and static reflection. PHPStan is the static analysis tool used by Rector. | ||
|
||
This brings a few limitations and affects your choices of how you can run Rector. | ||
|
||
## PHP Language Level | ||
|
||
The PHP process that runs Rector code also needs to load and parse your source code. So, you need to run a version of PHP that is both compatible with your project code as well as Rector. | ||
|
||
This means you cannot run Rector on PHP 8 to work on a project that uses language features limited to previous PHP versions. | ||
|
||
> One example for this is [array and string offset access using curly braces](https://www.php.net/manual/de/migration74.deprecated.php#migration74.deprecated.core.array-string-access-curly-brace). | ||
|
||
But also the other way round, you cannot run Rector on PHP 7 to analyze a project with PHP 8 language features, like [constructor property promotion](https://stitcher.io/blog/constructor-promotion-in-php-8). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That should be doable, as promoted properties were added to php-parsed for PHP 7 too There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't use Docker. Could we somehow verify it in a GitHub Action to be sure and have future validation for new php-parser release? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rectorphp/getrector-com#269 contains an example of PHP 8 code with constructor property promotion that could not be processed with the PHP 7.3 Docker image, and so the Docker image was upgraded to use PHP 8 in #5536. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. I forgot to add following PR must be merged first, as code is now included manually #5665 |
||
|
||
Together with Rector's minimum PHP 7.3 version requirement, this might limit your options of how to run Rector. | ||
|
||
Note that using an older version of Rector might allow you to run a version of PHP <7.3. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That cannot be recommended, as that is older version with many missing features and invalid config loading. That could lead to random report on already fixed bugs. Docker with PHP 7.1 downgraded and prefixed Rector should be single solution for all non-composer installs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For example, with PHP 7.2, you will simply get So, if you really want to encourage people not to work with older versions (rather give Docker a try), we should state that more clearly. By the way, what is "PHP 7.1 downgraded"? I just found https://github.com/rectorphp/rector-php71, but as far as I can see, its mentioned nowhere in the README or There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is current state, yes. But that's wrong. Our goal is that this command: composer require --dev rector/rector Installs 0.9.x even for PHP 7.1 and also for version conflicts. |
||
|
||
## Code Collisions | ||
|
||
Since Rector needs to load the code it works on in addition to the code it needs to run itself, conflicts may occur. | ||
|
||
One practical example of this is when you're running Rector on a project that uses an older version of Symfony. Since Rector uses a few Symfony Components itself, you may either run into problems when classes are re-declared, or when Rector happens to load and later (by accident) run parts of Symfony from your project installation. In the later case, the mix-up of different versions may lead to generally unpredictable results. | ||
|
||
To remedy this situation, [Rector Prefixed](https://github.com/rectorphp/rector-prefixed) has been created. Basically, it takes Rector and all of its dependencies and moves _all_ that code under a unique namespace prefix. | ||
|
||
(Add a note here: Drawbacks? Why isn't Rector-prefixed _the_ recommended approach?) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Anyone? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's meta information users don't need. See PHPStan for example, the release produces such version. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am afraid I cannot make sense of your comment in this context 😞. Is this referring to just why Rector-prefixed is not the primary recommended approach? (What are the drawbacks of using it?) Or are you suggesting to remove the explanation what Rector-prefixed is about at all? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
👍 Unless Docker, the install command for Rector should be always the same: composer install rector/rector Composer itself should resolve if the version is prefixed (because of conflicts), or PHP 7.1 (because of lower PHP version). We should handle the release and tagging, instead of telling user the information mentioned above. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, I'll have to let that sink in a bit... I've tried such thing (offer several versions with "smart" tags/versions, let Composer find the "best" one) with internal packages in the past, and it turned out not to be possible in all cases, required a lot of effort to get it right and resulted in unintuitive version numbers (not what you'd expect from SemVer). Not to say it's not doable, just ... But back to the initial question – what's the drawback of using |
||
|
||
## Installation Options | ||
|
||
With these limitations in mind, here are your installation options. | ||
|
||
### 1. Direct Composer install | ||
|
||
```bash | ||
composer require rector/rector --dev | ||
``` | ||
|
||
Composer will try to find a version of Rector compatible with your current version of PHP. Also, since it will resolve a _single_ version for every dependency your project and Rector have in common, code collisions will not occur. | ||
|
||
The downside is that having Rector installed this way might affect the dependency versions your project will be able to use. | ||
|
||
### 2. Use Composer to install Rector Prefixed | ||
|
||
```bash | ||
composer require rector/rector-prefixed --dev | ||
``` | ||
|
||
Since Rector Prefixed brings along most of the code it needs in a separate namespace, the impact and resulting limitations for your project's requirements are minimal. | ||
|
||
As of writing, the only additional dependency requirement is on `phpstan/phpstan`. But apart from that, Composer should be able to install it with whatever packages your project requires. | ||
mpdude marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
If you need Rector `^0.9.0`, however, you need to run at least PHP 7.3. | ||
|
||
### 3. Use Docker to run Rector | ||
|
||
When you're using a different version of PHP than the desired version of Rector supports, you can run one of the Docker images we provide. | ||
|
||
See the [documentation on running in Docker](/docs/how_to_run_rector_in_docker.md) on how to do this. | ||
|
||
There is one caveat, though: As of writing, the [official Rector Docker images](https://hub.docker.com/r/rector/rector/tags?page=1&ordering=last_updated) are built with PHP 8.0 and use the non-prefixed version of Rector. | ||
|
||
So, you might run into problems with these images when your code uses language features not compatible with PHP 8, or when there is a collision between your project's and Rector's dependencies. | ||
|
||
[Work is underway](https://github.com/rectorphp/rector/pull/5667) to improve this situation and to provide Docker images with alternative PHP versions and/or a prefixed version of Rector. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be resolved before adding this docs There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, you decide... but that's the state of affairs right now, so it might help people to understand the situation and we can remove this any time the corresponding PR is merged. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm affraid this would create much more ways to install Rector and add more salt to the injure.
Anything else is wrong on our side and putting burden on the users. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nonwithstanding what I said above, I agree. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be resolved before adding this docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above.