Skip to content

Module code adapted for PHPStan #213

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.

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

Merged
merged 1 commit into from
Aug 3, 2025
Merged

Conversation

TavoNiievez
Copy link
Member

No description provided.

@W0rma
Copy link
Contributor

W0rma commented Aug 2, 2025

The branch works fine for my own symfony project.

I plan to do the review within the next days.

Comment on lines +55 to +58
* @param object|class-string $mixed
* @return EntityRepository<object>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @param object|class-string $mixed
* @return EntityRepository<object>
* @phpstan-template T of object
* @phpstan-param T|class-string<T> $mixed
* @phpstan-return EntityRepository<T>

Use generics should make the type hints even more precise. WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

The truth is... I'm not sure.

I intentionally avoided using @phpstan-- annotations because, although I know it's possible to better express types with templates and generics, I think it makes the code more difficult to read for someone who isn't familiar with these concepts and tools, while also tying the type description too closely to a specific tool.

In other words, I would agree to implement them in a piece of code if and only if the simplification they provide outweighs the complexity they add.

@TavoNiievez TavoNiievez force-pushed the phpstan branch 5 times, most recently from aa8de78 to ab37721 Compare August 3, 2025 16:30
@TavoNiievez TavoNiievez force-pushed the phpstan branch 3 times, most recently from bf01524 to 869ab99 Compare August 3, 2025 20:56
@TavoNiievez TavoNiievez merged commit 784f1b0 into Codeception:main Aug 3, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants