Fixed Type hint on entityColumnsToJson is incorrect

Affected version
2.2.2

Xon

Well-known member
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

Well-known member
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

XenForo bug fixer bot
Staff member
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

XenForo developer
Staff member
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.
 
Top