disco
disco copied to clipboard
bitExpert
Rewrite using native Attributes instead of annotations.
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:
-
nameoption van the Parameter should correspond with the name of one of the arguments - multiple Parameters with the same
nameshould 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?
@netiul if it makes things easier, I'd be fine supporting only named parameters.
Coverage decreased (-2.3%) to 97.739% when pulling 48d452baa7d80c613dac2b431249a73371d1a843 on netiul:php8-rewrite into c3659ff55bb610cffb3a41fd5146d2d64fdf658f on bitExpert:master.
@shochdoerfer It's actually not very hard to support both. This is basically all that's needed, but it might be a bit confusing in userland to have both ways availabe...
@shochdoerfer I've have updated the top post with a summary of the main changes and some points up for discussing.
@netiul awesome work! Thanks a lot for tackling this.
Not allowing built-in types as return type alias makes sense to me. That seems to be a "bug" in the previous versions that I missed.
Parameter validation also makes sense to me. I would even go that far and get rid of the positional logic and "just" keep the named parameter logic to reduce potential problems when parameters are re-ordered any types don't match anymore. Since PHP 8 gives us the ability to use named parameters it makes sense to make use of them. Not that I like them in code but for the configuration that sounds fine. Given that, checking that parameter names exist and are unique in the attribute configuration makes absolute sense. I also like the idea of the no default value check.
@heiglandreas any other validations, you would like to see? I hear you are big Disco fan, so may have opinions :)
For sure butwon't be able to voice them for another week. So if you don't want to wait that's fine with me 😉
For me the whole Parameter declaration is not 💯% clear as most of it seems to duplicate internal functionality.
default and require are already declarable via the function parameter declaration (OK: Default only via native types) and can be retrieved via Reflection. I would not require people to declare things in duplicate, once in the parameter-definition and once in the function signature, as that will lead to discrepancies. And I can't see why one would declare a different default value via the function signature and the parameter declaration. Same applies for whether a parameter is required or not.
So for me the only declaration that introduces something, that can not be declared via the function signature, would be the key parameter, that defines a configuration key whose value will be passed as parameter. Using named parameters here makes absolutely sense to me. Validating that all required parameters are set via a Parameter-annotation would then be the only thing left to do.
I would also only go for named parameters only. The key name has to be mandatory IMO.
When there are multiple parameter declarations for the same name I would use the "last" one to win and raise a warning.
Question: Can we use aliases from the DI within the configuration?
$testConfig = [
'db' => '@sqlite',
];
$prodConfig = [
'db' => '@postgresql',
];
#[Configuration]
class MyConfiguration
{
#[Parameter(
name: 'db',
key: 'db',
)]
public function mySampleService(PDO $db) : SampleService
{
$service = new SampleService();
$service->setDatabase($test);
return $service;
}
#[Bean]
#[Alias(name: 'postgresql')]
public function productionDatabase(): PDO
{
// Define Postgresql service
}
#[Bean]
#[Alias(name: 'sqlite')]
public function testDatabase(): PDO
{
// Define SQLite-Service
}
}
And I'm absolutely in favour of not allowing native types as type-aliases.
Yes! Sorry for the late reply :)
Thank for the feedback @heiglandreas. It makes totally sense to use the functions signature to declare defaults and what parameter is required. Funny how one can be easily fixated on an existing path :laughing:
About "last parameter with the same name wins", shouldn't we just hard fail at compile time? It's not really something that can only be detected at runtime.
Yeah. failing at compile time makes more sense. There can only be one name entry. If there are more, that needs to be fixed.