-
-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve containers types to use iterable objects #299
base: master
Are you sure you want to change the base?
Conversation
# Conflicts: # src/Container.php # tests/Unit/ContainerTest.php
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #299 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 158 160 +2
===========================================
Files 11 11
Lines 460 467 +7
===========================================
+ Hits 460 467 +7
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall good. One change is needed to satisfy psalm.
Co-authored-by: Sergei Predvoditelev <[email protected]>
@xepozz need a line for CHANGELOG. Also, it looks like BC-breaking change. |
Done. I can keep it for 2.0 |
} | ||
|
||
public function dataInvalidTags(): array | ||
{ | ||
return [ | ||
[ | ||
'/^Invalid tags configuration: tag should be string, 42 given\.$/', | ||
'Invalid tags configuration: tag should be string, array given.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That does not look right. The problem here is 42
, right?
|
||
$id = (string) $id; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not need type-casting. Above check that $id is string.
@@ -323,10 +325,12 @@ private function validateDefinition(mixed $definition, ?string $id = null): void | |||
/** | |||
* @throws InvalidConfigException | |||
*/ | |||
private function validateMeta(array $meta): void | |||
private function validateMeta(iterable $meta): void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validateMeta
always receive array. Not need iterable.
foreach ($meta as $key => $value) { | ||
$key = (string)$key; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$key = (string)$key; |
Not need.
} | ||
|
||
$tags = $this->tags[$tag]; | ||
$tags = $tags instanceof Traversable ? iterator_to_array($tags, true) : $tags; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better convert iterable to array before foreach
@@ -58,5 +58,5 @@ public function getDefinitions(): array; | |||
* @return array Extensions for the container services. Each array key is the name of the service to be modified | |||
* and a corresponding value is callable doing the job. | |||
*/ | |||
public function getExtensions(): array; | |||
public function getExtensions(): iterable; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need fix phpdoc.
@@ -42,7 +42,7 @@ interface ServiceProviderInterface | |||
* @return array Definitions for the container. Each array key is the name of the service (usually it is | |||
* an interface name), and a corresponding value is a service definition. | |||
*/ | |||
public function getDefinitions(): array; | |||
public function getDefinitions(): iterable; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need fix phpdoc.
{ | ||
if ($this->validate) { | ||
foreach ($tags as $tag => $services) { | ||
if (!is_string($tag)) { | ||
throw new InvalidConfigException( | ||
sprintf( | ||
'Invalid tags configuration: tag should be string, %s given.', | ||
$tag | ||
get_debug_type($services) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_debug_type($services) | |
get_debug_type($tag) |
Added ability to use iterable parameters.
For example, now you cannot use Generator as tags values.