[DependencyInjection] Fix phpdoc for $configurator in Autoconfigure attribute #59820
Conversation
|
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
|
Hey! Oh no, it looks like you have made this PR towards a branch that is not maintained anymore. :/ Cheers! Carsonbot |
1b43559 to
c9b5a4b
Compare
c9b5a4b to
d9f9e20
Compare
d9f9e20 to
4d68e58
Compare
| * @param bool|null $shared Whether to declare the service as shared | ||
| * @param bool|null $autowire Whether to declare the service as autowired | ||
| * @param array<string, mixed>|null $properties The properties to define when creating the service | ||
| * @param array{non-empty-string|class-string, non-empty-string}|non-empty-string|null $configurator A PHP function, reference or an array containing a class/Reference and a method to call after the service is fully initialized |
There was a problem hiding this comment.
let's remove the empty- prefixes, they don't provide much
and let's merge class-string into string:
| * @param array{non-empty-string|class-string, non-empty-string}|non-empty-string|null $configurator A PHP function, reference or an array containing a class/Reference and a method to call after the service is fully initialized | |
| * @param array{string, string}|string|null $configurator A PHP function, reference or an array containing a class/reference and a method to call after the service is fully initialized |
There was a problem hiding this comment.
Hello @nicolas-grekas, thanks for the review!
I thought a non-empty string (as the first argument) would make sense since an empty string would trigger a RuntimeException in the PhpDumper :) . But anyway, using an empty string here doesn’t really make sense, so…
I've applied the changes you suggested. :)
4d68e58 to
58bba2b
Compare
58bba2b to
ce03562
Compare
|
Thank you @jfranclin. |
Since SF7.1, in Autoconfigure attribute, the $configurator phpdoc seems wrong :
* @param array<class-string, string>|string|null $configurator A PHP function, reference or an array containing a class/Reference and a method to call after the service is fully initializedThe doc is asking for a class-string key and a string value. However, this is not always the case because the first parameter could be a service, like
@foowhich is not a class-string. Moreover, the second parameter should not be empty.