Not adding prefixes to names after use statements

[marekdedic]

Closes #456

I am new to the codebase and it isn't exactly simple, so please check if this breaks anything...

[marekdedic]

Ok, so I'm going through the failing unit tests and fixing them, but there are a few cases that need comments:

• e2e-032: I'm honestly confused by what this one is supposed to do - I just added an exception for Isolated\Symfony\Component\Finder\Finder...

[marekdedic]

I think this is ready. Please review the changes. I'd focus on stuff around whitelisting as I am still a little fuzzy about the details of how it's supposed to work. And also e2e test 32 where I had to put in a special workaround - I'm not really sure whether there isn't done bigger underlying issue I'm missing completely or is it's just caused by using php-scoper on itself.

BTW all the tests really helped to get this into shape, thanks for being diligent about them!

[theofidry]

Sorry for the tardiness @marekdedic, I'll try to check it out ASAP

[marekdedic]

Thanks, don't worry, I know what it's like with volunteer-run projects 🙂

[theofidry]

I did not bother with a Blackfire profile but I can't see any notable speed difference so LGTM, thanks a lot for the impressive work!

I'd focus on stuff around whitelisting as I am still a little fuzzy about the details of how it's supposed to work.

Regarding the whitelisting it's something I would like to change as the terminology is both deceptive and inappropriate (without going too far into diversity stuff I am not too fond of the whitelist/blacklist terminology).

Ideally I would like to come with terms that can properly express that in one case the symbols are left unchanged (e.g. for "whitelist-files" the files are not changed at all) and in the other they are still scoped but an alias is added to allow using its non-scoped symbol (more details on this in https://github.com/humbug/php-scoper#implementation-insights)

And also e2e test 32 where I had to put in a special workaround - I'm not really sure whether there isn't done bigger underlying issue I'm missing completely or is it's just caused by using php-scoper on itself.

I am a bit confused by that one as well. In case this is unclear:

• PHP-Scoper ships with a Finder which is scoped
• Since PHP-Scoper loads directly the PHP config file, although there is other ways, in order to leverage the Finder without requiring the user to autoload special code PHP-Scoper exposes its finder (see bin/php-scoper:)

// Exposes the finder used by PHP-Scoper PHAR to allow its usage in the configuration file.
if (false === class_exists(IsolatedFinder::class)) {
    class_alias(Finder::class, IsolatedFinder::class);
}

However the above would still be scoped and to avoid that I am relying on a patcher in the config:

        static function (string $filePath, string $prefix, string $contents): string {
            if ('bin/php-scoper' !== $filePath) {
                return $contents;
            }

            return str_replace(
                '\\'.$prefix.'\Isolated\Symfony\Component\Finder\Finder::class',
                '\Isolated\Symfony\Component\Finder\Finder::class',
                $contents
            );
        },

The fixture 032 is to make sure this process works, i.e. that the isolated finder is exposed and usable.

BTW all the tests really helped to get this into shape, thanks for being diligent about them!

Glad they helped out, they are annoying to write and a bit slow but hopefully they pay for themselves

[marekdedic]

I agree that the term "whitelisted" can be really confusing for this use. If I may, I'd suggest ignored/excluded for files that are completely untouched. For the other use, maybe something like "add-alias-to-original"?

I fixed both your suggestions, thank you for them. Is there anything more to do on my part? (e.g. regarding the Finder or whitelisting?)

[theofidry]

I'd suggest ignored/excluded for files that are completely untouched. For the other use, maybe something like "add-alias-to-original"?

I think those are good suggestions

I fixed both your suggestions, thank you for them. Is there anything more to do on my part? (e.g. regarding the Finder or whitelisting?)

I think the Finder can wait; it's all good I just wanted to do a patch release before merging this :)

[theofidry]

Thanks a lot @marekdedic

[TomasVotruba]

Thank you @marekdedic 👍

[TomasVotruba]

@theofidry Hi Theo, this feature would be very helpful with solving #440 (comment)

Too bad patch release was before this and not after :D
Any ETA on phar release?

[TomasVotruba]

@theofidry Thank you for such a fast action 👏
I'm gonna try it on Rector right now

[TomasVotruba]

I tried the new released and it crashed previously working Rector build with 0.14.0

It seems it still prefixed short single namespaces:
https://github.com/rectorphp/rector/blob/363a910cf79487391977d43b193a5ad7bcb697a0/vendor/nikic/php-parser/lib/PhpParser/Builder/Declaration.php

Here is failing CI: https://github.com/rectorphp/rector/runs/2550133646

It should be like this:

declare (strict_types=1);
namespace PhpParser\Builder;

-use RectorPrefix20210510\PhpParser;
+use PhpParser;
use PhpParser\BuilderHelpers;
abstract class Declaration implements PhpParser\Builder

The PhpParser is excluded here:

https://github.com/rectorphp/rector-src/blob/a7db1ad94df4879885b9106773d2f8335cb40e49/utils/compiler/src/PhpScoper/StaticEasyPrefixer.php#L28 via whitelist option https://github.com/rectorphp/rector-src/blob/main/scoper.php#L37

It seems the bug is caused by this PR

[marekdedic]

Hi @TomasVotruba, I am a bit lost in your code and cannot reproduce the issue. Could you please narrow it down to some minimal reproducible example?

Thanks.