Automatic PHPStan extension loading in rector

[staabm]

Feature request

In https://github.com/rectorphp/rector-src/pull/4769 rector dropped automatic loading of PHPStan extensions, because it seems to much work to maintain the feature.

As of this linked PR one needs to manually register PHPExtensions to rectors' internal PHPStan , so it will utilize the additional sources of type-knowledge. Its a pity that we have the luxury of the Extension Installer for PHPStan, but don't have it anymore for rector - as manually keeping a list of extensions is a maintenance pain.

my local testing suggests, to get it running again I need to add a phpstan.neon to rector which contains a includes: config with a php file like:

<?php

declare(strict_types=1);

use PHPStan\Command\InceptionNotSuccessfulException;
use PHPStan\ExtensionInstaller\GeneratedConfig;

$additionalConfigFiles = [];

if (class_exists('PHPStan\ExtensionInstaller\GeneratedConfig')) {
    $generatedConfigReflection = new ReflectionClass('PHPStan\ExtensionInstaller\GeneratedConfig');
    $generatedConfigDirectory = dirname($generatedConfigReflection->getFileName());
    foreach (GeneratedConfig::EXTENSIONS as $name => $extensionConfig) {
        foreach ($extensionConfig['extra']['includes'] ?? [] as $includedFile) {
            if (!is_string($includedFile)) {
                $errorOutput->writeLineFormatted(sprintf('Cannot include config from package %s, expecting string file path but got %s', $name, gettype($includedFile)));
                throw new InceptionNotSuccessfulException();
            }
            $includedFilePath = null;
            if (isset($extensionConfig['relative_install_path'])) {
                $includedFilePath = sprintf('%s/%s/%s', $generatedConfigDirectory, $extensionConfig['relative_install_path'], $includedFile);
                if (!is_file($includedFilePath) || !is_readable($includedFilePath)) {
                    $includedFilePath = null;
                }
            }

            if ($includedFilePath === null) {
                $includedFilePath = sprintf('%s/%s', $extensionConfig['install_path'], $includedFile);
            }
            if (!is_file($includedFilePath) || !is_readable($includedFilePath)) {
                $errorOutput->writeLineFormatted(sprintf('Config file %s does not exist or isn\'t readable', $includedFilePath));
                throw new InceptionNotSuccessfulException();
            }
            $additionalConfigFiles[] = $includedFilePath;
        }
    }
}

return ['includes' => $additionalConfigFiles];

this means I copied the logic from CommandHelper

could we move this part into a new method, so I can call it from my php-file which gets loaded via includes: instead of duplicating the logic?

Did PHPStan help you today? Did it make you happy in any way?

No response

[ondrejmirtes]

rector dropped automatic loading of PHPStan extensions

I don't get it, why? What was the work needed to maintain it?

[staabm]

What was the work needed to maintain it?

I can't prove this point. its what was mentioned in the PR description which dropped the feature by Tomas.

in other comments, it was mentioned that loading of PHPStan extensions is not that useful, as rector is not interessting in most parts PHPStan extensions provide. rector is only interessted in general config and type inference, not rules and other stuff.

my interpretation is, that the rector project is trying to focus on what is really essential for the 1.0 release and therefore is trying to reduce as much "clutter" as possible from the rector core.

in my opinion, from a tool user DX perspective it still can make sense to let the configs be autoloaded though.

[staabm]

we are still in discussion and trying possible alternatives. I will report back after we concluded how it should/can work

[ondrejmirtes]

Would be actually nice to involve me in the discussion, I believe I'd have some useful input into it ;)

[staabm]

I would love your input.

we are currently trying to build a custom "extension loader" which only loads those phpstan extension files which are really necessary for our rector case. this only works for our own needs and will not lead to a generic solution.

what do you have in mind? :-)

[ondrejmirtes]

I need to understand what's so complex and problematic about this area before I can think of a solution. Looks like you're trying to build a nuclear plant (=really complex and unnecessary solution).

[staabm]

ok, lets go back to the drawing board

in rector its sometimes usefull to pass the phpstan config into rector e.g.

• for dynamicConstantNames so it does not remove code which compares constants against literal strings (because always true/false)
• for scanDirectory so it finds the right classes at the right path
• bootstrapFiles for custom autoloaders
• ...

I feel its usefull, because otherwise I would need to repeat this configs in separate config files for rector. especially in legacy applications where we don't have a consistent PSR autoloading it would otherwise take a lot of redundant configuration to get the symbol resolving configured properly.

rector does not care about e.g. PHPStan rules though, as it does not report errors, but is only working on the plain type inference. btw: rector does most of the time also not care about phpdoc types.. I can see a trend to native types only, because its more safe.

this means it is still helpful to register PHPStan extensions within rector which improve type inference - e.g. *ReturnTypeExtensions.

tomas has another argument that rector should not autoload PHPStan extensions automatically because some extensions require additional configuration and therefore the "PHPStan config" for rector gets more complicated then it needs to be. he therefore suggests one should manual register the PHPStan config files within rector which are really required (which leads to 2 different sets of config files).

I am not yet sure how a good solution can look like which takes all the given aspects into account.

[ondrejmirtes]

Sure, we agree that these extensions are needed for precise type inference which benefits both PHPStan and Rector.

I'm not sure in which cases Rector want these extensions disabled, what are downsides?

should not autoload PHPStan extensions automatically because some extensions require additional configuration

If that's the case these extensions usually don't do anything unless the configuration is provided. Meaning the process doesn't crash without the configuration.

Also it's in the best interest of the user, if they use both PHPStan and Rector, to share the relevant configuration between them.

So - I'm still puzzled why Rector doesn't want to auto-discover PHPStan extensions by default (if the user actually uses phpstan/extension-installer), I haven't seen yet any downside to that.

[staabm]

I don't know more then whats already mentioned in https://github.com/phpstan/extension-installer/issues/78 and https://github.com/phpstan/extension-installer/issues/78

@TomasVotruba do you have anything to add?