Refactor closure container poc

[liamduckett]

Hey,

I was looking #54628 and spotted a few small potential static analysis related improvements.

cc @peterfox (very cool feature by the way!)

[liamduckett]

Failure doesn't look related to my changes?

[taylorotwell]

I am closing this pull request because it lacks sufficient explanation, tests, or both. It is difficult for us to merge pull requests without these things because the change may introduce breaking changes to the framework.

Feel free to re-submit your change with a thorough explanation of the feature and tests - integration tests are preferred over unit tests. Please include it's benefit to end users; the reasons it does not break any existing features; how it makes building web applications easier, etc.

Thanks!

[liamduckett]

Apologies @taylorotwell - should have explained what I was doing here...

Unsure whether to open a new PR, or comment on this one - so I've gone for the easier option first.

I've made a few code quality changes (covered below), functionality stays exactly the same.

1. 'void' method 'bindBasedOnClosureReturnTypes' result used

// src/Illuminate/Container/Container.php (284)
if ($abstract instanceof Closure) {
    return $this->bindBasedOnClosureReturnTypes(
        $abstract, $concrete, $shared
    );
} 

This snippet return the result of a method which returns void. The return has been moved to after the expression.

if ($abstract instanceof Closure) {
   $this->bindBasedOnClosureReturnTypes($abstract, concrete, $shared);

   return;
}

2. Unused parameter 'concrete'. The parameter's value is overwritten immediately.

// src/Illuminate/Container/Container.php (480)
protected function bindBasedOnClosureReturnTypes($abstract, $concrete = null, $shared = false)
{
    $abstracts = $this->closureReturnTypes($abstract);

    $concrete = $abstract;

    foreach ($abstracts as $abstract) {
        $this->bind($abstract, $concrete, $shared);
    }
}

The $concrete parameter doesn't need to be here - it's overridden immediately.

// src/Illuminate/Container/Container.php (480)
protected function bindBasedOnClosureReturnTypes($abstract, $shared = false)
{
    $abstracts = $this->closureReturnTypes($abstract);

    $concrete = $abstract;

    foreach ($abstracts as $abstract) {
        $this->bind($abstract, $concrete, $shared);
    }
}

3. Disallow passing a string $abstract to bindBasedOnClosureReturnTypes

// src/Illuminate/Container/Container.php (472)
/**
 * Register a binding with the container based on the given Closure's return types.
 *
 * @param  \Closure|string  $abstract
 * @param  \Closure|string|null  $concrete
 * @param  bool  $shared
 * @return void
 */
protected function bindBasedOnClosureReturnTypes($abstract, $concrete = null, $shared = false)
{
    $abstracts = $this->closureReturnTypes($abstract);

    $concrete = $abstract;

    foreach ($abstracts as $abstract) {
        $this->bind($abstract, $concrete, $shared);
    }
}

// src/Illuminate/Container/Container.php (480)
protected function closureReturnTypes(Closure $closure)
{
    // ...
}

closureReturnTypes only accepts a Closure, therefore passing a string $abstract to bindBasedOnClosureReturnTypes would result in a type error.

// src/Illuminate/Container/Container.php (472)
/**
 * Register a binding with the container based on the given Closure's return types.
 *
 * @param  \Closure  $abstract
 * @param  \Closure|string|null  $concrete
 * @param  bool  $shared
 * @return void
 */
protected function bindBasedOnClosureReturnTypes($abstract, $concrete = null, $shared = false)
{
    $abstracts = $this->closureReturnTypes($abstract);

    $concrete = $abstract;

    foreach ($abstracts as $abstract) {
        $this->bind($abstract, $concrete, $shared);
    }
}

// src/Illuminate/Container/Container.php (480)
protected function closureReturnTypes(Closure $closure)
{
    // ...
}

4. Add types to new methods

Maybe less applicable now that the feature has been out in the wild? The goal was to enforce types at runtime for the newly introduced methods - which I believe aligns with the frameworks attitude towards typing (please correct me if I'm wrong).