feature #33497 [Contracts] Add parameter type declarations to contrac…

…ts (derrabus) This PR was merged into the 5.0-dev branch. Discussion ---------- [Contracts] Add parameter type declarations to contracts | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #32179 | License | MIT | Doc PR | N/A This PR proposes to create a php 7.2 version of the contracts that maintains BC with Symfony 4. The PR suggests to bump the contracts version to ~~1.2~~ 2.0 on the master branch. We would still be able to maintain the contracts 1.1 branch on Symfony's 4.4 branch, should we need to patch the current contracts in the future. This move would allow us to add parameter type declarations to existing contracts interfaces and make use of them in Symfony 5. Especially the Translation and EventDispatcher components benefit a lot from this bump, imho. Contracts that will be added on the road to Symfony 6 wouldn't be restricted to the capabilities of php 7.1, which would be another benefit in my opinion. ~~<sup>1</sup> Test currently fail because the translator is called with `null` as translation key. That possibility should be deprecated imho.~~ Commits ------- 4de3773 Add parameter type declarations to contracts.
symfony · Nov 9, 2019 · 9aa7492 · 9aa7492
2 parents 69d436b + 4de3773
commit 9aa7492
Show file tree

Hide file tree

Showing 29 changed files with 90 additions and 184 deletions.
diff --git a/composer.json b/composer.json
@@ -23,9 +23,10 @@
         "twig/twig": "^2.10|^3.0",
         "psr/cache": "~1.0",
         "psr/container": "^1.0",
+        "psr/event-dispatcher": "^1.0",
         "psr/link": "^1.0",
         "psr/log": "~1.0",
-        "symfony/contracts": "^1.1.7|^2",
+        "symfony/contracts": "^2",
         "symfony/polyfill-ctype": "~1.8",
         "symfony/polyfill-intl-grapheme": "~1.0",
         "symfony/polyfill-intl-icu": "~1.0",

diff --git a/...le/FrameworkBundle/Tests/DependencyInjection/Compiler/DataCollectorTranslatorPassTest.php b/...le/FrameworkBundle/Tests/DependencyInjection/Compiler/DataCollectorTranslatorPassTest.php
@@ -108,7 +108,7 @@ public function getNotImplementingTranslatorBagInterfaceTranslatorClassNames()
 
 class TranslatorWithTranslatorBag implements TranslatorInterface
 {
-    public function trans($id, array $parameters = [], $domain = null, $locale = null): string
+    public function trans(string $id, array $parameters = [], string $domain = null, string $locale = null): string
     {
     }
 }
diff --git a/src/Symfony/Component/EventDispatcher/Debug/TraceableEventDispatcher.php b/src/Symfony/Component/EventDispatcher/Debug/TraceableEventDispatcher.php
@@ -18,7 +18,6 @@
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\Stopwatch\Stopwatch;
-use Symfony\Contracts\EventDispatcher\Event;
 use Symfony\Contracts\Service\ResetInterface;
 
 /**
@@ -129,12 +128,8 @@ public function hasListeners(string $eventName = null)
     /**
      * {@inheritdoc}
      */
-    public function dispatch($event, string $eventName = null): object
+    public function dispatch(object $event, string $eventName = null): object
     {
-        if (!\is_object($event)) {
-            throw new \TypeError(sprintf('Argument 1 passed to "%s::dispatch()" must be an object, %s given.', EventDispatcherInterface::class, \gettype($event)));
-        }
-
         $eventName = $eventName ?? \get_class($event);
 
         if (null === $this->callStack) {
@@ -143,7 +138,7 @@ public function dispatch($event, string $eventName = null): object
 
         $currentRequestHash = $this->currentRequestHash = $this->requestStack && ($request = $this->requestStack->getCurrentRequest()) ? spl_object_hash($request) : '';
 
-        if (null !== $this->logger && ($event instanceof Event || $event instanceof StoppableEventInterface) && $event->isPropagationStopped()) {
+        if (null !== $this->logger && $event instanceof StoppableEventInterface && $event->isPropagationStopped()) {
             $this->logger->debug(sprintf('The "%s" event is already stopped. No listeners have been called.', $eventName));
         }
 

diff --git a/src/Symfony/Component/EventDispatcher/Debug/WrappedListener.php b/src/Symfony/Component/EventDispatcher/Debug/WrappedListener.php
@@ -15,7 +15,6 @@
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\Stopwatch\Stopwatch;
 use Symfony\Component\VarDumper\Caster\ClassStub;
-use Symfony\Contracts\EventDispatcher\Event;
 
 /**
  * @author Fabien Potencier <fabien@symfony.com>
@@ -121,7 +120,7 @@ public function __invoke(object $event, string $eventName, EventDispatcherInterf
             $e->stop();
         }
 
-        if (($event instanceof Event || $event instanceof StoppableEventInterface) && $event->isPropagationStopped()) {
+        if ($event instanceof StoppableEventInterface && $event->isPropagationStopped()) {
             $this->stoppedPropagation = true;
         }
     }

diff --git a/src/Symfony/Component/EventDispatcher/EventDispatcher.php b/src/Symfony/Component/EventDispatcher/EventDispatcher.php
@@ -13,7 +13,6 @@
 
 use Psr\EventDispatcher\StoppableEventInterface;
 use Symfony\Component\EventDispatcher\Debug\WrappedListener;
-use Symfony\Contracts\EventDispatcher\Event;
 
 /**
  * The EventDispatcherInterface is the central point of Symfony's event listener system.
@@ -46,12 +45,8 @@ public function __construct()
     /**
      * {@inheritdoc}
      */
-    public function dispatch($event, string $eventName = null): object
+    public function dispatch(object $event, string $eventName = null): object
     {
-        if (!\is_object($event)) {
-            throw new \TypeError(sprintf('Argument 1 passed to "%s::dispatch()" must be an object, %s given.', EventDispatcherInterface::class, \gettype($event)));
-        }
-
         $eventName = $eventName ?? \get_class($event);
 
         if (null !== $this->optimized && null !== $eventName) {
@@ -226,7 +221,7 @@ public function removeSubscriber(EventSubscriberInterface $subscriber)
      */
     protected function callListeners(iterable $listeners, string $eventName, object $event)
     {
-        $stoppable = $event instanceof Event || $event instanceof StoppableEventInterface;
+        $stoppable = $event instanceof StoppableEventInterface;
 
         foreach ($listeners as $listener) {
             if ($stoppable && $event->isPropagationStopped()) {

diff --git a/src/Symfony/Component/EventDispatcher/EventDispatcherInterface.php b/src/Symfony/Component/EventDispatcher/EventDispatcherInterface.php
@@ -22,11 +22,6 @@
  */
 interface EventDispatcherInterface extends ContractsEventDispatcherInterface
 {
-    /**
-     * {@inheritdoc}
-     */
-    public function dispatch($event, string $eventName = null): object;
-
     /**
      * Adds an event listener that listens on the specified events.
      *

diff --git a/src/Symfony/Component/EventDispatcher/ImmutableEventDispatcher.php b/src/Symfony/Component/EventDispatcher/ImmutableEventDispatcher.php
@@ -28,7 +28,7 @@ public function __construct(EventDispatcherInterface $dispatcher)
     /**
      * {@inheritdoc}
      */
-    public function dispatch($event, string $eventName = null): object
+    public function dispatch(object $event, string $eventName = null): object
     {
         return $this->dispatcher->dispatch($event, $eventName);
     }

diff --git a/src/Symfony/Component/EventDispatcher/composer.json b/src/Symfony/Component/EventDispatcher/composer.json
@@ -17,7 +17,7 @@
     ],
     "require": {
         "php": "^7.2.9",
-        "symfony/event-dispatcher-contracts": "^1.1|^2"
+        "symfony/event-dispatcher-contracts": "^2"
     },
     "require-dev": {
         "symfony/dependency-injection": "^4.4|^5.0",
@@ -33,7 +33,7 @@
     },
     "provide": {
         "psr/event-dispatcher-implementation": "1.0",
-        "symfony/event-dispatcher-implementation": "1.1"
+        "symfony/event-dispatcher-implementation": "2.0"
     },
     "suggest": {
         "symfony/dependency-injection": "",

diff --git a/src/Symfony/Component/Form/composer.json b/src/Symfony/Component/Form/composer.json
@@ -23,7 +23,7 @@
         "symfony/polyfill-ctype": "~1.8",
         "symfony/polyfill-mbstring": "~1.0",
         "symfony/property-access": "^5.0",
-        "symfony/service-contracts": "~1.1"
+        "symfony/service-contracts": "^1.1|^2"
     },
     "require-dev": {
         "doctrine/collections": "~1.0",

diff --git a/src/Symfony/Component/String/composer.json b/src/Symfony/Component/String/composer.json
@@ -20,7 +20,7 @@
         "symfony/polyfill-intl-grapheme": "~1.0",
         "symfony/polyfill-intl-normalizer": "~1.0",
         "symfony/polyfill-mbstring": "~1.0",
-        "symfony/translation-contracts": "^1.1|^2.0"
+        "symfony/translation-contracts": "^1.1|^2"
     },
     "autoload": {
         "psr-4": { "Symfony\\Component\\String\\": "" },

diff --git a/src/Symfony/Component/Translation/DataCollectorTranslator.php b/src/Symfony/Component/Translation/DataCollectorTranslator.php
@@ -47,9 +47,9 @@ public function __construct(TranslatorInterface $translator)
     /**
      * {@inheritdoc}
      */
-    public function trans($id, array $parameters = [], $domain = null, $locale = null)
+    public function trans(?string $id, array $parameters = [], string $domain = null, string $locale = null)
     {
-        $trans = $this->translator->trans($id, $parameters, $domain, $locale);
+        $trans = $this->translator->trans($id = (string) $id, $parameters, $domain, $locale);
         $this->collectMessage($locale, $domain, $id, $trans, $parameters);
 
         return $trans;
@@ -58,7 +58,7 @@ public function trans($id, array $parameters = [], $domain = null, $locale = nul
     /**
      * {@inheritdoc}
      */
-    public function setLocale($locale)
+    public function setLocale(string $locale)
     {
         $this->translator->setLocale($locale);
     }
@@ -119,13 +119,12 @@ public function getCollectedMessages()
         return $this->messages;
     }
 
-    private function collectMessage(?string $locale, ?string $domain, ?string $id, string $translation, ?array $parameters = [])
+    private function collectMessage(?string $locale, ?string $domain, string $id, string $translation, ?array $parameters = [])
     {
         if (null === $domain) {
             $domain = 'messages';
         }
 
-        $id = (string) $id;
         $catalogue = $this->translator->getCatalogue($locale);
         $locale = $catalogue->getLocale();
         $fallbackLocale = null;

diff --git a/src/Symfony/Component/Translation/LoggingTranslator.php b/src/Symfony/Component/Translation/LoggingTranslator.php
@@ -44,9 +44,9 @@ public function __construct(TranslatorInterface $translator, LoggerInterface $lo
     /**
      * {@inheritdoc}
      */
-    public function trans($id, array $parameters = [], $domain = null, $locale = null)
+    public function trans(?string $id, array $parameters = [], string $domain = null, string $locale = null)
     {
-        $trans = $this->translator->trans($id, $parameters, $domain, $locale);
+        $trans = $this->translator->trans($id = (string) $id, $parameters, $domain, $locale);
         $this->log($id, $domain, $locale);
 
         return $trans;
@@ -55,7 +55,7 @@ public function trans($id, array $parameters = [], $domain = null, $locale = nul
     /**
      * {@inheritdoc}
      */
-    public function setLocale($locale)
+    public function setLocale(string $locale)
     {
         $prev = $this->translator->getLocale();
         $this->translator->setLocale($locale);
@@ -107,13 +107,12 @@ public function __call(string $method, array $args)
     /**
      * Logs for missing translations.
      */
-    private function log(?string $id, ?string $domain, ?string $locale)
+    private function log(string $id, ?string $domain, ?string $locale)
     {
         if (null === $domain) {
             $domain = 'messages';
         }
 
-        $id = (string) $id;
         $catalogue = $this->translator->getCatalogue($locale);
         if ($catalogue->defines($id, $domain)) {
             return;

diff --git a/src/Symfony/Component/Translation/Translator.php b/src/Symfony/Component/Translation/Translator.php
@@ -144,7 +144,7 @@ public function addResource(string $format, $resource, string $locale, string $d
     /**
      * {@inheritdoc}
      */
-    public function setLocale($locale)
+    public function setLocale(string $locale)
     {
         $this->assertValidLocale($locale);
         $this->locale = $locale;
@@ -190,9 +190,9 @@ public function getFallbackLocales(): array
     /**
      * {@inheritdoc}
      */
-    public function trans($id, array $parameters = [], $domain = null, $locale = null)
+    public function trans(?string $id, array $parameters = [], string $domain = null, string $locale = null)
     {
-        if ('' === $id = (string) $id) {
+        if (null === $id || '' === $id) {
             return '';
         }
 

diff --git a/src/Symfony/Component/Translation/composer.json b/src/Symfony/Component/Translation/composer.json
@@ -18,7 +18,7 @@
     "require": {
         "php": "^7.2.9",
         "symfony/polyfill-mbstring": "~1.0",
-        "symfony/translation-contracts": "^1.1.6|^2"
+        "symfony/translation-contracts": "^2"
     },
     "require-dev": {
         "symfony/config": "^4.4|^5.0",
@@ -40,7 +40,7 @@
         "symfony/yaml": "<4.4"
     },
     "provide": {
-        "symfony/translation-implementation": "1.0"
+        "symfony/translation-implementation": "2.0"
     },
     "suggest": {
         "symfony/config": "",

diff --git a/src/Symfony/Contracts/Cache/composer.json b/src/Symfony/Contracts/Cache/composer.json
@@ -16,7 +16,7 @@
         }
     ],
     "require": {
-        "php": "^7.1.3",
+        "php": "^7.2.9",
         "psr/cache": "^1.0"
     },
     "suggest": {
@@ -28,7 +28,7 @@
     "minimum-stability": "dev",
     "extra": {
         "branch-alias": {
-            "dev-master": "1.1-dev"
+            "dev-master": "2.0-dev"
         }
     }
 }
diff --git a/src/Symfony/Contracts/EventDispatcher/Event.php b/src/Symfony/Contracts/EventDispatcher/Event.php
@@ -13,84 +13,42 @@
 
 use Psr\EventDispatcher\StoppableEventInterface;
 
-if (interface_exists(StoppableEventInterface::class)) {
+/**
+ * Event is the base class for classes containing event data.
+ *
+ * This class contains no event data. It is used by events that do not pass
+ * state information to an event handler when an event is raised.
+ *
+ * You can call the method stopPropagation() to abort the execution of
+ * further listeners in your event listener.
+ *
+ * @author Guilherme Blanco <guilhermeblanco@hotmail.com>
+ * @author Jonathan Wage <jonwage@gmail.com>
+ * @author Roman Borschel <roman@code-factory.org>
+ * @author Bernhard Schussek <bschussek@gmail.com>
+ * @author Nicolas Grekas <p@tchwork.com>
+ */
+class Event implements StoppableEventInterface
+{
+    private $propagationStopped = false;
+
     /**
-     * Event is the base class for classes containing event data.
-     *
-     * This class contains no event data. It is used by events that do not pass
-     * state information to an event handler when an event is raised.
-     *
-     * You can call the method stopPropagation() to abort the execution of
-     * further listeners in your event listener.
-     *
-     * @author Guilherme Blanco <guilhermeblanco@hotmail.com>
-     * @author Jonathan Wage <jonwage@gmail.com>
-     * @author Roman Borschel <roman@code-factory.org>
-     * @author Bernhard Schussek <bschussek@gmail.com>
-     * @author Nicolas Grekas <p@tchwork.com>
+     * {@inheritdoc}
      */
-    class Event implements StoppableEventInterface
+    public function isPropagationStopped(): bool
     {
-        private $propagationStopped = false;
-
-        /**
-         * Returns whether further event listeners should be triggered.
-         */
-        public function isPropagationStopped(): bool
-        {
-            return $this->propagationStopped;
-        }
-
-        /**
-         * Stops the propagation of the event to further event listeners.
-         *
-         * If multiple event listeners are connected to the same event, no
-         * further event listener will be triggered once any trigger calls
-         * stopPropagation().
-         */
-        public function stopPropagation(): void
-        {
-            $this->propagationStopped = true;
-        }
+        return $this->propagationStopped;
     }
-} else {
+
     /**
-     * Event is the base class for classes containing event data.
-     *
-     * This class contains no event data. It is used by events that do not pass
-     * state information to an event handler when an event is raised.
+     * Stops the propagation of the event to further event listeners.
      *
-     * You can call the method stopPropagation() to abort the execution of
-     * further listeners in your event listener.
-     *
-     * @author Guilherme Blanco <guilhermeblanco@hotmail.com>
-     * @author Jonathan Wage <jonwage@gmail.com>
-     * @author Roman Borschel <roman@code-factory.org>
-     * @author Bernhard Schussek <bschussek@gmail.com>
-     * @author Nicolas Grekas <p@tchwork.com>
+     * If multiple event listeners are connected to the same event, no
+     * further event listener will be triggered once any trigger calls
+     * stopPropagation().
      */
-    class Event
+    public function stopPropagation(): void
     {
-        private $propagationStopped = false;
-
-        /**
-         * Returns whether further event listeners should be triggered.
-         */
-        public function isPropagationStopped(): bool
-        {
-            return $this->propagationStopped;
-        }
-
-        /**
-         * Stops the propagation of the event to further event listeners.
-         *
-         * If multiple event listeners are connected to the same event, no
-         * further event listener will be triggered once any trigger calls
-         * stopPropagation().
-         */
-        public function stopPropagation(): void
-        {
-            $this->propagationStopped = true;
-        }
+        $this->propagationStopped = true;
     }
 }