[WIP] Di instance compiler #1112

Closed
wants to merge 21 commits into
from

Conversation

Projects
None yet
4 participants
Contributor

prolic commented Apr 28, 2012

No description provided.

Member

Ocramius commented Apr 28, 2012

Will rebase on your branch and continue work there :)
Anyway, I will work only on the dumper for quite a bit. At the end of all this, I think the generator will need no definitions at all :)

Looks fine, but it will conflict with some of my conflicts... Can you pull my branch in this one eventually? :)

Nevermind, will do the rebase myself tomorrow :)

+ /**
+ * Construct, configure, and return a PHP classfile code generation object
+ *
+ * Creates a Zend\CodeGenerator\Php\PhpFile object that has
@Maks3w

Maks3w Apr 30, 2012

Member

Code\Generator

+ list($class, $method) = explode('->', $constructor, 2);
+
+ if (!class_exists($class)) {
+ throw new \InvalidArgumentException('No class found: ' . $class);
@Maks3w

Maks3w Apr 30, 2012

Member

Should be this Exception\InvalidArgumentException?

+ $creation = sprintf('$object = $this->' . $factoryGetter . '()->%s();', $method);
+ }
+ } else {
+ throw new \InvalidArgumentException('Invalid instantiator supplied for class: ' . $name);
@Maks3w

Maks3w Apr 30, 2012

Member

This too

@Ocramius

Ocramius Apr 30, 2012

Member

Not sure about the exceptions yet. I think I'll have to ask Ralph about that...

@prolic

prolic Apr 30, 2012

Contributor

I will take care of all exceptions, never mind. This subcomponent is not ready yet :-)

+ }
+
+ } else {
+ $resolvedParams[$index] = new Dumper\ScalarParameter(null);
@Ocramius

Ocramius Apr 30, 2012

Member

This one causes serious troubles for now. I am working on it, but it causes lots of ->setSomething(null) in the generated code.
@ralphschindler I stripped it in the last commits, yet I am unsure it is correct. What is the correct way of finding out if a setter/method has to be called?

+ }
+ }
+ }
+ foreach ($supertypeInjectionMethods as $supertype => $supertypeInjectionMethod) {
@Ocramius

Ocramius Apr 30, 2012

Member

@ralphschindler this one causes duplicate injections such as ->setEventManager($this->get('Zend\\EventManager\\EventManager')) being called 2, 3, N times in the generated code. That seems to be a problem in Zend\Di\Di too. Ideas about how to get the duplicate calls removed? Can we somehow match the calls and strip the ones that are equal? The dumper does not need runtime performance, but a fix in Zend\Di\Di would be cool too :)

@ralphschindler

ralphschindler Apr 30, 2012

Member

Actually, the way matthew has it setup, EventManager must always be new (its set as not-shared). So that behavior is correct, I think.

@Ocramius

Ocramius Apr 30, 2012

Member

Yeah, but skipping EventManager, following happens (unsure if example is correct, but that is what I'm currently experiencing):

class A
{
    public function setFoo(Foo $foo)
    {
    }
}

class B extends A {}

$di->get('B'); // $instance->setFoo($di->get('Foo')) is called 2 times!
@Ocramius

Ocramius Apr 30, 2012

Member

Attached failing test at zendframework/zf2#1117

Member

Ocramius commented May 4, 2012

@prolic can you close this one? We'll have to rebase on @ralphschindler's work anyway...

@prolic prolic closed this May 4, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment