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.

Sign up for GitHub

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

Dev: Remove the image ignore errors in phpstan-baseline.php #7765

Merged
merged 5 commits into from
Aug 6, 2023
+62 −80
Merged

Dev: Remove the image ignore errors in phpstan-baseline.php #7765

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5435ea3
Dev: Remove the image error in phpstan-baseline.php
ping-yee Aug 2, 2023
8e42ddf
Dev: Add the return type of image exception.
ping-yee Aug 2, 2023
773ee48
Dev: modify the return type to the comments.
ping-yee Aug 3, 2023
eac79d8
Dev: change the return type for the exception.
ping-yee Aug 4, 2023
7ecc708
Dev: Define the image exception.
ping-yee Aug 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 0 additions & 80 deletions phpstan-baseline.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1366,71 +1366,6 @@
'count' => 1,
'path' => __DIR__ . '/system/I18n/TimeLegacy.php',
];
$ignoreErrors[] = [
'message' => '#^Method CodeIgniter\\\\Images\\\\Exceptions\\\\ImageException\\:\\:forEXIFUnsupported\\(\\) has no return type specified\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Images/Exceptions/ImageException.php',
];
$ignoreErrors[] = [
'message' => '#^Method CodeIgniter\\\\Images\\\\Exceptions\\\\ImageException\\:\\:forFileNotSupported\\(\\) has no return type specified\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Images/Exceptions/ImageException.php',
];
$ignoreErrors[] = [
'message' => '#^Method CodeIgniter\\\\Images\\\\Exceptions\\\\ImageException\\:\\:forImageProcessFailed\\(\\) has no return type specified\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Images/Exceptions/ImageException.php',
];
$ignoreErrors[] = [
'message' => '#^Method CodeIgniter\\\\Images\\\\Exceptions\\\\ImageException\\:\\:forInvalidDirection\\(\\) has no return type specified\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Images/Exceptions/ImageException.php',
];
$ignoreErrors[] = [
'message' => '#^Method CodeIgniter\\\\Images\\\\Exceptions\\\\ImageException\\:\\:forInvalidImageCreate\\(\\) has no return type specified\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Images/Exceptions/ImageException.php',
];
$ignoreErrors[] = [
'message' => '#^Method CodeIgniter\\\\Images\\\\Exceptions\\\\ImageException\\:\\:forInvalidImageLibraryPath\\(\\) has no return type specified\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Images/Exceptions/ImageException.php',
];
$ignoreErrors[] = [
'message' => '#^Method CodeIgniter\\\\Images\\\\Exceptions\\\\ImageException\\:\\:forInvalidPath\\(\\) has no return type specified\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Images/Exceptions/ImageException.php',
];
$ignoreErrors[] = [
'message' => '#^Method CodeIgniter\\\\Images\\\\Exceptions\\\\ImageException\\:\\:forMissingAngle\\(\\) has no return type specified\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Images/Exceptions/ImageException.php',
];
$ignoreErrors[] = [
'message' => '#^Method CodeIgniter\\\\Images\\\\Exceptions\\\\ImageException\\:\\:forMissingImage\\(\\) has no return type specified\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Images/Exceptions/ImageException.php',
];
$ignoreErrors[] = [
'message' => '#^Method CodeIgniter\\\\Images\\\\Exceptions\\\\ImageException\\:\\:forSaveFailed\\(\\) has no return type specified\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Images/Exceptions/ImageException.php',
];
$ignoreErrors[] = [
'message' => '#^Method CodeIgniter\\\\Images\\\\Handlers\\\\BaseHandler\\:\\:_text\\(\\) has no return type specified\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Images/Handlers/BaseHandler.php',
];
$ignoreErrors[] = [
'message' => '#^Method CodeIgniter\\\\Images\\\\Handlers\\\\BaseHandler\\:\\:ensureResource\\(\\) has no return type specified\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Images/Handlers/BaseHandler.php',
];
$ignoreErrors[] = [
'message' => '#^Method CodeIgniter\\\\Images\\\\Handlers\\\\BaseHandler\\:\\:reproportion\\(\\) has no return type specified\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Images/Handlers/BaseHandler.php',
];
$ignoreErrors[] = [
'message' => '#^Property CodeIgniter\\\\Images\\\\Handlers\\\\BaseHandler\\:\\:\\$image \\(CodeIgniter\\\\Images\\\\Image\\) in empty\\(\\) is not falsy\\.$#',
'count' => 1,
Expand All @@ -1441,21 +1376,6 @@
'count' => 2,
'path' => __DIR__ . '/system/Images/Handlers/ImageMagickHandler.php',
];
$ignoreErrors[] = [
'message' => '#^Method CodeIgniter\\\\Images\\\\Handlers\\\\ImageMagickHandler\\:\\:_text\\(\\) has no return type specified\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Images/Handlers/ImageMagickHandler.php',
];
$ignoreErrors[] = [
'message' => '#^Method CodeIgniter\\\\Images\\\\Handlers\\\\ImageMagickHandler\\:\\:ensureResource\\(\\) has no return type specified\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Images/Handlers/ImageMagickHandler.php',
];
$ignoreErrors[] = [
'message' => '#^Method CodeIgniter\\\\Images\\\\Handlers\\\\ImageMagickHandler\\:\\:supportedFormatCheck\\(\\) has no return type specified\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Images/Handlers/ImageMagickHandler.php',
];
$ignoreErrors[] = [
'message' => '#^PHPDoc type string\\|null of property CodeIgniter\\\\Images\\\\Handlers\\\\ImageMagickHandler\\:\\:\\$resource is not covariant with PHPDoc type resource\\|null of overridden property CodeIgniter\\\\Images\\\\Handlers\\\\BaseHandler\\:\\:\\$resource\\.$#',
'count' => 1,
Expand Down
20 changes: 10 additions & 10 deletions system/Images/Exceptions/ImageException.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,52 +16,52 @@

class ImageException extends FrameworkException implements ExceptionInterface
{
public static function forMissingImage()
public static function forMissingImage(): ImageException
{
return new static(lang('Images.sourceImageRequired'));
}

public static function forFileNotSupported()
public static function forFileNotSupported(): ImageException
{
return new static(lang('Images.fileNotSupported'));
}

public static function forMissingAngle()
public static function forMissingAngle(): ImageException
{
return new static(lang('Images.rotationAngleRequired'));
}

public static function forInvalidDirection(?string $dir = null)
public static function forInvalidDirection(?string $dir = null): ImageException
{
return new static(lang('Images.invalidDirection', [$dir]));
}

public static function forInvalidPath()
public static function forInvalidPath(): ImageException
{
return new static(lang('Images.invalidPath'));
}

public static function forEXIFUnsupported()
public static function forEXIFUnsupported(): ImageException
{
return new static(lang('Images.exifNotSupported'));
}

public static function forInvalidImageCreate(?string $extra = null)
public static function forInvalidImageCreate(?string $extra = null): ImageException
{
return new static(lang('Images.unsupportedImageCreate') . ' ' . $extra);
}

public static function forSaveFailed()
public static function forSaveFailed(): ImageException
{
return new static(lang('Images.saveFailed'));
}

public static function forInvalidImageLibraryPath(?string $path = null)
public static function forInvalidImageLibraryPath(?string $path = null): ImageException
{
return new static(lang('Images.libPathInvalid', [$path]));
}

public static function forImageProcessFailed()
public static function forImageProcessFailed(): ImageException
{
return new static(lang('Images.imageProcessFailed'));
}
Expand Down
6 changes: 5 additions & 1 deletion system/Images/Handlers/BaseHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ public function withFile(string $path)

/**
* Make the image resource object if needed
*
* @return void
*/
abstract protected function ensureResource();

Expand Down Expand Up @@ -422,6 +424,8 @@ public function text(string $text, array $options = [])

/**
* Handler-specific method for overlaying text on an image.
*
* @return void
*/
abstract protected function _text(string $text, array $options = []);

Expand Down Expand Up @@ -723,7 +727,7 @@ public function __call(string $name, array $args = [])
* This function lets us re-proportion the width/height
* if users choose to maintain the aspect ratio when resizing.
*/
protected function reproportion()
protected function reproportion(): void
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, can you make this as PHPDoc instead? I mistakenly written yesterday in your earlier PR that it is protected so we can add the : void directly. I meant it is final. With your change it will violate LSP for those people extending the methods.

Please revise the same with the other protected methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the image exception return type also be written in the comments?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favor of leaving the exception return types in place. Framework exceptions are explicitly for framework code - so even though they aren't marked @internal they still should not be extended - and certainly not their names constructors.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MGatner then I think we should instead use new self over new static

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MGatner @paulbalandan

Framework exceptions are explicitly for framework code - so even though they aren't marked @internal they still should not be extended

It is not documented, and I did not know that.
In fact, we could extend these exceptions.

So I oppose this breaking change. It is better to add @internal or final in 4.4 branch, and add return type.

{
if (($this->width === 0 && $this->height === 0) || $this->image()->origWidth === 0 || $this->image()->origHeight === 0 || (! ctype_digit((string) $this->width) && ! ctype_digit((string) $this->height)) || ! ctype_digit((string) $this->image()->origWidth) || ! ctype_digit((string) $this->image()->origHeight)) {
return;
Expand Down
6 changes: 3 additions & 3 deletions system/Images/Handlers/ImageMagickHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ protected function getResourcePath()
*
* @throws Exception
*/
protected function ensureResource()
protected function ensureResource(): void
{
$this->getResourcePath();

Expand All @@ -304,7 +304,7 @@ protected function ensureResource()
*
* @throws ImageException
*/
protected function supportedFormatCheck()
protected function supportedFormatCheck(): void
{
switch ($this->image()->imageType) {
case IMAGETYPE_WEBP:
Expand All @@ -320,7 +320,7 @@ protected function supportedFormatCheck()
*
* @throws Exception
*/
protected function _text(string $text, array $options = [])
protected function _text(string $text, array $options = []): void
{
$xAxis = 0;
$yAxis = 0;
Expand Down