auraphp / Aura.Di

Dependency Injection System
MIT License
349 stars 63 forks source link

Added support for variadic constructor parameters #167

Closed marcusjhdon closed 6 years ago

marcusjhdon commented 6 years ago

I've committed the changes you requested, but I don't have PHP 5.5 on my machine to test, so I'll have to leave that to you :)

harikt commented 6 years ago

HHVM is the only failure now. I have send an email to our user group asking about hhvm and whether it is good to drop or not : https://groups.google.com/d/msg/auraphp/NDz0aboCF4Q/6k_lH3zjAwAJ .

We can wait for a few days, and merge if there is no issues.

Hope that helps all.

far-blue commented 6 years ago

Maybe I missed something but isn't the problem with the HHVM build simply the type hinting on the variadic parameter in the unit tests? That wouldn't require dropping HHVM support, simply skipping the tests for HHVM.

harikt commented 6 years ago

@far-blue yes, hhvm is not supporting variadic parameter in php and I don't think it will. So claiming it to be hhvm compatible seems false in my opinion.

marcusjhdon commented 6 years ago

I've added some checks to skip the unit tests and variadic support in HHVM. All tests are now passing.

harikt commented 6 years ago

Hi @pmjones , If you don't have any objections, I will merge this in coming days.

harikt commented 6 years ago

Thank you @marcusjhdon .

harikt commented 6 years ago

Just to verify I did the tests as in the issue #164 . And it fails. It only have one value in the array.

<?php
require __DIR__ . '/vendor/autoload.php';

class Test {
    public function __construct(...$items) {
        print_r($items);
    }
}

class Config extends Aura\Di\ContainerConfig {
    public function define(Aura\Di\Container $di)  {
        $di->params[Test::class]['items'] = ['one', 'two'];
        $di->set('test', $di->lazyNew(Test::class));
    }
}

/*
I was wrong with the examples

class Config extends Aura\Di\ContainerConfig {
    public function define(Aura\Di\Container $di)  {
        $di->params[Test::class]['items'] = 'one';
        $di->params[Test::class]['items'] = 'two';
        $di->set('test', $di->lazyNew(Test::class));
    }
}

class Config extends Aura\Di\ContainerConfig {
    public function define(Aura\Di\Container $di)  {
        $di->params[Test::class][] = 'one';
        $di->params[Test::class][] = 'two';
        $di->set('test', $di->lazyNew(Test::class));
    }
}

*/

(new Aura\Di\ContainerBuilder())
    ->newConfiguredInstance([Config::class])
    ->get('test');

Result :

Array
(
    [0] => one
)

Expected :

Array ( [0] => one [1] => two )


I was wrong with the examples. Also do we consider setters or is it ignored for now ?
harikt commented 6 years ago

UPDATE : I was wrong with the test example. Also do we consider setters now? or is it ignored ?

This PR looks god and I will make the changes.