Prevent inaccessible links from being displayed in LinkFormatter

First draft proposal on the approach.
Tests still todo.

Tests should pass with this, let's see.
New tests validating the added access_check still todo.

Is this approach decent? It is somewhat similar to what EntityReferenceFormatterBase does.
I thought about adding this in the field storage/instance setting but \Drupal\link\Plugin\Field\FieldType\LinkItem doesn't have such access check.
Only \Drupal\Core\Field\FieldItemList has an access check but it applies to the whole item list which wouldn't be viable AFAICS.
Another way I think would be to use #access_callback, similar to what \Drupal\Core\Url::toRenderArray does, but then the link field (wrapper) would be rendered, even if all items are inaccessible. The only emptiness check happens in \Drupal\Core\Field\FormatterBase::view.

Hello @larowlan, what does that mean?
Do you consider this as a security issue? (I don't think it is but... idk)
Thanks

Note that I wrote this patch initially with the idea to make it non-disruptive.
If the goal is to apply access checks by default then we may need to reverse the logic.

Though I wonder, is there a reason why \Drupal\Core\Url::toRenderArray has the access check, but \Drupal\Core\Url::toString doesn't?
Couldn't we include this access check in toString by default? Perhaps via an "access" key set to TRUE by default in \Drupal\Core\Url::$options which we can disable if needed?).

Reroll for 10.1.x and adapted the unit tests.

Functional tests are still needed but it would be great to get some feedback on the whole issue/approach before moving forward with that.
As-is, the access check happens in a similar way to EntityReferenceFormatterBase but I wonder if this access check should even happen at this level, and in this way (or perhaps using a #pre_render or #access_callback)...
This could extend way beyond link formatters.

Fixing PHPStan issues.

MR created (PS: Nightwatch tests seem unstable, I had to re-run).

Please check #4 and #13, I feel like this issue extends beyond link formatters so leaving to needs work...
Here is a static patch from MR for composer.

Rebased and moving to review, to get feedback on the approach, thanks.

Thanks, though I wasn't looking for a code review yet (tests and upgrade path still todo) but rather opinions on the overall approach / scope as it may extend way beyond link formatters.
See #13.

I agree with #28, displaying inaccessible URLs is a totally valid use case.
So indeed this is not a bug, and a formatter settings does make sense.
When I created this issue, I was confused why \Drupal\Core\Url::toRenderArray had the access check, but \Drupal\Core\Url::toString didn't. But I see now that #3343153: Remove deprecated code from \Drupal\Core\Url removed ::toRenderArray.

I also agree with all review comments so if anyone wants to address those, please feel free. Or I will find some time later this week.
Cheers.

Answering to #24:
1. If I'm not mistaken, this looks like a crude attempt to check entity access.
AccessManager::checkNamedRoute I used here is a much cleaner alternative.
2 & 3. The access cacheability is collected with the current MR here in the formatter, but we indeed still need a test here to validate the access is respected and the cacheability as well if possible.
I'll work on that next.

Applied suggestions, attaching MR snapshot and also patch that applies to 11.3.

Rebased on main, and addressed the review threads in the MR. Ready for another round of review.

All great suggestions, thank you @marcoscano. Applied, and I refactored the kernel test.

I did find a real blocker though: with access_check on, a link pointing at a deleted entity makes the page uncacheable, I added a test proving it. If you are already running this patch, this affects you.
FWIW, here is Claude code's assessment:

AccessManager::checkNamedRoute() catches ParamNotConvertedException and returns AccessResult::forbidden()->setCacheMaxAge(0). That lands on the field build and bubbles up: with the same items and only the setting differing, max-age goes from -1 to 0.

Core's other consumers avoid this rather than handle it: DefaultMenuLinkTreeManipulators::menuLinkCheckAccess() is structurally identical, but menu_link_content deletes links to an entity in hook_entity_predelete(). Link fields have no such cleanup, and no route index to build one from, so this is the first consumer that actually hits it.

Working around it in the formatter would be wrong: the exception carries no reason, so a permanently missing entity is indistinguishable from the dynamic case core is guarding against. #3190542: Layout Builder overrides section storage sets local tasks block cache max-age to 0 on content entity pages without overrides enabled hit the same chain and fixed it by stopping the converter from failing. Here the converter is right, the entity really is gone, so the fix has to be that a failed conversion can express a stable "not found".

A prototype of that: an optional interface letting a converter state the cacheability of a NULL return, attached to the exception by ParamConverterManager and read by AccessManager, with EntityConverter returning the entity type's list cache tags. The dangling link then comes back cacheable, tagged <entity_type>_list, and still hidden. The existing Core ParamConverter, Access and Routing tests stay green.

So I don't think this can land as it stands: one dangling link anywhere on a page silently disables page caching, which is a worse trade than the broken links the setting removes.
Shall I create a new issue and postpone this one on it?
In light of this I will remove the patch from the site I maintain, where it was guarding exactly one inaccessible link after inspecting values in DB.