Skip to content

Commit

Permalink
minor #28321 [Routing] Fixed the interface description of the url gen…
Browse files Browse the repository at this point in the history
…erator interface (Toflar)

This PR was merged into the 2.8 branch.

Discussion
----------

[Routing] Fixed the interface description of the url generator interface

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

The `UrlGenerator` has always been able to return `null`. Many tests assert this for many years but the interface actually always only allowed a `string` return.

Examples for tests:

- https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php#L206
- https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php#L217
- https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php#L471

So I think I would not consider this change as a BC break but rather a doc fix because it seems like `null` has always been an accepted return value.

Commits
-------

d2e9e0b Fixed the interface description of the url generator interface
  • Loading branch information
fabpot committed Sep 4, 2018
2 parents cf359c2 + d2e9e0b commit 487f8ac
Showing 1 changed file with 1 addition and 1 deletion.
Expand Up @@ -73,7 +73,7 @@ interface UrlGeneratorInterface extends RequestContextAwareInterface
* @param mixed $parameters An array of parameters
* @param int $referenceType The type of reference to be generated (one of the constants)
*
* @return string The generated URL
* @return string|null The generated URL
*
* @throws RouteNotFoundException If the named route doesn't exist
* @throws MissingMandatoryParametersException When some parameters are missing that are mandatory for the route
Expand Down

0 comments on commit 487f8ac

Please sign in to comment.