Skip to content

Tweak return type declarations and related CI checks #60702

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 5, 2025
Merged

Tweak return type declarations and related CI checks #60702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 34 additions & 7 deletions .github/expected-missing-return-types.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ git checkout src/Symfony/Contracts/Service/ResetInterface.php
(echo "$head" && echo && git diff -U2 src/ | grep '^index ' -v) > .github/expected-missing-return-types.diff
git checkout composer.json src/

diff --git a/src/Symfony/Bridge/Twig/Test/Traits/RuntimeLoaderProvider.php b/src/Symfony/Bridge/Twig/Test/Traits/RuntimeLoaderProvider.php
--- a/src/Symfony/Bridge/Twig/Test/Traits/RuntimeLoaderProvider.php
+++ b/src/Symfony/Bridge/Twig/Test/Traits/RuntimeLoaderProvider.php
@@ -21,5 +21,5 @@ trait RuntimeLoaderProvider
* @return void
*/
- protected function registerTwigRuntimeLoader(Environment $environment, FormRenderer $renderer)
+ protected function registerTwigRuntimeLoader(Environment $environment, FormRenderer $renderer): void
{
$loader = $this->createMock(RuntimeLoaderInterface::class);
diff --git a/src/Symfony/Component/BrowserKit/AbstractBrowser.php b/src/Symfony/Component/BrowserKit/AbstractBrowser.php
--- a/src/Symfony/Component/BrowserKit/AbstractBrowser.php
+++ b/src/Symfony/Component/BrowserKit/AbstractBrowser.php
Expand Down Expand Up @@ -48,7 +58,7 @@ diff --git a/src/Symfony/Component/BrowserKit/AbstractBrowser.php b/src/Symfony/
diff --git a/src/Symfony/Component/Config/Definition/Builder/NodeDefinition.php b/src/Symfony/Component/Config/Definition/Builder/NodeDefinition.php
--- a/src/Symfony/Component/Config/Definition/Builder/NodeDefinition.php
+++ b/src/Symfony/Component/Config/Definition/Builder/NodeDefinition.php
@@ -94,5 +94,5 @@ abstract class NodeDefinition implements NodeParentInterface
@@ -115,5 +115,5 @@ abstract class NodeDefinition implements NodeParentInterface
* @return NodeParentInterface|NodeBuilder|self|ArrayNodeDefinition|VariableNodeDefinition
*/
- public function end(): NodeParentInterface
Expand All @@ -58,21 +68,21 @@ diff --git a/src/Symfony/Component/Config/Definition/Builder/NodeDefinition.php
diff --git a/src/Symfony/Component/Console/Command/Command.php b/src/Symfony/Component/Console/Command/Command.php
--- a/src/Symfony/Component/Console/Command/Command.php
+++ b/src/Symfony/Component/Console/Command/Command.php
@@ -163,5 +163,5 @@ class Command
@@ -201,5 +201,5 @@ class Command implements SignalableCommandInterface
* @return void
*/
- protected function configure()
+ protected function configure(): void
{
}
@@ -195,5 +195,5 @@ class Command
@@ -233,5 +233,5 @@ class Command implements SignalableCommandInterface
* @return void
*/
- protected function interact(InputInterface $input, OutputInterface $output)
+ protected function interact(InputInterface $input, OutputInterface $output): void
{
}
@@ -211,5 +211,5 @@ class Command
@@ -249,5 +249,5 @@ class Command implements SignalableCommandInterface
* @return void
*/
- protected function initialize(InputInterface $input, OutputInterface $output)
Expand Down Expand Up @@ -474,14 +484,14 @@ diff --git a/src/Symfony/Component/HttpKernel/KernelInterface.php b/src/Symfony/
diff --git a/src/Symfony/Component/Routing/Loader/AttributeClassLoader.php b/src/Symfony/Component/Routing/Loader/AttributeClassLoader.php
--- a/src/Symfony/Component/Routing/Loader/AttributeClassLoader.php
+++ b/src/Symfony/Component/Routing/Loader/AttributeClassLoader.php
@@ -253,5 +253,5 @@ abstract class AttributeClassLoader implements LoaderInterface
@@ -277,5 +277,5 @@ abstract class AttributeClassLoader implements LoaderInterface
* @return string
*/
- protected function getDefaultRouteName(\ReflectionClass $class, \ReflectionMethod $method)
+ protected function getDefaultRouteName(\ReflectionClass $class, \ReflectionMethod $method): string
{
$name = str_replace('\\', '_', $class->name).'_'.$method->name;
@@ -355,5 +355,5 @@ abstract class AttributeClassLoader implements LoaderInterface
@@ -379,5 +379,5 @@ abstract class AttributeClassLoader implements LoaderInterface
* @return void
*/
- abstract protected function configureRoute(Route $route, \ReflectionClass $class, \ReflectionMethod $method, object $attr);
Expand Down Expand Up @@ -578,7 +588,7 @@ diff --git a/src/Symfony/Component/Translation/Extractor/ExtractorInterface.php
diff --git a/src/Symfony/Component/TypeInfo/Tests/Fixtures/DummyWithPhpDoc.php b/src/Symfony/Component/TypeInfo/Tests/Fixtures/DummyWithPhpDoc.php
--- a/src/Symfony/Component/TypeInfo/Tests/Fixtures/DummyWithPhpDoc.php
+++ b/src/Symfony/Component/TypeInfo/Tests/Fixtures/DummyWithPhpDoc.php
@@ -15,5 +15,5 @@ final class DummyWithPhpDoc
@@ -50,5 +50,5 @@ final class DummyWithPhpDoc
* @return Dummy
*/
- public function getNextDummy(mixed $dummy): mixed
Expand Down Expand Up @@ -610,6 +620,23 @@ diff --git a/src/Symfony/Component/VarDumper/Dumper/DataDumperInterface.php b/sr
- public function dump(Data $data);
+ public function dump(Data $data): ?string;
}
diff --git a/src/Symfony/Component/VarDumper/Test/VarDumperTestTrait.php b/src/Symfony/Component/VarDumper/Test/VarDumperTestTrait.php
--- a/src/Symfony/Component/VarDumper/Test/VarDumperTestTrait.php
+++ b/src/Symfony/Component/VarDumper/Test/VarDumperTestTrait.php
@@ -49,5 +49,5 @@ trait VarDumperTestTrait
* @return void
*/
- public function assertDumpEquals(mixed $expected, mixed $data, int $filter = 0, string $message = '')
+ public function assertDumpEquals(mixed $expected, mixed $data, int $filter = 0, string $message = ''): void
{
$this->assertSame($this->prepareExpectation($expected, $filter), $this->getDump($data, null, $filter), $message);
@@ -57,5 +57,5 @@ trait VarDumperTestTrait
* @return void
*/
- public function assertDumpMatchesFormat(mixed $expected, mixed $data, int $filter = 0, string $message = '')
+ public function assertDumpMatchesFormat(mixed $expected, mixed $data, int $filter = 0, string $message = ''): void
{
$this->assertStringMatchesFormat($this->prepareExpectation($expected, $filter), $this->getDump($data, null, $filter), $message);
diff --git a/src/Symfony/Contracts/Translation/LocaleAwareInterface.php b/src/Symfony/Contracts/Translation/LocaleAwareInterface.php
--- a/src/Symfony/Contracts/Translation/LocaleAwareInterface.php
+++ b/src/Symfony/Contracts/Translation/LocaleAwareInterface.php
Expand Down
34 changes: 15 additions & 19 deletions .github/patch-types.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

use PHPUnit\Framework\TestCase;

$mode = $argv[1] ?? 'patch';
if ('lint' !== $mode && false === getenv('SYMFONY_PATCH_TYPE_DECLARATIONS')) {
echo "Please define the SYMFONY_PATCH_TYPE_DECLARATIONS env var when running this script.\n";
Expand All @@ -18,17 +20,16 @@

switch (true) {
case false !== strpos($file, '/src/Symfony/Component/Cache/Traits/Redis'):
case false !== strpos($file, '/src/Symfony/Component/Cache/Traits/Relay'):
if (!str_ends_with($file, 'Proxy.php')) {
break;
}
// no break;
continue 2;
case false !== strpos($file, '/vendor/'):
case false !== strpos($file, '/src/Symfony/Bridge/Doctrine/Logger/DbalLogger.php'):
case false !== strpos($file, '/src/Symfony/Bridge/Doctrine/Middleware/Debug/'):
case false !== strpos($file, '/src/Symfony/Bridge/Doctrine/Tests/Fixtures/LegacyQueryMock.php'):
case false !== strpos($file, '/src/Symfony/Bridge/PhpUnit/'):
case false !== strpos($file, '/src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Validation/Article.php'):
case false !== strpos($file, '/src/Symfony/Component/Cache/Tests/Fixtures/DriverWrapper.php'):
case false !== strpos($file, '/src/Symfony/Component/Config/Tests/Fixtures/BadFileName.php'):
case false !== strpos($file, '/src/Symfony/Component/Config/Tests/Fixtures/BadParent.php'):
case false !== strpos($file, '/src/Symfony/Component/Config/Tests/Fixtures/ParseError.php'):
Expand All @@ -38,52 +39,46 @@
case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/compositetype_classes.php'):
case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/intersectiontype_classes.php'):
case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/MultipleArgumentsOptionalScalarNotReallyOptional.php'):
case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeDeclarationsPass/IntersectionConstructor.php'):
case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Fixtures/NewInInitializer.php'):
case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Fixtures/ParentNotExists.php'):
case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Fixtures/Preload/'):
case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/BadClasses/MissingParent.php'):
case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/'):
case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Fixtures/TestServiceSubscriberIntersectionWithTrait.php'):
case false !== strpos($file, '/src/Symfony/Component/ErrorHandler/Tests/Fixtures/'):
case false !== strpos($file, '/src/Symfony/Component/HttpClient/Internal/'):
case false !== strpos($file, '/src/Symfony/Component/Form/Tests/Fixtures/Answer.php'):
case false !== strpos($file, '/src/Symfony/Component/Form/Tests/Fixtures/Number.php'):
case false !== strpos($file, '/src/Symfony/Component/Form/Tests/Fixtures/Suit.php'):
case false !== strpos($file, '/src/Symfony/Component/HttpClient/Internal/') && str_contains($file, 'V5'):
case false !== strpos($file, '/src/Symfony/Component/PropertyAccess/Tests/Fixtures/AsymmetricVisibility.php'):
case false !== strpos($file, '/src/Symfony/Component/PropertyInfo/Tests/Fixtures/'):
case false !== strpos($file, '/src/Symfony/Component/PropertyInfo/Tests/Fixtures/Php81Dummy.php'):
case false !== strpos($file, '/src/Symfony/Component/Runtime/Internal/ComposerPlugin.php'):
case false !== strpos($file, '/src/Symfony/Component/Security/Http/Tests/Fixtures/IsGrantedAttributeMethodsWithClosureController.php'):
case false !== strpos($file, '/src/Symfony/Component/Security/Http/Tests/Fixtures/IsGrantedAttributeWithClosureController.php'):
case false !== strpos($file, '/src/Symfony/Component/Serializer/Tests/Fixtures/'):
case false !== strpos($file, '/src/Symfony/Component/Serializer/Tests/Normalizer/Features/ObjectOuter.php'):
case false !== strpos($file, '/src/Symfony/Component/Validator/Tests/Constraints/Fixtures/WhenTestWithClosure.php'):
case false !== strpos($file, '/src/Symfony/Component/Validator/Tests/Fixtures/NestedAttribute/Entity.php'):
case false !== strpos($file, '/src/Symfony/Component/VarDumper/Tests/Fixtures/NotLoadableClass.php'):
case false !== strpos($file, '/src/Symfony/Component/VarDumper/Tests/Fixtures/ReflectionIntersectionTypeFixture.php'):
case false !== strpos($file, '/src/Symfony/Component/VarDumper/Tests/Fixtures/ReflectionUnionTypeWithIntersectionFixture.php'):
case false !== strpos($file, '/src/Symfony/Component/VarDumper/Tests/Fixtures/VirtualProperty.php'):
case false !== strpos($file, '/src/Symfony/Component/VarExporter/Internal'):
case false !== strpos($file, '/src/Symfony/Component/VarExporter/Tests/Fixtures/'):
case false !== strpos($file, '/src/Symfony/Component/Cache/Traits/RelayProxy.php'):
case false !== strpos($file, '/src/Symfony/Contracts/Service/Test/ServiceLocatorTest.php'):
case false !== strpos($file, '/src/Symfony/Contracts/Service/Test/ServiceLocatorTestCase.php'):
case false !== strpos($file, '/src/Symfony/Contracts/'):
continue 2;
}

class_exists($class);

if ('lint' !== $mode) {
if ('lint' !== $mode || is_subclass_of($class, TestCase::class)) {
continue;
}

$refl = new \ReflectionClass($class);
foreach ($refl->getMethods() as $method) {
if (
$method->getReturnType()
|| str_contains($method->getDocComment(), '@return')
|| (str_contains($method->getDocComment(), '@return') && str_contains($method->getDocComment(), 'resource'))
|| '__construct' === $method->getName()
|| '__destruct' === $method->getName()
|| '__clone' === $method->getName()
|| $method->getDeclaringClass()->getName() !== $class
|| str_contains($method->getDeclaringClass()->getName(), '\\Test\\')
|| str_contains($method->getDeclaringClass()->getName(), '\\Tests\\')
|| str_contains($method->getDeclaringClass()->getName(), '\\Test\\') && str_starts_with($method->getName(), 'test')
) {
continue;
}
Expand All @@ -95,6 +90,7 @@ class_exists($class);
if ($missingReturnTypes) {
echo \count($missingReturnTypes)." missing return types on interfaces\n\n";
echo implode("\n", $missingReturnTypes);
echo "\n";
exit(1);
}

Expand Down
5 changes: 3 additions & 2 deletions .github/workflows/unit-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ jobs:

# Create local composer packages for each patched components and reference them in composer.json files when cross-testing components
if [[ ! "${{ matrix.mode }}" = *-deps ]]; then
php .github/build-packages.php HEAD^ $SYMFONY_VERSION src/Symfony/Bridge/PhpUnit
php .github/build-packages.php HEAD^ $SYMFONY_VERSION src/Symfony/Bridge/PhpUnit
else
echo SYMFONY_DEPRECATIONS_HELPER=weak >> $GITHUB_ENV
cp composer.json composer.json.orig
Expand Down Expand Up @@ -154,9 +154,10 @@ jobs:
run: |
patch -sp1 < .github/expected-missing-return-types.diff
git add .
sed -i 's/ *"\*\*\/Tests\/",//' composer.json
composer install -q --optimize-autoloader || composer install --optimize-autoloader
SYMFONY_PATCH_TYPE_DECLARATIONS='force=2&php=8.2' php .github/patch-types.php
git checkout src/Symfony/Contracts/Service/ResetInterface.php
git checkout composer.json src/Symfony/Contracts/Service/ResetInterface.php
SYMFONY_PATCH_TYPE_DECLARATIONS='force=2&php=8.2' php .github/patch-types.php # ensure the script is idempotent
git checkout src/Symfony/Contracts/Service/ResetInterface.php
git diff --exit-code
Expand Down
3 changes: 3 additions & 0 deletions src/Symfony/Bridge/Twig/Test/Traits/RuntimeLoaderProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@

trait RuntimeLoaderProvider
{
/**
* @return void
*/
protected function registerTwigRuntimeLoader(Environment $environment, FormRenderer $renderer)
{
$loader = $this->createMock(RuntimeLoaderInterface::class);
Expand Down
25 changes: 0 additions & 25 deletions src/Symfony/Bundle/FrameworkBundle/Tests/Kernel/MicroKernelTraitTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
use PHPUnit\Framework\TestCase;
use Psr\Log\NullLogger;
use Symfony\Bundle\FrameworkBundle\Console\Application;
use Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait;
use Symfony\Component\Console\Attribute\AsCommand;
use Symfony\Component\Console\Input\ArrayInput;
use Symfony\Component\Console\Output\BufferedOutput;
Expand Down Expand Up @@ -186,27 +185,3 @@ public function testDefaultKernel()
$this->assertSame('OK', $response->getContent());
}
}

abstract class MinimalKernel extends Kernel
{
use MicroKernelTrait;

private string $cacheDir;

public function __construct(string $cacheDir)
{
parent::__construct('test', false);

$this->cacheDir = sys_get_temp_dir().'/'.$cacheDir;
}

public function getCacheDir(): string
{
return $this->cacheDir;
}

public function getLogDir(): string
{
return $this->cacheDir;
}
}
39 changes: 39 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Tests/Kernel/MinimalKernel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bundle\FrameworkBundle\Tests\Kernel;

use Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait;
use Symfony\Component\HttpKernel\Kernel;

abstract class MinimalKernel extends Kernel
{
use MicroKernelTrait;

private string $cacheDir;

public function __construct(string $cacheDir)
{
parent::__construct('test', false);

$this->cacheDir = sys_get_temp_dir().'/'.$cacheDir;
}

public function getCacheDir(): string
{
return $this->cacheDir;
}

public function getLogDir(): string
{
return $this->cacheDir;
}
}
6 changes: 6 additions & 0 deletions src/Symfony/Component/VarDumper/Test/VarDumperTestTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,17 @@ protected function tearDownVarDumper(): void
$this->varDumperConfig['flags'] = null;
}

/**
* @return void
*/
public function assertDumpEquals(mixed $expected, mixed $data, int $filter = 0, string $message = '')
{
$this->assertSame($this->prepareExpectation($expected, $filter), $this->getDump($data, null, $filter), $message);
}

/**
* @return void
*/
public function assertDumpMatchesFormat(mixed $expected, mixed $data, int $filter = 0, string $message = '')
{
$this->assertStringMatchesFormat($this->prepareExpectation($expected, $filter), $this->getDump($data, null, $filter), $message);
Expand Down