\Drupal\Core\Url::getInternalPath() cannot be deprecated

I think this depends on #2339219: [meta] Finalize URL generation API (naming, docs, deprecation) being sorted

Url->getInternalPath() seems to be used 21 times in core.

Should it really be deprecated?

Changing parent issue.

Started working on this issue.

Should it be something like this or is the fix too simple?

I removed unrelated changes + fixed a lot of failures, but I'm not sure this is the right way..

Patch attached.

I know RC1 is out, but there is still a small change we can remove this function. See #2205673-147: [META] Remove all @deprecated functions marked "remove before 8.0"

1. +++ b/core/lib/Drupal/Core/Path/PathMatcher.php
   @@ -101,7 +101,7 @@ public function isFrontPage() {
   +        $this->isCurrentFrontPage = ($url->getRouteName() && $url->toString() === $this->getFrontPagePath());
   

   $url->toString() should be trimmed.

2. +++ b/core/lib/Drupal/Core/Utility/LinkGenerator.php
   +++ b/core/lib/Drupal/Core/Utility/LinkGenerator.php
   @@ -132,8 +132,7 @@ public function generate($text, Url $url) {
   
   +++ b/core/modules/config_translation/src/ConfigNamesMapper.php
   @@ -250,7 +250,7 @@ public function getOverviewRoute() {
      public function getOverviewPath() {
   

   I am not sure if this function also require tests?

   Need tests.

3. +++ b/core/modules/config_translation/src/ConfigNamesMapper.php
   @@ -213,7 +213,7 @@ protected function processRoute(Route $route) {
      public function getBasePath() {
   

   Needs tests.

4. +++ b/core/modules/path/path.module
   @@ -83,7 +83,7 @@ function path_entity_base_field_info(EntityTypeInterface $entity_type) {
   +  $path = $translation->urlInfo()->setOption('alias', TRUE)->toString();
   +  $conditions = array('source' => $path, 'langcode' => $translation->language()->getId());
   

   We are ending system_path with '/' and someplaces we are not using. why?

5. +++ b/core/modules/path/src/Plugin/Field/FieldType/PathItem.php
   @@ -59,7 +59,7 @@ public function postSave($update) {
   @@ -72,7 +72,7 @@ public function postSave($update) {
   

   Need tests.

Also, we would require to think about getInternalPath was not terurning the paths ending with slash whereas. OLD Code

if (!isset($this->internalPath)) {
-      $this->internalPath = $this->urlGenerator()->getPathFromRoute($this->getRouteName(), $this->getRouteParameters());
-    }
-    return $this->internalPath; 

I have done some minor changes in existing patch and submitting here to review.

Main thing, this issue should be considered as a Major .

In the patch above, the function toString() does not give any results. That has to be replaced with specific values that include RouteName and the RouteParameters. Following are the Test files where changes have to be made in addition to the changes made in this attached patch, and then we can completely remove the getInternalPath() function usage from /core/lib/Drupal/Core/Url.php.

Test Paths :
/core/modules/aggregator/src/Tests/AggregatorRenderingTest.php
/core/modules/system/src/Tests/Menu/BreadcrumbTest.php
/core/tests/Drupal/Tests/Core/UnroutedUrlTest.php
/core/tests/Drupal/Tests/Core/UrlTest.php

Putting this issue in needs review state

Since this is just a wrapper on UrlGenerator::getPathFromRoute() I don't think there is much gain if we are just swapping the one for the other.

I think we just need to change or remove the deprecation notice now.

Until every system that relies on an internal path can instead use a Url object, we need this.
One example: \Drupal\Core\Path\AliasStorageInterface::save().

I don't think using a Url object or not in method calls matters here, I think the real issue is that path processing and aliases are coupled to the "internal" path so we cannot remove that capability from the generator if we want to be able to e.g. create a path alias for a given route.

If we are not removing it from the generator, there is no point in removing the wrapper method from Url.

This only affects phpdoc, not code

Could use an issue summary update, and a check for affected change records

Hm. Remove @deprecated entirely? Or target for 9 instead?

Tagging rc eligible since this is just about docs now.

Please update the issue summary.

Committed b219839 and pushed to 8.0.x. Thanks!