Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
This repository was archived by the owner on Oct 18, 2020. It is now read-only.

Conversation

tabuna
Copy link
Contributor

@tabuna tabuna commented Oct 17, 2019

Hi, this is a great package, but I think we can improve the experience with it.

In order for the IDE to have tips, you need to add a few comments, which will greatly facilitate life. In the same way they solve the problem in laravel itself. For example: Log, Hash, URL, etc...

This PR adds comments to the main methods.

Hi, this is a great package, but I think we can improve the experience with it.

In order for the IDE to have tips, you need to add a few comments, which will greatly facilitate life.
In the same way they solve the problem in laravel itself.
For example:
https://github.com/laravel/framework/blob/f61b787d88505d94d4ca691d43307518aed3dbd9/src/Illuminate/Support/Facades/Log.php#L5-L20

This PR adds comments to the main methods.
@d13r
Copy link
Owner

d13r commented Oct 17, 2019

I've always just used IDE Helper, but seems reasonable to add them (as discussed in #205).

I wonder if there's a way to have them maintained automatically, or at least add a unit test to ensure they're kept up-to-date... I might look into that before merging it.

Cheers

@tabuna
Copy link
Contributor Author

tabuna commented Oct 17, 2019

Oh, I did not look at the previous issue. Sorry.
If this is your condition, then I can do it. It’s easy to automate how you like this kind of comment.

Something like this (Not yet tests):

function checkMethodDocBlock(string $facade, string $class)
{
    $class = new ReflectionClass($class);
    $methods = $class->getMethods(ReflectionMethod::IS_PUBLIC);

    $methods = collect($methods)->map(function (ReflectionMethod $method) {
        return in_array($method->name, [
            '__construct',
            '__destruct',
            '__call',
            '__callStatic',
            '__get',
            '__set',
            '__isset',
            '__unset',
            '__sleep',
            '__wakeup',
            '__toString',
            '__invoke',
            '__set_state',
            '__clone',
            '__debugInfo',
        ], true) ? null : $method;
    })->filter()->map(function (ReflectionMethod $method){


        /** @var ReflectionParameter[] $parameters */
        $parameters = $method->getParameters() ?? [];

        $parameters = array_map(function (ReflectionParameter $parameter){
            $rawParams =  sprintf('%s $%s', optional($parameter->getType())->getName(), $parameter->getName());

            return trim($rawParams);
        },$parameters);

        $arguments = sprintf('(%s)',  implode(', ', $parameters));

        $docMethod = sprintf('@method static %s %s%s',
            $method->getReturnType(),
            $method->getName(),
            $arguments
        );

        return preg_replace('/\s+/', ' ', $docMethod);
    });


    $facadeDocs = (new ReflectionClass($facade))->getDocComment();

    $methods->each(function (string $method) use ($facadeDocs){

        if(!Str::contains($facadeDocs, $method)){
            echo "Not found: $method \n";
        }
    });

}

Usage and result:

checkMethodDocBlock(Breadcrumbs::class, BreadcrumbsManager::class);
Not found: @method static void for(string $name, callable $callback) 
Not found: @method static void register(string $name, callable $callback) 
Not found: @method static void before(callable $callback) 
Not found: @method static void after(callable $callback) 
Not found: @method static bool exists(string $name) 
Not found: @method static Collection generate(string $name, $params) 
Not found: @method static HtmlString view(string $view, string $name, $params) 
Not found: @method static HtmlString render(string $name, $params) 
Not found: @method static current() 
Not found: @method static void setCurrentRoute(string $name, $params) 
Not found: @method static void clearCurrentRoute() 
Not found: @method static macro($name, $macro) 
Not found: @method static mixin($mixin, $replace) 
Not found: @method static hasMacro($name) 

Of the minuses, it can be noted that it also captured the Macro trait.

Please tell me should I continue to do this? Hoping to merge?

@d13r
Copy link
Owner

d13r commented Oct 18, 2019

Yeah that looks awesome, thanks!

PS I think it's good to include the macro trait. (I should probably add it to the API Reference section of the readme too.)

@d13r d13r merged commit b7a24a5 into d13r:master Oct 20, 2019
@tabuna tabuna deleted the patch-1 branch October 23, 2019 08:37
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.