Skip to content
This repository has been archived by the owner on Sep 8, 2023. It is now read-only.

Parsing support for class+method callable and added --filename option #3

Merged
merged 2 commits into from
Mar 4, 2019

Conversation

vaites
Copy link
Contributor

@vaites vaites commented Feb 28, 2019

Laravel suggest to add macros using service providers. If you create the macro using a closure or a funcion the helper works well but fails if you use a callable with an array of class+method the helper thows an exception:

In MacrosCommand.php line 98:

Symfony\Component\Debug\Exception\FatalThrowableError]                                      
Type error: ReflectionFunction::__construct() expects parameter 1 to be string, array given

Example code:

<?php

namespace App\Providers;

use Illuminate\Support\Collection;
use Illuminate\Support\ServiceProvider;

class CollectionServiceProvider extends ServiceProvider
{
    public function register()
    {
        Collection::macro('test1', [$this, 'test1']);
        Collection::macro('test2', [static::class, 'test2']);
    }

    /**
     * Test
     *
     * @return int
     */
    public function test1()
    {
        return time();
    }

    /**
     * Static test
     *
     * @return int
     */
    public static function test2()
    {
        return time();
    }
}

This makes the code more readable, so this PR fixes the exception thrown by the helper trying parse this behaviour.

I also added the --filename= option to command (like Laravel IDE Helper) that allows users to change the filename and path without publishing any additional config file (perfect for composer scripts).

Thanks @KristofMorva

@vaites vaites changed the title Parsing support for class+method callable Parsing support for class+method callable and added --filename option Mar 1, 2019
@KristofMorva
Copy link
Owner

Looks good, thanks! ;)

@KristofMorva KristofMorva merged commit 33632e7 into KristofMorva:master Mar 4, 2019
@vaites
Copy link
Contributor Author

vaites commented Mar 4, 2019

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants