Merge a0b90a3 into 1621ced

PHP-DI · Jun 12, 2017 · b9c82de · b9c82de
2 parents 1621ced + a0b90a3
commit b9c82de
Show file tree

Hide file tree

Showing 12 changed files with 331 additions and 38 deletions.
diff --git a/composer.json b/composer.json
@@ -27,7 +27,9 @@
         "php": ">=7.0.0",
         "psr/container": "^1.0",
         "php-di/invoker": "^2.0",
-        "php-di/phpdoc-reader": "^2.0.1"
+        "php-di/phpdoc-reader": "^2.0.1",
+        "roave/better-reflection": "^1.2",
+        "nikic/php-parser": "^2.0|^3.0"
     },
     "require-dev": {
         "phpunit/phpunit": "~5.7",

diff --git a/doc/performances.md b/doc/performances.md
@@ -82,12 +82,18 @@ Currently PHP-DI does not traverse directories to find autowired or annotated cl
 
 Please note that the following definitions are not compiled (yet):
 
-- [factory definitions](php-definitions.md#factories)
 - [decorator definitions](php-definitions.md#decoration)
 - [wildcard definitions](php-definitions.md#wildcards)
 
 Those definitions will still work perfectly, they will simply not get a performance boost when using a compiled container.
 
+On the other hand factory definitions (either defined with closures or with class factories) are supported in the compiled container. However please note that if you are using closures as factories:
+
+- you should not use `$this` inside closures
+- you should not import variables inside the closure using the `use` keyword, like in `function () use ($foo) { ...`
+
+These limitations exist because the code of each closure is copied into the compiled container. It is safe to say that you should probably not do these things even if you do not compile the container.
+
 ### How it works
 
 PHP-DI will read definitions from your [configuration](php-definitions.md). When the container is compiled, PHP code will be generated based on those definitions.

diff --git a/doc/php-definitions.md b/doc/php-definitions.md
@@ -99,7 +99,7 @@ However **this is not recommended** as that object will be created *for every PH
 
 ### Factories
 
-Factories are **PHP callables** that return the instance. They allow to define objects *lazily*, i.e. they will be created only when actually used.
+Factories are **PHP callables** that return the instance. They allow to define objects *lazily*, i.e. each object will be created only when actually needed.
 
 Here is an example using a closure:
 
@@ -197,7 +197,7 @@ Please note:
 
 - `factory([FooFactory::class, 'build'])`: if `build()` is a **static** method then the object will not be created: `FooFactory::build()` will be called statically (as one would expect)
 - you can set any container entry name in the array, e.g. `DI\factory(['foo_bar_baz', 'build'])` (or alternatively: `DI\factory('foo_bar_baz::build')`), allowing you to configure `foo_bar_baz` and its dependencies like any other object
-- as a factory can be any PHP callable, you can use invokable objects, too: `DI\factory(InvocableFooFactory::class)` (or alternatively: `DI\factory('invocable_foo_factory')`, if it's defined in the container)
+- as a factory can be any PHP callable, you can use invokable objects, too: `DI\factory(InvokableFooFactory::class)` (or alternatively: `DI\factory('invokable_foo_factory')`, if it's defined in the container)
 
 #### Retrieving the name of the requested entry
 

diff --git a/src/CompiledContainer.php b/src/CompiledContainer.php
@@ -4,7 +4,17 @@
 
 namespace DI;
 
+use DI\Compiler\RequestedEntryHolder;
 use DI\Definition\Definition;
+use DI\Definition\Exception\InvalidDefinition;
+use DI\Invoker\FactoryParameterResolver;
+use Invoker\Exception\NotCallableException;
+use Invoker\Exception\NotEnoughParametersException;
+use Invoker\Invoker;
+use Invoker\InvokerInterface;
+use Invoker\ParameterResolver\AssociativeArrayResolver;
+use Invoker\ParameterResolver\NumericArrayResolver;
+use Invoker\ParameterResolver\ResolverChain;
 
 /**
  * Compiled version of the dependency injection container.
@@ -13,6 +23,11 @@
  */
 abstract class CompiledContainer extends Container
 {
+    /**
+     * @var InvokerInterface
+     */
+    private $factoryInvoker;
+
     /**
      * {@inheritdoc}
      */
@@ -74,4 +89,33 @@ protected function setDefinition(string $name, Definition $definition)
         // every time, which kinds of defeats the performance gains of the compiled container
         throw new \LogicException('You cannot set a definition at runtime on a compiled container. You can either put your definitions in a file, disable compilation or ->set() a raw value directly (PHP object, string, int, ...) instead of a PHP-DI definition.');
     }
+
+    /**
+     * Invoke the given callable.
+     */
+    protected function resolveFactory($callable, $entryName, array $extraParameters = [])
+    {
+        // Initialize the factory resolver
+        if (! $this->factoryInvoker) {
+            $parameterResolver = new ResolverChain([
+                new AssociativeArrayResolver,
+                new FactoryParameterResolver($this->delegateContainer),
+                new NumericArrayResolver,
+            ]);
+
+            $this->factoryInvoker = new Invoker($parameterResolver, $this->delegateContainer);
+        }
+
+        $parameters = [$this->delegateContainer, new RequestedEntryHolder($entryName)];
+
+        $parameters = array_merge($parameters, $extraParameters);
+
+        try {
+            return $this->factoryInvoker->call($callable, $parameters);
+        } catch (NotCallableException $e) {
+            throw new InvalidDefinition("Entry \"$entryName\" cannot be resolved: factory " . $e->getMessage());
+        } catch (NotEnoughParametersException $e) {
+            throw new InvalidDefinition("Entry \"$entryName\" cannot be resolved: " . $e->getMessage());
+        }
+    }
 }
diff --git a/src/Compiler.php b/src/Compiler.php
@@ -4,6 +4,8 @@
 
 namespace DI;
 
+use BetterReflection\Reflection\ReflectionFunction;
+use BetterReflection\SourceLocator\Exception\TwoClosuresOneLine;
 use DI\Compiler\ObjectCreationCompiler;
 use DI\Definition\AliasDefinition;
 use DI\Definition\ArrayDefinition;
@@ -18,6 +20,7 @@
 use DI\Definition\StringDefinition;
 use DI\Definition\ValueDefinition;
 use InvalidArgumentException;
+use PhpParser\Node\Expr\Closure;
 
 /**
  * Compiles the container into PHP code much more optimized for performances.
@@ -43,13 +46,19 @@ class Compiler
      */
     private $methods = [];
 
+    /**
+     * @var bool
+     */
+    private $autowiringEnabled;
+
     /**
      * Compile the container.
      *
      * @return string The compiled container class name.
      */
-    public function compile(DefinitionSource $definitionSource, string $fileName) : string
+    public function compile(DefinitionSource $definitionSource, string $fileName, bool $autowiringEnabled) : string
     {
+        $this->autowiringEnabled = $autowiringEnabled;
         $this->containerClass = basename($fileName, '.php');
 
         // Validate that it's a valid class name
@@ -88,6 +97,8 @@ public function compile(DefinitionSource $definitionSource, string $fileName) :
     }
 
     /**
+     * @throws DependencyException
+     * @throws InvalidDefinition
      * @return string The method name
      */
     private function compileDefinition(string $entryName, Definition $definition) : string
@@ -138,6 +149,31 @@ private function compileDefinition(string $entryName, Definition $definition) :
                 $compiler = new ObjectCreationCompiler($this);
                 $code = $compiler->compile($definition);
                 $code .= "\n        return \$object;";
+                break;
+            case $definition instanceof FactoryDefinition:
+                $value = $definition->getCallable();
+
+                // Custom error message to help debugging
+                $isInvokableClass = is_string($value) && class_exists($value) && method_exists($value, '__invoke');
+                if ($isInvokableClass && !$this->autowiringEnabled) {
+                    throw new InvalidDefinition(sprintf(
+                        'Entry "%s" cannot be compiled. Invokable classes cannot be automatically resolved if autowiring is disabled on the container, you need to enable autowiring or define the entry manually.',
+                        $entryName
+                    ));
+                }
+
+                $definitionParameters = '';
+                if (!empty($definition->getParameters())) {
+                    $definitionParameters = ', ' . $this->compileValue($definition->getParameters());
+                }
+
+                $code = sprintf(
+                    'return $this->resolveFactory(%s, %s%s);',
+                    $this->compileValue($value),
+                    var_export($entryName, true),
+                    $definitionParameters
+                );
+
                 break;
             default:
                 // This case should not happen (so it cannot be tested)
@@ -182,6 +218,10 @@ public function compileValue($value) : string
             return "[\n$value        ]";
         }
 
+        if ($value instanceof \Closure) {
+            return $this->compileClosure($value);
+        }
+
         return var_export($value, true);
     }
 
@@ -210,13 +250,13 @@ private function isCompilable($value)
 
             return 'A decorator definition was found but decorators cannot be compiled';
         }
-        if ($value instanceof FactoryDefinition) {
-            return 'A factory definition was found but factories cannot be compiled';
-        }
         // All other definitions are compilable
         if ($value instanceof Definition) {
             return true;
         }
+        if ($value instanceof \Closure) {
+            return true;
+        }
         if (is_object($value)) {
             return 'An object was found but objects cannot be compiled';
         }
@@ -226,4 +266,32 @@ private function isCompilable($value)
 
         return true;
     }
+
+    private function compileClosure(\Closure $value) : string
+    {
+        try {
+            $reflection = ReflectionFunction::createFromClosure($value);
+        } catch (TwoClosuresOneLine $e) {
+            throw new InvalidDefinition('Cannot compile closures when two closures are defined on the same line', 0, $e);
+        }
+
+        /** @var Closure $ast */
+        $ast = $reflection->getAst();
+
+        // Force all closures to be static (add the `static` keyword), i.e. they can't use
+        // $this, which makes sense since their code is copied into another class.
+        $ast->static = true;
+
+        // Check if the closure imports variables with `use`
+        if (! empty($ast->uses)) {
+            throw new InvalidDefinition('Cannot compile closures which import variables using the `use` keyword');
+        }
+
+        $code = (new \PhpParser\PrettyPrinter\Standard)->prettyPrint([$reflection->getAst()]);
+
+        // Trim spaces and the last `;`
+        $code = trim($code, "\t\n\r;");
+
+        return $code;
+    }
 }
diff --git a/src/Compiler/RequestedEntryHolder.php b/src/Compiler/RequestedEntryHolder.php
@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DI\Compiler;
+
+use DI\Factory\RequestedEntry;
+
+/**
+ * @author Matthieu Napoli <matthieu@mnapoli.fr>
+ */
+class RequestedEntryHolder implements RequestedEntry
+{
+    /**
+     * @var string
+     */
+    private $name;
+
+    public function __construct(string $name)
+    {
+        $this->name = $name;
+    }
+
+    public function getName() : string
+    {
+        return $this->name;
+    }
+}
diff --git a/src/ContainerBuilder.php b/src/ContainerBuilder.php
@@ -144,7 +144,12 @@ public function build()
         $containerClass = $this->containerClass;
 
         if ($this->compileToFile) {
-            $containerClass = (new Compiler)->compile($source, $this->compileToFile);
+            $containerClass = (new Compiler)->compile(
+                $source,
+                $this->compileToFile,
+                $this->useAutowiring || $this->useAnnotations
+            );
+
             // Only load the file if it hasn't been already loaded
             // (the container can be created multiple times in the same process)
             if (!class_exists($containerClass, false)) {

diff --git a/tests/IntegrationTest/CompiledContainerTest.php b/tests/IntegrationTest/CompiledContainerTest.php
@@ -6,7 +6,6 @@
 
 use DI\ContainerBuilder;
 use function DI\create;
-use function DI\factory;
 
 /**
  * Tests specific to the compiled container.
@@ -72,24 +71,6 @@ public function anonymous_classes_cannot_be_compiled()
         $builder->build();
     }
 
-    /**
-     * @test
-     * @expectedException \DI\Definition\Exception\InvalidDefinition
-     * @expectedExceptionMessage Entry "stdClass" cannot be compiled: A factory definition was found but factories cannot be compiled
-     */
-    public function factories_nested_in_other_definitions_cannot_be_compiled()
-    {
-        $builder = new ContainerBuilder;
-        $builder->addDefinitions([
-            \stdClass::class => create()
-                ->property('foo', factory(function () {
-                    return 'hello';
-                })),
-        ]);
-        $builder->compile(self::generateCompilationFileName());
-        $builder->build();
-    }
-
     /**
      * @test
      * @expectedException \DI\Definition\Exception\InvalidDefinition
@@ -156,3 +137,10 @@ public function entries_cannot_be_overridden_by_definitions_in_the_compiled_cont
         $container->set('foo', create(ContainerSetTest\Dummy::class));
     }
 }
+
+namespace DI\Test\IntegrationTest\CompiledContainerTest;
+
+class Property
+{
+    public $foo;
+}
diff --git a/tests/IntegrationTest/Definitions/FactoryDefinition/config.inc b/tests/IntegrationTest/Definitions/FactoryDefinition/config.inc
@@ -0,0 +1,7 @@
+<?php
+
+// This is a separated file so that StyleCI doesn't detect it as PHP and doesn't try to reformat it
+
+return [
+    'factory' => function () { return 'foo'; }, 'factory2' => function () { return 'bar'; },
+];