feature #2022 Inject Twig by default instead of EngineInterface for S…

…ymfony >= 4.3 (sarcher)

This PR was squashed before being merged into the 2.6-dev branch (closes #2022).

Discussion
----------

Inject Twig by default instead of EngineInterface for Symfony >= 4.3

This addresses #2020 by:

* Changing the default `services.templating` value from `templating` to `twig` if Symfony is >= 4.3 (it can still be overridden in `symfony/templating` is present
* Removing the type hint from `TwigExceptionController` and `ViewHandler` to allow either `Twig\Environment` or `EngineInterface` to be passed
* Borrowing the bridge code from TwigBundle to handle the new Twig template lookup via try/catch

Seems to work in 4.2 and 4.3, I tried it in both; we'll see what the CI says.

Not sure this is the best approach but I needed something working and so far so good. Please let me know if this needs to look different. I considered just having two separate classes for with/without the templating component, but this is far less code to maintain.

Commits
-------

e6f8eb5 Inject Twig by default instead of EngineInterface for Symfony >= 4.3

Controller/TemplatingExceptionController.php

@@ -16,6 +16,7 @@
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\Templating\EngineInterface;
 use Symfony\Component\Templating\TemplateReferenceInterface;
+use Twig\Environment;
 
 abstract class TemplatingExceptionController extends ExceptionController
 {
@@ -25,8 +26,18 @@ public function __construct(
         ViewHandlerInterface $viewHandler,
         ExceptionValueMap $exceptionCodes,
         $showException,
-        EngineInterface $templating
+        $templating
     ) {
+        if (!$templating instanceof EngineInterface && !$templating instanceof Environment) {
+            throw new \TypeError(sprintf(
+                'The fourth argument of %s must be an instance of %s or %s, but %s was given.',
+                __METHOD__,
+                EngineInterface::class,
+                Environment::class,
+                is_object($templating) ? get_class($templating) : gettype($templating)
+            ));
+        }
+
         parent::__construct($viewHandler, $exceptionCodes, $showException);
 
         $this->templating = $templating;

Controller/TwigExceptionController.php

@@ -12,6 +12,10 @@
 namespace FOS\RestBundle\Controller;
 
 use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\Templating\EngineInterface;
+use Twig\Environment;
+use Twig\Error\LoaderError;
+use Twig\Loader\ExistsLoaderInterface;
 
 /**
  * Custom ExceptionController that uses the view layer and supports HTTP response status code mapping.
@@ -48,14 +52,20 @@ protected function findTemplate(Request $request, $statusCode, $showException)
         // For error pages, try to find a template for the specific HTTP status code and format
         if (!$showException) {
             $template = sprintf('@Twig/Exception/%s%s.%s.twig', $name, $statusCode, $format);
-            if ($this->templating->exists($template)) {
+            if (
+                ($this->templating instanceof EngineInterface && $this->templating->exists($template)) ||
+                ($this->templating instanceof Environment && $this->templateExists($template))
+            ) {
                 return $template;
             }
         }
 
         // try to find a template for the given format
         $template = sprintf('@Twig/Exception/%s.%s.twig', $name, $format);
-        if ($this->templating->exists($template)) {
+        if (
+            ($this->templating instanceof EngineInterface && $this->templating->exists($template)) ||
+            ($this->templating instanceof Environment && $this->templateExists($template))
+        ) {
             return $template;
         }
 
@@ -64,4 +74,27 @@ protected function findTemplate(Request $request, $statusCode, $showException)
 
         return sprintf('@Twig/Exception/%s.html.twig', $showException ? 'exception_full' : $name);
     }
+
+    /**
+     * See if a template exists using the modern Twig mechanism.
+     *
+     * This code is based on TwigBundle and should be removed when the minimum required
+     * version of Twig is >= 3.0. See src/Symfony/Bundle/TwigBundle/Controller/ExceptionController.php
+     */
+    private function templateExists(string $template): bool
+    {
+        $loader = $this->templating->getLoader();
+        if ($loader instanceof ExistsLoaderInterface || method_exists($loader, 'exists')) {
+            return $loader->exists($template);
+        }
+
+        try {
+            $loader->getSourceContext($template)->getCode();
+
+            return true;
+        } catch (LoaderError $e) {
+        }
+
+        return false;
+    }
 }

DependencyInjection/Compiler/TwigExceptionPass.php

@@ -28,7 +28,7 @@ public function process(ContainerBuilder $container)
         if ($container->hasDefinition('fos_rest.exception_listener') &&
             null === $container->getDefinition('fos_rest.exception_listener')->getArgument(0)
         ) {
-            if (isset($container->getParameter('kernel.bundles')['TwigBundle']) && $container->has('templating.engine.twig')) {
+            if (isset($container->getParameter('kernel.bundles')['TwigBundle']) && ($container->has('templating.engine.twig') || $container->has('twig'))) {
                 // only use this when TwigBundle is enabled and the deprecated SF templating integration is used
                 $controller = Kernel::VERSION_ID >= 40100 ? 'fos_rest.exception.twig_controller::showAction' : 'fos_rest.exception.twig_controller:showAction';
             } else {
@@ -39,7 +39,11 @@ public function process(ContainerBuilder $container)
         }
 
         if (!$container->has('templating.engine.twig')) {
-            $container->removeDefinition('fos_rest.exception.twig_controller');
+            if ($container->has('twig') && $container->has('fos_rest.exception.twig_controller')) {
+                $container->findDefinition('fos_rest.exception.twig_controller')->replaceArgument(3, $container->findDefinition('twig'));
+            } else {
+                $container->removeDefinition('fos_rest.exception.twig_controller');
+            }
         }
     }
 }

View/ViewHandler.php

@@ -22,6 +22,7 @@
 use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
 use Symfony\Component\Templating\EngineInterface;
 use Symfony\Component\Templating\TemplateReferenceInterface;
+use Twig\Environment;
 
 /**
  * View may be used in controllers to build up a response in a format agnostic way
@@ -122,7 +123,7 @@ class ViewHandler implements ConfigurableViewHandlerInterface
     public function __construct(
         UrlGeneratorInterface $urlGenerator,
         Serializer $serializer,
-        EngineInterface $templating = null,
+        $templating,
         RequestStack $requestStack,
         array $formats = null,
         $failedValidationCode = Response::HTTP_BAD_REQUEST,
@@ -132,6 +133,15 @@ public function __construct(
         $defaultEngine = 'twig',
         array $options = []
     ) {
+        if (null !== $templating && !$templating instanceof EngineInterface && !$templating instanceof Environment) {
+            throw new \TypeError(sprintf(
+                'If provided, the templating engine must be an instance of %s or %s, but %s was given.',
+                EngineInterface::class,
+                Environment::class,
+                get_class($templating)
+            ));
+        }
+
         $this->urlGenerator = $urlGenerator;
         $this->serializer = $serializer;
         $this->templating = $templating;