Not a bug XF\Repository\ChangeLog :: findChangeLogsByContentType should type hint array|string rather than just string

DragonByte Tech

Well-known member
Affected version
2.0.10
If a developer uses custom content types as well as the change log system, we may wish to fetch change logs by multiple content types. While this works, phpStorm will throw a warning because the function is type hinted to only accept strings.

I realise the function itself also indicates it's meant to be a single content type, but in the interest of not breaking BC you probably don't want to rename the function.

No actual functionality / code needs to change since the Finder system already accepts an array here.


Fillip
 
Probably should be string[]|string (ie an array of strings vs a single string) rather than an array of arbitrary items or strings.
 
Last edited:
This is a bit of a philosophical issue, but I don't think we'll be making a specific change here. It's definitely not something we would consider a bug. That the function supports an array is more of a technical quirk than an intentional design (for the function in question). The naming of the function (and param) and the type hint should make it clear that the function expects a single, scalar input. When we have functions that take a scalar or array explicitly, these are usually marked up in code to make it clear (with is_array checks or (array)$var type casting).

You're right that we could change this function, though I imagine that would also apply to a number of other functions, so hence I'm taking a more general approach in saying that we are fairly unlikely to make changes of this nature without clear need to. By introducing a single-multi behavior change to a function like this, it can add significant complexity if future changes require acting against the content type passed, for example.

Given that you have a requirement which isn't really covered by the function--and it's a simple function--I think it's best that you implement something that works specifically for you. (In this instance, it probably just means instantiating the finder directly.)
 
Back
Top Bottom