Add Decoder and Encoder thrown exception

VincentLanglet commented 6 months ago

Hi @olivervogel, as suggested in https://github.com/Intervention/image/pull/1214#issuecomment-1848577030, I'm now using the version 3 of intervention/image. But there is still no phpdoc about possibly thrown exception.

Keeping a @throws phpdoc up to date is useful

For the user to know which method can thrown exception, and which exception are thrown
For PHPStorm to report uncaught exceptions
For static analysis tools (like PHPStan) to also report uncaught exception

Since there is way too much exception in this tool, I started with the most frequent: EncoderException and DecoderException.

In my case this would be helpful to tell PHPStan/Phpstorm that I need to try/catch

ImageInterface::encode method
ImageInterface::* modifiers methods
ImageManager::read method

If you're interested, I can continue

with other exception
with a PR introducing the check done by phpstan (https://phpstan.org/blog/bring-your-exceptions-under-control)

I just recommend this to be in another PR to minimize the size.

olivervogel commented 6 months ago

Hey @VincentLanglet , thanks for your PR. The PHPDoc tags for the different exception types could be really helpful.

If you're interested, I can continue

Absolutely, any help is very welcome.

I only have one request. Even if some see it as nitpicking, I would suggest keeping the @return tag as the last item in the block. Apart from that, it already looks wonderful.

VincentLanglet commented 6 months ago

I only have one request. Even if some see it as nitpicking, I would suggest keeping the @return tag as the last item in the block. Apart from that, it already looks wonderful.

Sure, it's done in https://github.com/Intervention/image/pull/1303/commits/ba39c13d2b264c815026f39829dae00b0d818c63 commit.

Hey @VincentLanglet , thanks for your PR. The PHPDoc tags for the different exception types could be really helpful.

This can be checked by PHPStan, I added the config in https://github.com/Intervention/image/pull/1303/commits/1ed31f3cdf48043ee3ca799d5d6af9c23cdec203#diff-a4ce6ffd8cb697aecebba7af3c784ed29078f645544c069ec7fecbe9417e2a04

If you're interested, I can continue

Absolutely, any help is very welcome.

I propose to merge this PR avoiding having a too big change (which is already not small) and I'll do same for others exceptions in another pr if it's ok for you @olivervogel

olivervogel commented 6 months ago

This all looks good. I have made a few minor adjustments and otherwise adopted it as it is.

Intervention / image

Add Decoder and Encoder thrown exception #1303