Skip to content
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.

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

[ProxyManagerBridge] Polyfill for unmaintained version #32992

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion composer.json
Expand Up @@ -123,7 +123,8 @@
"Symfony\\Component\\": "src/Symfony/Component/"
},
"classmap": [
"src/Symfony/Component/Intl/Resources/stubs"
"src/Symfony/Component/Intl/Resources/stubs",
"src/Symfony/Bridge/ProxyManager/Legacy/ProxiedMethodReturnExpression.php"
],
"exclude-from-classmap": [
"**/Tests/"
Expand Down
Expand Up @@ -112,6 +112,10 @@ public function getProxyCode(Definition $definition)
);
}

if (version_compare(self::getProxyManagerVersion(), '2.5', '<')) {
$code = str_replace(' \Closure::bind(function ', ' \Closure::bind(static function ', $code);
}

return $code;
}

Expand Down
@@ -0,0 +1,73 @@
<?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 ProxyManager\Generator\Util;

use Composer\Autoload\ClassLoader;
use ProxyManager\Version;

if (class_exists(Version::class) && version_compare(\defined(Version::class.'::VERSION') ? Version::VERSION : Version::getVersion(), '2.5', '<')) {
/**
* Utility class to generate return expressions in method, given a method signature.
*
* This is required since return expressions may be forbidden by the method signature (void).
*
* @author Marco Pivetta <ocramius@gmail.com>
* @license MIT
*
* @see https://github.com/Ocramius/ProxyManager
*/
final class ProxiedMethodReturnExpression
{
public static function generate(string $returnedValueExpression, ?\ReflectionMethod $originalMethod): string
{
$originalReturnType = null === $originalMethod ? null : $originalMethod->getReturnType();

$originalReturnTypeName = null === $originalReturnType ? null : $originalReturnType->getName();

if ('void' === $originalReturnTypeName) {
return $returnedValueExpression.";\nreturn;";
}

return 'return '.$returnedValueExpression.';';
}
}
} else {
// Fallback to the original class by unregistering this file from composer class loader
$getComposerClassLoader = static function ($functionLoader) use (&$getComposerClassLoader) {
if (\is_array($functionLoader)) {
$functionLoader = $functionLoader[0];
}
if (!\is_object($functionLoader)) {
return;
}
if ($functionLoader instanceof ClassLoader) {
return $functionLoader;
}
if ($functionLoader instanceof \Symfony\Component\Debug\DebugClassLoader) {
return $getComposerClassLoader($functionLoader->getClassLoader());
}
if ($functionLoader instanceof \Symfony\Component\ErrorHandler\DebugClassLoader) {
return $getComposerClassLoader($functionLoader->getClassLoader());
}
};

$classLoader = null;
$functions = spl_autoload_functions();
while (null === $classLoader && $functions) {
$classLoader = $getComposerClassLoader(array_shift($functions));
}
$getComposerClassLoader = null;

jderusse marked this conversation as resolved.
Show resolved Hide resolved
if (null !== $classLoader) {
$classLoader->addClassMap([ProxiedMethodReturnExpression::class => null]);
$classLoader->loadClass(ProxiedMethodReturnExpression::class);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modifying the configuration of the main composer autoloader at runtime is a very bad idea: it cancels optimizations performed by @nicolas-grekas to ensure that the whole composer config stays in shared memory (as it triggers copy-on-write on the classmap).
Also, is it compatible with the classmap-authoritative mode of composer ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This happens only when a proxy is dumped, which doesn't happen at runtime, so the performance consideration is not an issue actually.

}
}
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Bridge\ProxyManager\Tests\LazyProxy\PhpDumper;

use PHPUnit\Framework\TestCase;
use ProxyManager\Version;
use Symfony\Bridge\ProxyManager\LazyProxy\PhpDumper\ProxyDumper;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
Expand Down Expand Up @@ -63,6 +64,20 @@ public function testGetProxyCode()
);
}

public function testStaticBinding()
{
if (!class_exists(Version::class) || version_compare(\defined(Version::class.'::VERSION') ? Version::VERSION : Version::getVersion(), '2.1', '<')) {
$this->markTestSkipped('ProxyManager prior to version 2.1 does not support static binding');
}

$definition = new Definition(__CLASS__);
$definition->setLazy(true);

$code = $this->dumper->getProxyCode($definition);

$this->assertStringContainsString('\Closure::bind(static function (\PHPUnit\Framework\TestCase $instance) {', $code);
}

public function testDeterministicProxyCode()
{
$definition = new Definition(__CLASS__);
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bridge/ProxyManager/composer.json
Expand Up @@ -25,6 +25,7 @@
},
"autoload": {
"psr-4": { "Symfony\\Bridge\\ProxyManager\\": "" },
"classmap": [ "Legacy/ProxiedMethodReturnExpression.php" ],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be ported in the main composer file ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, didn't see. 👍

"exclude-from-classmap": [
"/Tests/"
]
Expand Down