Document current limitations #5678

mpdude · 2021-02-25T08:51:24Z

There are some current limitations on how Rector can be used that made me struggle as a first-time user (see #5593).

Here is an attempt to write them up in a way that I think would have saved me some problems on my first attempts.

Please review them carefully to make sure the technical aspects are correct – my understanding is still limited.

I'd say this closes #5593.

docs/limitations.md

mpdude · 2021-02-26T06:44:07Z

docs/limitations.md

+
+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?)


That's meta information users don't need. See PHPStan for example, the release produces such version.
It should be as part of Rector release too. Fully downgraded and prefixed version, e.g. under sub tag, 0.9.50.71

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?

Or are you suggesting to remove the explanation what Rector-prefixed is about at all?

👍

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.

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 -prefixed, at least for the average user? Is it just that it is less tested, does it bring other problems (for users, not developers)?

TomasVotruba · 2021-02-26T09:49:42Z

docs/limitations.md

+
+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.


This should be resolved before adding this docs

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.

I'm affraid this would create much more ways to install Rector and add more salt to the injure.
Our goal should be the other ways around:

have 1 way to install with composer

have 1 way to install with Docker

Anything else is wrong on our side and putting burden on the users.

Nonwithstanding what I said above, I agree.

TomasVotruba · 2021-02-26T09:49:49Z

docs/limitations.md

+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.


This should be resolved before adding this docs

TomasVotruba · 2021-02-26T09:50:34Z

docs/limitations.md

+
+> 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).


That should be doable, as promoted properties were added to php-parsed for PHP 7 too

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?

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.

I see. I forgot to add following PR must be merged first, as code is now included manually #5665

TomasVotruba · 2021-02-26T09:52:16Z

docs/limitations.md

+
+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.


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.

composer require --dev rector/rector is the recommended, primary approach and the only one written directly in the README. This will – without further notice or warning – install an older version of Rector if only that meets your (platform) requirements.

For example, with PHP 7.2, you will simply get 0.8.56.

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 docs.

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.
The rector-prefixed and rector-php71 won't exist as they do now, but only as subtag of 0.9.x-y

mpdude · 2021-02-26T15:29:17Z

We've had some interesting exchanges in the annotations/comments above, thanks for that 🙏🏻.

I guess the question now is:

Do you want to resolve other pending issues first, and then come back here and see how much of the things still apply? I'm fine with putting this PR on the back burner for the time being.

Or would it be fine to finalize this here, and maybe come back later for updates when the issues are resolved? Of course, things that are just plain wrong (technically) should be fixed.

Keep in mind that most of what's documented here is meant as an additional, background information. It's the information I wish I had known when I ran into #5593 as a first-time user.

mpdude · 2021-03-11T06:49:58Z

#5784 shows that the issues described here have been resolved, so I am closing this.

TomasVotruba · 2021-03-11T10:58:41Z

Thank you 👍

rectorphp/rector-src@c9fe3ca [Transform] Allow transform phpVersion() to withPhpVersion() on RectorConfigBuilderRector (#5678)

mpdude force-pushed the document-limitations branch from 456d2d5 to 68248fb Compare February 25, 2021 08:53

Document current limitations

b27c4b9

mpdude force-pushed the document-limitations branch from 68248fb to b27c4b9 Compare February 25, 2021 08:54

samsonasik reviewed Feb 26, 2021

View reviewed changes

docs/limitations.md Show resolved Hide resolved

mpdude commented Feb 26, 2021

View reviewed changes

TomasVotruba reviewed Feb 26, 2021

View reviewed changes

This was referenced Mar 3, 2021

How does Rector interfere/require with my project's libraries and/or PHP version? #5593

Closed

Add first "system level" (full-scale integration) tests #5784

Closed

mpdude closed this Mar 11, 2021

TomasVotruba added a commit that referenced this pull request Mar 3, 2024

Updated Rector to commit c9fe3caacced982e1c21d7203e9ea0f1bb2092c8

6986e1f

rectorphp/rector-src@c9fe3ca [Transform] Allow transform phpVersion() to withPhpVersion() on RectorConfigBuilderRector (#5678)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Document current limitations #5678

Document current limitations #5678

mpdude commented Feb 25, 2021 •

edited

mpdude Feb 26, 2021

TomasVotruba Feb 26, 2021

mpdude Feb 26, 2021

TomasVotruba Feb 26, 2021

mpdude Feb 26, 2021

TomasVotruba Feb 26, 2021

mpdude Feb 26, 2021

TomasVotruba Feb 26, 2021

mpdude Feb 26, 2021 •

edited

TomasVotruba Feb 26, 2021

mpdude Feb 26, 2021

TomasVotruba Feb 26, 2021

TomasVotruba Feb 26, 2021 •

edited

mpdude Feb 26, 2021 •

edited

TomasVotruba Feb 26, 2021 •

edited

TomasVotruba Feb 26, 2021 •

edited

mpdude Feb 26, 2021 •

edited

TomasVotruba Feb 26, 2021 •

edited

mpdude commented Feb 26, 2021

mpdude commented Mar 11, 2021

TomasVotruba commented Mar 11, 2021


		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?)


		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.


		> 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).


		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.

Document current limitations #5678

Document current limitations #5678

Conversation

mpdude commented Feb 25, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpdude Feb 26, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomasVotruba Feb 26, 2021 • edited

Choose a reason for hiding this comment

mpdude Feb 26, 2021 • edited

Choose a reason for hiding this comment

TomasVotruba Feb 26, 2021 • edited

Choose a reason for hiding this comment

TomasVotruba Feb 26, 2021 • edited

Choose a reason for hiding this comment

mpdude Feb 26, 2021 • edited

Choose a reason for hiding this comment

TomasVotruba Feb 26, 2021 • edited

Choose a reason for hiding this comment

mpdude commented Feb 26, 2021

mpdude commented Mar 11, 2021

TomasVotruba commented Mar 11, 2021

mpdude commented Feb 25, 2021 •

edited

mpdude Feb 26, 2021 •

edited

TomasVotruba Feb 26, 2021 •

edited

mpdude Feb 26, 2021 •

edited

TomasVotruba Feb 26, 2021 •

edited

TomasVotruba Feb 26, 2021 •

edited

mpdude Feb 26, 2021 •

edited

TomasVotruba Feb 26, 2021 •

edited