Skip to content

Make use of static reflection from PHPStan#5665

Merged
TomasVotruba merged 21 commits intomasterfrom
static-reflection-config
Feb 28, 2021
Merged

Make use of static reflection from PHPStan#5665
TomasVotruba merged 21 commits intomasterfrom
static-reflection-config

Conversation

@TomasVotruba
Copy link
Copy Markdown
Member

@TomasVotruba TomasVotruba commented Feb 23, 2021

Closes #3490

Now Rector autoloads all the code that analyses. That means all your projects has to be loaded with composer, custom autoload or included. That can lead to side-effects like these:

// in Rector
include __DIR__ . '/some_file.php';
// some_file.php
mysql_connect('...');

Fatal error: canont connect to mysql

The only way to avoid this is to use static reflections. That means the code is parsed, so Rector knows about mysql_connect function call, but it is not included and does not produce side effects. Win win 🎉


Todo

  • break rename test without autoload
  • replace ClassExistenceStaticHelper::doesClassLikeExists() with ReflectionProvider->hasClass()
  • replace class_exists() with ReflectionProvider->hasClass()
  • fix AnnotationReader to work with native reflections

SourceLocatorFactory

Tests

  • make test pass again
  • make test pass without autoload
  • replace is_a(), instanceof, $objectType->hasMethod(), $objectType->hasProperty() with ClassReflection API

@TomasVotruba TomasVotruba force-pushed the static-reflection-config branch 2 times, most recently from 3f80faa to c3c3084 Compare February 23, 2021 17:18
@TomasVotruba TomasVotruba force-pushed the static-reflection-config branch 6 times, most recently from 2f3a909 to 93672f4 Compare February 23, 2021 21:22
@TomasVotruba TomasVotruba changed the title [WIP] Make use of static reflectoin [WIP] Make use of static reflection Feb 23, 2021
@TomasVotruba TomasVotruba force-pushed the static-reflection-config branch 2 times, most recently from f036a30 to d8bd4e3 Compare February 23, 2021 23:40
@TomasVotruba TomasVotruba changed the title [WIP] Make use of static reflection [WIP] Make use of static reflection from PHPStan Feb 23, 2021
@TomasVotruba TomasVotruba force-pushed the static-reflection-config branch 4 times, most recently from 4c9a89e to 0885286 Compare February 24, 2021 00:41
@TomasVotruba TomasVotruba force-pushed the static-reflection-config branch 9 times, most recently from ad3c3dc to b9cddc0 Compare February 25, 2021 01:21
@TomasVotruba
Copy link
Copy Markdown
Member Author

Is this solved by your contribution?

@mpdude
Copy link
Copy Markdown
Contributor

mpdude commented Mar 4, 2021

@TomasVotruba #5772 seems to solve the issue I had with running a non-scoped Rector (from Docker) on a Symfony 2.3 project (a comment I initially left here but moved to the PR).

But could you review my question above?

If it is right what I am writing, we could update the Docker build process to only create PHP 8.0 images and to omit the scoping step... /cc @JanMikes

@TomasVotruba
Copy link
Copy Markdown
Member Author

But could you review my question above?

I got lost in the long text. Could you narrow it down what exactly is your issue?

@mpdude
Copy link
Copy Markdown
Contributor

mpdude commented Mar 5, 2021

Under which conditions would Rector still use dynamic reflection, so that code would have to be loaded/parsed by the running PHP interpreter and would possibly end up as a declared class/function/... in the current process?

(Maybe also @ondrejmirtes could answer this?)

@ondrejmirtes
Copy link
Copy Markdown
Contributor

It depends on how the ReflectionProvider instance looks like in Rector. In PHPStan by default it's a chain of various ReflectionProvider implementations. In layman's terms, if a class is reachable through a correctly configured Composer autoloader, a runtime reflection will be used. Otherwise static reflection is used.

@TomasVotruba
Copy link
Copy Markdown
Member Author

Rector is using the same ReflectionProvider service, and adding files and directories from rector.php and paths arguments from vendor/bin/rector process src to SourceLocator.

@mpdude
Copy link
Copy Markdown
Contributor

mpdude commented Mar 5, 2021

(How) can I run Rector with a version of PHPStan that is not packed up as a Phar?

@ondrejmirtes
Copy link
Copy Markdown
Contributor

Nope, PHPStan is only distributed as PHAR. What are your needs?

@mpdude
Copy link
Copy Markdown
Contributor

mpdude commented Mar 5, 2021

Debug into Rector, see when/how it uses ReflectionProvider and subclasses, being able to tinker around with that code to understand if/when/how/why runtime reflection comes into play.

(To understand when/why scoping Rector is needed, and to understand when/why Rector needs to run with the same PHP version as the project it analyzes.)

@ondrejmirtes
Copy link
Copy Markdown
Contributor

Xdebug works even with PHARs.

@mpdude
Copy link
Copy Markdown
Contributor

mpdude commented Mar 5, 2021

... but changing code (commenting out things etc) does not.

@mpdude
Copy link
Copy Markdown
Contributor

mpdude commented Mar 5, 2021

But I'll take this as "there is no easy way to install a non-Phar source version of PHPStan" and deal with it 😉

@ondrejmirtes
Copy link
Copy Markdown
Contributor

You can always clone phpstan-src alongside a project and run plain PHPStan using ../phpstan-src/phpstan analyse ... on a codebase, but not in combination with Rector.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Taking advantage of static reflection

3 participants