Add @trigger_error() to deprecated url/link EntityInterface methods

+++ b/core/modules/hal/src/Normalizer/ContentEntityNormalizer.php
@@ -186,10 +186,9 @@ public function denormalize($data, $class, $format = NULL, array $context = [])
-      return $entity->url('canonical', []);
+      return '';

Looking at the code in \Drupal\Core\Entity\Entity::toUrl() shouldn't we be calling that here to give things that implement uri_callback a chance?

Ah I see the following code is in the url method already...

    // While self::toUrl() will throw an exception if the entity has no id,
    // the expected result for a URL is always a string.
    if ($this->id() === NULL || !$this->hasLinkTemplate($rel)) {
      return '';
    }

So yeah the whole not having a link template situation is meh and the change in #8 seems good to me.

👏

+++ b/core/modules/comment/src/CommentTypeForm.php
@@ -158,7 +158,7 @@ public function save(array $form, FormStateInterface $form_state) {
-    $edit_link = $this->entity->toLink($this->t('Edit'))->toString();
+    $edit_link = $this->entity->toLink($this->t('Edit'), 'edit-form')->toString();

I think this change will break contrib. We need to think a bit harder about config entities ::toLink() method.

  public function testUrlInfo($rel, $options) {
    $entity = $this->getEntity(Entity::class, [], ['toUrl']);
    $entity->expects($this->once())
      ->method('toUrl')
      ->with($rel, $options);

    $entity->toUrl($rel, $options);
  }

test needs renaming. And so does providerTestUrlInfo - or maybe this should be the legacy urlInfo test as with the current change it does not appear to be testing anything and we need a legacy test to for urlInfo().

We also don't appear to have any test coverage of the ::link() deprecation.

The existing testUrl and testUrlEmpty are good coverage for the url method.

Ah phew... I thought somehow the behaviour of toLink() was changing. So things seems okay - yes inconsistent but that is already the case and not caused by this patch. It does mean we have to review all the link() -> toLink() conversions very carefully - especially any that are generic.

Thanks @Berdir. Looks like all feedback has been addressed.

I think it worth mentioning that the defaults for configuration entities have changed on the deprecation message for ::link(). This seems something that couyld catch people out and documenting this is helpful.

Also the ::url() deprecation notice could be helpful and mention the return type change from string to a Url object and to use toString() if that is needed. The other thing is not handling new entities.

I think these notices need to help people avoid errors in their code.

I suggest:

@trigger_error("EntityInterface::link() is deprecated in Drupal 8.0.0 and will be removed in Drupal 9.0.0. EntityInterface::toLink() instead. Note, the default relationship for configuration entities changes from 'edit-form' to 'canonical'. See https://www.drupal.org/node/2614344", E_USER_DEPRECATED);

@trigger_error('EntityInterface::url() is deprecated in Drupal 8.0.0 and will be removed in Drupal 9.0.0. EntityInterface::toUrl() instead. Note, a \Drupal\Core\Url object is returned. See https://www.drupal.org/node/2614344', E_USER_DEPRECATED);

Yes the lines are long but the detail is really helpful.

Thanks @Berdir - let's get this done. This has been rtbc in #18 and was only sent back for improvements to the deprecation messages.

Committed 60505f3 and pushed to 8.7.x. Thanks!