laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.52k stars 11.02k forks source link

PHPDoc param is multitype #38217

Closed carlos-mora closed 3 years ago

carlos-mora commented 3 years ago

Description:

Enhance type of param in Illuminate/Cache/Repository.php

    /**
     * Retrieve an item from the cache by key.
     *
     * @param  string  $key
     * @param  mixed  $default
     * @return mixed
     */
    public function get($key, $default = null)
    {

In get function, $key param must be declared as dual typed string|array

The same issue in put method.

Proposal

Change the @param to list the allowed types:

    /**
     * Retrieve an item from the cache by key.
     *
     * @param  string|array  $key
     * @param  mixed  $default
     * @return mixed
     */
    public function get($key, $default = null)
    {
driesvints commented 3 years ago

The current types are correct and adhere to the PSR simplecache interface.

carlos-mora commented 3 years ago

The current types adhere to the PSR simplecache interface, so it's ok, but I don't fully agree that the types are correct, at least according to the function's code. The real behavoir of the function's code accepts and handles 'array' as the first param, therefore there will be client code that invoques it with an array as first param and will fire PHP linters issues. Obviously it's not an easy question to solve: Adhere to the PSR or make the real code work with linters. It seems to me as a convention clashing, because right param specs are also a conventional use. KR