php-imagine / Imagine

PHP Object Oriented image manipulation library
https://imagine.readthedocs.io
Other
4.42k stars 530 forks source link

Deprecation messages in symfony 5.4 #809

Closed jokaorgua closed 2 years ago

jokaorgua commented 2 years ago

Issue description

Deprecation messages when using symfony 5.4

Method "ArrayAccess::offsetGet()" might add "mixed" as a native return type declaration in the future. Do the same in implementation "Imagine\Image\Metadata\MetadataBag" now to avoid errors or add an explicit @return annotation to suppress this message.

Method "ArrayAccess::offsetUnset()" might add "void" as a native return type declaration in the future. Do the same in implementation "Imagine\Image\Metadata\MetadataBag" now to avoid errors or add an explicit @return annotation to suppress this message.

What version of Imagine are you using?

1.2.4

What's the PHP version you are using?

8.0.13

What's the imaging library you are using [gd/imagick/gmagick/any]?

gd

What's the imaging library configuration

does not matter

Minimal PHP code to reproduce the error:

run any tests

ausi commented 2 years ago

This problem should already be fixed by https://github.com/php-imagine/Imagine/pull/768 because we added #[\ReturnTypeWillChange] attributes there.

@jokaorgua Can you please verify if these deprecation messages are gone in the current dev-develop version?

jokaorgua commented 2 years ago

@ausi I've tried dev-develop version and deprecation messages still exist

ReturnTypeWillChange was introduced in 8.1 and I'm using 8.0.13. I think that is the problem.

ausi commented 2 years ago

This might be a bug in Symfony 5.4 then I think. The Symfony ErrorHandler Component should not report a deprecation for the return type if the attribute #[\ReturnTypeWillChange] is present (even in PHP 8.0).

jokaorgua commented 2 years ago

@ausi I can not find a code in ErrorHandler component which should react to ReturnTypeWillChange

could you provide a link where it is said that Symfony 5.4 on PHP prior to 8.1 must not trigger a deprecation message?

ausi commented 2 years ago

could you provide a link where it is said that Symfony 5.4 on PHP prior to 8.1 must not trigger a deprecation message?

This is just my opinion, but I think Symfony might agree that a deprecation does not make sense in this case.

That code where the attributes schould be handled is somewhere aroud here I think: https://github.com/symfony/symfony/blob/dcf09d6c0a194f5f36ec963eb6fef92d573db576/src/Symfony/Component/ErrorHandler/DebugClassLoader.php#L563

ruudk commented 2 years ago

I created a PR to add the missing types.

ausi commented 2 years ago

I didn’t notice that adding the return types via PHPDoc (in contrast to real return types) also fixes the issue, so your PR is correct IMO. Thank you!

jokaorgua commented 2 years ago

@ausi symfony thinks that [#\ReturnTypeWillChange] is not enough and phpdoc must be added

https://github.com/symfony/symfony/issues/44929

ausi commented 2 years ago

symfony thinks that [#\ReturnTypeWillChange] is not enough and phpdoc must be added

Makes sense! 👍

jokaorgua commented 2 years ago

@ausi will this be merged into master and new release created?

mlocati commented 2 years ago

We'd first fix these issues: https://github.com/php-imagine/Imagine/milestone/1

ausi commented 2 years ago

@jokaorgua I assume so, see #814

Regarding release, see https://github.com/php-imagine/Imagine/pull/768#issuecomment-1011083965

mlocati commented 2 years ago

Fixed.