Merge pull request #105 from symfony-cmf/code-cleanup

visbility cleanup

Author: dbu

UPGRADE-2.0.md

@@ -38,3 +38,11 @@
 
  * The `subjectObject` and `translatedSubjectObject` with their related methods
    are renamed to `subject` and `translatedSubject`.
+
+## Customization
+
+A lot of previously protected fields and some methods where moved to private, 
+as they are not meant to represent part of the contract. If you have a valid 
+use case to extend one of the classes, please explain in a github issue and
+we will see whether that property should be made protected or whether ther is a
+cleaner sulution.

src/AutoRouteManager.php

@@ -23,17 +23,17 @@ class AutoRouteManager
     /**
      * @var AdapterInterface
      */
-    protected $adapter;
+    private $adapter;
 
     /**
      * @var UriGeneratorInterface
      */
-    protected $uriGenerator;
+    private $uriGenerator;
 
     /**
      * @var DefunctRouteHandlerInterface
      */
-    protected $defunctRouteHandler;
+    private $defunctRouteHandler;
 
     /**
      * @var UriContextCollection[]
@@ -43,7 +43,7 @@ class AutoRouteManager
     /**
      * @var UriContextCollectionBuilder
      */
-    protected $collectionBuilder;
+    private $collectionBuilder;
 
     public function __construct(
         AdapterInterface $adapter,

src/ConflictResolver/AutoIncrementConflictResolver.php

@@ -24,8 +24,17 @@
  */
 class AutoIncrementConflictResolver implements ConflictResolverInterface
 {
-    protected $adapter;
-    protected $index;
+    /**
+     * @var AdapterInterface
+     */
+    private $adapter;
+
+    /**
+     * To count the increment, increased until conflic is resolved.
+     *
+     * @var int
+     */
+    private $index;
 
     /**
      * Construct the conflict resolver using the given adapter.
@@ -56,13 +65,13 @@ public function resolveConflict(UriContext $uriContext)
      * Tell if the given URI for the given context is conflicting with another
      * route.
      */
-    protected function isUriConflicting($uri, UriContext $uriContext)
+    private function isUriConflicting($uri, UriContext $uriContext)
     {
         return null !== $uriContext->getCollection()->getAutoRouteByUri($uri)
             || null !== $this->adapter->findRouteForUri($uri, $uriContext);
     }
 
-    protected function incrementUri($uri)
+    private function incrementUri($uri)
     {
         return sprintf('%s-%s', $uri, ++$this->index);
     }

src/DefunctRouteHandler/DelegatingDefunctRouteHandler.php

@@ -25,14 +25,23 @@
  */
 class DelegatingDefunctRouteHandler implements DefunctRouteHandlerInterface
 {
-    protected $serviceRegistry;
-    protected $adapter;
+    /**
+     * Used to find the old route action.
+     *
+     * @var ServiceRegistry
+     */
+    private $serviceRegistry;
+
+    /**
+     * @var AdapterInterface
+     */
+    private $adapter;
 
     /**
-     * @param ServiceRegistry auto routing service registry (for getting old route action)
-     * @param AdapterInterface auto routing backend adapter
-     * @param MetadataFactory  auto routing metadata factory
+     * @var MetadataFactory
      */
+    private $metadataFactory;
+
     public function __construct(
         MetadataFactory $metadataFactory,
         AdapterInterface $adapter,

src/DefunctRouteHandler/LeaveRedirectDefunctRouteHandler.php

@@ -20,7 +20,7 @@ class LeaveRedirectDefunctRouteHandler implements DefunctRouteHandlerInterface
     /**
      * @var AdapterInterface
      */
-    protected $adapter;
+    private $adapter;
 
     public function __construct(AdapterInterface $adapter)
     {

src/DefunctRouteHandler/RemoveDefunctRouteHandler.php

@@ -20,7 +20,7 @@ class RemoveDefunctRouteHandler implements DefunctRouteHandlerInterface
     /**
      * @var AdapterInterface
      */
-    protected $adapter;
+    private $adapter;
 
     /**
      * @param AdapterInterface

src/Mapping/ClassMetadata.php

@@ -24,12 +24,12 @@ class ClassMetadata extends MergeableClassMetadata
     /**
      * @var array
      */
-    protected $tokenProviders = [];
+    private $tokenProviders = [];
 
     /**
      * @var array
      */
-    protected $conflictResolver = ['name' => 'throw_exception', 'options' => []];
+    private $conflictResolver = ['name' => 'throw_exception', 'options' => []];
 
     /**
      * Defunct route handler, default to remove.

src/ServiceRegistry.php

@@ -13,9 +13,20 @@
 
 class ServiceRegistry
 {
-    protected $tokenProviders = [];
-    protected $conflictResolvers = [];
-    protected $defunctRouteHandlers = [];
+    /**
+     * @var TokenProviderInterface[]
+     */
+    private $tokenProviders = [];
+
+    /**
+     * @var ConflictResolverInterface[]
+     */
+    private $conflictResolvers = [];
+
+    /**
+     * @var DefunctRouteHandlerInterface[]
+     */
+    private $defunctRouteHandlers = [];
 
     /**
      * Return the named token provider.
@@ -60,7 +71,7 @@ public function getConflictResolver($name)
      *
      * @throws \InvalidArgumentException if the named token provider does not exist
      *
-     * @return ConflictResolverInterface
+     * @return DefunctRouteHandlerInterface
      */
     public function getDefunctRouteHandler($name)
     {

src/UriContext.php

@@ -26,16 +26,54 @@
  */
 class UriContext
 {
-    protected $collection;
-    protected $translatedSubject;
-    protected $locale;
-    protected $uri;
-    protected $autoRoute;
-    protected $uriSchema;
-    protected $tokenProviderConfigs;
-    protected $conflictResolverConfig;
-    protected $subjectMetadata;
-    protected $defaults;
+    /**
+     * @var UriContextCollection
+     */
+    private $collection;
+
+    /**
+     * @var string
+     */
+    private $uriSchema;
+
+    /**
+     * @var array
+     */
+    private $defaults;
+
+    /**
+     * @var array
+     */
+    private $tokenProviderConfigs;
+
+    /**
+     * @var array
+     */
+    private $conflictResolverConfig;
+
+    /**
+     * The locale of the translatedSubject.
+     *
+     * If null, the subject is not translated.
+     *
+     * @var string
+     */
+    private $locale;
+
+    /**
+     * @var object
+     */
+    private $translatedSubject;
+
+    /**
+     * @var string
+     */
+    private $uri;
+
+    /**
+     * @var AutoRouteInterface
+     */
+    private $autoRoute;
 
     /**
      * Construct the context.

src/UriContextCollection.php

@@ -18,12 +18,12 @@
  */
 class UriContextCollection
 {
-    protected $subject;
+    private $subject;
 
     /**
      * @var UriContext[]
      */
-    protected $uriContexts = [];
+    private $uriContexts = [];
 
     /**
      * Construct the collection for the given subject.

src/UriContextCollectionBuilder.php

@@ -12,6 +12,7 @@
 namespace Symfony\Cmf\Component\RoutingAuto;
 
 use Metadata\MetadataFactoryInterface;
+use Symfony\Cmf\Component\RoutingAuto\Mapping\MetadataFactory;
 
 /**
  * Class responsible for creating all the implied UriContext objects for
@@ -22,14 +23,14 @@
 class UriContextCollectionBuilder
 {
     /**
-     * @var MetadataFactoryInterface
+     * @var MetadataFactory
      */
-    protected $metadataFactory;
+    private $metadataFactory;
 
     /**
      * @var AdapterInterface
      */
-    protected $adapter;
+    private $adapter;
 
     public function __construct(MetadataFactoryInterface $metadataFactory, AdapterInterface $adapter)
     {

src/UriGenerator.php

@@ -20,12 +20,11 @@
  */
 class UriGenerator implements UriGeneratorInterface
 {
-    protected $serviceRegistry;
-
     /**
-     * @param MetadataFactory   the metadata factory
-     * @param ServiceRegistry  the auto route service registry
+     * @var ServiceRegistry The auto route service registry
      */
+    private $serviceRegistry;
+
     public function __construct(
         ServiceRegistry $serviceRegistry
     ) {
@@ -62,7 +61,7 @@ public function generateUri(UriContext $uriContext)
 
             $tokenValue = $tokenProvider->provideValue($uriContext, $tokenProviderOptions);
 
-            $isEmpty = empty($tokenValue) || $tokenValue == '/';
+            $isEmpty = empty($tokenValue) || '/' === $tokenValue;
 
             if ($isEmpty && false === $tokenProviderOptions['allow_empty']) {
                 throw new \InvalidArgumentException(sprintf(
@@ -87,7 +86,7 @@ public function generateUri(UriContext $uriContext)
 
         $uri = strtr($uriSchema, $tokens);
 
-        if (substr($uri, 0, 1) !== '/') {
+        if ($uri[0] !== '/') {
             throw new \InvalidArgumentException(sprintf(
                 'Generated non-absolute URI "%s" for object "%s"',
                 $uri, get_class($uriContext->getSubject())
@@ -104,12 +103,10 @@ public function resolveConflict(UriContext $uriContext)
     {
         $conflictResolverConfig = $uriContext->getConflictResolverConfig();
         $conflictResolver = $this->serviceRegistry->getConflictResolver(
-            $conflictResolverConfig['name'],
-            $conflictResolverConfig['options']
+            $conflictResolverConfig['name']
         );
-        $uri = $conflictResolver->resolveConflict($uriContext);
 
-        return $uri;
+        return $conflictResolver->resolveConflict($uriContext);
     }
 
     /**

tests/Unit/Adapter/EventDispatchingAdapterTest.php

@@ -59,11 +59,14 @@ class EventDispatchingAdapterTest extends \PHPUnit_Framework_TestCase
      */
     private $autoRoute2;
 
+    /**
+     * @var object
+     */
     private $content;
 
     public function setUp()
     {
-        $this->realAdapter = $this->prophesize('Symfony\Cmf\Component\RoutingAuto\AdapterInterface');
+        $this->realAdapter = $this->prophesize(AdapterInterface::class);
         $this->dispatcher = new EventDispatcher();
         $this->adapter = new EventDispatchingAdapter(
             $this->realAdapter->reveal(),
@@ -72,9 +75,9 @@ public function setUp()
 
         $this->subscriber = new EventDispatchingAdapterSubscriber();
         $this->dispatcher->addSubscriber($this->subscriber);
-        $this->uriContext = $this->prophesize('Symfony\Cmf\Component\RoutingAuto\UriContext');
-        $this->autoRoute = $this->prophesize('Symfony\Cmf\Component\RoutingAuto\Model\AutoRouteInterface');
-        $this->autoRoute2 = $this->prophesize('Symfony\Cmf\Component\RoutingAuto\Model\AutoRouteInterface');
+        $this->uriContext = $this->prophesize(UriContext::class);
+        $this->autoRoute = $this->prophesize(AutoRouteInterface::class);
+        $this->autoRoute2 = $this->prophesize(AutoRouteInterface::class);
         $this->content = new \stdClass();
     }
 
@@ -83,7 +86,7 @@ public function testCreateAutoRoute()
         $this->realAdapter->createAutoRoute($this->uriContext->reveal(), 'fr')->willReturn($this->autoRoute->reveal());
         $this->adapter->createAutoRoute($this->uriContext->reveal(), 'fr');
         $this->assertNotNull($this->subscriber->createEvent);
-        $this->assertInstanceOf('Symfony\Cmf\Component\RoutingAuto\Event\AutoRouteCreateEvent', $this->subscriber->createEvent);
+        $this->assertInstanceOf(AutoRouteCreateEvent::class, $this->subscriber->createEvent);
         $this->assertSame($this->autoRoute->reveal(), $this->subscriber->createEvent->getAutoRoute());
         $this->assertSame($this->uriContext->reveal(), $this->subscriber->createEvent->getUriContext());
     }
@@ -95,7 +98,7 @@ public function testMigrateAutoRouteChildren()
             $this->autoRoute2->reveal()
         );
         $this->assertNotNull($this->subscriber->migrateEvent);
-        $this->assertInstanceOf('Symfony\Cmf\Component\RoutingAuto\Event\AutoRouteMigrateEvent', $this->subscriber->migrateEvent);
+        $this->assertInstanceOf(AutoRouteMigrateEvent::class, $this->subscriber->migrateEvent);
         $this->assertSame($this->autoRoute->reveal(), $this->subscriber->migrateEvent->getSrcAutoRoute());
         $this->assertSame($this->autoRoute2->reveal(), $this->subscriber->migrateEvent->getDestAutoRoute());
     }