refactor: enforce strict typing

[mairo744]

Q	A
Documentation	no
Bugfix	no
BC Break	yes
New Feature	no
RFC	no
QA	yes

Description

This PR introduces strict typing enforcement using Rector.

• Adds Rector to the project and configures it to enforce strict typing rules.
• Applies readonly properties where applicable.
• Adds scalar and class parameter types, return types, and property types throughout the codebase.
• Updates psalm-baseline.xml to remove ignored issues that are now resolved through typing improvements.

[Xerkus]

This is potentially problematic due to no constraints on swoole versions. We test with a single swoole version while there are several major releases. This might be broken and we wouldn't even know it.

[Ocramius]

Wait, ext-swoole is not even listed under require?!

[Xerkus]

@Ocramius what is worse it can be swoole or openswoole, which diverged and later is pretty much in maintenance only state for a while.

I did bring this up in TSC meeting last year although discussion went nowhere at the time iirc

[gsteel]

We need to drop support for OpenSwoole IMO and require ext-swoole at specific versions for the next major.

I don't think we need a TSC vote to do that do we?

[Ocramius]

The worst that can happen is blocking off an upgrade path: constraint reduction can always be done, while widening is more "definitive"

[mairo744]

Thanks everyone for the feedback.

I’ve updated this in #144 to use Swoole v6, which is now installed and tested in CI. Everything passes and the typing changes work correctly.

Let me know if there’s anything else needed to move this forward.

[Ocramius]

@mairo744 I'd say that a rebase of this one is needed then :-)

[Xerkus]

It was rebased.

For the record, I do not like addition of QA tooling to composer.json and to package workflows like here. It is something that needs to be done organization wide via shared CI workflows and actions to be consistent everywhere.

[mairo744]

I automated this using Rector and added it to the CI workflow to prevent newly added code from violating these rules. I think this is important to maintain code quality.

That said, I agree it would be better to handle this organization-wide through shared CI workflows for consistency.
I also noticed that dependency checking is currently missing, which I believe is an important part of QA as well.

Would you be able to bring this up with the Laminas TSC, or let me know how I can help move it forward? I’d really like to contribute, but I’m not sure what the best next step is.

[Ocramius]

I'm fine with adding tooling that is locked here: the maintainers of mezzio/mezzio-swoole can kick it out at any time, if needed.

Introducing it in a more generalized way would be nice, and can be proposed for the Laminas CI Matrix Action, IMO: I'd love to see it happen, and I never found the time to do it.

[Xerkus]

Psalm is not swoole aware so I do not have high confidence. I will need to carefully review this manually.

@mairo744 is this blocking any more work you want to do on this component?

[mairo744]

Thanks for taking the time to review this.

I can hold off on the Rector improvements for now and move to dependency analysis or other work. I think Psalm should be able to check most of the Swoole code with swoole/ide-helper, but I agree that manual review is important.

Looking forward to your feedback

[Ocramius]

This is a lot of tiny BC breaks all over the place, but IMO should be go in as-is.

@Xerkus I think you should press the merge button, when happy about it.

[Xerkus]

Added couple more type declarations in tests and other minor QA improvements.

[Xerkus]

@mairo744 thank you