test: #3486376 Extend Symfony DebugClassLoader to report missing cross-module @return types (90c0add2) · Commits · project / drupal

core/.deprecation-ignore.txt

+6 −0

Original line number	Diff line number	Diff line
		@@ -2,6 +2,12 @@
		# deprecated code.
		# See https://www.drupal.org/node/3285162 for more details.

		# Cross-module return type deprecations within Drupal core. Downstream
		# implementations should add return types first, then we can remove this.
		%Method "Drupal\\[^"]+::[^"]+" might add "[^"]+" as a native return type declaration in the future. Do the same in (?:implementation\|child class) "Drupal\\(Core\|basic_auth\|big_pipe\|block\|block_content\|breakpoint\|ckeditor5\|comment\|config\|config_translation\|contact\|content_moderation\|content_translation\|contextual\|datetime\|datetime_range\|dblog\|dynamic_page_cache\|editor\|field\|field_ui\|file\|filter\|help\|image\|jsonapi\|language\|layout_builder\|link\|locale\|media\|media_library\|menu_link_content\|menu_ui\|migrate\|migrate_drupal\|mysql\|mysqli\|navigation\|node\|options\|package_manager\|path\|path_alias\|pgsql\|responsive_image\|rest\|search\|serialization\|shortcut\|sqlite\|system\|taxonomy\|telephone\|text\|toolbar\|update\|user\|views\|views_ui\|workflows\|workspaces\|workspaces_ui)\\[^"]+" now to avoid errors or add an explicit @return annotation to suppress this message%
		# Test modules that we skip for now, but can be fixed any time.
		%Method "Drupal\\[^"]+::[^"]+" might add "[^"]+" as a native return type declaration in the future. Do the same in (?:implementation\|child class) "Drupal\\(Tests\\(Core\|block\|block_content\|comment\|editor\|field\|file\|filter\|image\|language\|layout_builder\|media\|menu_link_content\|node\|path_alias\|responsive_image\|search\|shortcut\|system\|taxonomy\|user\|views\|workflows\|workspaces)\|FunctionalTests\\Rest\|test_.\|._test\|._test_.\|ckeditor5_plugin_elements_subset\|dummydb\|search_embedded_form\|search_extra_type)\\[^"]+" now to avoid errors or add an explicit @return annotation to suppress this message%

		# @todo Remove when we no longer support PHPUnit 11.
		%The "PHPUnit\\Framework\\TestCase::__construct" method is considered internal.* You should not extend it from "Drupal\\[^"]+"%
		%The "PHPUnit\\Framework\\TestCase::__construct" method is considered final.* You should not extend it from "Drupal\\[^"]+"%

core/tests/Drupal/KernelTests/Core/Entity/Sql/TestableSqlContentEntityStorage.php

+1 −1

Original line number	Diff line number	Diff line
		@@ -58,7 +58,7 @@ public function setTableMapping(TableMappingInterface $table_mapping): void {
		*
		* {@inheritdoc}
		*/
		public function getTableMapping(?array $storage_definitions = NULL) {
		public function getTableMapping(?array $storage_definitions = NULL): TableMappingInterface {
		return $this->tableMapping;
		}

core/tests/Drupal/KernelTests/RouteProvider.php

+7 −5

Original line number	Diff line number	Diff line
		@@ -8,6 +8,8 @@
		use Drupal\Core\Routing\PreloadableRouteProviderInterface;
		use Drupal\Core\Routing\RouteProvider as RouteProviderBase;
		use Symfony\Component\HttpFoundation\Request;
		use Symfony\Component\Routing\Route;
		use Symfony\Component\Routing\RouteCollection;

		/**
		* Rebuilds the router when the provider is instantiated.
		@@ -40,14 +42,14 @@ protected function lazyLoadItself(): RouteProviderBase {
		/**
		* {@inheritdoc}
		*/
		public function getRouteCollectionForRequest(Request $request) {
		public function getRouteCollectionForRequest(Request $request): RouteCollection {
		return $this->lazyLoadItself()->getRouteCollectionForRequest($request);
		}

		/**
		* {@inheritdoc}
		*/
		public function getRouteByName($name) {
		public function getRouteByName($name): Route {
		return $this->lazyLoadItself()->getRouteByName($name);
		}

		@@ -61,7 +63,7 @@ public function preLoadRoutes($names) {
		/**
		* {@inheritdoc}
		*/
		public function getRoutesByNames($names) {
		public function getRoutesByNames($names): Route\|array {
		return $this->lazyLoadItself()->getRoutesByNames($names);
		}

		@@ -75,7 +77,7 @@ public function getCandidateOutlines(array $parts) {
		/**
		* {@inheritdoc}
		*/
		public function getRoutesByPattern($pattern) {
		public function getRoutesByPattern($pattern): RouteCollection {
		return $this->lazyLoadItself()->getRoutesByPattern($pattern);
		}

		@@ -89,7 +91,7 @@ public function routeProviderRouteCompare(array $a, array $b) {
		/**
		* {@inheritdoc}
		*/
		public function getAllRoutes() {
		public function getAllRoutes(): array {
		return $this->lazyLoadItself()->getAllRoutes();
		}

core/tests/Drupal/TestTools/ErrorHandler/DrupalDebugClassLoader.php

0 → 100644

+116 −0

Original line number	Diff line number	Diff line
		<?php

		declare(strict_types=1);

		namespace Drupal\TestTools\ErrorHandler;

		use Symfony\Component\ErrorHandler\DebugClassLoader;

		/**
		* Extends Symfony's DebugClassLoader for Drupal-aware vendor boundaries.
		*
		* Symfony's DebugClassLoader uses the first namespace segment as the vendor
		* boundary for return type deprecation checks. Since all Drupal code shares
		* "Drupal\" as the first segment, cross-extension return type deprecations
		* (e.g., contrib extending core) are never triggered.
		*
		* This subclass treats each Drupal extension (the second namespace segment)
		* as the vendor boundary, enabling return type deprecation notices when an
		* extension overrides a method from a different extension without adding
		* native return types.
		*
		* @internal
		*/
		class DrupalDebugClassLoader extends DebugClassLoader {

		/**
		* Cached ReflectionProperty for DebugClassLoader::$returnTypes.
		*/
		private static \ReflectionProperty $returnTypesProperty;

		/**
		* Cached ReflectionProperty for DebugClassLoader::$patchTypes.
		*/
		private static \ReflectionProperty $patchTypesProperty;

		/**
		* {@inheritdoc}
		*/
		public function checkAnnotations(\ReflectionClass $refl, string $class): array {
		$deprecations = parent::checkAnnotations($refl, $class);

		// Only process non-trait Drupal classes.
		if (!str_starts_with($class, 'Drupal\\') \|\| trait_exists($class, FALSE)) {
		return $deprecations;
		}

		self::$returnTypesProperty ??= new \ReflectionProperty(DebugClassLoader::class, 'returnTypes');
		$returnTypes = self::$returnTypesProperty->getValue()[$class] ?? [];

		self::$patchTypesProperty ??= new \ReflectionProperty(DebugClassLoader::class, 'patchTypes');
		if (!$returnTypes \|\| empty(self::$patchTypesProperty->getValue($this)['deprecations'])) {
		return $deprecations;
		}

		$classExtension = self::getExtensionName($class);
		$className = str_contains($class, "@anonymous\0")
		? (get_parent_class($class) ?: key(class_implements($class)) ?: 'class') . '@anonymous'
		: $class;

		foreach ($returnTypes as $method => $returnTypeData) {
		[$normalizedType, , $declaringClass] = $returnTypeData;

		// Skip if no cross-extension boundary: empty declaring class (magic
		// methods), same class (own @return), or same/non-Drupal extension.
		$declaringExtension = self::getExtensionName($declaringClass);
		if ($declaringClass === '' \|\| $declaringClass === $class \|\| $declaringExtension === NULL \|\| $classExtension === $declaringExtension) {
		continue;
		}

		// Skip if not actually overridden, or already has a native return type.
		$methodRefl = $refl->getMethod($method);
		if ($methodRefl->class !== $class \|\| $methodRefl->hasReturnType()) {
		continue;
		}

		// Skip if the method's docblock contains @deprecated or @return.
		$docComment = $methodRefl->getDocComment();
		if ($docComment !== FALSE && (str_contains($docComment, '@deprecated') \|\| str_contains($docComment, '@return'))) {
		continue;
		}

		$deprecations[] = \sprintf(
		'Method "%s::%s()" might add "%s" as a native return type declaration in the future. Do the same in %s "%s" now to avoid errors or add an explicit @return annotation to suppress this message.',
		$declaringClass,
		$method,
		$normalizedType,
		interface_exists($declaringClass) ? 'implementation' : 'child class',
		$className,
		);
		}

		return $deprecations;
		}

		/**
		* Extracts the Drupal extension name from a fully qualified class name.
		*
		* @param string $class
		* The fully qualified class name.
		*
		* @return string\|null
		* The extension name, or NULL for non-Drupal classes.
		*/
		private static function getExtensionName(string $class): ?string {
		if (!str_starts_with($class, 'Drupal\\')) {
		return NULL;
		}

		$parts = explode('\\', $class);
		if (isset($parts[2]) && $parts[1] === 'Tests') {
		return $parts[2];
		}
		return $parts[1] ?? NULL;
		}

		}

core/tests/Drupal/Tests/TestTools/ErrorHandler/DrupalDebugClassLoaderTest.php

0 → 100644

+170 −0

Original line number	Diff line number	Diff line
		<?php

		declare(strict_types=1);

		namespace Drupal\Tests\TestTools\ErrorHandler;

		use Drupal\TestTools\ErrorHandler\DrupalDebugClassLoader;
		use PHPUnit\Framework\Attributes\CoversClass;
		use PHPUnit\Framework\Attributes\Group;
		use PHPUnit\Framework\Attributes\TestWith;
		use PHPUnit\Framework\TestCase;

		/**
		* Tests the DrupalDebugClassLoader.
		*/
		#[CoversClass(DrupalDebugClassLoader::class)]
		#[Group('TestTools')]
		class DrupalDebugClassLoaderTest extends TestCase {

		/**
		* A DrupalDebugClassLoader instance for testing.
		*/
		private DrupalDebugClassLoader $loader;

		/**
		* {@inheritdoc}
		*/
		protected function setUp(): void {
		parent::setUp();
		$this->loader = new DrupalDebugClassLoader(function (): void {});
		}

		/**
		* Tests that cross-module return type deprecations are generated.
		*/
		public function testCrossModuleReturnTypeDeprecation(): void {
		$deprecations = $this->getReturnTypeDeprecations('Drupal\drupal_debug_test_other\ChildWithoutReturnType');
		$this->assertSame(['Method "Drupal\drupal_debug_test_core\ParentWithReturn::testMethod()" might add "string" as a native return type declaration in the future. Do the same in child class "Drupal\drupal_debug_test_other\ChildWithoutReturnType" now to avoid errors or add an explicit @return annotation to suppress this message.'], $deprecations);
		}

		/**
		* Tests scenarios that should NOT trigger cross-module deprecations.
		*/
		#[TestWith(['Drupal\drupal_debug_test_core\SameModuleChild'])]
		#[TestWith(['Drupal\drupal_debug_test_other\ChildWithNativeReturnType'])]
		#[TestWith(['Drupal\drupal_debug_test_other\ChildWithReturnAnnotation'])]
		#[TestWith(['Drupal\drupal_debug_test_other\ChildWithDeprecatedMethod'])]
		#[TestWith(['Drupal\drupal_debug_test_other\ChildWithoutOverride'])]
		public function testNoDeprecation(string $class): void {
		$this->assertEmpty($this->getReturnTypeDeprecations($class));
		}

		/**
		* Returns only the return-type deprecations for a given class.
		*/
		private function getReturnTypeDeprecations(string $class): array {
		$deprecations = $this->loader->checkAnnotations(new \ReflectionClass($class), $class);
		return array_values(array_filter($deprecations, fn($d) => str_contains($d, 'might add')));
		}

		}

		namespace Drupal\drupal_debug_test_core;

		/**
		* Fixture: parent class with @return annotations but no native return types.
		*/
		class ParentWithReturn {

		/**
		* @return string
		* A test string.
		*/
		public function testMethod() {
		return 'test';
		}

		/**
		* @return int
		* A test integer.
		*/
		public function anotherMethod() {
		return 42;
		}

		}

		/**
		* Fixture: child in the same module as the parent.
		*/
		class SameModuleChild extends ParentWithReturn {

		/**
		* {@inheritdoc}
		*/
		public function testMethod() {
		return 'same module';
		}

		}

		namespace Drupal\drupal_debug_test_other;

		use Drupal\drupal_debug_test_core\ParentWithReturn;

		/**
		* Fixture: cross-module child without native return type.
		*/
		class ChildWithoutReturnType extends ParentWithReturn {

		/**
		* {@inheritdoc}
		*/
		public function testMethod() {
		return 'overridden';
		}

		}

		/**
		* Fixture: cross-module child with native return type.
		*/
		class ChildWithNativeReturnType extends ParentWithReturn {

		/**
		* {@inheritdoc}
		*/
		public function testMethod(): string {
		return 'overridden';
		}

		}

		/**
		* Fixture: cross-module child with own @return annotation.
		*/
		class ChildWithReturnAnnotation extends ParentWithReturn {

		/**
		* @return string
		* A test string.
		*/
		public function testMethod() {
		return 'overridden';
		}

		}

		/**
		* Fixture: cross-module child with deprecated method.
		*/
		class ChildWithDeprecatedMethod extends ParentWithReturn {

		/**
		* @deprecated in drupal:11.0.0 and is removed from drupal:12.0.0.
		* Use something else instead.
		* @see https://www.drupal.org/node/9999999
		*/
		public function testMethod() {
		return 'overridden';
		}

		}

		/**
		* Fixture: cross-module child that does not override the method.
		*/
		class ChildWithoutOverride extends ParentWithReturn {

		}