UnsavedRelationList::first() / last() return false instead of null when list is empty

[kinglozzer]

Pull request: https://github.com/silverstripe/silverstripe-framework/pull/11085

Affected Version

5.x

Description

UnsavedRelationList::first() and UnsavedRelationList::last() return false instead of null when the list is empty. This is inconsistent with DataList and co, which return null. Cause is the use of reset() and end():

https://github.com/silverstripe/silverstripe-framework/blob/809f9e7ae06e4e3999ef2670aa322aacdddb7def/src/ORM/UnsavedRelationList.php#L238-L248

https://github.com/silverstripe/silverstripe-framework/blob/809f9e7ae06e4e3999ef2670aa322aacdddb7def/src/ORM/UnsavedRelationList.php#L255-L262

I suppose there’s a minor BC risk to changing this, open to feedback on whether this is appropriate for a patch release.

[GuySartorelli]

I'm of the opinion that false is just outright wrong here and should have always been null, and that anyone relying on false will/should be doing so only as a falsy check where null will behave the same way (i.e. if (!$returnValue)).

If this is new as of CMS 5, then I doubt anyone else has even noticed the changed return type.

tl;dr: I'd support this being patched rather than waiting for a major release.

But we should probably get at least one more opinion. @silverstripe/core-team any other opinions?

[michalkleiner]

patch 👍

[madmatt]

+1 for patch

[kinglozzer]

Thanks all, PR raised

[GuySartorelli]

Merged. Will be automatically tagged and merged up by GitHub actions