From 5832e762583f439db62f49e6f9a1825e8a29d2b1 Mon Sep 17 00:00:00 2001 From: "John Paul E. Balandan, CPA" Date: Sat, 8 Aug 2026 14:53:15 +0800 Subject: [PATCH] refactor: cleanup the Images library --- phpstan.dist.neon | 1 - system/Images/Handlers/BaseHandler.php | 28 ++----- system/Images/Handlers/GDHandler.php | 79 ++++++++----------- system/Images/Handlers/ImageMagickHandler.php | 30 +------ utils/phpstan-baseline/argument.type.neon | 12 +-- utils/phpstan-baseline/loader.neon | 3 +- .../missingType.iterableValue.neon | 12 +-- .../phpstan-baseline/property.phpDocType.neon | 7 +- utils/phpstan-baseline/varTag.type.neon | 8 -- 9 files changed, 49 insertions(+), 131 deletions(-) delete mode 100644 utils/phpstan-baseline/varTag.type.neon diff --git a/phpstan.dist.neon b/phpstan.dist.neon index 2e05af3201a5..71e4151f6f3d 100644 --- a/phpstan.dist.neon +++ b/phpstan.dist.neon @@ -18,7 +18,6 @@ parameters: - app/Views/errors/html/* - system/Commands/Generators/Views/* - system/Debug/Toolbar/Views/toolbar.tpl.php - - system/Images/Handlers/GDHandler.php - system/Test/Mock/MockCommon.php - system/ThirdParty/* - system/Validation/Views/single.php diff --git a/system/Images/Handlers/BaseHandler.php b/system/Images/Handlers/BaseHandler.php index 9301b1573c23..ee8822568515 100644 --- a/system/Images/Handlers/BaseHandler.php +++ b/system/Images/Handlers/BaseHandler.php @@ -20,7 +20,9 @@ use Config\Images; /** - * Base image handling implementation + * Base image handling implementation. + * + * @template T of object */ abstract class BaseHandler implements ImageHandlerInterface { @@ -90,7 +92,7 @@ abstract class BaseHandler implements ImageHandlerInterface /** * Default options for text watermarking. * - * @var array + * @var array */ protected $textDefaults = [ 'fontPath' => null, @@ -110,7 +112,7 @@ abstract class BaseHandler implements ImageHandlerInterface /** * Image types with support for transparency. * - * @var array + * @var list */ protected $supportTransparency = [ IMAGETYPE_PNG, @@ -120,7 +122,7 @@ abstract class BaseHandler implements ImageHandlerInterface /** * Temporary image used by the different engines. * - * @var resource|null + * @var T|null */ protected $resource; @@ -193,7 +195,6 @@ protected function image(): Image throw ImageException::forMissingImage(); } - // Verify the loaded image is an Image instance if (! $this->image instanceof Image) { throw ImageException::forInvalidPath(); } @@ -203,7 +204,6 @@ protected function image(): Image throw ImageException::forFileNotSupported(); } - // Note that the image has been verified $this->verified = true; return $this->image; @@ -214,7 +214,7 @@ protected function image(): Image * Good for extending the system or doing things this library * is not intended to do. * - * @return resource + * @return T */ public function getResource() { @@ -246,7 +246,6 @@ public function withResource() */ public function resize(int $width, int $height, bool $maintainRatio = false, string $masterDim = 'auto') { - // If the target width/height match the source, then we have nothing to do here. if ($this->image()->origWidth === $width && $this->image()->origHeight === $height) { return $this; } @@ -316,28 +315,20 @@ public function convert(int $imageType) */ public function rotate(float $angle) { - // Allowed rotation values - $degs = [ - 90.0, - 180.0, - 270.0, - ]; + $degs = [90.0, 180.0, 270.0]; if (! in_array($angle, $degs, true)) { throw ImageException::forMissingAngle(); } - // cast angle as an int, for our use $angle = (int) $angle; - // Reassign the width and height if ($angle === 90 || $angle === 270) { $temp = $this->height; $this->width = $this->height; $this->height = $temp; } - // Call the Handler-specific version. $this->_rotate($angle); return $this; @@ -568,8 +559,6 @@ protected function calcAspectRatio($width, $height = null, $origWidth = 0, $orig throw new InvalidArgumentException('You must supply the parameters: origWidth, origHeight.'); } - // If $height is null, then we have it easy. - // Calc based on full image size and be done. if ($height === null) { $height = ($width / $origWidth) * $origHeight; @@ -724,7 +713,6 @@ protected function reproportion() return; } - // Sanitize $this->width = (int) $this->width; $this->height = (int) $this->height; diff --git a/system/Images/Handlers/GDHandler.php b/system/Images/Handlers/GDHandler.php index 01c05384c744..2ba97437edda 100644 --- a/system/Images/Handlers/GDHandler.php +++ b/system/Images/Handlers/GDHandler.php @@ -15,15 +15,16 @@ use CodeIgniter\Images\Exceptions\ImageException; use Config\Images; +use GdImage; /** * Image handler for GD package + * + * @extends BaseHandler */ class GDHandler extends BaseHandler { /** - * Constructor. - * * @param Images|null $config * * @throws ImageException @@ -43,17 +44,13 @@ public function __construct($config = null) */ protected function _rotate(int $angle): bool { - // Create the image handle $srcImg = $this->createImage(); - // Set the background color // This won't work with transparent PNG files so we are // going to have to figure out how to determine the color // of the alpha channel in a future release. - $white = imagecolorallocate($srcImg, 255, 255, 255); - // Rotate it! $destImg = imagerotate($srcImg, $angle, $white); $this->resource = $destImg; @@ -110,23 +107,23 @@ protected function _flip(string $direction) /** * Get GD version * - * @return mixed + * @return string */ public function getVersion() { - if (function_exists('gd_info')) { - $gdVersion = @gd_info(); - - return preg_replace('/\D/', '', $gdVersion['GD Version']); + if (! function_exists('gd_info')) { + return ''; // @codeCoverageIgnore } - return false; + $gdVersion = @gd_info(); + + return preg_replace('/\D/', '', $gdVersion['GD Version']) ?? ''; } /** * Resizes the image. * - * @return GDHandler + * @return $this */ public function _resize(bool $maintainRatio = false) { @@ -136,7 +133,7 @@ public function _resize(bool $maintainRatio = false) /** * Crops the image. * - * @return GDHandler + * @return $this */ public function _crop() { @@ -154,7 +151,6 @@ protected function process(string $action) $origHeight = $this->image()->origHeight; if ($action === 'crop') { - // Reassign the source width/height if cropping $origWidth = $this->width; $origHeight = $this->height; @@ -165,7 +161,6 @@ protected function process(string $action) $this->image()->origWidth = $this->width; } - // Create the image handle $src = $this->createImage(); if (function_exists('imagecreatetruecolor')) { @@ -178,7 +173,6 @@ protected function process(string $action) $dest = $create($this->width, $this->height); - // for png and webp we can actually preserve transparency if (in_array($this->image()->imageType, $this->supportTransparency, true)) { imagealphablending($dest, false); imagesavealpha($dest, true); @@ -209,7 +203,7 @@ public function save(?string $target = null, int $quality = 90): bool // If no new resource has been created, then we're // simply copy the existing one. - if (empty($this->resource) && $quality === 100) { + if ($this->resource === null && $quality === 100) { if ($original === null) { return true; } @@ -222,7 +216,6 @@ public function save(?string $target = null, int $quality = 90): bool $this->ensureResource(); - // for png and webp we can actually preserve transparency if (in_array($this->image()->imageType, $this->supportTransparency, true)) { imagepalettetotruecolor($this->resource); imagealphablending($this->resource, false); @@ -284,10 +277,7 @@ public function save(?string $target = null, int $quality = 90): bool /** * Create Image Resource * - * This simply creates an image resource handle - * based on the type of image being processed - * - * @return bool|resource + * @return GdImage */ protected function createImage(string $path = '', string $imageType = '') { @@ -312,7 +302,6 @@ protected function createImage(string $path = '', string $imageType = '') protected function ensureResource() { if ($this->resource === null) { - // if valid image type, make corresponding image resource $this->resource = $this->getImageResource( $this->image()->getPathname(), $this->image()->imageType, @@ -326,7 +315,7 @@ protected function ensureResource() * @param string $path Image path * @param int $imageType Image type * - * @return bool|resource + * @return GdImage * * @throws ImageException */ @@ -338,32 +327,42 @@ protected function getImageResource(string $path, int $imageType) throw ImageException::forInvalidImageCreate(lang('Images.gifNotSupported')); } - return imagecreatefromgif($path); + $resource = imagecreatefromgif($path); + break; case IMAGETYPE_JPEG: if (! function_exists('imagecreatefromjpeg')) { throw ImageException::forInvalidImageCreate(lang('Images.jpgNotSupported')); } - return imagecreatefromjpeg($path); + $resource = imagecreatefromjpeg($path); + break; case IMAGETYPE_PNG: if (! function_exists('imagecreatefrompng')) { throw ImageException::forInvalidImageCreate(lang('Images.pngNotSupported')); } - return @imagecreatefrompng($path); + $resource = @imagecreatefrompng($path); + break; case IMAGETYPE_WEBP: if (! function_exists('imagecreatefromwebp')) { throw ImageException::forInvalidImageCreate(lang('Images.webpNotSupported')); } - return imagecreatefromwebp($path); + $resource = imagecreatefromwebp($path); + break; default: throw ImageException::forInvalidImageCreate('Ima'); } + + if ($resource === false) { + throw ImageException::forInvalidImageCreate($path); // @codeCoverageIgnore + } + + return $resource; } /** @@ -386,10 +385,7 @@ protected function _text(string $text, array $options = []) $options['hOffset'] *= -1; } - // Set font width and height - // These are calculated differently depending on - // whether we are using the true type font or not - if (! empty($options['fontPath'])) { + if (($options['fontPath'] ?? '') !== '') { if (function_exists('imagettfbbox')) { $temp = imagettfbbox($options['fontSize'], 0, $options['fontPath'], $text); $temp = $temp[2] - $temp[0]; @@ -408,11 +404,9 @@ protected function _text(string $text, array $options = []) $options['fontheight'] = $fontheight; $options['fontwidth'] = $fontwidth; - // Set base X and Y axis values $xAxis = $options['hOffset'] + $options['padding']; $yAxis = $options['vOffset'] + $options['padding']; - // Set vertical alignment if ($options['vAlign'] === 'middle') { // Don't apply padding when you're in the middle of the image. $yAxis += ($this->image()->origHeight / 2) + ($fontheight / 2) - $options['padding'] - $fontheight - $options['shadowOffset']; @@ -420,7 +414,6 @@ protected function _text(string $text, array $options = []) $yAxis = ($this->image()->origHeight - $fontheight - $options['shadowOffset'] - ($fontheight / 2)) - $yAxis; } - // Set horizontal alignment if ($options['hAlign'] === 'right') { $xAxis += ($this->image()->origWidth - ($fontwidth * strlen($text)) - $options['shadowOffset']) - (2 * $options['padding']); } elseif ($options['hAlign'] === 'center') { @@ -431,7 +424,6 @@ protected function _text(string $text, array $options = []) $options['yAxis'] = $yAxis; if ($options['withShadow']) { - // Offset from text $options['xShadow'] = $xAxis + $options['shadowOffset']; $options['yShadow'] = $yAxis + $options['shadowOffset']; @@ -444,17 +436,15 @@ protected function _text(string $text, array $options = []) /** * Handler-specific method for overlaying text on an image. * - * @param bool $isShadow Whether we are drawing the dropshadow or actual text + * @param array $options + * @param bool $isShadow Whether we are drawing the dropshadow or actual text + * + * @return void */ protected function textOverlay(string $text, array $options = [], bool $isShadow = false) { $src = $this->createImage(); - /* Set RGB values for shadow - * - * Get the rest of the string and split it into 2-length - * hex values: - */ $opacity = (int) ($options['opacity'] * 127); // Allow opacity to be applied to the text @@ -473,8 +463,7 @@ protected function textOverlay(string $text, array $options = [], bool $isShadow $xAxis = $isShadow ? $options['xShadow'] : $options['xAxis']; $yAxis = $isShadow ? $options['yShadow'] : $options['yAxis']; - // Add the shadow to the source image - if (! empty($options['fontPath'])) { + if (($options['fontPath'] ?? '') !== '') { // We have to add fontheight because imagettftext locates the bottom left corner, not top-left corner. imagettftext($src, $options['fontSize'], 0, (int) $xAxis, (int) ($yAxis + $options['fontheight']), $color, $options['fontPath'], $text); } else { diff --git a/system/Images/Handlers/ImageMagickHandler.php b/system/Images/Handlers/ImageMagickHandler.php index c2b24c81f272..3215c84f1ab3 100644 --- a/system/Images/Handlers/ImageMagickHandler.php +++ b/system/Images/Handlers/ImageMagickHandler.php @@ -24,16 +24,11 @@ /** * Image handler for Imagick extension. + * + * @extends BaseHandler */ class ImageMagickHandler extends BaseHandler { - /** - * Stores Imagick instance. - * - * @var Imagick|null - */ - protected $resource; - /** * Constructor. * @@ -60,14 +55,12 @@ public function __construct($config = null) protected function ensureResource() { if (! $this->resource instanceof Imagick) { - // Verify that we have a valid image $this->image(); try { $this->resource = new Imagick(); $this->resource->readImage($this->image()->getPathname()); - // Check for valid image if ($this->resource->getImageWidth() === 0 || $this->resource->getImageHeight() === 0) { throw ImageException::forInvalidImageCreate($this->image()->getPathname()); } @@ -119,12 +112,10 @@ protected function process(string $action, int $quality = 100) $yAxis, ); - // Reset canvas to cropped size $this->resource->setImagePage(0, 0, 0, 0); break; } - // Handle transparency for supported image types if (in_array($this->image()->imageType, $this->supportTransparency, true) && $this->resource->getImageAlphaChannel() === Imagick::ALPHACHANNEL_UNDEFINED) { $this->resource->setImageAlphaChannel(Imagick::ALPHACHANNEL_OPAQUE); @@ -146,7 +137,6 @@ protected function process(string $action, int $quality = 100) public function _resize(bool $maintainRatio = false) { if ($maintainRatio) { - // If maintaining a ratio, we need a custom approach $this->ensureResource(); // Use thumbnailImage which preserves an aspect ratio @@ -155,7 +145,6 @@ public function _resize(bool $maintainRatio = false) return $this; } - // Use the common process() method for normal resizing return $this->process('resize'); } @@ -168,7 +157,6 @@ public function _resize(bool $maintainRatio = false) */ public function _crop() { - // Use the common process() method for cropping $result = $this->process('crop'); // Handle a case where crop dimensions exceed the original image size @@ -177,15 +165,12 @@ public function _crop() $imgHeight = $this->resource->getImageHeight(); if ($this->xAxis >= $imgWidth || $this->yAxis >= $imgHeight) { - // Create transparent background $background = new Imagick(); $background->newImage($this->width, $this->height, new ImagickPixel('transparent')); $background->setImageFormat($this->resource->getImageFormat()); - // Composite our image on the background $background->compositeImage($this->resource, Imagick::COMPOSITE_OVER, 0, 0); - // Replace our resource $this->resource = $background; } } @@ -205,11 +190,9 @@ protected function _rotate(int $angle) { $this->ensureResource(); - // Create transparent background $this->resource->setImageBackgroundColor(new ImagickPixel('transparent')); $this->resource->rotateImage(new ImagickPixel('transparent'), $angle); - // Reset canvas dimensions $this->resource->setImagePage($this->resource->getImageWidth(), $this->resource->getImageHeight(), 0, 0); return $this; @@ -226,10 +209,8 @@ protected function _flatten(int $red = 255, int $green = 255, int $blue = 255) { $this->ensureResource(); - // Create background $bg = new ImagickPixel("rgb({$red},{$green},{$blue})"); - // Create a new canvas with the background color $canvas = new Imagick(); $canvas->newImage( $this->resource->getImageWidth(), @@ -238,7 +219,6 @@ protected function _flatten(int $red = 255, int $green = 255, int $blue = 255) $this->resource->getImageFormat(), ); - // Composite our image on the background $canvas->compositeImage( $this->resource, Imagick::COMPOSITE_OVER, @@ -246,7 +226,6 @@ protected function _flatten(int $red = 255, int $green = 255, int $blue = 255) 0, ); - // Replace our resource with the flattened version $this->resource->clear(); $this->resource = $canvas; @@ -393,13 +372,11 @@ protected function _text(string $text, array $options = []) $draw->setFillColor(new ImagickPixel("rgba({$r},{$g},{$b},{$opacity})")); } - // Calculate text positioning $imgWidth = $this->resource->getImageWidth(); $imgHeight = $this->resource->getImageHeight(); $xAxis = 0; $yAxis = 0; - // Default padding $padding = $options['padding'] ?? 0; if (isset($options['hAlign'])) { @@ -436,7 +413,7 @@ protected function _text(string $text, array $options = []) break; case 'bottom': - // Note: Vertical offset is inverted for bottom alignment as per original implementation + // A negative offset pushes the text further up from the bottom edge. $yAxis = $vOffset < 0 ? $imgHeight + $vOffset - $padding : $imgHeight - $vOffset - $padding; break; } @@ -470,7 +447,6 @@ protected function _text(string $text, array $options = []) ); } - // Draw the main text $this->resource->annotateImage( $draw, $xAxis, diff --git a/utils/phpstan-baseline/argument.type.neon b/utils/phpstan-baseline/argument.type.neon index 7fbc636537e7..6f504bc90f90 100644 --- a/utils/phpstan-baseline/argument.type.neon +++ b/utils/phpstan-baseline/argument.type.neon @@ -1,4 +1,4 @@ -# total 68 errors +# total 64 errors parameters: ignoreErrors: @@ -142,16 +142,6 @@ parameters: count: 1 path: ../../tests/system/Honeypot/HoneypotTest.php - - - message: '#^Parameter \#1 \$image of function imagecolorat expects GdImage, resource given\.$#' - count: 2 - path: ../../tests/system/Images/GDHandlerTest.php - - - - message: '#^Parameter \#1 \$image of function imagecolorsforindex expects GdImage, resource given\.$#' - count: 2 - path: ../../tests/system/Images/GDHandlerTest.php - - message: '#^Parameter \#2 \$message of method CodeIgniter\\Log\\Handlers\\ChromeLoggerHandler\:\:handle\(\) expects string, stdClass given\.$#' count: 1 diff --git a/utils/phpstan-baseline/loader.neon b/utils/phpstan-baseline/loader.neon index 32ad4dacc2f5..f4f70532bf2d 100644 --- a/utils/phpstan-baseline/loader.neon +++ b/utils/phpstan-baseline/loader.neon @@ -1,4 +1,4 @@ -# total 1808 errors +# total 1799 errors includes: - argument.type.neon @@ -23,4 +23,3 @@ includes: - return.type.neon - staticMethod.notFound.neon - ternary.shortNotAllowed.neon - - varTag.type.neon diff --git a/utils/phpstan-baseline/missingType.iterableValue.neon b/utils/phpstan-baseline/missingType.iterableValue.neon index 28aeaabd8f58..78b41e9422dd 100644 --- a/utils/phpstan-baseline/missingType.iterableValue.neon +++ b/utils/phpstan-baseline/missingType.iterableValue.neon @@ -1,4 +1,4 @@ -# total 1139 errors +# total 1137 errors parameters: ignoreErrors: @@ -3267,16 +3267,6 @@ parameters: count: 1 path: ../../system/Images/Handlers/BaseHandler.php - - - message: '#^Property CodeIgniter\\Images\\Handlers\\BaseHandler\:\:\$supportTransparency type has no value type specified in iterable type array\.$#' - count: 1 - path: ../../system/Images/Handlers/BaseHandler.php - - - - message: '#^Property CodeIgniter\\Images\\Handlers\\BaseHandler\:\:\$textDefaults type has no value type specified in iterable type array\.$#' - count: 1 - path: ../../system/Images/Handlers/BaseHandler.php - - message: '#^Method CodeIgniter\\Images\\Image\:\:getProperties\(\) return type has no value type specified in iterable type array\.$#' count: 1 diff --git a/utils/phpstan-baseline/property.phpDocType.neon b/utils/phpstan-baseline/property.phpDocType.neon index 0ba558aded06..148b167e3e40 100644 --- a/utils/phpstan-baseline/property.phpDocType.neon +++ b/utils/phpstan-baseline/property.phpDocType.neon @@ -1,4 +1,4 @@ -# total 44 errors +# total 43 errors parameters: ignoreErrors: @@ -162,11 +162,6 @@ parameters: count: 1 path: ../../system/HTTP/IncomingRequest.php - - - message: '#^PHPDoc type Imagick\|null of property CodeIgniter\\Images\\Handlers\\ImageMagickHandler\:\:\$resource is not the same as PHPDoc type resource\|null of overridden property CodeIgniter\\Images\\Handlers\\BaseHandler\:\:\$resource\.$#' - count: 1 - path: ../../system/Images/Handlers/ImageMagickHandler.php - - message: '#^PHPDoc type string of property CodeIgniter\\Session\\Handlers\\FileHandler\:\:\$savePath is not the same as PHPDoc type array\\|string of overridden property CodeIgniter\\Session\\Handlers\\BaseHandler\:\:\$savePath\.$#' count: 1 diff --git a/utils/phpstan-baseline/varTag.type.neon b/utils/phpstan-baseline/varTag.type.neon deleted file mode 100644 index d2244f5a31ab..000000000000 --- a/utils/phpstan-baseline/varTag.type.neon +++ /dev/null @@ -1,8 +0,0 @@ -# total 2 errors - -parameters: - ignoreErrors: - - - message: '#^PHPDoc tag @var with type Imagick is not subtype of type resource\.$#' - count: 2 - path: ../../tests/system/Images/ImageMagickHandlerTest.php