Skip to content

[PropertyAccess] Add type-hints to public interfaces and classes #32234

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.

Sign up for GitHub

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 5, 2019
Merged

[PropertyAccess] Add type-hints to public interfaces and classes #32234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions UPGRADE-5.0.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,11 @@ Process
$process = Process::fromShellCommandline('ls -l');
```

PropertyAccess
--------------

* Removed support of passing `null` as 2nd argument of `PropertyAccessor::createCache()` method (`$defaultLifetime`), pass `0` instead.
Copy link
Member

Choose a reason for hiding this comment

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

This should also be done in #32269 (we always document the deprecation and its removal in the same branch).

Copy link
Member

Choose a reason for hiding this comment

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

Done in a71ee27


Routing
-------

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1704,7 +1704,7 @@ private function registerCacheConfiguration(array $config, ContainerBuilder $con

if (!$container->getParameter('kernel.debug')) {
$propertyAccessDefinition->setFactory([PropertyAccessor::class, 'createCache']);
$propertyAccessDefinition->setArguments([null, null, $version, new Reference('logger', ContainerInterface::IGNORE_ON_INVALID_REFERENCE)]);
$propertyAccessDefinition->setArguments([null, 0, $version, new Reference('logger', ContainerInterface::IGNORE_ON_INVALID_REFERENCE)]);
Copy link
Member

Choose a reason for hiding this comment

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

If that change is necessary, your int type-hint was a small breaking change. We could make the type-hint nullable, but I also prefer int over ?int in that case.

I'd suggest that we backport this change to FrameworkBundle 4.4 along with a deprecation in PropertyAccessor::createCache() that is triggered if null is passed as $defaultLifetime.

$propertyAccessDefinition->addTag('cache.pool', ['clearer' => 'cache.system_clearer']);
$propertyAccessDefinition->addTag('monolog.logger', ['channel' => 'cache']);
} else {
Expand Down
24 changes: 8 additions & 16 deletions src/Symfony/Component/PropertyAccess/PropertyAccessor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ public function setValue(&$objectOrArray, $propertyPath, $value)
}
}

private static function throwInvalidArgumentException($message, $trace, $i, $propertyPath)
private static function throwInvalidArgumentException(string $message, array $trace, $i, $propertyPath)
{
// the type mismatch is not caused by invalid arguments (but e.g. by an incompatible return type hint of the writer method)
if (0 !== strpos($message, 'Argument ')) {
Expand Down Expand Up @@ -274,7 +274,7 @@ public function isWritable($objectOrArray, $propertyPath)
* @throws UnexpectedTypeException if a value within the path is neither object nor array
* @throws NoSuchIndexException If a non-existing index is accessed
*/
private function readPropertiesUntil($zval, PropertyPathInterface $propertyPath, $lastIndex, $ignoreInvalidIndices = true)
private function readPropertiesUntil(array $zval, PropertyPathInterface $propertyPath, int $lastIndex, bool $ignoreInvalidIndices = true)
{
if (!\is_object($zval[self::VALUE]) && !\is_array($zval[self::VALUE])) {
throw new UnexpectedTypeException($zval[self::VALUE], $propertyPath, 0);
Expand Down Expand Up @@ -383,7 +383,7 @@ private function readIndex($zval, $index)
*
* @throws NoSuchPropertyException If $ignoreInvalidProperty is false and the property does not exist or is not public
*/
private function readProperty($zval, $property, bool $ignoreInvalidProperty = false)
private function readProperty(array $zval, string $property, bool $ignoreInvalidProperty = false)
{
if (!\is_object($zval[self::VALUE])) {
throw new NoSuchPropertyException(sprintf('Cannot read property "%s" from an array. Maybe you intended to write the property path as "[%1$s]" instead.', $property));
Expand Down Expand Up @@ -430,12 +430,9 @@ private function readProperty($zval, $property, bool $ignoreInvalidProperty = fa
/**
* Guesses how to read the property value.
*
* @param string $class
* @param string $property
*
* @return array
*/
private function getReadAccessInfo($class, $property)
private function getReadAccessInfo(string $class, string $property)
{
$key = str_replace('\\', '.', $class).'..'.$property;

Expand Down Expand Up @@ -520,7 +517,7 @@ private function getReadAccessInfo($class, $property)
*
* @throws NoSuchIndexException If the array does not implement \ArrayAccess or it is not an array
*/
private function writeIndex($zval, $index, $value)
private function writeIndex(array $zval, $index, $value)
{
if (!$zval[self::VALUE] instanceof \ArrayAccess && !\is_array($zval[self::VALUE])) {
throw new NoSuchIndexException(sprintf('Cannot modify index "%s" in object of type "%s" because it doesn\'t implement \ArrayAccess.', $index, \get_class($zval[self::VALUE])));
Expand All @@ -538,7 +535,7 @@ private function writeIndex($zval, $index, $value)
*
* @throws NoSuchPropertyException if the property does not exist or is not public
*/
private function writeProperty($zval, $property, $value)
private function writeProperty(array $zval, string $property, $value)
{
if (!\is_object($zval[self::VALUE])) {
throw new NoSuchPropertyException(sprintf('Cannot write property "%s" to an array. Maybe you should write the property path as "[%1$s]" instead?', $property));
Expand Down Expand Up @@ -579,7 +576,7 @@ private function writeProperty($zval, $property, $value)
* @param string $addMethod The add*() method
* @param string $removeMethod The remove*() method
*/
private function writeCollection($zval, $property, $collection, $addMethod, $removeMethod)
private function writeCollection(array $zval, string $property, $collection, string $addMethod, string $removeMethod)
{
// At this point the add and remove methods have been found
$previousValue = $this->readProperty($zval, $property);
Expand Down Expand Up @@ -817,16 +814,11 @@ private function getPropertyPath($propertyPath): PropertyPath
/**
* Creates the APCu adapter if applicable.
*
* @param string $namespace
* @param int $defaultLifetime
* @param string $version
* @param LoggerInterface|null $logger
*
* @return AdapterInterface
*
* @throws \LogicException When the Cache Component isn't available
*/
public static function createCache($namespace, $defaultLifetime, $version, LoggerInterface $logger = null)
public static function createCache(string $namespace, int $defaultLifetime, string $version, LoggerInterface $logger = null)
{
if (!class_exists('Symfony\Component\Cache\Adapter\ApcuAdapter')) {
throw new \LogicException(sprintf('The Symfony Cache component must be installed to use %s().', __METHOD__));
Expand Down
6 changes: 3 additions & 3 deletions src/Symfony/Component/PropertyAccess/PropertyPath.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ public function getElements()
/**
* {@inheritdoc}
*/
public function getElement($index)
public function getElement(int $index)
{
if (!isset($this->elements[$index])) {
throw new OutOfBoundsException(sprintf('The index %s is not within the property path', $index));
Expand All @@ -182,7 +182,7 @@ public function getElement($index)
/**
* {@inheritdoc}
*/
public function isProperty($index)
public function isProperty(int $index)
{
if (!isset($this->isIndex[$index])) {
throw new OutOfBoundsException(sprintf('The index %s is not within the property path', $index));
Expand All @@ -194,7 +194,7 @@ public function isProperty($index)
/**
* {@inheritdoc}
*/
public function isIndex($index)
public function isIndex(int $index)
{
if (!isset($this->isIndex[$index])) {
throw new OutOfBoundsException(sprintf('The index %s is not within the property path', $index));
Expand Down
16 changes: 8 additions & 8 deletions src/Symfony/Component/PropertyAccess/PropertyPathBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public function __construct($path = null)
* @param int $length The length of the appended piece
* If 0, the full path is appended
*/
public function append($path, $offset = 0, $length = 0)
public function append($path, int $offset = 0, int $length = 0)
{
if (\is_string($path)) {
$path = new PropertyPath($path);
Expand All @@ -66,7 +66,7 @@ public function append($path, $offset = 0, $length = 0)
*
* @param string $name The name of the appended index
*/
public function appendIndex($name)
public function appendIndex(string $name)
{
$this->elements[] = $name;
$this->isIndex[] = true;
Expand All @@ -77,7 +77,7 @@ public function appendIndex($name)
*
* @param string $name The name of the appended property
*/
public function appendProperty($name)
public function appendProperty(string $name)
{
$this->elements[] = $name;
$this->isIndex[] = false;
Expand All @@ -91,7 +91,7 @@ public function appendProperty($name)
*
* @throws OutOfBoundsException if offset is invalid
*/
public function remove($offset, $length = 1)
public function remove(int $offset, int $length = 1)
{
if (!isset($this->elements[$offset])) {
throw new OutOfBoundsException(sprintf('The offset %s is not within the property path', $offset));
Expand All @@ -113,7 +113,7 @@ public function remove($offset, $length = 1)
*
* @throws OutOfBoundsException If the offset is invalid
*/
public function replace($offset, $length, $path, $pathOffset = 0, $pathLength = 0)
public function replace(int $offset, int $length, $path, int $pathOffset = 0, int $pathLength = 0)
{
if (\is_string($path)) {
$path = new PropertyPath($path);
Expand Down Expand Up @@ -146,7 +146,7 @@ public function replace($offset, $length, $path, $pathOffset = 0, $pathLength =
*
* @throws OutOfBoundsException If the offset is invalid
*/
public function replaceByIndex($offset, $name = null)
public function replaceByIndex(int $offset, string $name = null)
{
if (!isset($this->elements[$offset])) {
throw new OutOfBoundsException(sprintf('The offset %s is not within the property path', $offset));
Expand All @@ -167,7 +167,7 @@ public function replaceByIndex($offset, $name = null)
*
* @throws OutOfBoundsException If the offset is invalid
*/
public function replaceByProperty($offset, $name = null)
public function replaceByProperty(int $offset, string $name = null)
{
if (!isset($this->elements[$offset])) {
throw new OutOfBoundsException(sprintf('The offset %s is not within the property path', $offset));
Expand Down Expand Up @@ -233,7 +233,7 @@ public function __toString()
* @param int $cutLength The length of the removed chunk
* @param int $insertionLength The length of the inserted chunk
*/
private function resize($offset, $cutLength, $insertionLength)
private function resize(int $offset, int $cutLength, int $insertionLength)
{
// Nothing else to do in this case
if ($insertionLength === $cutLength) {
Expand Down
6 changes: 3 additions & 3 deletions src/Symfony/Component/PropertyAccess/PropertyPathInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public function getElements();
*
* @throws Exception\OutOfBoundsException If the offset is invalid
*/
public function getElement($index);
public function getElement(int $index);

/**
* Returns whether the element at the given index is a property.
Expand All @@ -71,7 +71,7 @@ public function getElement($index);
*
* @throws Exception\OutOfBoundsException If the offset is invalid
*/
public function isProperty($index);
public function isProperty(int $index);

/**
* Returns whether the element at the given index is an array index.
Expand All @@ -82,5 +82,5 @@ public function isProperty($index);
*
* @throws Exception\OutOfBoundsException If the offset is invalid
*/
public function isIndex($index);
public function isIndex(int $index);
}