Fixed Type hint on entityColumnsToJson is incorrect

[Xon]

Affected version

2.2.2

entityColumnsToJson has the return type of array but the function it calls tableColumnsToJson is array|bool is there any reason this couldn't just return an null to signal to exit instead of 'true' ? (ie so this could be replaced with nullable support). Or just make the installStepRunner convert an empty array as a terminal signal?

 

[Xon]

This is noticeable because modern phpstorm will suggest a return type of :array when using entityColumnsToJson in an installer step. Which throws an error if encountered

 

[XF Bug Bot]

Thank you for reporting this issue, it has now been resolved. We are aiming to include any changes that have been made in a future XF release (2.2.4).

Change log:

Update phpdoc on entityColumnsToJson method to indicate the correct return type.

There may be a delay before changes are rolled out to the XenForo Community.

 

[Chris D]

I think we'll just flip to array|bool on the php doc to reflect the correct return type which is likely the safest thing to do.

If we were writing the method now we would likely make tableColumnsToJson simply return; rather than return true; and then that would be ready for using a nullable return type if/when we support PHP 7.1. That said, that's somewhat of a moot point as we won't be retroactively giving return types to existing methods anyway. But ultimately not something that's likely worth making big changes to now.