Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions src/Utils/ArrayHash.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
/**
* Provides objects to work as array.
* @template T
* @implements \IteratorAggregate<array-key, T>
* @implements \ArrayAccess<array-key, T>
*/
class ArrayHash extends \stdClass implements \ArrayAccess, \Countable, \IteratorAggregate
{
Expand Down
4 changes: 3 additions & 1 deletion src/Utils/ArrayList.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
/**
* Provides the base class for a generic list (items can be accessed by index).
* @template T
* @implements \IteratorAggregate<int, T>
Copy link
Member

Choose a reason for hiding this comment

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

Isn't that duplicate information?

Copy link
Contributor

Choose a reason for hiding this comment

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

It is, but just for getIterator(). Implementing IteratorAggregate also passes generic type to Traversable. Without it Traversable type is mixed. Not sure if this specific case is problem, but generally speaking, generic interface should be always implemented.

Generic type from getIterator() should be removed, as it is already defined by IteratorAggregate

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the explanation. Can I ask @xificurk to remove the annotation from getIterator()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a bit more complicated...

Latest PHPStan can infer key/value types even without the @implements annotation. Actually, the correct annotations on methods like getIterator() are the key to get this working.

The @implements annotations on class come into play only when checking compatibility between e.g. ArrayList<Foo> and Traversable<string>.

Example with method annotations only:
https://phpstan.org/r/0ecafc0e-1f6a-422b-8121-601992d34b25

Example with class annotation only:
https://phpstan.org/r/ec2e29f0-53a0-45c8-9030-dcfb5cb3ab57

* @implements \ArrayAccess<int, T>
*/
class ArrayList implements \ArrayAccess, \Countable, \IteratorAggregate
{
Expand All @@ -25,7 +27,7 @@ class ArrayList implements \ArrayAccess, \Countable, \IteratorAggregate

/**
* Transforms array to ArrayList.
* @param array<T> $array
* @param list<T> $array
*/
public static function from(array $array): static
{
Expand Down
1 change: 1 addition & 0 deletions src/Utils/Arrays.php
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ public static function flatten(array $array, bool $preserveKeys = false): array

/**
* Checks if the array is indexed in ascending order of numeric keys from zero, a.k.a list.
* @return ($value is list ? true : false)
Copy link
Member

Choose a reason for hiding this comment

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

What's it good for?

Copy link
Contributor

Choose a reason for hiding this comment

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

If variable is of type list it will always return true. Informs user that check is effectively useless.

But it should probably be @return ($value is list ? true : bool)

Copy link
Member

Choose a reason for hiding this comment

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

I understand what it does, I ask what good it does :-) I just need an example.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here it is. If condition is always true.
https://phpstan.org/r/37d792a0-1fe7-488a-8818-15048810f6c0

Copy link
Contributor

Choose a reason for hiding this comment

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

Sadly it is only half of the solution and it seems phpstan does not support assert for list. Even after isList() is given value array
https://phpstan.org/r/c26d6864-a46a-4d81-9fad-54dbc75d7e83

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mabar @enumag Why do you think the return type should be @return ($value is list ? true : **bool**)? Can you give me example of value type which is not list, but the method will return true?

This change was actually the reason, why the examples you've posted seemingly did not work. The correct conditional return annotation is enough for phpstan to correctly detect always true/false conditions & perform type narrowing.

https://phpstan.org/r/c3f4cdc0-3417-42d8-9e5f-24f1c722b01c

Copy link
Contributor

@enumag enumag Mar 20, 2023

Choose a reason for hiding this comment

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

@xificurk Because $value can be a list without PHPStan being aware of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@enumag Can you create example in phpstan playground?

btw, isn't it exactly the case of the last if-else block in my example? PHPStan can't know if the value is list or not, but if it is, the isList() method will always return true, otherwise false.

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried a few things and yeah you're right, false is enough there. PHPStan does treat unknown array as a possible list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For completeness, here is a good example for it: https://phpstan.org/r/8f789bd9-8fbe-477c-87e1-e7c2cad3d6f2

*/
public static function isList(mixed $value): bool
{
Expand Down
24 changes: 19 additions & 5 deletions src/Utils/Image.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@
* @method void stringUp($font, $x, $y, string $s, $col)
* @method void trueColorToPalette(bool $dither, $ncolors)
* @method array ttfText($size, $angle, $x, $y, $color, string $fontfile, string $text)
* @property-read int $width
* @property-read int $height
* @property-read positive-int $width
* @property-read positive-int $height
Copy link
Member

Choose a reason for hiding this comment

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

What's it good for?

Copy link
Contributor

Choose a reason for hiding this comment

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

Mostly for compatibility with other classes that already expect width and height to be >= 0. e.g. for database entity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As any type (narrowing) hint, it's good to make code more readable and eliminate useless checks.

If you rely on static analysis and narrow down the types enough, checks like this become useless and can be safely eliminated.

Dimensions are often used in various calculations. Knowing that those are actually positive integers, implies e.g.:

  • their ratio is well defined and also positive number
  • the area (number of pixels in image) is positive integer

* @property-read \GdImage $imageResource
*/
class Image
Expand Down Expand Up @@ -220,6 +220,8 @@ private static function invokeSafe(string $func, string $arg, string $message, s

/**
* Creates a new true color image of the given dimensions. The default color is black.
* @param positive-int $width
* @param positive-int $height
* @throws Nette\NotSupportedException if gd extension is not loaded
*/
public static function fromBlank(int $width, int $height, ?array $color = null): static
Expand Down Expand Up @@ -247,6 +249,7 @@ public static function fromBlank(int $width, int $height, ?array $color = null):

/**
* Returns the type of image from file.
* @return ImageType::*|null
*/
public static function detectTypeFromFile(string $file, &$width = null, &$height = null): ?int
{
Expand All @@ -257,6 +260,7 @@ public static function detectTypeFromFile(string $file, &$width = null, &$height

/**
* Returns the type of image from string.
* @return ImageType::*|null
*/
public static function detectTypeFromString(string $s, &$width = null, &$height = null): ?int
{
Expand All @@ -267,6 +271,8 @@ public static function detectTypeFromString(string $s, &$width = null, &$height

/**
* Returns the file extension for the given `ImageType::XXX` constant.
* @param ImageType::* $type
* @return value-of<self::Formats>
Copy link
Member

Choose a reason for hiding this comment

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

What's @return annotation good for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For me the most important thing out of this is specifying that the output extension is non-empty string, which is useful when you want to use the extension further. I guess @return non-empty-string could cover most of the use-cases, but why shouldn't we make the return type as narrow as possible?

Copy link
Member

Choose a reason for hiding this comment

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

Now, as a return annotation, this should probably be ImageType::*, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, because it returns one of the string values contained in self::Formats, not ImageType::* value.

*/
public static function typeToExtension(int $type): string
{
Expand All @@ -280,6 +286,7 @@ public static function typeToExtension(int $type): string

/**
* Returns the `ImageType::XXX` constant for given file extension.
* @return ImageType::*
*/
public static function extensionToType(string $extension): int
{
Expand All @@ -295,6 +302,7 @@ public static function extensionToType(string $extension): int

/**
* Returns the mime type for the given `ImageType::XXX` constant.
* @param ImageType::* $type
*/
public static function typeToMimeType(int $type): string
{
Expand All @@ -314,6 +322,7 @@ public function __construct(\GdImage $image)

/**
* Returns image width.
* @return positive-int
*/
public function getWidth(): int
{
Expand All @@ -323,6 +332,7 @@ public function getWidth(): int

/**
* Returns image height.
* @return positive-int
*/
public function getHeight(): int
{
Expand Down Expand Up @@ -351,7 +361,7 @@ public function getImageResource(): \GdImage

/**
* Scales an image. Width and height accept pixels or percent.
* @param self::OrSmaller|self::OrBigger|self::Stretch|self::Cover|self::ShrinkOnly $mode
* @param int-mask-of<self::OrSmaller|self::OrBigger|self::Stretch|self::Cover|self::ShrinkOnly> $mode
*/
public function resize(int|string|null $width, int|string|null $height, int $mode = self::OrSmaller): static
{
Expand Down Expand Up @@ -388,7 +398,7 @@ public function resize(int|string|null $width, int|string|null $height, int $mod

/**
* Calculates dimensions of resized image. Width and height accept pixels or percent.
* @param self::OrSmaller|self::OrBigger|self::Stretch|self::Cover|self::ShrinkOnly $mode
* @param int-mask-of<self::OrSmaller|self::OrBigger|self::Stretch|self::Cover|self::ShrinkOnly> $mode
*/
public static function calculateSize(
int $srcWidth,
Expand Down Expand Up @@ -534,7 +544,7 @@ public function sharpen(): static

/**
* Puts another image into this image. Left and top accepts pixels or percent.
* @param int $opacity 0..100
* @param int<0, 100> $opacity 0..100
*/
public function place(self $image, int|string $left = 0, int|string $top = 0, int $opacity = 100): static
{
Expand Down Expand Up @@ -596,6 +606,7 @@ public function place(self $image, int|string $left = 0, int|string $top = 0, in

/**
* Saves image to the file. Quality is in the range 0..100 for JPEG (default 85), WEBP (default 80) and AVIF (default 30) and 0..9 for PNG (default 9).
* @param ImageType::*|null $type
* @throws ImageException
*/
public function save(string $file, ?int $quality = null, ?int $type = null): void
Expand All @@ -607,6 +618,7 @@ public function save(string $file, ?int $quality = null, ?int $type = null): voi

/**
* Outputs image to string. Quality is in the range 0..100 for JPEG (default 85), WEBP (default 80) and AVIF (default 30) and 0..9 for PNG (default 9).
* @param ImageType::* $type
*/
public function toString(int $type = ImageType::JPEG, ?int $quality = null): string
{
Expand All @@ -627,6 +639,7 @@ public function __toString(): string

/**
* Outputs image to browser. Quality is in the range 0..100 for JPEG (default 85), WEBP (default 80) and AVIF (default 30) and 0..9 for PNG (default 9).
* @param ImageType::* $type
* @throws ImageException
*/
public function send(int $type = ImageType::JPEG, ?int $quality = null): void
Expand All @@ -639,6 +652,7 @@ public function send(int $type = ImageType::JPEG, ?int $quality = null): void
/**
* Outputs image to browser or file.
* @throws ImageException
* @param ImageType::* $type
*/
private function output(int $type, ?int $quality, ?string $file = null): void
{
Expand Down
30 changes: 22 additions & 8 deletions src/Utils/Paginator.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,30 @@
* @property int $page
* @property-read int $firstPage
* @property-read int|null $lastPage
* @property-read int $firstItemOnPage
* @property-read int $lastItemOnPage
* @property-read int<0,max> $firstItemOnPage
* @property-read int<0,max> $lastItemOnPage
* @property int $base
* @property-read bool $first
* @property-read bool $last
* @property-read int|null $pageCount
* @property int $itemsPerPage
* @property int|null $itemCount
* @property-read int $offset
* @property-read int|null $countdownOffset
* @property-read int $length
* @property-read int<0,max>|null $pageCount
* @property positive-int $itemsPerPage
* @property int<0,max>|null $itemCount
* @property-read int<0,max> $offset
* @property-read int<0,max>|null $countdownOffset
* @property-read int<0,max> $length
*/
class Paginator
{
use Nette\SmartObject;

private int $base = 1;

/** @var positive-int */
private int $itemsPerPage = 1;

private int $page = 1;

/** @var int<0, max>|null */
private ?int $itemCount = null;


Expand Down Expand Up @@ -81,6 +86,7 @@ public function getLastPage(): ?int

/**
* Returns the sequence number of the first element on the page
* @return int<0, max>
*/
public function getFirstItemOnPage(): int
{
Expand All @@ -92,6 +98,7 @@ public function getFirstItemOnPage(): int

/**
* Returns the sequence number of the last element on the page
* @return int<0, max>
*/
public function getLastItemOnPage(): int
{
Expand Down Expand Up @@ -120,6 +127,7 @@ public function getBase(): int

/**
* Returns zero-based page number.
* @return int<0, max>
*/
protected function getPageIndex(): int
{
Expand Down Expand Up @@ -152,6 +160,7 @@ public function isLast(): bool

/**
* Returns the total number of pages.
* @return int<0, max>|null
*/
public function getPageCount(): ?int
{
Expand All @@ -173,6 +182,7 @@ public function setItemsPerPage(int $itemsPerPage): static

/**
* Returns the number of items to display on a single page.
* @return positive-int
*/
public function getItemsPerPage(): int
{
Expand All @@ -192,6 +202,7 @@ public function setItemCount(?int $itemCount = null): static

/**
* Returns the total number of items.
* @return int<0, max>|null
*/
public function getItemCount(): ?int
{
Expand All @@ -201,6 +212,7 @@ public function getItemCount(): ?int

/**
* Returns the absolute index of the first item on current page.
* @return int<0, max>
*/
public function getOffset(): int
{
Expand All @@ -210,6 +222,7 @@ public function getOffset(): int

/**
* Returns the absolute index of the first item on current page in countdown paging.
* @return int<0, max>|null
*/
public function getCountdownOffset(): ?int
{
Expand All @@ -221,6 +234,7 @@ public function getCountdownOffset(): ?int

/**
* Returns the number of items on current page.
* @return int<0, max>
*/
public function getLength(): int
{
Expand Down
4 changes: 4 additions & 0 deletions src/Utils/Random.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ final class Random
/**
* Generates a random string of given length from characters specified in second argument.
* Supports intervals, such as `0-9` or `A-Z`.
*
* @param positive-int $length
* @param non-empty-string $charlist
* @return non-empty-string
*/
public static function generate(int $length = 10, string $charlist = '0-9a-z'): string
{
Expand Down
2 changes: 2 additions & 0 deletions src/Utils/Strings.php
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,7 @@ public static function trim(string $s, string $charlist = self::TrimCharacters):

/**
* Pads a UTF-8 string to given length by prepending the $pad string to the beginning.
* @param non-empty-string $pad
*/
public static function padLeft(string $s, int $length, string $pad = ' '): string
{
Expand All @@ -426,6 +427,7 @@ public static function padLeft(string $s, int $length, string $pad = ' '): strin

/**
* Pads UTF-8 string to given length by appending the $pad string to the end.
* @param non-empty-string $pad
*/
public static function padRight(string $s, int $length, string $pad = ' '): string
{
Expand Down
5 changes: 5 additions & 0 deletions src/Utils/Validators.php
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ public static function everyIs(iterable $values, string $expected): bool

/**
* Checks if the value is an integer or a float.
* @return ($value is int|float ? true : false)
*/
public static function isNumber(mixed $value): bool
{
Expand All @@ -221,6 +222,7 @@ public static function isNumber(mixed $value): bool

/**
* Checks if the value is an integer or a integer written in a string.
* @return ($value is non-empty-string ? bool : ($value is int ? true : false))
*/
public static function isNumericInt(mixed $value): bool
{
Expand All @@ -230,6 +232,7 @@ public static function isNumericInt(mixed $value): bool

/**
* Checks if the value is a number or a number written in a string.
* @return ($value is non-empty-string ? bool : ($value is int|float ? true : false))
*/
public static function isNumeric(mixed $value): bool
{
Expand Down Expand Up @@ -257,6 +260,7 @@ public static function isUnicode(mixed $value): bool

/**
* Checks if the value is 0, '', false or null.
* @return ($value is 0|''|false|null ? true : false)
*/
public static function isNone(mixed $value): bool
{
Expand All @@ -274,6 +278,7 @@ public static function isMixed(): bool
/**
* Checks if a variable is a zero-based integer indexed array.
* @deprecated use Nette\Utils\Arrays::isList
* @return ($value is list ? true : false)
*/
public static function isList(mixed $value): bool
{
Expand Down