Create method RouteProvider::routeExists()

Patch works great. Nice API addition.

Needs a re-roll and a change record.

i can't push to the issue fork but here's another reroll as a patch

Because this re-roll affected only a comment (preserving "Tests…" instead of the older "Test"), setting back to RTBC.

Thanks for the reroll, looked at the patch again and a couple more questions:

1. Do we need an interface with this method for RouteProvider to implement?

2. Should we add a return type hint here since it's new code?

Hi Catch, #18
I have reviewed the patch @#16 and it works great but it still needs some improvements.

For your queries
1. Do we need an interface with this method for RouteProvider to implement?
Yes, This method should be defined inside RouteProviderInterface. Its good to have it inside RouteProviderInterface as we can use it to extended the functionality across system.

2. Should we add a return type hint here since it's new code?
Type hint can help as it's a way of self documentation of method and also tightly restrict developers to write good quality code.

Here is the patch according to #18

public function routeExists($name)
This needs to be typehinted both function and parameter

public function routeExists($name)
Parameter needs typehinting

All instances of that outside the tests.

This adds type declarations and uses casting instead of count() to check whether the array is empty.

If the merge request is to to used, I think the person who created the merge request needs to update its target branch to point to 11.x.

Fix test.

CR looks good, updated branch numbers.

Think this is a good addition.

Hitting retest to see if we can display a green run.

The random test failure has already been logged: #3376563: Random test fail in Drupal\Tests\Component\Utility\RandomTest::testRandomMachineNamesUniqueness

Slight improvement, imo. Not a deal breaker for me.

public function routeExists(string $name): bool {
  return !empty($this->getRoutesByNames([$name]));
}

Unless we anticipate a lot of calls to routeExists with an empty string then let's just assume the supplied string is a potential route and then explicitly declare true if not empty rather than casting.

I'm triaging RTBC issues.

I read the Issue summary, comments and the change record. I found that all questions have been answered. The CR is complete and has a before and after section which makes it easy for the reader. It is a bit brief for my tastes but it does not require a change.

I looked at the patch and ran the tests.

1. +++ b/core/tests/Drupal/KernelTests/Core/Routing/RouteProviderTest.php
   @@ -655,7 +655,7 @@ public function testRouteCaching() {
   +   * Tests RouteProvider::getRouteByName(), RouteProvider::getRoutesByNames(), and RouteProvider::routeExists().
   

   The summary line is to be 80 characters max. This could be changed by using @covers for the methods being tested.

2. +++ b/core/tests/Drupal/KernelTests/Core/Routing/RouteProviderTest.php
   @@ -667,6 +667,11 @@ public function testRouteByName() {
   +    $this->assertFalse($provider->routeExists(''));
   

   And since a change is being made, this could use a simple comment to help the next person working on this.

I decided to make a patch for the above and at the same time I have added the suggestion in #42.

All green so back to RTBC

I have created a merge request with the patch in #43 targeting 11.x. All tests are passing.

CR is outdated.

Also why does RouteProvider not implement RouteProvider Interface??

Issue summary also appears incomplete as this seems like an API change that probably can only go into a major as it would be an immediate break in a minor

RouteProvider implements CacheableRouteProviderInterface which implements RouteProviderInterface.

I suppose to prevent a breaking change to the API, the method could be added to the class but not the interface. The interface would then be updated with the next major release.