yiisoft / definitions Goto Github PK

What steps will reproduce the problem?

1. Define a spreaded parameter in constructor like this: __construct(...$test)
2. Define its value in config: ['__construct()' => ['test' => [1, 2]]]

What is the expected result?

$test in constructor contains [1, 2] value

What do you get instead?

$test in constructor contains [0 => [1, 2]] value

Additional info

What steps will reproduce the problem?

Having the sample code:

class A
{
    public function __construct()
    {
        throw new RuntimeException('Something happened on initialization');
    }
}

class B
{
    public function __construct(private ?A $a = null)
    {
    }
}

$container->get(B::class);

What is the expected result?

The exception is thrown

What do you get instead?

Instance of B with a wrong dependency, and no exception is thrown. It' a hell to debug this situation.

Additional info

This code is responsible for the specified behavior. I'm pretty sure container definitions must not have a deal with application-level exceptions. In my opinion in the case when a dependency has a definition, and an exception is thrown on its resolving, this exception must not be catched by container/definition resolver, and doesn't matter if this dependency is optional or not.

At the beginnig of the function u r checking defined dependencies for specified class (it's ok):

if (isset(self::$dependencies[$class])) {
        return self::$dependencies[$class];
}

but at the end of function u r return all dependencies.

self::$dependencies[$class] = $dependencies;

return $dependencies;

I have not watched use of this method, but looks like bug.

Moved from yiisoft/factory#134

Factory should not resolve parameters such as method(?string $var) by seting $var to null.

Move DefinitionStorage from yiisoft/di, because it can be reused by a custom user's container.

I'd remove var_export usage because it's insecure to disclose array's values into the world/logs.
Better to use keys or variable types.

What steps will reproduce the problem?

class Foo
{
    public function __construct(public Bar $bar)
    {
    }
}

class Bar
{
    public function __construct(public string $value = 'default')
    {
    }
}

$config = ContainerConfig::create()
    ->withDefinitions([
        Foo::class => [
            'class' => Foo::class,
            '__construct()' => [
                'baz' => Reference::to('bar'), // Typo here
            ],
        ],
        'bar' => [
            'class' => Bar::class,
            '__construct()' => [
                'value' => 'custom',
            ],
        ],
    ]);

$container = new Container($config);

var_dump($container->get(Foo::class));

What is the expected result?

Exception with message, like:
Unknown parameter $baz in Foo::__construct() method. Available paraemeters: $bar

What do you get instead?

New instance of Foo with wrong Bar

class Foo#11 (1) {
  public Bar $bar =>
  class Bar#10 (1) {
    public string $value =>
    string(7) "default"
  }
}

Additional info

Now we cache extracted dependencies in the ArrayDefinitionBulder https://github.com/yiisoft/definitions/blob/master/src/Infrastructure/ArrayDefinitionBuilder.php#L161
We should move it to DefinitionExtractor to reuse it (for example in the DefinitionStorage::has()).

In this case https://github.com/yiisoft/definitions/blob/master/src/Infrastructure/DefinitionExtractor.php#L57, we shouldn't throw NotFoundExeption because this exception is a scope of a container. Use NotInstantiableClassException instead. Also, I think we should move NotFoundException to a container and factory.

I propose to add an alternative syntax for a simplified notation of the constructor and its arguments.
For example:

use \Yiisoft\Definitions\ArrayDefinition;

$definition = ArrayDefinition::fromConfig([
    'class' => MyServiceInterface::class,
    '__construct()' => [42], 
    '$propertyName' => 'value',
    'setName()' => ['Alex'],
]);

So it can be written like this:

use \Yiisoft\Definitions\ArrayDefinition;

$definition = ArrayDefinition::fromConfig([
    MyServiceInterface::class,
    [42], 
    'propertyName' => 'value',
    'setName()' => ['Alex'],
]);

Here the unnamed first (0) parameter (string) is the name of the class.
The second unnamed (1) parameter (array, if present) is the constructor arguments.

Raise an error if unnamed parameters are present along with "__construct()" or "class".

Also, the '$' notation here may be redundant and looks like a developer error visually (lack of double quotes).

This improvement will simplify development and reduce the amount of code.

https://github.com/yiisoft/definitions/actions/runs/5735242606/job/15542667285?pr=58

Replace ArrayDefinition::setReferenceContainer() to ArrayDefinition::withReferenceContainer().

Cache definitions objects, so you can use them in another context where you don't need a separate container for references.

See yiisoft/factory#145 (comment)

We use a hardcoded Injector to resolve CallableDefinition dependencies. I think it is not a good idea.
We should implement this in the package itself. We can use DefinitionResolver::resolveArray() and DefinitionExtractor::fromMethod() methods for this purpose if we make DefinitionExtractor::fromMethod() public.