Skip to content

Fix module sort comparator and drop Drupal 8/9 dead code in autoloader - #1036

Merged
mglaman merged 2 commits into
mainfrom
audit/1d-autoloader-cleanup
Sep 8, 2026
Merged

Fix module sort comparator and drop Drupal 8/9 dead code in autoloader#1036
mglaman merged 2 commits into
mainfrom
audit/1d-autoloader-cleanup

Conversation

@mglaman

@mglaman mglaman commented Aug 5, 2026

Copy link
Copy Markdown
Owner

Part 5 of 9 in the legacy-code audit stack (on top of #1042).

What changed

  • Module sort comparator fixed. The usort callback in DrupalAutoloader returned 10 or 0 and ignored its second operand, so it was not a valid comparator and the order of test modules was implementation-defined. On PHP 8 it did not sort test modules last at all. Replaced with a valid comparator that reliably loads _test extensions after regular ones.
  • Why the sort exists. Test modules stub functions from their parent module behind function_exists() guards. If the test module's .module file loads first, the parent's unconditional declaration is a compile error that loadAndCatchErrors() cannot intercept. This was the original blazy_test fix from 2019. The comment now says so.
  • Removed the Drupal 8/9-only PhpUnit8\ClassWriter shim (the class was removed in Drupal 10) and the Drush 8 directory-depth branch (composer requires drush ^11 || ^12 || ^13).

Known consequence

Service definitions are last-writer-wins by ID. With the ordering now deterministic, a test module that redefines a production service ID always wins in the service map, where before the winner depended on filesystem order. Core's own collisions are against core.services.yml, which is processed first regardless, and the pattern is rare in contrib. Tracked as a follow-up together with a path-based test-module check, since _test in the name misses test extensions under tests/ directories and catches real modules like ab_tests.

Testing

Full suite, self-analysis, and phpcs are green.

🤖 Generated with Claude Code

@mglaman
mglaman force-pushed the audit/1d-autoloader-cleanup branch from 8463308 to af47a4d Compare August 5, 2026 17:15
@mglaman
mglaman force-pushed the audit/1d-autoloader-cleanup branch from af47a4d to 822edb0 Compare August 5, 2026 19:13
@mglaman
mglaman force-pushed the audit/1d-autoloader-cleanup branch from 822edb0 to 9bac357 Compare August 5, 2026 19:33
@mglaman
mglaman force-pushed the audit/1d-autoloader-cleanup branch from 9bac357 to 65d4b55 Compare September 8, 2026 17:42
@mglaman
mglaman changed the base branch from audit/1c-latent-fixes to audit/1c2-legacy-cleanups September 8, 2026 17:44
@mglaman
mglaman force-pushed the audit/1d-autoloader-cleanup branch from 65d4b55 to 3c6afad Compare September 8, 2026 18:15
Base automatically changed from audit/1c2-legacy-cleanups to main September 8, 2026 18:27
@mglaman
mglaman force-pushed the audit/1d-autoloader-cleanup branch from 3c6afad to f391e31 Compare September 8, 2026 18:27
The usort callback never returned a negative value, so test-module
ordering was implementation-defined. Replace it with a valid comparator
that sorts _test extensions last. Remove the PhpUnit8 ClassWriter shim
(removed in Drupal 10) and the Drush 8 directory-depth branch (composer
requires drush ^11 || ^12 || ^13).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mglaman
mglaman force-pushed the audit/1d-autoloader-cleanup branch from f391e31 to 1f4686e Compare September 8, 2026 18:27
…he comparator

The sort exists so test modules' .module files load after their parent
module's. Test modules stub parent functions behind function_exists()
guards, and loading them first turns the parent's declaration into a
compile error the bootstrap cannot catch. Namespaces are keyed by module
name, so order never affected them.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@mglaman
mglaman marked this pull request as ready for review September 8, 2026 18:58
@mglaman
mglaman merged commit 3bb9715 into main Sep 8, 2026
19 of 20 checks passed
@mglaman
mglaman deleted the audit/1d-autoloader-cleanup branch September 9, 2026 14:55
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