Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using array_key_exists() on objects is deprecated when using paginate() with object target #294

Open
ThauEx opened this issue Jul 16, 2021 · 9 comments

Comments

@ThauEx
Copy link

ThauEx commented Jul 16, 2021

When using the paginate() method of the Paginator class with an object as target, it throws the following deprecation:

array_key_exists(): Using array_key_exists() on objects is deprecated. Use isset() or property_exists() instead in vendor/knplabs/knp-components/src/Knp/Component/Pager/Pagination/AbstractPagination.php line 163

It can be fixed with the following change:

    public function offsetExists($offset): bool
    {
        if ($this->items instanceof \ArrayIterator) {
            return array_key_exists($offset, iterator_to_array($this->items));
        }

-        return array_key_exists($offset, $this->items);
+        return isset($this->items[$offset]);
    }

Since paginate has the following description for the $target argument:

  • @param mixed $target anything what needs to be paginated
    I would say this is a bug.

Should I open a PR for that?

@garak
Copy link
Collaborator

garak commented Jul 16, 2021

Can you provide an example of using paginate with an object?

@ThauEx
Copy link
Author

ThauEx commented Jul 16, 2021

I don't have any sample code, but it could be something like ArrayCollection from the doctrine collections.

@garak
Copy link
Collaborator

garak commented Jul 16, 2021

That's already working, as it's working with QueryBuilder objects and with many other objects.

@ThauEx
Copy link
Author

ThauEx commented Jul 16, 2021

I know that it works with ArrayCollection, but php 7.4 deprecated using array_key_exists on objects. This is the reason, why I opened this issue.
Together with the KnpPaginatorBundle and Twig, offsetExists gets called somewhere and triggers the mentioned deprecation message.

@garak
Copy link
Collaborator

garak commented Jul 16, 2021

A PR is welcome

@ThauEx
Copy link
Author

ThauEx commented Jul 18, 2021

Done 🙂

@W0rma
Copy link
Contributor

W0rma commented Aug 1, 2024

@garak The linked PR was closed more than one year ago with the following remark:

I'm closing this since we don't support PHP 7.4 anymore

Should this issue be closed as well?

@garak
Copy link
Collaborator

garak commented Aug 1, 2024

I'm not really sure.
In theory, it could be still a problem if someone would use the paginator with a custom object not implementing ArrayIterator.
We still have an inconsistency here.

@garak
Copy link
Collaborator

garak commented Aug 3, 2024

Maybe we can try this:

$items = get_mangled_object_vars($this->items);

return array_key_exists($offset, $items);

A PR is still welcome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants