Merge pull request #611 from api-platform/quality

Fix some Scrutinizer issues

src/Bridge/Doctrine/Orm/Filter/SearchFilter.php

@@ -294,11 +294,11 @@ public function getDescription(string $resourceClass) : array
                     ];
                 }
             } elseif ($metadata->hasAssociation($field)) {
-                $association = $field;
                 $filterParameterNames = [
                     $property,
                     $property.'[]',
                 ];
+
                 foreach ($filterParameterNames as $filterParameterName) {
                     $description[$filterParameterName] = [
                         'property' => $property,

src/Bridge/Doctrine/Orm/Metadata/Property/DoctrineOrmPropertyMetadataFactory.php

@@ -14,6 +14,7 @@
 use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
 use ApiPlatform\Core\Metadata\Property\PropertyMetadata;
 use Doctrine\Common\Persistence\ManagerRegistry;
+use Doctrine\ORM\Mapping\ClassMetadataInfo;
 
 /**
  * Use Doctrine metadata to populate the identifier property.
@@ -56,7 +57,13 @@ public function create(string $resourceClass, string $property, array $options =
         foreach ($identifiers as $identifier) {
             if ($identifier === $property) {
                 $propertyMetadata = $propertyMetadata->withIdentifier(true);
-                $propertyMetadata = $propertyMetadata->withWritable($doctrineClassMetadata->isIdentifierNatural());
+                if ($doctrineClassMetadata instanceof ClassMetadataInfo) {
+                    $writable = $doctrineClassMetadata->isIdentifierNatural();
+                } else {
+                    $writable = false;
+                }
+
+                $propertyMetadata = $propertyMetadata->withWritable($writable);
 
                 break;
             }

src/Bridge/Doctrine/Orm/Util/QueryChecker.php

@@ -44,17 +44,7 @@ public static function hasHavingClause(QueryBuilder $queryBuilder) : bool
      */
     public static function hasRootEntityWithForeignKeyIdentifier(QueryBuilder $queryBuilder, ManagerRegistry $managerRegistry) : bool
     {
-        foreach ($queryBuilder->getRootEntities() as $rootEntity) {
-            $rootMetadata = $managerRegistry
-                ->getManagerForClass($rootEntity)
-                ->getClassMetadata($rootEntity);
-
-            if ($rootMetadata->containsForeignIdentifier) {
-                return true;
-            }
-        }
-
-        return false;
+        return self::hasRootEntityWithIdentifier($queryBuilder, $managerRegistry, true);
     }
 
     /**
@@ -66,13 +56,27 @@ public static function hasRootEntityWithForeignKeyIdentifier(QueryBuilder $query
      * @return bool
      */
     public static function hasRootEntityWithCompositeIdentifier(QueryBuilder $queryBuilder, ManagerRegistry $managerRegistry) : bool
+    {
+        return self::hasRootEntityWithIdentifier($queryBuilder, $managerRegistry, false);
+    }
+
+    /**
+     * Detects if the root entity has the given identifier.
+     *
+     * @param QueryBuilder    $queryBuilder
+     * @param ManagerRegistry $managerRegistry
+     * @param bool            $isForeign
+     *
+     * @return bool
+     */
+    private static function hasRootEntityWithIdentifier(QueryBuilder $queryBuilder, ManagerRegistry $managerRegistry, bool $isForeign) : bool
     {
         foreach ($queryBuilder->getRootEntities() as $rootEntity) {
             $rootMetadata = $managerRegistry
                 ->getManagerForClass($rootEntity)
                 ->getClassMetadata($rootEntity);
 
-            if ($rootMetadata->isIdentifierComposite) {
+            if ($isForeign ? $rootMetadata->isIdentifierComposite : $rootMetadata->containsForeignIdentifier) {
                 return true;
             }
         }

src/Bridge/Doctrine/Orm/Util/QueryNameGenerator.php

@@ -32,7 +32,7 @@ abstract class QueryNameGenerator
      *
      * @return string
      */
-    public static function generateJoinAlias($association)
+    public static function generateJoinAlias(string $association) : string
     {
         return sprintf('%s_%s', $association, uniqid());
     }
@@ -44,7 +44,7 @@ public static function generateJoinAlias($association)
      *
      * @return string
      */
-    public static function generateParameterName($name)
+    public static function generateParameterName(string $name) : string
     {
         return sprintf('%s_%s', $name, uniqid());
     }
@@ -58,11 +58,8 @@ public static function generateParameterName($name)
      *
      * @return ClassMetadata
      */
-    public static function getClassMetadataFromJoinAlias(
-        $alias,
-        QueryBuilder $queryBuilder,
-        ManagerRegistry $managerRegistry
-    ) {
+    public static function getClassMetadataFromJoinAlias(string $alias, QueryBuilder $queryBuilder, ManagerRegistry $managerRegistry) : ClassMetadata
+    {
         $rootEntities = $queryBuilder->getRootEntities();
         $rootAliases = $queryBuilder->getRootAliases();
 
@@ -127,7 +124,7 @@ public static function getClassMetadataFromJoinAlias(
      *
      * @return string
      */
-    public static function getJoinRelationship(Join $join)
+    public static function getJoinRelationship(Join $join) : string
     {
         static $relationshipProperty = null;
         static $initialized = false;
@@ -149,7 +146,7 @@ public static function getJoinRelationship(Join $join)
      *
      * @return string
      */
-    public static function getJoinAlias(Join $join)
+    public static function getJoinAlias(Join $join) : string
     {
         static $aliasProperty = null;
         static $initialized = false;
@@ -171,7 +168,7 @@ public static function getJoinAlias(Join $join)
      *
      * @return string[]
      */
-    public static function getOrderByParts(OrderBy $orderBy)
+    public static function getOrderByParts(OrderBy $orderBy) : array
     {
         static $partsProperty = null;
         static $initialized = false;

src/Bridge/FosUser/EventListener.php

@@ -52,7 +52,7 @@ public function onKernelView(GetResponseForControllerResultEvent $event)
                 break;
 
             default:
-                $this->userManager->updateUser($user, false);
+                $this->userManager->updateUser($user);
                 break;
         }
     }

src/Bridge/NelmioApiDoc/Extractor/AnnotationsProvider/ApiPlatformProvider.php

@@ -14,6 +14,7 @@
 use ApiPlatform\Core\Api\FilterCollection;
 use ApiPlatform\Core\Bridge\NelmioApiDoc\Parser\ApiPlatformParser;
 use ApiPlatform\Core\Bridge\Symfony\Routing\OperationMethodResolverInterface;
+use ApiPlatform\Core\Documentation\Documentation;
 use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
 use ApiPlatform\Core\Metadata\Resource\Factory\ResourceNameCollectionFactoryInterface;
 use ApiPlatform\Core\Metadata\Resource\ResourceMetadata;
@@ -50,22 +51,31 @@ public function __construct(ResourceNameCollectionFactoryInterface $resourceName
      */
     public function getAnnotations() : array
     {
-        $annotations = [];
-        $hydraDoc = $this->documentationNormalizer->normalize($this->resourceNameCollectionFactory->create());
+        $hydraDoc = $this->documentationNormalizer->normalize(new Documentation($this->resourceNameCollectionFactory->create()));
         $entrypointHydraDoc = $this->getResourceHydraDoc($hydraDoc, '#Entrypoint');
 
+        if (empty($hydraDoc) || null === $entrypointHydraDoc) {
+            return [];
+        }
+
+        $annotations = [];
         foreach ($this->resourceNameCollectionFactory->create() as $resourceClass) {
             $resourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
 
             $prefixedShortName = ($iri = $resourceMetadata->getIri()) ? $iri : '#'.$resourceMetadata->getShortName();
             $resourceHydraDoc = $this->getResourceHydraDoc($hydraDoc, $prefixedShortName);
+            if (null === $resourceHydraDoc) {
+                continue;
+            }
 
-            if ($hydraDoc) {
-                foreach ($resourceMetadata->getCollectionOperations() as $operationName => $operation) {
+            if (null !== $collectionOperations = $resourceMetadata->getCollectionOperations()) {
+                foreach ($collectionOperations as $operationName => $operation) {
                     $annotations[] = $this->getApiDoc(true, $resourceClass, $resourceMetadata, $operationName, $resourceHydraDoc, $entrypointHydraDoc);
                 }
+            }
 
-                foreach ($resourceMetadata->getItemOperations() as $operationName => $operation) {
+            if (null !== $itemOperations = $resourceMetadata->getItemOperations()) {
+                foreach ($itemOperations as $operationName => $operation) {
                     $annotations[] = $this->getApiDoc(false, $resourceClass, $resourceMetadata, $operationName, $resourceHydraDoc);
                 }
             }

src/Bridge/NelmioApiDoc/Parser/ApiPlatformParser.php

@@ -82,7 +82,7 @@ public function parse(array $item) : array
         list($io, $resourceClass, $operationName) = explode(':', $item['class'], 3);
         $resourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
 
-        $classOperations = $this->getGroupsForItemAndCollectionOperation($resourceMetadata, $resourceClass, $io, $operationName);
+        $classOperations = $this->getGroupsForItemAndCollectionOperation($resourceMetadata, $resourceClass, $operationName);
         if (!empty($classOperations['serializer_groups'])) {
             return $this->getPropertyMetadata($resourceMetadata, $resourceClass, $io, [], $classOperations);
         }
@@ -123,26 +123,27 @@ private function parseResource(ResourceMetadata $resourceMetadata, string $resou
      *
      * @param ResourceMetadata $resourceMetadata
      * @param string           $resourceClass
-     * @param string           $io
      * @param string[]         $visited
      *
      * @return array
      */
-    private function getGroupsForItemAndCollectionOperation(ResourceMetadata $resourceMetadata, string $resourceClass, string $io, string $operationName, array $visited = []) : array
+    private function getGroupsForItemAndCollectionOperation(ResourceMetadata $resourceMetadata, string $resourceClass, string $operationName, array $visited = []) : array
     {
         $visited[] = $resourceClass;
 
-        $options = [];
-
-        $operation['denormalization_context'] = array_merge($resourceMetadata->getItemOperationAttribute($operationName, 'denormalization_context', []), $resourceMetadata->getCollectionOperationAttribute($operationName, 'denormalization_context', []));
-        $operation['normalization_context'] = array_merge($resourceMetadata->getItemOperationAttribute($operationName, 'normalization_context', []), $resourceMetadata->getCollectionOperationAttribute($operationName, 'normalization_context', []));
+        $operation = [
+            'denormalization_context' => array_merge($resourceMetadata->getItemOperationAttribute($operationName, 'denormalization_context', []), $resourceMetadata->getCollectionOperationAttribute($operationName, 'denormalization_context', [])),
+            'normalization_context' => array_merge($resourceMetadata->getItemOperationAttribute($operationName, 'normalization_context', []), $resourceMetadata->getCollectionOperationAttribute($operationName, 'normalization_context', [])),
+        ];
 
-        $options['serializer_groups'] = !empty($operation['normalization_context']) ? $operation['normalization_context']['groups'] : [];
+        $options = [
+            'serializer_groups' => !empty($operation['normalization_context']) ? $operation['normalization_context']['groups'] : [],
+        ];
 
         $options['serializer_groups'] = array_merge(
-                $options['serializer_groups'],
-                !empty($operation['denormalization_context']) ? $operation['denormalization_context']['groups'] : []
-            );
+            $options['serializer_groups'],
+            !empty($operation['denormalization_context']) ? $operation['denormalization_context']['groups'] : []
+        );
 
         return $options;
     }

src/Bridge/Symfony/PropertyInfo/Metadata/Property/PropertyInfoPropertyMetadataFactory.php

@@ -37,13 +37,16 @@ public function __construct(PropertyInfoExtractorInterface $propertyInfo, Proper
      */
     public function create(string $resourceClass, string $name, array $options = []) : PropertyMetadata
     {
-        if (null !== $this->decorated) {
+        if (null === $this->decorated) {
+            $propertyMetadata = new PropertyMetadata();
+        } else {
             try {
                 $propertyMetadata = $this->decorated->create($resourceClass, $name, $options);
             } catch (PropertyNotFoundException $propertyNotFoundException) {
                 $propertyMetadata = new PropertyMetadata();
             }
         }
+
         if (null === $propertyMetadata->getType()) {
             $types = $this->propertyInfo->getTypes($resourceClass, $name, $options);
             if (isset($types[0])) {

src/Bridge/Symfony/PropertyInfo/Metadata/Property/PropertyInfoPropertyNameCollectionFactory.php

@@ -11,6 +11,7 @@
 
 namespace ApiPlatform\Core\Bridge\Symfony\PropertyInfo\Metadata\Property;
 
+use ApiPlatform\Core\Exception\RuntimeException;
 use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
 use ApiPlatform\Core\Metadata\Property\PropertyNameCollection;
 use Symfony\Component\PropertyInfo\PropertyInfoExtractorInterface;
@@ -36,6 +37,11 @@ public function __construct(PropertyInfoExtractorInterface $propertyInfo)
      */
     public function create(string $resourceClass, array $options = []) : PropertyNameCollection
     {
-        return new PropertyNameCollection($this->propertyInfo->getProperties($resourceClass, $options));
+        $properties = $this->propertyInfo->getProperties($resourceClass, $options);
+        if (null === $properties) {
+            throw new RuntimeException(sprintf('There is no PropertyInfo extractor supporting the class "%s".', $resourceClass));
+        }
+
+        return new PropertyNameCollection($properties);
     }
 }

src/Bridge/Symfony/Routing/ApiLoader.php

@@ -69,12 +69,16 @@ public function load($data, $type = null)
                 throw new InvalidResourceException(sprintf('Resource %s has no short name defined.', $resourceClass));
             }
 
-            foreach ($resourceMetadata->getCollectionOperations() as $operationName => $operation) {
-                $this->addRoute($routeCollection, $resourceClass, $operationName, $operation, $resourceShortName, true);
+            if (null !== $collectionOperations = $resourceMetadata->getCollectionOperations()) {
+                foreach ($collectionOperations as $operationName => $operation) {
+                    $this->addRoute($routeCollection, $resourceClass, $operationName, $operation, $resourceShortName, true);
+                }
             }
 
-            foreach ($resourceMetadata->getItemOperations() as $operationName => $operation) {
-                $this->addRoute($routeCollection, $resourceClass, $operationName, $operation, $resourceShortName, false);
+            if (null !== $itemOperations = $resourceMetadata->getItemOperations()) {
+                foreach ($itemOperations as $operationName => $operation) {
+                    $this->addRoute($routeCollection, $resourceClass, $operationName, $operation, $resourceShortName, false);
+                }
             }
         }
 

src/Bridge/Symfony/Routing/IriConverter.php

@@ -119,7 +119,8 @@ private function getIdentifiersFromItem($item) : array
         foreach ($this->propertyNameCollectionFactory->create($resourceClass) as $propertyName) {
             $propertyMetadata = $this->propertyMetadataFactory->create($resourceClass, $propertyName);
 
-            if (!$propertyMetadata->isIdentifier()) {
+            $identifier = $propertyMetadata->isIdentifier();
+            if (null === $identifier || false === $identifier) {
                 continue;
             }
 
@@ -180,15 +181,10 @@ private function getRouteName(string $resourceClass, bool $collection) : string
             $methods = $route->getMethods();
 
             if ($resourceClass === $currentResourceClass && null !== $operation && (empty($methods) || in_array('GET', $methods))) {
-                $found = true;
-                break;
+                return $routeName;
             }
         }
 
-        if (!isset($found)) {
-            throw new InvalidArgumentException(sprintf('No route associated with the type "%s".', $resourceClass));
-        }
-
-        return $routeName;
+        throw new InvalidArgumentException(sprintf('No route associated with the type "%s".', $resourceClass));
     }
 }

src/Hydra/DocumentationNormalizer.php

@@ -116,8 +116,7 @@ public function normalize($object, $format = null, array $context = [])
 
             foreach ($this->propertyNameCollectionFactory->create($resourceClass, $context) as $propertyName) {
                 $propertyMetadata = $this->propertyMetadataFactory->create($resourceClass, $propertyName);
-
-                if ($propertyMetadata->isIdentifier() && !$propertyMetadata->isWritable()) {
+                if (true === $propertyMetadata->isIdentifier() && false === $propertyMetadata->isWritable()) {
                     continue;
                 }
 

src/JsonLd/ContextBuilder.php

@@ -85,7 +85,7 @@ public function getEntrypointContext(int $referenceType = UrlGeneratorInterface:
      */
     public function getResourceContext(string $resourceClass, int $referenceType = UrlGeneratorInterface::ABS_PATH) : array
     {
-        $context = $this->getBaseContext($referenceType, $referenceType);
+        $context = $this->getBaseContext($referenceType);
         $resourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
         $prefixedShortName = sprintf('#%s', $resourceMetadata->getShortName());
 

src/Swagger/DocumentationNormalizer.php

@@ -185,6 +185,7 @@ public function normalize($object, $format = null, array $context = [])
                     }
                 }
             } catch (InvalidArgumentException $e) {
+                continue;
             }
 
             $classes[] = $class;

src/Util/Reflection.php

@@ -18,7 +18,7 @@
  *
  * @author Kévin Dunglas <dunglas@gmail.com>
  */
-class Reflection
+final class Reflection
 {
     const ACCESSOR_PREFIXES = ['get', 'is', 'has', 'can'];
     const MUTATOR_PREFIXES = ['set', 'add', 'remove'];
@@ -38,23 +38,4 @@ public function getProperty($methodName)
             return $matches[2];
         }
     }
-
-    /**
-     * Gets the {@see \ReflectionProperty} from the class or its parent.
-     *
-     * @param \ReflectionClass $reflectionClass
-     * @param string           $attributeName
-     *
-     * @return \ReflectionProperty
-     */
-    private function getReflectionProperty(\ReflectionClass $reflectionClass, $attributeName)
-    {
-        if ($reflectionClass->hasProperty($attributeName)) {
-            return $reflectionClass->getProperty($attributeName);
-        }
-
-        if ($parent = $reflectionClass->getParentClass()) {
-            return $this->getReflectionProperty($parent, $attributeName);
-        }
-    }
 }