Skip to content

fix: conditional (BC) removal in RemoveAliasManagerCacheMethodCallsRector#405

Merged
bbrala merged 2 commits into
palantirnet:mainfrom
bbrala:fix/3600789-alias-manager-cache-bc
Jul 7, 2026
Merged

fix: conditional (BC) removal in RemoveAliasManagerCacheMethodCallsRector#405
bbrala merged 2 commits into
palantirnet:mainfrom
bbrala:fix/3600789-alias-manager-cache-bc

Conversation

@bbrala

@bbrala bbrala commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

Problem

RemoveAliasManagerCacheMethodCallsRector removed calls to AliasManager::setCacheKey() / writeCache() unconditionally. As reported by Berdir on #3600789, that breaks backward compatibility on Drupal < 11.3.

These methods only became no-ops at 11.3.0, when the path alias preload cache was replaced by a Fiber-based bulk-lookup strategy (CR 3496369, CR 3532412). Before 11.3.0 they performed real caching work, so deleting the call outright silently drops the alias preload caching for modules that still support < 11.3.

Fix

The rector now extends AbstractDrupalCoreRector (version-gated on 11.3.0):

  • BC support enabled → the call is wrapped in a DeprecationHelper::backwardsCompatibleCall() with a no-op current callable, so the caching still runs on Drupal < 11.3 and is skipped on 11.3+:

    \Drupal\Component\Utility\DeprecationHelper::backwardsCompatibleCall(
        \Drupal::VERSION, '11.3.0',
        fn() => null,
        fn() => $aliasManager->setCacheKey('/some/path'),
    );
  • BC support disabled (or minimum supported core ≥ 11.3) → the call is removed as before.

Verified against core source: backwardsCompatibleCall() takes two required callable params and invokes one based on the version compare; a fn() => null current callable is valid and its return is discarded in statement context. No deprecation notice fires because the deprecated method is only invoked on the < 11.3 branch. The shared createBcCallOnExpr() helper can't produce this shape (it needs a replacement Expr; this is a whole-statement deletion), so the wrapper node is built in the rector, keeping the Expression-level match.

Real-world validation (contrib search)

Every contrib module that actually calls these methods on an AliasManager has already added a manual BC guard — confirming unconditional removal is wrong and the conditional approach is correct:

  • redirectif ($this->aliasManager instanceof AliasManager && version_compare(\Drupal::VERSION, '11.3', '<')) (Berdir's MR !196)
  • domain_path / domain_unique_path_alias — alias-manager decorators guarding the passthrough with if (method_exists($this->inner, 'setCacheKey'))
  • seo_urls — calls its own SeoUrlManager (not AliasManager), so the type guard correctly skips it

Contrib writeCache() hits are almost all ModuleHandler::writeCache() and other unrelated methods, not AliasManager — correctly ignored by the type guard.

Tests

  • fixture-bc/ — proves the BC wrapper output (basic + concrete receiver).
  • fixture/ — proves plain removal when BC is disabled.
  • fixture-below-version/ — proves the rector does not fire when target Drupal < 11.3.

Full suite green (699 tests), PHPStan clean, php-cs-fixer clean.

🤖 Generated with Claude Code

bbrala added 2 commits July 5, 2026 07:22
…unconditionally [#3600789]

RemoveAliasManagerCacheMethodCallsRector removed calls to
AliasManager::setCacheKey()/writeCache() unconditionally, which breaks
backward compatibility on Drupal < 11.3. These methods only became no-ops
in 11.3.0 (when the path alias preload cache was replaced by a Fiber-based
bulk-lookup strategy); before 11.3.0 they performed real caching work.

The rector now extends AbstractDrupalCoreRector. When BC support is enabled
the call is wrapped in a DeprecationHelper::backwardsCompatibleCall() with a
no-op current callable, so the caching still runs on Drupal < 11.3 and is
skipped on 11.3+. When BC support is disabled the call is removed as before.

Reported by Berdir.
@bbrala bbrala merged commit 72a674f into palantirnet:main Jul 7, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant