[ErrorHandler] Improve fileLinkFormat handling

- Avoid repeating file link format guessing (logic is already in FileLinkFormatter class) - Always set a fileLinkFormat to a FileLinkFormatter object to handle path mappings properly
symfony · Oct 16, 2023 · 510b77b · 510b77b
1 parent eadd41a
commit 510b77b
Show file tree

Hide file tree

Showing 26 changed files with 242 additions and 138 deletions.
diff --git a/UPGRADE-6.4.md b/UPGRADE-6.4.md
@@ -152,6 +152,7 @@ HttpKernel
  * [BC break] Add native return types to `TraceableEventDispatcher` and to `MergeExtensionConfigurationPass`
  * Deprecate `Kernel::stripComments()`
  * Deprecate `UriSigner`, use `UriSigner` from the HttpFoundation component instead
+ * Deprecate `FileLinkFormatter`, use `FileLinkFormatter` from the ErrorHandler component instead
 
 Messenger
 ---------

diff --git a/src/Symfony/Bridge/Twig/Command/DebugCommand.php b/src/Symfony/Bridge/Twig/Command/DebugCommand.php
@@ -22,8 +22,8 @@
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Style\SymfonyStyle;
+use Symfony\Component\ErrorHandler\ErrorRenderer\FileLinkFormatter;
 use Symfony\Component\Finder\Finder;
-use Symfony\Component\HttpKernel\Debug\FileLinkFormatter;
 use Twig\Environment;
 use Twig\Loader\ChainLoader;
 use Twig\Loader\FilesystemLoader;

diff --git a/src/Symfony/Bridge/Twig/Extension/CodeExtension.php b/src/Symfony/Bridge/Twig/Extension/CodeExtension.php
@@ -11,7 +11,7 @@
 
 namespace Symfony\Bridge\Twig\Extension;
 
-use Symfony\Component\HttpKernel\Debug\FileLinkFormatter;
+use Symfony\Component\ErrorHandler\ErrorRenderer\FileLinkFormatter;
 use Twig\Extension\AbstractExtension;
 use Twig\TwigFilter;
 

diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/CodeExtensionTest.php b/src/Symfony/Bridge/Twig/Tests/Extension/CodeExtensionTest.php
@@ -13,7 +13,7 @@
 
 use PHPUnit\Framework\TestCase;
 use Symfony\Bridge\Twig\Extension\CodeExtension;
-use Symfony\Component\HttpKernel\Debug\FileLinkFormatter;
+use Symfony\Component\ErrorHandler\ErrorRenderer\FileLinkFormatter;
 
 class CodeExtensionTest extends TestCase
 {

diff --git a/src/Symfony/Bundle/FrameworkBundle/Command/DebugAutowiringCommand.php b/src/Symfony/Bundle/FrameworkBundle/Command/DebugAutowiringCommand.php
@@ -21,7 +21,7 @@
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Style\SymfonyStyle;
 use Symfony\Component\DependencyInjection\Attribute\Target;
-use Symfony\Component\HttpKernel\Debug\FileLinkFormatter;
+use Symfony\Component\ErrorHandler\ErrorRenderer\FileLinkFormatter;
 
 /**
  * A console command for autowiring information.

diff --git a/src/Symfony/Bundle/FrameworkBundle/Command/RouterDebugCommand.php b/src/Symfony/Bundle/FrameworkBundle/Command/RouterDebugCommand.php
@@ -22,7 +22,7 @@
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Style\SymfonyStyle;
-use Symfony\Component\HttpKernel\Debug\FileLinkFormatter;
+use Symfony\Component\ErrorHandler\ErrorRenderer\FileLinkFormatter;
 use Symfony\Component\Routing\RouteCollection;
 use Symfony\Component\Routing\RouterInterface;
 

diff --git a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/TextDescriptor.php b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/TextDescriptor.php
@@ -26,8 +26,8 @@
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
 use Symfony\Component\DependencyInjection\Reference;
+use Symfony\Component\ErrorHandler\ErrorRenderer\FileLinkFormatter;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
-use Symfony\Component\HttpKernel\Debug\FileLinkFormatter;
 use Symfony\Component\Routing\Route;
 use Symfony\Component\Routing\RouteCollection;
 

diff --git a/src/Symfony/Bundle/FrameworkBundle/Console/Helper/DescriptorHelper.php b/src/Symfony/Bundle/FrameworkBundle/Console/Helper/DescriptorHelper.php
@@ -16,7 +16,7 @@
 use Symfony\Bundle\FrameworkBundle\Console\Descriptor\TextDescriptor;
 use Symfony\Bundle\FrameworkBundle\Console\Descriptor\XmlDescriptor;
 use Symfony\Component\Console\Helper\DescriptorHelper as BaseDescriptorHelper;
-use Symfony\Component\HttpKernel\Debug\FileLinkFormatter;
+use Symfony\Component\ErrorHandler\ErrorRenderer\FileLinkFormatter;
 
 /**
  * @author Jean-François Simon <jeanfrancois.simon@sensiolabs.com>

diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/debug_prod.php b/src/Symfony/Bundle/FrameworkBundle/Resources/config/debug_prod.php
@@ -11,8 +11,8 @@
 
 namespace Symfony\Component\DependencyInjection\Loader\Configurator;
 
+use Symfony\Component\ErrorHandler\ErrorRenderer\FileLinkFormatter;
 use Symfony\Component\HttpKernel\Debug\ErrorHandlerConfigurator;
-use Symfony\Component\HttpKernel\Debug\FileLinkFormatter;
 use Symfony\Component\HttpKernel\EventListener\DebugHandlersListener;
 
 return static function (ContainerConfigurator $container) {

diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Console/Descriptor/TextDescriptorTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Console/Descriptor/TextDescriptorTest.php
@@ -12,7 +12,7 @@
 namespace Symfony\Bundle\FrameworkBundle\Tests\Console\Descriptor;
 
 use Symfony\Bundle\FrameworkBundle\Console\Descriptor\TextDescriptor;
-use Symfony\Component\HttpKernel\Debug\FileLinkFormatter;
+use Symfony\Component\ErrorHandler\ErrorRenderer\FileLinkFormatter;
 use Symfony\Component\Routing\Route;
 
 class TextDescriptorTest extends AbstractDescriptorTestCase

diff --git a/src/Symfony/Bundle/WebProfilerBundle/Resources/config/profiler.php b/src/Symfony/Bundle/WebProfilerBundle/Resources/config/profiler.php
@@ -17,7 +17,7 @@
 use Symfony\Bundle\WebProfilerBundle\Csp\ContentSecurityPolicyHandler;
 use Symfony\Bundle\WebProfilerBundle\Csp\NonceGenerator;
 use Symfony\Bundle\WebProfilerBundle\Twig\WebProfilerExtension;
-use Symfony\Component\HttpKernel\Debug\FileLinkFormatter;
+use Symfony\Component\ErrorHandler\ErrorRenderer\FileLinkFormatter;
 use Symfony\Component\VarDumper\Dumper\HtmlDumper;
 
 return static function (ContainerConfigurator $container) {

diff --git a/src/Symfony/Bundle/WebProfilerBundle/composer.json b/src/Symfony/Bundle/WebProfilerBundle/composer.json
@@ -18,8 +18,8 @@
     "require": {
         "php": ">=8.1",
         "symfony/config": "^5.4|^6.0|^7.0",
-        "symfony/framework-bundle": "^5.4|^6.0|^7.0",
-        "symfony/http-kernel": "^6.3|^7.0",
+        "symfony/framework-bundle": "^6.2|^7.0",
+        "symfony/http-kernel": "^6.4|^7.0",
         "symfony/routing": "^5.4|^6.0|^7.0",
         "symfony/twig-bundle": "^5.4|^6.0|^7.0",
         "twig/twig": "^2.13|^3.0.4"

diff --git a/src/Symfony/Component/ErrorHandler/ErrorRenderer/FileLinkFormatter.php b/src/Symfony/Component/ErrorHandler/ErrorRenderer/FileLinkFormatter.php
@@ -0,0 +1,115 @@
+<?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\Component\ErrorHandler\ErrorRenderer;
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
+
+/**
+ * Formats debug file links.
+ *
+ * @author Jérémy Romey <jeremy@free-agent.fr>
+ *
+ * @final
+ */
+class FileLinkFormatter
+{
+    private array|false $fileLinkFormat;
+    private ?RequestStack $requestStack = null;
+    private ?string $baseDir = null;
+    private \Closure|string|null $urlFormat;
+
+    /**
+     * @param string|\Closure $urlFormat the URL format, or a closure that returns it on-demand
+     */
+    public function __construct(string|array $fileLinkFormat = null, RequestStack $requestStack = null, string $baseDir = null, string|\Closure $urlFormat = null)
+    {
+        $fileLinkFormat ??= $_ENV['SYMFONY_IDE'] ?? $_SERVER['SYMFONY_IDE'] ?? '';
+
+        if (!\is_array($f = $fileLinkFormat)) {
+            $f = (ErrorRendererInterface::IDE_LINK_FORMATS[$f] ?? $f) ?: \ini_get('xdebug.file_link_format') ?: get_cfg_var('xdebug.file_link_format') ?: 'file://%f#L%l';
+            $i = strpos($f, '&', max(strrpos($f, '%f'), strrpos($f, '%l'))) ?: \strlen($f);
+            $fileLinkFormat = [substr($f, 0, $i)] + preg_split('/&([^>]++)>/', substr($f, $i), -1, \PREG_SPLIT_DELIM_CAPTURE);
+        }
+
+        $this->fileLinkFormat = $fileLinkFormat;
+        $this->requestStack = $requestStack;
+        $this->baseDir = $baseDir;
+        $this->urlFormat = $urlFormat;
+    }
+
+    /**
+     * @return string|false
+     */
+    public function format(string $file, int $line): string|bool
+    {
+        if ($fmt = $this->getFileLinkFormat()) {
+            for ($i = 1; isset($fmt[$i]); ++$i) {
+                if (str_starts_with($file, $k = $fmt[$i++])) {
+                    $file = substr_replace($file, $fmt[$i], 0, \strlen($k));
+                    break;
+                }
+            }
+
+            return strtr($fmt[0], ['%f' => $file, '%l' => $line]);
+        }
+
+        return false;
+    }
+
+    /**
+     * @internal
+     */
+    public function __sleep(): array
+    {
+        $this->fileLinkFormat = $this->getFileLinkFormat();
+
+        return ['fileLinkFormat'];
+    }
+
+    /**
+     * @internal
+     */
+    public static function generateUrlFormat(UrlGeneratorInterface $router, string $routeName, string $queryString): ?string
+    {
+        try {
+            return $router->generate($routeName).$queryString;
+        } catch (\Throwable) {
+            return null;
+        }
+    }
+
+    private function getFileLinkFormat(): array|false
+    {
+        if ($this->fileLinkFormat) {
+            return $this->fileLinkFormat;
+        }
+
+        if ($this->requestStack && $this->baseDir && $this->urlFormat) {
+            $request = $this->requestStack->getMainRequest();
+
+            if ($request instanceof Request && (!$this->urlFormat instanceof \Closure || $this->urlFormat = ($this->urlFormat)())) {
+                return [
+                    $request->getSchemeAndHttpHost().$this->urlFormat,
+                    $this->baseDir.\DIRECTORY_SEPARATOR, '',
+                ];
+            }
+        }
+
+        return false;
+    }
+}
+
+if (!class_exists(\Symfony\Component\HttpKernel\Debug\FileLinkFormatter::class, false)) {
+    class_alias(FileLinkFormatter::class, \Symfony\Component\HttpKernel\Debug\FileLinkFormatter::class);
+}
diff --git a/src/Symfony/Component/ErrorHandler/ErrorRenderer/HtmlErrorRenderer.php b/src/Symfony/Component/ErrorHandler/ErrorRenderer/HtmlErrorRenderer.php
@@ -15,7 +15,6 @@
 use Symfony\Component\ErrorHandler\Exception\FlattenException;
 use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpFoundation\Response;
-use Symfony\Component\HttpKernel\Debug\FileLinkFormatter;
 use Symfony\Component\HttpKernel\Log\DebugLoggerConfigurator;
 use Symfony\Component\VarDumper\Cloner\Data;
 use Symfony\Component\VarDumper\Dumper\HtmlDumper;
@@ -37,7 +36,7 @@ class HtmlErrorRenderer implements ErrorRendererInterface
 
     private bool|\Closure $debug;
     private string $charset;
-    private string|array|FileLinkFormatter|false $fileLinkFormat;
+    private FileLinkFormatter $fileLinkFormat;
     private ?string $projectDir;
     private string|\Closure $outputBuffer;
     private ?LoggerInterface $logger;
@@ -52,10 +51,7 @@ public function __construct(bool|callable $debug = false, string $charset = null
     {
         $this->debug = \is_bool($debug) ? $debug : $debug(...);
         $this->charset = $charset ?: (\ini_get('default_charset') ?: 'UTF-8');
-        $fileLinkFormat ??= $_ENV['SYMFONY_IDE'] ?? $_SERVER['SYMFONY_IDE'] ?? null;
-        $this->fileLinkFormat = \is_string($fileLinkFormat)
-            ? (ErrorRendererInterface::IDE_LINK_FORMATS[$fileLinkFormat] ?? $fileLinkFormat ?: false)
-            : ($fileLinkFormat ?: \ini_get('xdebug.file_link_format') ?: get_cfg_var('xdebug.file_link_format') ?: false);
+        $this->fileLinkFormat = $fileLinkFormat instanceof FileLinkFormatter ? $fileLinkFormat : new FileLinkFormatter($fileLinkFormat);
         $this->projectDir = $projectDir;
         $this->outputBuffer = \is_string($outputBuffer) ? $outputBuffer : $outputBuffer(...);
         $this->logger = $logger;
@@ -210,15 +206,6 @@ private function getFileRelative(string $file): ?string
         return null;
     }
 
-    private function getFileLink(string $file, int $line): string|false
-    {
-        if ($fmt = $this->fileLinkFormat) {
-            return \is_string($fmt) ? strtr($fmt, ['%f' => $file, '%l' => $line]) : $fmt->format($file, $line);
-        }
-
-        return false;
-    }
-
     /**
      * Formats a file path.
      *
@@ -242,11 +229,9 @@ private function formatFile(string $file, int $line, string $text = null): strin
             $text .= ' at line '.$line;
         }
 
-        if (false !== $link = $this->getFileLink($file, $line)) {
-            return sprintf('<a href="%s" title="Click to open this file" class="file_link">%s</a>', $this->escape($link), $text);
-        }
+        $link = $this->fileLinkFormat->format($file, $line);
 
-        return $text;
+        return sprintf('<a href="%s" title="Click to open this file" class="file_link">%s</a>', $this->escape($link), $text);
     }
 
     /**

diff --git a/...nel/Tests/Debug/FileLinkFormatterTest.php → ...s/ErrorRenderer/FileLinkFormatterTest.php b/...nel/Tests/Debug/FileLinkFormatterTest.php → ...s/ErrorRenderer/FileLinkFormatterTest.php
@@ -9,12 +9,12 @@
  * file that was distributed with this source code.
  */
 
-namespace Symfony\Component\HttpKernel\Tests\Debug;
+namespace Symfony\Component\ErrorHandler\Tests\ErrorRenderer;
 
 use PHPUnit\Framework\TestCase;
+use Symfony\Component\ErrorHandler\ErrorRenderer\FileLinkFormatter;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\RequestStack;
-use Symfony\Component\HttpKernel\Debug\FileLinkFormatter;
 
 class FileLinkFormatterTest extends TestCase
 {
@@ -80,4 +80,44 @@ public function testSerialize()
     {
         $this->assertInstanceOf(FileLinkFormatter::class, unserialize(serialize(new FileLinkFormatter())));
     }
+
+    /**
+     * @dataProvider providePathMappings
+     */
+    public function testIdeFileLinkFormatWithPathMappingParameters($mappings)
+    {
+        $params = array_reduce($mappings, function ($c, $m) {
+            return "$c&".implode('>', $m);
+        }, '');
+        $sut = new FileLinkFormatter("vscode://file/%f:%l$params");
+        foreach ($mappings as $mapping) {
+            $fileGuest = $mapping['guest'].'file.php';
+            $fileHost = $mapping['host'].'file.php';
+            $this->assertSame("vscode://file/$fileHost:3", $sut->format($fileGuest, 3));
+        }
+    }
+
+    public static function providePathMappings()
+    {
+        yield 'single path mapping' => [
+            [
+                [
+                    'guest' => '/var/www/app/',
+                    'host' => '/user/name/project/',
+                ],
+            ],
+        ];
+        yield 'multiple path mapping' => [
+            [
+                [
+                    'guest' => '/var/www/app/',
+                    'host' => '/user/name/project/',
+                ],
+                [
+                    'guest' => '/var/www/app2/',
+                    'host' => '/user/name/project2/',
+                ],
+            ],
+        ];
+    }
 }
diff --git a/src/Symfony/Component/ErrorHandler/Tests/ErrorRenderer/HtmlErrorRendererTest.php b/src/Symfony/Component/ErrorHandler/Tests/ErrorRenderer/HtmlErrorRendererTest.php
@@ -54,4 +54,47 @@ public static function getRenderData(): iterable
             $expectedNonDebug,
         ];
     }
+
+    /**
+     * @dataProvider provideFileLinkFormats
+     */
+    public function testFileLinkFormat(\ErrorException $exception, string $fileLinkFormat, bool $withSymfonyIde, string $expected)
+    {
+        if ($withSymfonyIde) {
+            $_ENV['SYMFONY_IDE'] = $fileLinkFormat;
+        }
+        $errorRenderer = new HtmlErrorRenderer(true, null, $withSymfonyIde ? null : $fileLinkFormat);
+
+        $this->assertStringContainsString($expected, $errorRenderer->render($exception)->getAsString());
+    }
+
+    public static function provideFileLinkFormats(): iterable
+    {
+        $exception = new \ErrorException('Notice', 0, \E_USER_NOTICE);
+
+        yield 'file link format set as known IDE with SYMFONY_IDE' => [
+            $exception,
+            'vscode',
+            true,
+            'href="vscode://file/'.__DIR__,
+        ];
+        yield 'file link format set as a raw format with SYMFONY_IDE' => [
+            $exception,
+            'phpstorm://open?file=%f&line=%l',
+            true,
+            'href="phpstorm://open?file='.__DIR__,
+        ];
+        yield 'file link format set as known IDE without SYMFONY_IDE' => [
+            $exception,
+            'vscode',
+            false,
+            'href="vscode://file/'.__DIR__,
+        ];
+        yield 'file link format set as a raw format without SYMFONY_IDE' => [
+            $exception,
+            'phpstorm://open?file=%f&line=%l',
+            false,
+            'href="phpstorm://open?file='.__DIR__,
+        ];
+    }
 }