Skip to content

[DependencyInjection] Add types to private properties #41928

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

Merged
merged 1 commit into from
Jul 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
[DependencyInjection] Add types to private properties
  • Loading branch information
derrabus authored and nicolas-grekas committed Jul 3, 2021
commit 069da202f972cd04c8a460c48babc50c009e41be
6 changes: 3 additions & 3 deletions src/Symfony/Component/DependencyInjection/Alias.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ class Alias
{
private const DEFAULT_DEPRECATION_TEMPLATE = 'The "%alias_id%" service alias is deprecated. You should stop using it, as it will be removed in the future.';

private $id;
private $public;
private $deprecation = [];
private string $id;
private bool $public;
private array $deprecation = [];

public function __construct(string $id, bool $public = false)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
*/
final class AbstractArgument
{
private $text;
private $context;
private string $text;
private string $context = '';

public function __construct(string $text = '')
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ final class BoundArgument implements ArgumentInterface
public const DEFAULTS_BINDING = 1;
public const INSTANCEOF_BINDING = 2;

private static $sequence = 0;
private static int $sequence = 0;

private $value;
private $identifier;
private $used;
private $type;
private $file;
private mixed $value;
private ?int $identifier = null;
private ?bool $used = null;
private int $type;
private ?string $file;

public function __construct(mixed $value, bool $trackUsage = true, int $type = 0, string $file = null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
*/
trait ReferenceSetArgumentTrait
{
private $values;
private array $values;

/**
* @param Reference[] $values
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@
*/
class RewindableGenerator implements \IteratorAggregate, \Countable
{
private $generator;
private $count;
private \Closure $generator;
private \Closure|int $count;

public function __construct(callable $generator, int|callable $count)
{
$this->generator = $generator;
$this->count = $count;
$this->generator = $generator instanceof \Closure ? $generator : \Closure::fromCallable($generator);
$this->count = is_callable($count) && !$count instanceof \Closure ? \Closure::fromCallable($count) : $count;
}

public function getIterator(): \Traversable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
*/
class ServiceClosureArgument implements ArgumentInterface
{
private $values;
private array $values;

public function __construct(Reference $reference)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
*/
class ServiceLocator extends BaseServiceLocator
{
private $factory;
private $serviceMap;
private $serviceTypes;
private \Closure $factory;
private array $serviceMap;
private ?array $serviceTypes;

public function __construct(\Closure $factory, array $serviceMap, array $serviceTypes = null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class ServiceLocatorArgument implements ArgumentInterface
{
use ReferenceSetArgumentTrait;

private $taggedIteratorArgument;
private ?TaggedIteratorArgument $taggedIteratorArgument = null;

/**
* @param Reference[]|TaggedIteratorArgument $values
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@
*/
class TaggedIteratorArgument extends IteratorArgument
{
private $tag;
private $indexAttribute;
private $defaultIndexMethod;
private $defaultPriorityMethod;
private $needsIndexes = false;
private string $tag;
private mixed $indexAttribute;
private ?string $defaultIndexMethod;
private ?string $defaultPriorityMethod;
private bool $needsIndexes;

/**
* @param string $tag The name of the tag identifying the target services
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
*/
class ChildDefinition extends Definition
{
private $parent;
private string $parent;

/**
* @param string $parent The id of Definition instance to decorate
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ abstract class AbstractRecursivePass implements CompilerPassInterface
protected $container;
protected $currentId;

private $processExpressions = false;
private $expressionLanguage;
private $inExpression = false;
private bool $processExpressions = false;
private ExpressionLanguage $expressionLanguage;
private bool $inExpression = false;

/**
* {@inheritdoc}
Expand Down Expand Up @@ -194,7 +194,7 @@ protected function getReflectionMethod(Definition $definition, string $method)

private function getExpressionLanguage(): ExpressionLanguage
{
if (null === $this->expressionLanguage) {
if (!isset($this->expressionLanguage)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why not nullable?

Copy link
Contributor

@ro0NL ro0NL Jul 1, 2021

Choose a reason for hiding this comment

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

the private scope confused me. But from #41924 i figured this lazy loading tactic is probably slightly preferred, as we dont need to give in on type info.

👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Right. null was previously used for lazy initialization only and I'd like to avoid making properties nullable. If someone accesses this property before initialization, they made a mistake and PHP will raise an exception for us. This is a good thing.

if (!class_exists(ExpressionLanguage::class)) {
throw new LogicException('Unable to use expressions as the Symfony ExpressionLanguage component is not installed. Try running "composer require symfony/expression-language".');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,7 @@

final class AliasDeprecatedPublicServicesPass extends AbstractRecursivePass
{
private $tagName;

private $aliases = [];

public function __construct(string $tagName = 'container.private')
{
if (0 < \func_num_args()) {
trigger_deprecation('symfony/dependency-injection', '5.3', 'Configuring "%s" is deprecated.', __CLASS__);
}

$this->tagName = $tagName;
}
private array $aliases = [];

/**
* {@inheritdoc}
Expand All @@ -47,13 +36,13 @@ protected function processValue(mixed $value, bool $isRoot = false)
*/
public function process(ContainerBuilder $container)
{
foreach ($container->findTaggedServiceIds($this->tagName) as $id => $tags) {
foreach ($container->findTaggedServiceIds('container.private') as $id => $tags) {
if (null === $package = $tags[0]['package'] ?? null) {
throw new InvalidArgumentException(sprintf('The "package" attribute is mandatory for the "%s" tag on the "%s" service.', $this->tagName, $id));
throw new InvalidArgumentException(sprintf('The "package" attribute is mandatory for the "container.private" tag on the "%s" service.', $id));
}

if (null === $version = $tags[0]['version'] ?? null) {
throw new InvalidArgumentException(sprintf('The "version" attribute is mandatory for the "%s" tag on the "%s" service.', $this->tagName, $id));
throw new InvalidArgumentException(sprintf('The "version" attribute is mandatory for the "container.private" tag on the "%s" service.', $id));
}

$definition = $container->getDefinition($id);
Expand All @@ -62,7 +51,7 @@ public function process(ContainerBuilder $container)
}

$container
->setAlias($id, $aliasId = '.'.$this->tagName.'.'.$id)
->setAlias($id, $aliasId = '.container.private.'.$id)
->setPublic(true)
->setDeprecated($package, $version, 'Accessing the "%alias_id%" service directly from the container is deprecated, use dependency injection instead.');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@
*/
class AnalyzeServiceReferencesPass extends AbstractRecursivePass
{
private $graph;
private $currentDefinition;
private $onlyConstructorArguments;
private $hasProxyDumper;
private $lazy;
private $byConstructor;
private $byFactory;
private $definitions;
private $aliases;
private ServiceReferenceGraph $graph;
private ?Definition $currentDefinition = null;
private bool $onlyConstructorArguments;
private bool $hasProxyDumper;
private bool $lazy;
private bool $byConstructor;
private bool $byFactory;
private array $definitions;
private array $aliases;

/**
* @param bool $onlyConstructorArguments Sets this Service Reference pass to ignore method calls
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,18 @@
*/
class AutowirePass extends AbstractRecursivePass
{
private $types;
private $ambiguousServiceTypes;
private $autowiringAliases;
private $lastFailure;
private $throwOnAutowiringException;
private $decoratedClass;
private $decoratedId;
private $methodCalls;
private $getPreviousValue;
private $decoratedMethodIndex;
private $decoratedMethodArgumentIndex;
private $typesClone;
private array $types;
private array $ambiguousServiceTypes;
private array $autowiringAliases;
private ?string $lastFailure = null;
private bool $throwOnAutowiringException;
private ?string $decoratedClass = null;
private ?string $decoratedId = null;
private ?array $methodCalls = null;
private ?\Closure $getPreviousValue = null;
private ?int $decoratedMethodIndex = null;
private ?int $decoratedMethodArgumentIndex = null;
private ?self $typesClone = null;

public function __construct(bool $throwOnAutowireException = true)
{
Expand Down Expand Up @@ -460,7 +460,7 @@ private function createTypeAlternatives(ContainerBuilder $container, TypedRefere
if ($message = $this->getAliasesSuggestionForType($container, $type = $reference->getType())) {
return ' '.$message;
}
if (null === $this->ambiguousServiceTypes) {
if (!isset($this->ambiguousServiceTypes)) {
$this->populateAvailableTypes($container);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
*/
class CheckArgumentsValidityPass extends AbstractRecursivePass
{
private $throwExceptions;
private bool $throwExceptions;

public function __construct(bool $throwExceptions = true)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
*/
class CheckCircularReferencesPass implements CompilerPassInterface
{
private $currentPath;
private $checkedNodes;
private array $currentPath;
private array $checkedNodes;

/**
* Checks the ContainerBuilder object for circular references.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
*/
class CheckExceptionOnInvalidReferenceBehaviorPass extends AbstractRecursivePass
{
private $serviceLocatorContextIds = [];
private array $serviceLocatorContextIds = [];

/**
* {@inheritdoc}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ final class CheckTypeDeclarationsPass extends AbstractRecursivePass
'string' => true,
];

private $autoload;
private $skippedIds;
private bool $autoload;
private array $skippedIds;

private $expressionLanguage;
private ExpressionLanguage $expressionLanguage;

/**
* @param bool $autoload Whether services who's class in not loaded should be checked or not.
Expand Down Expand Up @@ -309,10 +309,6 @@ private function checkType(Definition $checkedDefinition, mixed $value, \Reflect

private function getExpressionLanguage(): ExpressionLanguage
{
if (null === $this->expressionLanguage) {
$this->expressionLanguage = new ExpressionLanguage(null, $this->container->getExpressionLanguageProviders());
}

return $this->expressionLanguage;
return $this->expressionLanguage ??= new ExpressionLanguage(null, $this->container->getExpressionLanguageProviders());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@
*/
class Compiler
{
private $passConfig;
private $log = [];
private $serviceReferenceGraph;
private PassConfig $passConfig;
private array $log = [];
private ServiceReferenceGraph $serviceReferenceGraph;

public function __construct()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,6 @@
*/
class DecoratorServicePass extends AbstractRecursivePass
{
private $innerId = '.inner';

public function __construct(?string $innerId = '.inner')
{
if (0 < \func_num_args()) {
trigger_deprecation('symfony/dependency-injection', '5.3', 'Configuring "%s" is deprecated.', __CLASS__);
}

$this->innerId = $innerId;
}

public function process(ContainerBuilder $container)
{
$definitions = new \SplPriorityQueue();
Expand Down Expand Up @@ -123,7 +112,7 @@ public function process(ContainerBuilder $container)

protected function processValue(mixed $value, bool $isRoot = false)
{
if ($value instanceof Reference && $this->innerId === (string) $value) {
if ($value instanceof Reference && '.inner' === (string) $value) {
return new Reference($this->currentId, $value->getInvalidBehavior());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@
*/
class InlineServiceDefinitionsPass extends AbstractRecursivePass
{
private $analyzingPass;
private $cloningIds = [];
private $connectedIds = [];
private $notInlinedIds = [];
private $inlinedIds = [];
private $notInlinableIds = [];
private $graph;
private ?AnalyzeServiceReferencesPass $analyzingPass;
private array $cloningIds = [];
private array $connectedIds = [];
private array $notInlinedIds = [];
private array $inlinedIds = [];
private array $notInlinableIds = [];
private ?ServiceReferenceGraph $graph = null;

public function __construct(AnalyzeServiceReferencesPass $analyzingPass = null)
{
Expand Down
Loading