Anteris-Dev / autotask-client

An HTTP client for the Autotask API built with PHP.
MIT License
23 stars 16 forks source link

Change return value of offsetGet() for child classes of DataTransferObjectCollection #46

Closed AntonioDiPassio-AppSys closed 1 year ago

AntonioDiPassio-AppSys commented 3 years ago

This applies to all *Collection classes, but I'll demonstrate with the ContactCollection class.

The offsetGet functions in the child classes are typehinted, but need to be made optional because the parent function can return null. Otherwise this will produce an error trying to return null

Should be changed to this: (notice the question mark)

    /**
     * Sets the proper return type for IDE completion.
     */
    public function offsetGet($offset): ?ContactEntity
    {
        return parent::offsetGet($offset);
    }
aidan-casey commented 2 years ago

@AntonioDiPassio-AppSys - There should be no scenario in which the collection is receiving null. Can you elaborate?

AntonioDiPassio-AppSys commented 2 years ago

This isn't about what the collection receives. The return value of the child function is declared as ContactEntity, but the parent function returns either a ContactEntity or null.

DataTranferObjectCollection line 39
public function offsetGet($offset)
{
    return $this->iterator[$offset] ?? null;
}

This will produce a TypeError when trying to get an element out of bounds

>>> $contacts = Autotask::contacts()->query()->where('emailAddress','eq','nonexistant@example.com')->get();
>>> $contacts->offsetGet(0);

TypeError: Anteris\Autotask\API\Contacts\ContactCollection::offsetGet(): Return value must be of type Anteris\Autotask\API\Contacts\ContactEntity, null returned