Given you have bean method configuration that looks like this:

/** @Bean({"alias" = "\My\Namespace\Service\ProductService"}) */
public function productService() : \My\Namespace\Service\ProductService
{
    return new \My\Namespace\Service\DefaultProductService();
}

The configured alias matches the return type definition - in this case either the name of an interface or a parent class. Since the alias is a hard-coded string it won't get updated during the usual refactorings (moving files to a different namespace and such) which is not what we want.

It would be nice to provide a sepcial keyword to let Disco automatically determine the return type declaration during the cached configuration generation process and automatically use the return type declaration as an alias, e.g.

/** @Bean({"alias" = "type"}) */
public function productService() : \My\Namespace\Service\ProductService
{
    return new \My\Namespace\Service\DefaultProductService();
}

That way I could access the bean like this:

$beanFactory->get('\My\Namespace\Service\ProductService');

This PR covers one step of #94: Getting 100% code coverage. I also made a few tests stricter.

During my work, I realized, that in its integration, there is still a possible unreachable code in Disco. In BeanMethod::generateMethod(), there is a else-block which can be tested when it is handled as a unit.

The method itself is only used in ConfigurationGenerator at line 167. But before it's invoked, the same checks as in BeanMethod::generateMethod() are also done in ConfigurationGenerator, which happens at line 150. It looks like the BeanMethod::generateMethod() else-block never can be reached during Disco's runtime.

Should I remove the else-block and its newly added test?

Currently Disco throws an exception when it finds an already declared alias. That is totally valid as long as the configuration is done in one single file.

As soon as you want to have a configuration split over different files that becomes complicated as you can not overwrite an alias that was declared in the base class within an extending class.

Given I want a configuration for my production system with an alias for a logger and I then want to extend that configuration for my dev-environment and want to implement a different logger.

Currently I need to create three classes. One base class with all the configs that will never change and one class for production config extending the base class and implementing all the aliases that will change between prod and dev. And I also create another config for the dev-counterpart of those changing aliases.

This commit allows to overwrite aliases in extending files only. So I can create my configuration for production and for dev I can extend that base class and overwrite the aliases I need to adapt.

This change should be absolute BC, no tests where adapted, only added.

When you want to extend a global configuration with a configuration for prod or dev-environment you might need to decalre a formerly private method with protected visibility. As that up until now required a method to have a Bean annotation that was actually not possible.

This Commit introduces a method-annotation "@Ignore" to mark protected methods to be ignored as Bean-methods

Currently, Disco has a code coverage of ~95%, but most of its coverage happened by hardcoded instance creations over bitExpert\Disco\AnnotationBeanFactory and bitExpert\Disco\AnnotationBeanFactoryUnitTest. In case of possible refactorings, does it make sense to add coverage by testing every class' methods separated in custom unit tests?

And why 100% code coverage?

I started to take a look into the uncovered code and asked myself if there may be unreachable code by writing tests.

For example: https://github.com/bitExpert/disco/blob/v0.8.0/src/bitExpert/Disco/BeanFactoryConfiguration.php#L146

if (!spl_autoload_register($this->proxyAutoloader, false)) {
    throw new RuntimeException(
        sprintf('Cannot register autoloader "%s"', get_class($this->proxyAutoloader))
    );
}

spl_autoload_register can return false in cases of an error, like using a function name, that doesn't exist, in form of a string, but I didn't find a way to make this fail with a callable autoloader class using __invoke(). I haven't try the same for spl_autoload_unregister yet, but I expect the same like in spl_autoload_register.

This PR:

• begins the creation of a self-sustaining documentation structure for the project. The README can then be dedicated to providing a project introduction/overview and a table of contents (of sorts).

This PR:

• adds instructions showing how to contribute to the project.

:information_desk_person: As part of the overhaul of the documentation, I want to make it as easy as possible for others to know how to contribute to the project.

Make sure that the current version of Disco can be installed on PHP 7.2. Also check for side-effects with the latest version of ProxyManager which is focused PHP 7.2 only.

Resolves #74

Implementation is very close to what we've discussed in the issue, except that I use a single @Alias for named and for return type aliases. It makes the implementation simpler ("aliases" attribute of bean can be a collection).

Like noted in the issue: the PR introduces a BC break

Optimize the README.md file to make it easier for beginners to get started. In addition I would love to see the following changes:

• add a rationale to explain why Disco makes sense and why/how it is different to the other DI containers in PHP
• highlight the different use-cases that can be solved with Disco
• highlight the different ways of injecting dependencies e.g. constructor injection, setter injection, interface injection
• improve the documentation of the bean configuration settings and how they affect each other
• fix some typos ;)

While experimenting with Disco and Expressive I realized that currently Disco is not able to work with Expressive for one reason: Expressive uses pseudo class names to query service instances from the service container. Since Disco tries to call a method with the same name on the config class this currently does not work because "" is not a valid character to be used for a method name.

To solve the issue we need a mapper in between to translate the invalid characters into valid ones.

This is an implementation of #131

Changes

Bean

#[Configuration]
class X
{
    #[Bean(
        singleton: true, 
        lazy: true, 
        scope: Bean::SCOPE_SESSION
    )]
    public function serviceDefinition(): Service {}
}

Use the Bean attribute to define a bean and set their singleton, lazy or scope option. Use the Parameter and Alias attribute to define parameters and aliases.

Alias

#[Configuration]
class X
{
    #[Bean]
    #[Alias(name: 'my.service.id')]
    #[TypeAlias]
    public function serviceDefinition(): SomeService { /* .... */ }
}

The purpose of the Alias attribute is to define a named alias. The name must be at least 1 character of length. To define the return type as an alias use #[TypeAlias]. Multiple Alias attribute are allowed, only 1 TypeAlias. Both Alias and TypeAlias only affect Bean definitions.

BeanPostProcessor

#[Configuration]
class X
{
    #[BeanPostProcessor]
    public function beanPostProcessor(): MyPostProcessor { /* .... */ }
}

To define a bean post processor apply #[BeanPostProcessor] attribute. Add #[Parameter] attributes to define parameters.

Parameter

#[Configuration]
class X
{
    #[BeanPostProcessor]
    #[Parameter(
        key: 'configKey',
        name: 'argName',
        default: 'some default value',
        required: false,
    )]
    public function beanPostProcessor(string $argName): MyPostProcessor { /* .... */ }

    #[Bean]
    #[Parameter(
        key: 'configKey',
        name: 'secondArg',
        default: 'some default value',
        required: false,
    )]
    #[Parameter(key: 'otherConfigKey', name: 'secondArg')]
    public function serviceDefinition(string $configValue, int $secondArg): MyPostProcessor { /* .... */ }
}

Attribute a Bean or BeanPostProcessor with 1 or multiple Parameter attributes. key used to look up the value in the configuration array. :warning: This is the equivalent of name in 0.x version. name has now a different purpose: name corresponds with the name of argument of the method definition. default and required act the same as in 0.x.

Disco will use the feature named arguments of PHP 8, to pass the arguments to the method call.

About positional parameters

At this moment the name option is not mandatory. When omitted the Parameter becomes a positional argument. Disco will pass positional arguments to the method function followed by the named arguments as required by PHP, see Example #16.

The mutual order of the positional parameters is of course important.

The base line is that the same rules and restrictions apply as when using and mixing named and positional arguments when calling the method yourself. Read more about it the PHP docs.

To be discussed

Builtin types as return type alias

In 0.x it was possible to define the return type alias when the actual return type was a builtin type like string, int, float, bool, array, object, callable or iterable. This is now not possible anymore. An exception will be thrown. An example of an invalid configuration:

#[Configuration]
class X
{
    #[Bean]
    #[TypeAlias]
    public function serviceDefinition(): callable { /* .... */ }
}

Parameter validation

At compile time, should we validate configured parameters with the method signature? Possible validations:

• name option van the Parameter should correspond with the name of one of the arguments
• multiple Parameters with the same name should not be allowed
• a bit more advanced: named Parameter should not correspond with an argument that's covered by a positional Parameter
• when argument in the method signature has no default value, the parameter should have a default value or be required...

Anything else?

With PHP 8 and having attributes support, configuring beans can be even more simpler. I don't expect to have a complete picture of what's needed, since I have only experience with discolight where I did this proposal (sort of). Anyway here's my take:

There are some options to walk this path:

1. Add support to the 0.x major versioning line
2. Replace configuration by annotation with attributes which of course would require a major version release: 1.x

I can't think of a good reason to go for option 2. Adding it to 0.x allows for a gradual upgrade path allowing to deprecate features which can eventually be removed in 1.x.

Adding this feature can be done by:

1. add support to current Annotation classes, or
2. creating new Attribute classes.

While new Attribute classes allows you to start with a clean slate, current Annotation classes can quite easily be adapted in order to be used as Attribute classes as well.

As option 1 is in my opinion the most valuable approach I'll go a bit further on that:

Add support to current Annotation classes

Example of how that can be achieved in case of the Alias class:

/**
 * @Annotation
 * @Target({"ANNOTATION"})
 * @Attributes({
 *   @Attribute("name", type = "string"),
 *   @Attribute("type", type = "bool"),
 * })
 */
#[\Attribute(\Attribute::TARGET_METHOD | \Attribute::IS_REPEATABLE)]
final class Alias
{
    /**
     * @var string
     */
    private $name;

    /**
     * @var bool
     */
    private $type;

    /**
     * Creates a new {@link \bitExpert\Disco\Annotations\Bean\Alias}.
     *
     * @param array $attributes
     * @throws AnnotationException
     */
    public function __construct(array $attributes = [], ?string $name = null, bool $type = false)
    {
        // When configured by annotations the $attributes['value'] is not empty, so
        // in that case override the $name and $type variables from the $attributes['value'] array.
        if (!empty($attributes['value'])) {
            $attributes = $attributes['value'];
            // Assign local variables the value from $attributes meanwhile making sure the keys exist
            // with at least the default value as defined in the constructor.
            ['name' => $name, 'type' => $type] = $attributes + ['name' => $name, 'type' => $type];
        }

        if (!is_bool($type)) {
            $type = AnnotationAttributeParser::parseBooleanValue($type);
        }

        if ($type && $name) {
            throw new AnnotationException('Type alias should not have a name!');
        }

        if (!$type && !$name) {
            throw new AnnotationException('Alias should either be a named alias or a type alias!');
        }

        $this->type = $type;
        $this->name = $name;
    }

    public function getName(): ?string
    {
        return $this->name;
    }

    public function isTypeAlias(): bool
    {
        return $this->type;
    }
}

class Config
{
    /**
     * Using named arguments
     */
    #[Bean]
    #[Alias(type: true)]
    #[Alias(name: 'my.service.id')]
    public function myService(): Service
    {}

    /**
     * Using ordered arguments
     */
    #[Bean]
    #[Alias([], null, true)]
    #[Alias([], 'my.service.id')]
    public function myOtherService(): OtherService
    {}
}

As you can see named arguments allow for a clean usage of existing Annotation classes.

What about nested attributes? In PHP's current implementation of attributes nesting is not supported, so explicitly configuring needs to be done on the same level, or instances of nested attributes/annotations should be able to be configured with scalars and instantiated in the "parent" attribute instance.

class Config {
    #[Bean(aliases: [
        ['type' => true],
        ['name' => 'my.service.id'],
    ])]
    public function myService(): Service
    {}
}

What does this means for each type of annotation?

Alias Allow configuration of

• name
• type

Bean Allow configuration of

• scope
• singleton
• lazy
• aliases discouraged, use (multiple) Alias attributes instead
• parameters discouraged, use (multiple) Parameter attributes instead

BeanPostProcessor Allow configuration of

• parameters discouraged, use (multiple) Parameter attributes instead

Configuration Just an identifier attribute

Parameter The ordering of parameters is important since the method is called with the parameters in the order they are configured. Nesting the parameters inside the Bean annotation provides a natural way of respecting the ordering of the arguments in the methods signature. But requiring attributes to be listed in a specific order feels a bit like a smell to me.

class Config {
    #[Bean(parameters: [
        ['name' => 'config.key1', 'default' => 'default value'],
        ['name' => 'config.key2', 'default' => 'other default value'],
    ])]
    public function myService($key1, $key2): Service
    {}

    // Correct ordering of the parameter attributes required, NOT IDEAL
    #[Bean]
    #[Parameter(name: 'config.key1', default => 'default value')]
    #[Parameter(name: 'config.key2', default => 'other default value')]
    public function myOtherService($key1, $key2): OtherService
    {}
}

Possible solution: When using the Parameter attribute, require a config key with the name of the argument so it can be used for calling the method with named arguments. The nicest would be something like this:

$parameters = [
    'config' => [
        'key1' => 'value of key1',
        'key2' => 'value of key2',
    ],
];
class Config {
    #[Bean]
    #[Parameter(name: 'arg2', key: 'config.key2', default => 'other default value')]
    #[Parameter(name: 'arg1', key: 'config.key1', default => 'default value')]
    public function myService($arg1, $arg2): Service
    {
        // $arg1 = 'value of key1;
        // $arg2 = 'value of key2;
    }
}

Unfortunately name is already in use as the name of the parameter key, but i.e. argname can perhaps be a good alternative.

How to incorporate this with the ConfigurationGenerator?

The doctrine AnnotationReader can be swapped out for a FallbackAttributeReader. The FallbackAttributeReader would prioritize attributes over Annotations.

I would not recommend mixing annotations and attributes on the same level.

Possible guidelines:

• When the Bean attribute is found on a method ignore any annotations.
• When the Bean attribute is found on a method look for Alias and Parameter attributes.
• When the Bean attribute has nested aliases prioritize these over configured Alias attributes.
• When the Bean attribute has nested parameters prioritize these over configured Parameter attributes.
• When the BeanPostProcessor attribute is found on a method ignore any annotations.
• When the Configuration attribute is found on a method ignore any annotations.

As disco is already hosted on github, wouldn't it be a great idea to serve the disco-documentation (that is currently only available via a local server) via github pages?

AFAIK that would require enabling the github-pages for the repo and setting the source to the docs-folder. Everything else should then work out from scratch. Probably linking the different pages needs to be taken care of.

Alternative would be to use the existing bookdown-setup to create the docs and deploy them to github pages…

As discussed with @Ocramius today it would make sense for Disco to be able to support autowiring for bean instances to reduce the amount of configuration code needed and to be able to get rid of traits for structuring the configuration code.

The following configuration:

/** @Bean */
protected function database() : Database
{
    return new Database('mysql://user:secret@localhost/mydb');
}

/** @Bean */
public function productService() : ProductService
{
    return new ProductService($this->database());
}

could then be simplified like this:

/** @Bean */
protected function database() : Database
{
    return new Database('mysql://user:secret@localhost/mydb');
}

/** @Bean */
abstract public function productService() : ProductService;

At a later stage we could move the methods into separate interfaces - to be able to get rid of the abstract keyword and ultimately get rid of the trait approach.

A few problems need to be solved:

• [ ] When using interfaces as configuration source, how to cope with method naming clashes?
• [ ] When there are multiple database instances available how would Disco decide which one to use? We would need a way to pass the method name or alias name to the bean configuration of the ProductService.
• [ ] For Constructor Injection things are simple, but to be able to decide which methods needs to be called for Setter Injection we might need annotations on the setter methods of the class. So far the goal of Disco was to keep annotations just in the configuration class. Maybe we can solve this differently?
• [ ] How to cope with primitive values that should be injected? Maybe we could rely on the same naming scheme - e.g. when the parameter of the bean definition method is called $dsn we check if the class has a parameter with that name as constructor argument, same for the setter methods.
• [ ] For primitives we obviously need to skip the whole injection logic.
• [ ] When adding this feature it might make sense to enable the production autoloader of Proxy Manager by default as the constant parsing could slow things down quite a bit.

PhpStorm supports a so-called Meta file helping with autocompletion for container calls. Add a (Phing) task to generate such a file for the given container configuration to enable autocomplete support for the return values of the BeanFactory::get() calls.