Skip to content

Conversation

@curry684
Copy link
Contributor

@curry684 curry684 commented Nov 29, 2025

Let's make all of us out there using PHPstan at max level happy, and provide the correct return types to all Findable functions!

For easy review: the file to check is at https://github.com/picqer/exact-php-client/pull/683/files#diff-866469e983d1b8abc730b4cb257101d62275a08e5936fd40fcc7bdbeb2225acc

The other 426 changed files are mass find/replace of the same thing 😆

I've tested the change on a huge project in which we were ignoring over 40 errors caused by typing issues, all disappeared, and caused a few new ones from now obsolete type checks (this is irrelevant for B/C purposes as it only affects projects at level max which already have the other issue here and there's no runtime code).

Please do check that I didn't miss any T|null cases or anything.

@remkobrenters
Copy link
Collaborator

So first of all: Thank you. Strict is good. Theoretically the change is okay with me. I wonder if we should not extend it a little by adding a template type to the Findable trait and have it return the TModel that where relevant (various find, filter and collection methods).

@curry684
Copy link
Contributor Author

curry684 commented Dec 1, 2025

That is exactly what this PR does? 😆

@remkobrenters
Copy link
Collaborator

Wow you are right. I overlooked it in the amount of changed files (did not show up in my search). Looks good to me like this. Thanks.

@rubenwebparking
Copy link

Is there a reason you didn't use @return static (or @return static[]) in the trait? Since static resolves to the class using the trait, it would achieve the same result with less boilerplate.

The @template approach is typically more useful when you need the type in multiple places—like generic collections (Collection), parameters, or factory methods using class-string.

@curry684
Copy link
Contributor Author

curry684 commented Dec 1, 2025

Yes, I believe it should also work like that without the @use annotations. I went for the generic approach because it is internally typesafe (@template T of Model) and because I didn't expect static to play nice with the more complex functions returning generators. From cursory testing I tried just now it works fine for our project but we're in practice only using filter(...) so I'm not entirely sure it works the same.

Given the boilerplate argument we might as well go for the static approach, I'm just not sure it's as flexible towards the future.

@curry684
Copy link
Contributor Author

curry684 commented Dec 1, 2025

Provided #684 as alternative approach.

@curry684
Copy link
Contributor Author

curry684 commented Dec 9, 2025

Closing this in favor of #684

@curry684 curry684 closed this Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants