Skip to content
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

PHP 8.1 notices #7430

Closed
kkmuffme opened this issue Aug 27, 2022 · 18 comments
Closed

PHP 8.1 notices #7430

kkmuffme opened this issue Aug 27, 2022 · 18 comments
Labels

Comments

@kkmuffme
Copy link

Bug Report

Subject Details
Rector version e.g. v0.14

Minimal PHP Code Causing Issue

  • all rules enabled
  • run rector with PHP 8.1 CLI
Constant FILE_TEXT is deprecated
phar:.../vendor/rector/rector/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/SourceLocator/SourceStubber/PhpStormStubs/CachingVisitor.php on line 162
Constant FILE_BINARY is deprecated
File: phar:.../vendor/rector/rector/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/SourceLocator/SourceStubber/PhpStormStubs/CachingVisitor.php on line 162

Since symfony/console 6.1: Relying on the static property $defaultName for setting a command name is deprecated. Add the RectorPrefix202208/Symfony/Component/Console/Attribute/AsCommand attribute to the RectorPrefix202208/Symfony/Component/Console/Command/DumpCompletionCommand class instead.
File: .../vendor/rector/rector/vendor/symfony/contracts/Deprecation/function.php on line 26
Since symfony/console 6.1: Relying on the static property $defaultDescription for setting a command description is deprecated. Add the RectorPrefix202208/Symfony/Component/Console/Attribute/AsCommand attribute to the RectorPrefix202208/Symfony/Component/Console/Command/DumpCompletionCommand class instead.
File: .../vendor/rector/rector/vendor/symfony/contracts/Deprecation/function.php on line 26

There may be more, I just grabbed the first I saw. Since they are in dependencies inside of rector I thought I report it here then upstream

@kkmuffme kkmuffme added the bug label Aug 27, 2022
@samsonasik
Copy link
Member

Could you narrow it down to specific rules and the code that cause it? For multiple rules applied, you can create failing test case PR to
https://github.com/rectorphp/rector-src/tree/main/tests/Issues

You can copy and modify on of the tests

@TomasVotruba
Copy link
Member

This has to be solved in the project themselves. We only use those here :)

@kkmuffme
Copy link
Author

kkmuffme commented Aug 29, 2022

symfony/symfony#47418

@TomasVotruba
Copy link
Member

The DumpCompletionCommand is a Symfony command.

@kkmuffme
Copy link
Author

Thanks, I reported both.
For the moment this means Rector is not compatible with PHP8.1 though, as even the most minimalistic config (any rule) has PHP notices.

3 line rector.php for confirming this issue:

return static function (\Rector\Config\RectorConfig $rectorConfig): void {
    $rectorConfig->rule(\Rector\CodeQuality\Rector\Include_\AbsolutizeRequireAndIncludePathRector::class);
};

@TomasVotruba
Copy link
Member

So is in Rector: https://github.com/rectorphp/rector/blob/main/vendor/symfony/console/Command/DumpCompletionCommand.php#L25

But Symfony checks the static property that is being downgraded to PHP 7.2. That's side effect of downgrade.

What can we do about it here?

@TomasVotruba
Copy link
Member

For the moment this means Rector is not compatible with PHP8.1 though, as even the most minimalistic config (any rule) has PHP notices.

It is :). The deprecation on framework side is false positive here.

@TomasVotruba
Copy link
Member

TomasVotruba commented Aug 29, 2022

@kkmuffme How does it change if you comment this line in your local Rector vendor?

@\trigger_error(($package || $version ? "Since {$package} {$version}: " : '') . ($args ? \vsprintf($message, $args) : $message), \E_USER_DEPRECATED);

We do not really need to bubble up Symfony deprecations to Rector release, so removing this function seems the way to go.

@kkmuffme
Copy link
Author

@TomasVotruba yes, then that notice is gone for symfony.
Could you please reopen this issue?

===

phpstan notice:
the line numbers don't match up with the latest phpstan version at all, is rector on the latest phpstan version? (as the issue might be fixed there already)

===

There are 2 phpstan.phar in rector, is this on purpose?
rector/vendor/bin/phpstan.phar
rector/vendor/phpstan/phpstan/phpstan.phar

@TomasVotruba
Copy link
Member

Could you create a new issue with the Symfony notice only? To keep it clean

@stof
Copy link

stof commented Aug 29, 2022

@TomasVotruba if you want to support PHP 7.2, why not using the Symfony 5.4 LTS which supports PHP 7.2 and so does not deprecate features that are necessary when using PHP 8- due to not being able to use attributes ?

@kkmuffme
Copy link
Author

kkmuffme commented Aug 29, 2022

@TomasVotruba phpstan issue is also fixed:
phpstan/phpstan#7873

Does rector use a custom error handler that picks this up despite snoozed/@ ? (or is this something in my PHP setup?)

@TomasVotruba
Copy link
Member

TomasVotruba commented Aug 29, 2022

@stof Because we run on Symfony 6 with most of the deps already. It's easier for us to the use latest deps in general. The downgrade should work propperly in general, not just for Rector. If someone uses Rector to downgrade their Symfony 6 project, it should be allright.

@TomasVotruba
Copy link
Member

TomasVotruba commented Aug 29, 2022

@kkmuffme > There are 2 phpstan.phar in rector, is this on purpose?

The vendor/bin/* in composer is usually simlinked bin files from the package itself.

@kkmuffme
Copy link
Author

@TomasVotruba #7430 (comment) What's your suggestion?
rector will throw 100s of notices with PHP 8.1 due to this

This has to be solved in the project themselves. We only use those here :)

😁

@TomasVotruba
Copy link
Member

TomasVotruba commented Aug 29, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants