Refactoring: type-safety along with some more simple code-paths

[boesing]

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

Description

This refactoring contains several changes:

• Removed psalm-baseline.xml
• Added array shapes where necessary
• Imported array shape from composer where possible
• overhaul of the Collection class
  • removed Collection#reduce as array_reduce stuff is really hard to read (IMHO)
  • dropped unused methods within the Collection class
  • added Collection#anySatisfies to verify if any of the elements satisfy a specific requirement
  • Make Collection generic so that it can hold any type of data
  • Removed ArrayAccess from Collection
    • This removes the necessity of adding these ReturnTypeWillChange attributes on PHP 8
    • Added Collection#has, Collection#get, Collection#set, Collection#remove instead
  • Added native typehint to Collection#count so we can remove ReturnTypeWillChange attribute
  • Added native typehint to Collection#getIterator so we can remove ReturnTypeWillChange attribute
  • Added Collection#toOrderedCollection to remove array keys when necessary
  • Added Collection#getKey to find a specific key by a known value
• Removed all usages of Collection#reduce
• Marked everything final
• Removed lazy callable factory for the PackageProviderDetectionFactory
• Added native type-hints and property-types wherever possible
• Fixed components which changed extra.laminas during versions while also providing multiple modules/config-provider

Overall, the behavior is still the same. This just adds plenty of type-safety.

BC break only due to the whole final and native property-, argument- and return-typehints.
I am not 100% sure if we have to create a new major release for this but if we want to do so, I am fine with that as well.

[froschdesign]

@boesing

BC break only due to the whole final and native property-, argument- and return-typehints.
I am not 100% sure if we have to create a new major release for this but if we want to do so, I am fine with that as well.

Are there any side effects with other components or their installation? If not, then create a new major version.

[Ocramius]

LGTM, but it does require a major version indeed.

I'd also endorse making everything that we can @internal, to prevent misuse: lots of very internal details in the Injector namespace, I think.

[boesing]

Are there any side effects with other components or their installation? If not, then create a new major version.

Not 100% sure if I understand that question. Featureset here is equal to the current version. But if we consider strict semver for composer plugins (which should only be used as composer plugins and not as libraries) the same way we do on libraries, then we should create a new major. But when it comes to the end-result this plugin has to achieve (adding components to config files after asking the user during composer install, composer require, composer remove or composer update), we do not need to create a new major.

[weierophinney]

There's some choices I'd have done differently, but it achieves the goal (better type-safety), which in turn will make it more maintainable, so 👍.

Considering that this is a package meant for consumption, not extension, I tend to agree that a new major is not needed; it achieves the same goals and has the same behavior for consumers after the refactor.

[boesing]

@weierophinney Please let me know what you would do differently. Always happy to get a different view.
I just wanted to get rid of that reduce - maybe another rewrite without the whole collection stuff would be better but I wanted to keep functionality consistent without the need to rewrite every test.

[weierophinney]

I just wanted to get rid of that reduce

See, I prefer reduce to each + reference. 😄

But probably the bigger refactor I'd do is to remove the collections stuff entirely; it was a solution in search of a problem at the time, and I think we could likely do this a lot more tightly with modern PHP than I did when I wrote this (more value objects, smaller public interface). But for now, this is a really good step towards better maintainability.

[boesing]

Okay. I guess releasing this as a new major will suffice. there are no dependants of this component and upgrading to the new major will be easy for those who are not using our code as library code. With the next major, everything is internal and starting with that, we can do whatever bc breaks we want to implement from an API perspective.

Cool for everyone?

I also added the other issues to the next major and I will work on this after my holidays which will start on 06-15. 👌🏻

[boesing]

Since I got no feedback, I guess we are good to go?
I will back a release on sunday so there is still some time left for feedback tho.

[Ocramius]

New major works 👍