brick / date-time

Date and time library for PHP
MIT License
321 stars 29 forks source link

Add @psalm-* annotations for better static analysis (compatible with PHPStan too) #97

Closed gnutix closed 3 months ago

gnutix commented 3 months ago

Below are the two types improvements I've noticed were missing when we've integrated the library in our application. I've not read every code signature to make sure everything was as precise as it could be though, so maybe other type improvements will come in the future.

TODO

BenMorel commented 3 months ago

Hi, good point, I actually added @psalm-return -1|0|1 to compareTo() in brick/math, but forgot to do the same here!

I'm curious about non-empty-string though, I've never come across a use case where this would be useful. Do you have an actual example code where knowing that the output of say toISOString() would help Psalm? I fail to see a real life use case where I'd check this against an empty string!

gnutix commented 3 months ago

An example that comes to mind would be indexing an array of Brick objects (let's say a LocalDate) by their ISO strings. If I have a function indexBy(array<T>, callable(T): non-empty-string): array<non-empty-string, T>, I want to make sure in the signature that it does not return an empty string, as indexing an array using one would produce unexpected results (the value would be overridden for each entry, and only the last one would remain).

Or if you have a method that expects some kind of "identifier", which could have invariants making sure it's not empty.

Sorry if it's not the most amazing use cases I'm thinking of right now, but that's something. ;)

BenMorel commented 3 months ago

Indeed that could make sense here. And it doesn't hurt ;-) Thanks for the example!

BenMorel commented 3 months ago

Please rebase to fix CS and I'll merge this! :+1:

Edit: looks like @psalm-return non-empty-string is missing on LocalDateRange?

BenMorel commented 3 months ago

Thank you, @gnutix!