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

[5.4] Arr::pluck - handle situation when itemKey is an object #19838

Merged
merged 2 commits into from
Jun 30, 2017
Merged

[5.4] Arr::pluck - handle situation when itemKey is an object #19838

merged 2 commits into from
Jun 30, 2017

Conversation

crlcu
Copy link
Contributor

@crlcu crlcu commented Jun 30, 2017

When using pluck on a collection who's items are Eloquent models and some of the fields are casted as date an error is thrown:

[2017-06-30 11:20:34] local.ERROR: ErrorException: Illegal offset type in /var/www/html/tpt/vendor/laravel/framework/src/Illuminate/Support/Arr.php:365
Stack trace:
#0 /var/www/html/tpt/vendor/laravel/framework/src/Illuminate/Support/Arr.php(365): Illuminate\Foundation\Bootstrap\HandleExceptions->handleError(2, 'Illegal offset ...', '/var/www/html/t...', 365, Array)
#1 /var/www/html/tpt/vendor/laravel/framework/src/Illuminate/Support/Collection.php(689): Illuminate\Support\Arr::pluck(Array, Array, Array)
#2 /var/www/html/tpt/app/Console/Commands/Test.php(48): Illuminate\Support\Collection->pluck('id', 'start_at')
#3 [internal function]: App\Console\Commands\Test->handle()
#4 /var/www/html/tpt/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(29): call_user_func_array(Array, Array)
#5 /var/www/html/tpt/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(87): Illuminate\Container\BoundMethod::Illuminate\Container\{closure}()
#6 /var/www/html/tpt/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(31): Illuminate\Container\BoundMethod::callBoundMethod(Object(Illuminate\Foundation\Application), Array, Object(Closure))
#7 /var/www/html/tpt/vendor/laravel/framework/src/Illuminate/Container/Container.php(531): Illuminate\Container\BoundMethod::call(Object(Illuminate\Foundation\Application), Array, Array, NULL)
#8 /var/www/html/tpt/vendor/laravel/framework/src/Illuminate/Console/Command.php(182): Illuminate\Container\Container->call(Array)
#9 /var/www/html/tpt/vendor/symfony/console/Command/Command.php(264): Illuminate\Console\Command->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Illuminate\Console\OutputStyle))
#10 /var/www/html/tpt/vendor/laravel/framework/src/Illuminate/Console/Command.php(167): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Illuminate\Console\OutputStyle))
#11 /var/www/html/tpt/vendor/symfony/console/Application.php(835): Illuminate\Console\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#12 /var/www/html/tpt/vendor/symfony/console/Application.php(200): Symfony\Component\Console\Application->doRunCommand(Object(App\Console\Commands\Test), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#13 /var/www/html/tpt/vendor/symfony/console/Application.php(124): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#14 /var/www/html/tpt/vendor/laravel/framework/src/Illuminate/Foundation/Console/Kernel.php(122): Symfony\Component\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#15 /var/www/html/tpt/artisan(35): Illuminate\Foundation\Console\Kernel->handle(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#16 {main}  

This commit is meant to fix this issue.

@taylorotwell
Copy link
Member

Why would we convert them to strings?

@crlcu
Copy link
Contributor Author

crlcu commented Jun 30, 2017

Because the key can either be an integer or a string, if the key is something else, in this case a Carbon object, an error is thrown.

@tillkruss tillkruss changed the title Arr::pluck - handle situation when itemKey is an object [5.4] Arr::pluck - handle situation when itemKey is an object Jun 30, 2017
@taylorotwell taylorotwell merged commit 900a39c into laravel:5.4 Jun 30, 2017
@Douglasdc3
Copy link
Contributor

This code will only work if the given class implements the magic __toString method otherwise it will throw PHP Error with the following test cases:

public function testPluckWithStdClass()
{
    $array = [
        ['start' => new \stdClass, 'end' => new \stdClass],
    ];
    Arr::pluck($array, 'end', 'start');
}

public function testPluckWithClass()
{
    $array = [
        ['start' => new Arr(), 'end' => new Arr()],
    ];
    Arr::pluck($array, 'end', 'start');
}

Output:


There were 2 errors:

1) Illuminate\Tests\Support\SupportArrTest::testPluckWithStdClass
Object of class stdClass could not be converted to string

/Users/kenandries/development/laravel/19838/src/Illuminate/Support/Arr.php:384
/Users/kenandries/development/laravel/19838/tests/Support/SupportArrTest.php:388

2) Illuminate\Tests\Support\SupportArrTest::testPluckWithClass
Object of class Illuminate\Support\Arr could not be converted to string

/Users/kenandries/development/laravel/19838/src/Illuminate/Support/Arr.php:384
/Users/kenandries/development/laravel/19838/tests/Support/SupportArrTest.php:396

From the php documentation: Since PHP 5.2.0, converting objects without __toString() method to string would cause E_RECOVERABLE_ERROR

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

Successfully merging this pull request may close these issues.

3 participants