Skip to content

Commit

Permalink
feature symfony#33497 [Contracts] Add parameter type declarations to …
Browse files Browse the repository at this point in the history
…contracts (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 | symfony#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
-------

bf515e1 Add parameter type declarations to contracts.
  • Loading branch information
nicolas-grekas committed Nov 8, 2019
2 parents 69cd750 + bf515e1 commit 65c2a11
Show file tree
Hide file tree
Showing 27 changed files with 98 additions and 174 deletions.
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

class StubTranslator 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
{
return '[trans]'.strtr($id, $parameters).'[/trans]';
}
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bridge/Twig/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
],
"require": {
"php": "^7.2.9",
"symfony/translation-contracts": "^1.1|^2",
"symfony/translation-contracts": "^2",
"twig/twig": "^2.10|^3.0"
},
"require-dev": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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) {
Expand All @@ -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));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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;
}
}
Expand Down
9 changes: 2 additions & 7 deletions src/Symfony/Component/EventDispatcher/EventDispatcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ interface EventDispatcherInterface extends ContractsEventDispatcherInterface
/**
* {@inheritdoc}
*/
public function dispatch($event, string $eventName = null): object;
public function dispatch(object $event, string $eventName = null): object;

/**
* Adds an event listener that listens on the specified events.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/EventDispatcher/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ public function testPostMaxSizeTranslation()

class DummyTranslator implements TranslatorInterface, LocaleAwareInterface
{
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
{
return 'translated max {{ max }}!';
}

public function setLocale($locale)
public function setLocale(string $locale)
{
}

Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/Form/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"symfony/http-kernel": "^4.4|^5.0",
"symfony/security-csrf": "^4.4|^5.0",
"symfony/translation": "^4.4|^5.0",
"symfony/translation-contracts": "^2",
"symfony/var-dumper": "^4.4|^5.0"
},
"conflict": {
Expand Down
9 changes: 4 additions & 5 deletions src/Symfony/Component/Translation/DataCollectorTranslator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down Expand Up @@ -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;
Expand Down
9 changes: 4 additions & 5 deletions src/Symfony/Component/Translation/LoggingTranslator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions src/Symfony/Component/Translation/Translator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 '';
}

Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/Translation/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -40,7 +40,7 @@
"symfony/yaml": "<4.4"
},
"provide": {
"symfony/translation-implementation": "1.0"
"symfony/translation-implementation": "2.0"
},
"suggest": {
"symfony/config": "",
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Contracts/Cache/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
}
],
"require": {
"php": "^7.1.3",
"php": "^7.2.9",
"psr/cache": "^1.0"
},
"suggest": {
Expand All @@ -28,7 +28,7 @@
"minimum-stability": "dev",
"extra": {
"branch-alias": {
"dev-master": "1.1-dev"
"dev-master": "2.0-dev"
}
}
}

0 comments on commit 65c2a11

Please sign in to comment.