Merge pull request #821 from meyerbaptiste/various_fixes_after_static…

…_analysis Various fixes after static analysis
api-platform · Nov 2, 2016 · 6cddcd2 · 6cddcd2
2 parents 2a4bca8 + 83da509
commit 6cddcd2
Show file tree

Hide file tree

Showing 40 changed files with 117 additions and 127 deletions.
diff --git a/src/Api/ResourceClassResolver.php b/src/Api/ResourceClassResolver.php
@@ -49,7 +49,7 @@ public function getResourceClass($value, string $resourceClass = null, bool $str
             $typeToFind = $type = $resourceClass;
         }
 
-        if (!$this->isResourceClass($typeToFind) || ($strict && isset($type) && $resourceClass !== $type)) {
+        if (($strict && isset($type) && $resourceClass !== $type) || !$this->isResourceClass($typeToFind)) {
             if (is_subclass_of($type, $resourceClass) && $this->isResourceClass($resourceClass)) {
                 return $type;
             }

diff --git a/src/Bridge/Doctrine/Orm/CollectionDataProvider.php b/src/Bridge/Doctrine/Orm/CollectionDataProvider.php
@@ -42,6 +42,8 @@ public function __construct(ManagerRegistry $managerRegistry, array $collectionE
 
     /**
      * {@inheritdoc}
+     *
+     * @throws RuntimeException
      */
     public function getCollection(string $resourceClass, string $operationName = null)
     {

diff --git a/src/Bridge/Doctrine/Orm/Extension/EagerLoadingExtension.php b/src/Bridge/Doctrine/Orm/Extension/EagerLoadingExtension.php
@@ -116,7 +116,7 @@ private function joinRelations(QueryBuilder $queryBuilder, string $resourceClass
 
             $queryBuilder->addSelect(sprintf('partial %s.{%s}', $associationAlias, implode(',', $select)));
 
-            $relationAlias = $relationAlias.++$j;
+            $relationAlias .= ++$j;
 
             $this->joinRelations($queryBuilder, $mapping['targetEntity'], $propertyMetadataOptions, $associationAlias, $relationAlias, $method === 'leftJoin');
         }

diff --git a/src/Bridge/Doctrine/Orm/Extension/FilterExtension.php b/src/Bridge/Doctrine/Orm/Extension/FilterExtension.php
@@ -47,7 +47,7 @@ public function applyToCollection(QueryBuilder $queryBuilder, QueryNameGenerator
         }
 
         foreach ($this->filters as $filterName => $filter) {
-            if (in_array($filterName, $resourceFilters) && $filter instanceof FilterInterface) {
+            if ($filter instanceof FilterInterface && in_array($filterName, $resourceFilters)) {
                 $filter->apply($queryBuilder, $queryNameGenerator, $resourceClass, $operationName);
             }
         }

diff --git a/src/Bridge/Doctrine/Orm/Filter/BooleanFilter.php b/src/Bridge/Doctrine/Orm/Filter/BooleanFilter.php
@@ -13,11 +13,8 @@
 
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
 use ApiPlatform\Core\Exception\InvalidArgumentException;
-use Doctrine\Common\Persistence\ManagerRegistry;
 use Doctrine\DBAL\Types\Type as DBALType;
 use Doctrine\ORM\QueryBuilder;
-use Psr\Log\LoggerInterface;
-use Symfony\Component\HttpFoundation\RequestStack;
 
 /**
  * Filters the collection by boolean values.
@@ -33,11 +30,6 @@
  */
 class BooleanFilter extends AbstractFilter
 {
-    public function __construct(ManagerRegistry $managerRegistry, RequestStack $requestStack, LoggerInterface $logger = null, array $properties = null)
-    {
-        parent::__construct($managerRegistry, $requestStack, $logger, $properties);
-    }
-
     /**
      * {@inheritdoc}
      */

diff --git a/src/Bridge/Doctrine/Orm/Filter/DateFilter.php b/src/Bridge/Doctrine/Orm/Filter/DateFilter.php
@@ -12,10 +12,7 @@
 namespace ApiPlatform\Core\Bridge\Doctrine\Orm\Filter;
 
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
-use Doctrine\Common\Persistence\ManagerRegistry;
 use Doctrine\ORM\QueryBuilder;
-use Psr\Log\LoggerInterface;
-use Symfony\Component\HttpFoundation\RequestStack;
 
 /**
  * Filters the collection by date intervals.
@@ -37,11 +34,6 @@ class DateFilter extends AbstractFilter
         'time' => true,
     ];
 
-    public function __construct(ManagerRegistry $managerRegistry, RequestStack $requestStack, LoggerInterface $logger = null, array $properties = null)
-    {
-        parent::__construct($managerRegistry, $requestStack, $logger, $properties);
-    }
-
     /**
      * {@inheritdoc}
      */
@@ -88,7 +80,7 @@ protected function filterProperty(string $property, $values, QueryBuilder $query
             list($alias, $field) = $this->addJoinsForNestedProperty($property, $alias, $queryBuilder, $queryNameGenerator);
         }
 
-        $nullManagement = isset($this->properties[$property]) ? $this->properties[$property] : null;
+        $nullManagement = $this->properties[$property] ?? null;
 
         if (self::EXCLUDE_NULL === $nullManagement) {
             $queryBuilder->andWhere($queryBuilder->expr()->isNotNull(sprintf('%s.%s', $alias, $field)));

diff --git a/src/Bridge/Doctrine/Orm/Filter/NumericFilter.php b/src/Bridge/Doctrine/Orm/Filter/NumericFilter.php
@@ -13,11 +13,8 @@
 
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
 use ApiPlatform\Core\Exception\InvalidArgumentException;
-use Doctrine\Common\Persistence\ManagerRegistry;
 use Doctrine\DBAL\Types\Type as DBALType;
 use Doctrine\ORM\QueryBuilder;
-use Psr\Log\LoggerInterface;
-use Symfony\Component\HttpFoundation\RequestStack;
 
 /**
  * Filters the collection by numeric values.
@@ -45,11 +42,6 @@ class NumericFilter extends AbstractFilter
         DBALType::SMALLINT => true,
     ];
 
-    public function __construct(ManagerRegistry $managerRegistry, RequestStack $requestStack, LoggerInterface $logger = null, array $properties = null)
-    {
-        parent::__construct($managerRegistry, $requestStack, $logger, $properties);
-    }
-
     /**
      * {@inheritdoc}
      */

diff --git a/src/Bridge/Doctrine/Orm/Filter/RangeFilter.php b/src/Bridge/Doctrine/Orm/Filter/RangeFilter.php
@@ -13,10 +13,7 @@
 
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
 use ApiPlatform\Core\Exception\InvalidArgumentException;
-use Doctrine\Common\Persistence\ManagerRegistry;
 use Doctrine\ORM\QueryBuilder;
-use Psr\Log\LoggerInterface;
-use Symfony\Component\HttpFoundation\RequestStack;
 
 /**
  * Filters the collection by range.
@@ -31,11 +28,6 @@ class RangeFilter extends AbstractFilter
     const PARAMETER_LESS_THAN = 'lt';
     const PARAMETER_LESS_THAN_OR_EQUAL = 'lte';
 
-    public function __construct(ManagerRegistry $managerRegistry, RequestStack $requestStack, LoggerInterface $logger = null, array $properties = null)
-    {
-        parent::__construct($managerRegistry, $requestStack, $logger, $properties);
-    }
-
     /**
      * {@inheritdoc}
      */

diff --git a/src/Bridge/Doctrine/Orm/Filter/SearchFilter.php b/src/Bridge/Doctrine/Orm/Filter/SearchFilter.php
@@ -168,9 +168,9 @@ private function getType(string $doctrineType) : string
     protected function filterProperty(string $property, $value, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, string $operationName = null)
     {
         if (
+            null === $value ||
             !$this->isPropertyEnabled($property) ||
-            !$this->isPropertyMapped($property, $resourceClass, true) ||
-            null === $value
+            !$this->isPropertyMapped($property, $resourceClass, true)
         ) {
             return;
         }

diff --git a/src/Bridge/Doctrine/Orm/ItemDataProvider.php b/src/Bridge/Doctrine/Orm/ItemDataProvider.php
@@ -17,6 +17,7 @@
 use ApiPlatform\Core\DataProvider\ItemDataProviderInterface;
 use ApiPlatform\Core\Exception\PropertyNotFoundException;
 use ApiPlatform\Core\Exception\ResourceClassNotSupportedException;
+use ApiPlatform\Core\Exception\RuntimeException;
 use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
 use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
 use Doctrine\Common\Persistence\ManagerRegistry;
@@ -53,6 +54,8 @@ public function __construct(ManagerRegistry $managerRegistry, PropertyNameCollec
 
     /**
      * {@inheritdoc}
+     *
+     * @throws RuntimeException
      */
     public function getItem(string $resourceClass, $id, string $operationName = null, bool $fetchData = false)
     {
@@ -68,6 +71,10 @@ public function getItem(string $resourceClass, $id, string $operationName = null
         }
 
         $repository = $manager->getRepository($resourceClass);
+        if (!method_exists($repository, 'createQueryBuilder')) {
+            throw new RuntimeException('The repository class must have a "createQueryBuilder" method.');
+        }
+
         $queryBuilder = $repository->createQueryBuilder('o');
         $queryNameGenerator = new QueryNameGenerator();
 

diff --git a/src/Bridge/Doctrine/Orm/Paginator.php b/src/Bridge/Doctrine/Orm/Paginator.php
@@ -12,7 +12,6 @@
 namespace ApiPlatform\Core\Bridge\Doctrine\Orm;
 
 use ApiPlatform\Core\DataProvider\PaginatorInterface;
-use Doctrine\ORM\Query;
 use Doctrine\ORM\Tools\Pagination\Paginator as DoctrineOrmPaginator;
 
 /**
@@ -24,11 +23,6 @@ final class Paginator implements \IteratorAggregate, PaginatorInterface
 {
     private $paginator;
 
-    /**
-     * @var Query
-     */
-    private $query;
-
     /**
      * @var int
      */
@@ -52,9 +46,9 @@ final class Paginator implements \IteratorAggregate, PaginatorInterface
     public function __construct(DoctrineOrmPaginator $paginator)
     {
         $this->paginator = $paginator;
-        $this->query = $paginator->getQuery();
-        $this->firstResult = $this->query->getFirstResult();
-        $this->maxResults = $this->query->getMaxResults();
+        $query = $paginator->getQuery();
+        $this->firstResult = $query->getFirstResult();
+        $this->maxResults = $query->getMaxResults();
         $this->totalItems = count($paginator);
     }
 

diff --git a/src/Bridge/Doctrine/Orm/Util/QueryChecker.php b/src/Bridge/Doctrine/Orm/Util/QueryChecker.php
@@ -132,16 +132,13 @@ public static function hasOrderByOnToManyJoin(QueryBuilder $queryBuilder, Manage
         }
 
         if (!empty($orderByAliases)) {
-            foreach ($joinParts as $rootAlias => $joins) {
+            foreach ($joinParts as $joins) {
                 foreach ($joins as $join) {
                     $alias = QueryJoinParser::getJoinAlias($join);
 
                     if (isset($orderByAliases[$alias])) {
                         $relationship = QueryJoinParser::getJoinRelationship($join);
-
-                        $relationshipParts = explode('.', $relationship);
-                        $parentAlias = $relationshipParts[0];
-                        $association = $relationshipParts[1];
+                        list($parentAlias, $association) = explode('.', $relationship);
 
                         $parentMetadata = QueryJoinParser::getClassMetadataFromJoinAlias($parentAlias, $queryBuilder, $managerRegistry);
 

diff --git a/src/Bridge/NelmioApiDoc/Parser/ApiPlatformParser.php b/src/Bridge/NelmioApiDoc/Parser/ApiPlatformParser.php
@@ -131,7 +131,7 @@ private function getGroupsContext(ResourceMetadata $resourceMetadata, string $op
         $groupsContext = $isNormalization ? 'normalization_context' : 'denormalization_context';
         $itemOperationAttribute = $resourceMetadata->getItemOperationAttribute($operationName, $groupsContext, ['groups' => []], true)['groups'];
         $collectionOperationAttribute = $resourceMetadata->getCollectionOperationAttribute($operationName, $groupsContext, ['groups' => []], true)['groups'];
-        $operation[$groupsContext]['groups'] = array_merge(isset($itemOperationAttribute) ? $itemOperationAttribute : [], isset($collectionOperationAttribute) ? $collectionOperationAttribute : []);
+        $operation[$groupsContext]['groups'] = array_merge($itemOperationAttribute ?? [], $collectionOperationAttribute ?? []);
 
         return $operation;
     }
@@ -141,6 +141,7 @@ private function getGroupsContext(ResourceMetadata $resourceMetadata, string $op
      *
      * @param ResourceMetadata $resourceMetadata
      * @param string           $operationName
+     * @param string           $io
      *
      * @return array
      */
@@ -213,15 +214,11 @@ private function parseProperty(ResourceMetadata $resourceMetadata, PropertyMetad
             'readonly' => !$propertyMetadata->isWritable(),
         ];
 
-        if (null === $type) {
-            $type = $propertyMetadata->getType();
-
-            if (null === $type) {
-                // Default to string
-                $data['dataType'] = DataTypes::STRING;
+        if (null === $type && null === $type = $propertyMetadata->getType()) {
+            // Default to string
+            $data['dataType'] = DataTypes::STRING;
 
-                return $data;
-            }
+            return $data;
         }
 
         if ($type->isCollection()) {

diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php b/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php
@@ -38,11 +38,11 @@ public function prepend(ContainerBuilder $container)
             return;
         }
 
-        if (!isset($frameworkConfiguration['serializer']) || !isset($frameworkConfiguration['serializer']['enabled'])) {
+        if (!isset($frameworkConfiguration['serializer'], $frameworkConfiguration['serializer']['enabled'])) {
             $container->prependExtensionConfig('framework', ['serializer' => ['enabled' => true]]);
         }
 
-        if (!isset($frameworkConfiguration['property_info']) || !isset($frameworkConfiguration['property_info']['enabled'])) {
+        if (!isset($frameworkConfiguration['property_info'], $frameworkConfiguration['property_info']['enabled'])) {
             $container->prependExtensionConfig('framework', ['property_info' => ['enabled' => true]]);
         }
     }

diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/DataProviderPass.php b/src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/DataProviderPass.php
@@ -49,7 +49,7 @@ private function registerDataProviders(ContainerBuilder $container, string $type
 
         foreach ($services as $serviceId => $tags) {
             foreach ($tags as $attributes) {
-                $priority = isset($attributes['priority']) ? $attributes['priority'] : 0;
+                $priority = $attributes['priority'] ?? 0;
                 $queue->insert(new Reference($serviceId), $priority);
             }
         }

diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/DoctrineQueryExtensionPass.php b/src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/DoctrineQueryExtensionPass.php
@@ -55,7 +55,7 @@ private function findSortedServices(ContainerBuilder $container, $tag)
         $extensions = [];
         foreach ($container->findTaggedServiceIds($tag) as $serviceId => $tags) {
             foreach ($tags as $tag) {
-                $priority = isset($tag['priority']) ? $tag['priority'] : 0;
+                $priority = $tag['priority'] ?? 0;
                 $extensions[$priority][] = new Reference($serviceId);
             }
         }

diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/FilterPass.php b/src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/FilterPass.php
@@ -27,6 +27,8 @@ final class FilterPass implements CompilerPassInterface
 {
     /**
      * {@inheritdoc}
+     *
+     * @throws RuntimeException
      */
     public function process(ContainerBuilder $container)
     {

diff --git a/src/Bridge/Symfony/Bundle/Resources/config/doctrine_orm.xml b/src/Bridge/Symfony/Bundle/Resources/config/doctrine_orm.xml
@@ -11,14 +11,14 @@
 
         <service id="api_platform.doctrine.orm.collection_data_provider" public="false" abstract="true">
             <argument type="service" id="doctrine" />
-            <argument type="collection"></argument> <!-- extensions -->
+            <argument type="collection" /> <!-- extensions -->
         </service>
 
         <service id="api_platform.doctrine.orm.item_data_provider" public="false" abstract="true">
             <argument type="service" id="doctrine" />
             <argument type="service" id="api_platform.metadata.property.name_collection_factory" />
             <argument type="service" id="api_platform.metadata.property.metadata_factory" />
-            <argument type="collection"></argument> <!-- extensions -->
+            <argument type="collection" /> <!-- extensions -->
         </service>
 
         <service id="api_platform.doctrine.orm.default.collection_data_provider" parent="api_platform.doctrine.orm.collection_data_provider" class="ApiPlatform\Core\Bridge\Doctrine\Orm\CollectionDataProvider">

diff --git a/...idge/Symfony/PropertyInfo/Metadata/Property/PropertyInfoPropertyNameCollectionFactory.php b/...idge/Symfony/PropertyInfo/Metadata/Property/PropertyInfoPropertyNameCollectionFactory.php
@@ -34,6 +34,8 @@ public function __construct(PropertyInfoExtractorInterface $propertyInfo)
 
     /**
      * {@inheritdoc}
+     *
+     * @throws RuntimeException
      */
     public function create(string $resourceClass, array $options = []) : PropertyNameCollection
     {

diff --git a/src/Bridge/Symfony/Routing/IriConverter.php b/src/Bridge/Symfony/Routing/IriConverter.php
@@ -61,7 +61,7 @@ public function getItemFromIri(string $iri, bool $fetchData = false)
             throw new InvalidArgumentException(sprintf('No route matches "%s".', $iri), $e->getCode(), $e);
         }
 
-        if (!isset($parameters['_api_resource_class']) || !isset($parameters['id'])) {
+        if (!isset($parameters['_api_resource_class'], $parameters['id'])) {
             throw new InvalidArgumentException(sprintf('No resource associated to "%s".', $iri));
         }
 

diff --git a/src/Bridge/Symfony/Routing/RouterOperationPathResolver.php b/src/Bridge/Symfony/Routing/RouterOperationPathResolver.php
@@ -33,6 +33,8 @@ public function __construct(RouterInterface $router, OperationPathResolverInterf
 
     /**
      * {@inheritdoc}
+     *
+     * @throws InvalidArgumentException
      */
     public function resolveOperationPath(string $resourceShortName, array $operation, bool $collection) : string
     {

diff --git a/src/Documentation/Documentation.php b/src/Documentation/Documentation.php
@@ -32,7 +32,7 @@ public function __construct(ResourceNameCollection $resourceNameCollection, stri
         $this->title = $title;
         $this->description = $description;
         $this->version = $version;
-        foreach ($formats as $format => $mimeTypes) {
+        foreach ($formats as $mimeTypes) {
             foreach ($mimeTypes as $mimeType) {
                 $this->mimeTypes[] = $mimeType;
             }

diff --git a/src/EventListener/AddFormatListener.php b/src/EventListener/AddFormatListener.php
@@ -38,6 +38,9 @@ public function __construct(Negotiator $negotiator, array $formats)
      * Sets the applicable format to the HttpFoundation Request.
      *
      * @param GetResponseEvent $event
+     *
+     * @throws NotFoundHttpException
+     * @throws NotAcceptableHttpException
      */
     public function onKernelRequest(GetResponseEvent $event)
     {
@@ -55,7 +58,7 @@ public function onKernelRequest(GetResponseEvent $event)
         } elseif (!isset($this->formats[$routeFormat])) {
             throw new NotFoundHttpException('Not Found');
         } else {
-            $mimeTypes = $request->getMimeTypes($routeFormat);
+            $mimeTypes = Request::getMimeTypes($routeFormat);
         }
 
         // First, try to guess the format from the Accept header

diff --git a/src/EventListener/ExceptionListener.php b/src/EventListener/ExceptionListener.php
@@ -27,8 +27,8 @@ public function onKernelException(GetResponseForExceptionEvent $event)
         $request = $event->getRequest();
         // Normalize exceptions only for routes managed by API Platform
         if (
-            (!$request->attributes->has('_api_resource_class') && !$request->attributes->has('_api_respond')) ||
-            'html' === $request->getRequestFormat(null)
+            'html' === $request->getRequestFormat(null) ||
+            (!$request->attributes->has('_api_resource_class') && !$request->attributes->has('_api_respond'))
         ) {
             return;
         }