Skip to content
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

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Prev Previous commit
Next Next commit
Fix types notation
  • Loading branch information
xepozz committed Apr 19, 2022
commit 66942d8eb3ec3e31929387b27f0f718aa2274ec1
16 changes: 10 additions & 6 deletions src/Container.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ final class Container implements ContainerInterface

/**
* @var array Tagged service IDs. The structure is `['tagID' => ['service1', 'service2']]`.
* @psalm-var array<string, list<string>>
* @psalm-var array<string, iterable<string>>
*/
private array $tags;

Expand Down Expand Up @@ -238,6 +238,7 @@ private function addDefinition(string $id, $definition): void
private function addDefinitions(iterable $config): void
vjik marked this conversation as resolved.
Show resolved Hide resolved
{
/** @var mixed $definition */
/** @psalm-suppress MixedAssignment */
foreach ($config as $id => $definition) {
if ($this->validate && !is_string($id)) {
throw new InvalidConfigException(
Expand All @@ -247,8 +248,8 @@ private function addDefinitions(iterable $config): void
)
);
}
/** @var string $id */

$id = (string) $id;
vjik marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

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.

$this->addDefinition($id, $definition);
}
}
Expand Down Expand Up @@ -331,7 +332,9 @@ private function validateDefinition($definition, ?string $id = null): void
private function validateMeta(iterable $meta): void
vjik marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

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.

{
/** @var mixed $value */
/** @psalm-suppress MixedAssignment */
foreach ($meta as $key => $value) {
$key = (string)$key;
arogachev marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$key = (string)$key;

Not need.

if (!in_array($key, self::ALLOWED_META, true)) {
throw new InvalidConfigException(
sprintf(
Expand Down Expand Up @@ -404,7 +407,7 @@ private function setTags(iterable $tags): void
throw new InvalidConfigException(
sprintf(
'Invalid tags configuration: tag should be string, %s given.',
$tag
$this->getVariableType($tag)
)
);
}
Expand All @@ -429,9 +432,9 @@ private function setTags(iterable $tags): void
}
}
}
/** @psalm-var array<string, list<string>>|Traversable $tags */
/** @psalm-var iterable<string, iterable<string>> $tags */

$this->tags = $tags instanceof Traversable ? iterator_to_array($tags) : $tags ;
$this->tags = $tags instanceof Traversable ? iterator_to_array($tags, true) : $tags ;
}

/**
Expand All @@ -446,8 +449,9 @@ private function setDefinitionTags(string $id, iterable $tags): void
}

$tags = $this->tags[$tag];
$tags = $tags instanceof Traversable ? iterator_to_array($tags) : $tags;
$tags = $tags instanceof Traversable ? iterator_to_array($tags, true) : $tags;
Copy link
Member

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

if (!in_array($id, $tags, true)) {
/** @psalm-suppress PossiblyInvalidArrayAssignment */
$this->tags[$tag][] = $id;
}
}
Expand Down