feature #28891 [TwigBundle] Deprecating support for legacy templates directories (yceruto)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[TwigBundle] Deprecating support for legacy templates directories

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | https://github.com/symfony/symfony-docs/pull/10500

go ahead with https://github.com/symfony/symfony/pull/28810#issuecomment-430245394

- [x] Fix tests

Commits
-------

8b390f346f Deprecating support for legacy templates directories

UPGRADE-4.2.md

@@ -257,6 +257,7 @@ TwigBundle
 ----------
 
  * The `transchoice` tag and filter have been deprecated, use the `trans` ones instead with a `%count%` parameter.
+ * Deprecated support for legacy templates directories `src/Resources/views/` and `src/Resources/<BundleName>/views/`, use `templates/` and `templates/bundles/<BundleName>/` instead.
 
 Validator
 ---------

UPGRADE-5.0.md

@@ -256,6 +256,7 @@ TwigBundle
 
  * The default value (`false`) of the `twig.strict_variables` configuration option has been changed to `%kernel.debug%`.
  * The `transchoice` tag and filter have been removed, use the `trans` ones instead with a `%count%` parameter.
+ * Removed support for legacy templates directories `src/Resources/views/` and `src/Resources/<BundleName>/views/`, use `templates/` and `templates/bundles/<BundleName>/` instead.
 
 Validator
 --------

src/Symfony/Bundle/SecurityBundle/composer.json

@@ -37,7 +37,7 @@
         "symfony/framework-bundle": "~4.2",
         "symfony/http-foundation": "~3.4|~4.0",
         "symfony/translation": "~3.4|~4.0",
-        "symfony/twig-bundle": "~3.4|~4.0",
+        "symfony/twig-bundle": "~4.2",
         "symfony/twig-bridge": "~3.4|~4.0",
         "symfony/process": "~3.4|~4.0",
         "symfony/validator": "~3.4|~4.0",
@@ -49,6 +49,7 @@
     },
     "conflict": {
         "symfony/browser-kit": "<4.2",
+        "symfony/twig-bundle": "<4.2",
         "symfony/var-dumper": "<3.4",
         "symfony/event-dispatcher": "<3.4",
         "symfony/framework-bundle": "<4.2",

src/Symfony/Bundle/TwigBundle/CHANGELOG.md

@@ -1,6 +1,11 @@
 CHANGELOG
 =========
 
+4.2.0
+-----
+
+ * deprecated support for legacy templates directories `src/Resources/views/` and `src/Resources/<BundleName>/views/`, use `templates/` and `templates/bundles/<BundleName>/` instead.
+
 4.1.0
 -----
 

src/Symfony/Bundle/TwigBundle/DependencyInjection/TwigExtension.php

@@ -74,6 +74,7 @@ class TwigExtension extends Extension
 
         $container->setParameter('twig.form.resources', $config['form_themes']);
         $container->setParameter('twig.default_path', $config['default_path']);
+        $defaultTwigPath = $container->getParameterBag()->resolveValue($config['default_path']);
 
         $envConfiguratorDefinition = $container->getDefinition('twig.configurator.environment');
         $envConfiguratorDefinition->replaceArgument(0, $config['date']['format']);
@@ -115,14 +116,18 @@ class TwigExtension extends Extension
         }
 
         if (file_exists($dir = $container->getParameter('kernel.root_dir').'/Resources/views')) {
+            if ($dir !== $defaultTwigPath) {
+                @trigger_error(sprintf('Templates directory "%s" is deprecated since Symfony 4.2, use "%s" instead.', $dir, $defaultTwigPath), E_USER_DEPRECATED);
+            }
+
             $twigFilesystemLoaderDefinition->addMethodCall('addPath', array($dir));
         }
         $container->addResource(new FileExistenceResource($dir));
 
-        if (file_exists($dir = $container->getParameterBag()->resolveValue($config['default_path']))) {
-            $twigFilesystemLoaderDefinition->addMethodCall('addPath', array($dir));
+        if (file_exists($defaultTwigPath)) {
+            $twigFilesystemLoaderDefinition->addMethodCall('addPath', array($defaultTwigPath));
         }
-        $container->addResource(new FileExistenceResource($dir));
+        $container->addResource(new FileExistenceResource($defaultTwigPath));
 
         if (!empty($config['globals'])) {
             $def = $container->getDefinition('twig');
@@ -164,15 +169,19 @@ class TwigExtension extends Extension
     {
         $bundleHierarchy = array();
         foreach ($container->getParameter('kernel.bundles_metadata') as $name => $bundle) {
+            $defaultOverrideBundlePath = $container->getParameterBag()->resolveValue($config['default_path']).'/bundles/'.$name;
+
             if (file_exists($dir = $container->getParameter('kernel.root_dir').'/Resources/'.$name.'/views')) {
+                @trigger_error(sprintf('Templates directory "%s" is deprecated since Symfony 4.2, use "%s" instead.', $dir, $defaultOverrideBundlePath), E_USER_DEPRECATED);
+
                 $bundleHierarchy[$name][] = $dir;
             }
             $container->addResource(new FileExistenceResource($dir));
 
-            if (file_exists($dir = $container->getParameterBag()->resolveValue($config['default_path']).'/bundles/'.$name)) {
-                $bundleHierarchy[$name][] = $dir;
+            if (file_exists($defaultOverrideBundlePath)) {
+                $bundleHierarchy[$name][] = $defaultOverrideBundlePath;
             }
-            $container->addResource(new FileExistenceResource($dir));
+            $container->addResource(new FileExistenceResource($defaultOverrideBundlePath));
 
             if (file_exists($dir = $bundle['path'].'/Resources/views')) {
                 $bundleHierarchy[$name][] = $dir;

src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/Fixtures/Resources/TwigBundle/views/layout.html.twig

@@ -1 +0,0 @@
-This is a layout

src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/Fixtures/Resources/views/layout.html.twig

@@ -1 +0,0 @@
-This is a layout

src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/TwigExtensionTest.php

@@ -193,11 +193,46 @@ class TwigExtensionTest extends TestCase
             array('namespaced_path1', 'namespace1'),
             array('namespaced_path2', 'namespace2'),
             array('namespaced_path3', 'namespace3'),
-            array(__DIR__.'/Fixtures/Resources/TwigBundle/views', 'Twig'),
             array(__DIR__.'/Fixtures/templates/bundles/TwigBundle', 'Twig'),
             array(realpath(__DIR__.'/../..').'/Resources/views', 'Twig'),
             array(realpath(__DIR__.'/../..').'/Resources/views', '!Twig'),
-            array(__DIR__.'/Fixtures/Resources/views'),
+            array(__DIR__.'/Fixtures/templates'),
+        ), $paths);
+    }
+
+    /**
+     * @group legacy
+     * @dataProvider getFormats
+     * @expectedDeprecation Templates directory "%s/Resources/TwigBundle/views" is deprecated since Symfony 4.2, use "%s/templates/bundles/TwigBundle" instead.
+     * @expectedDeprecation Templates directory "%s/Resources/views" is deprecated since Symfony 4.2, use "%s/templates" instead.
+     */
+    public function testLegacyTwigLoaderPaths($format)
+    {
+        $container = $this->createContainer(__DIR__.'/../Fixtures/templates');
+        $container->registerExtension(new TwigExtension());
+        $this->loadFromFile($container, 'full', $format);
+        $this->loadFromFile($container, 'extra', $format);
+        $this->compileContainer($container);
+
+        $def = $container->getDefinition('twig.loader.native_filesystem');
+        $paths = array();
+        foreach ($def->getMethodCalls() as $call) {
+            if ('addPath' === $call[0] && false === strpos($call[1][0], 'Form')) {
+                $paths[] = $call[1];
+            }
+        }
+
+        $this->assertEquals(array(
+            array('path1'),
+            array('path2'),
+            array('namespaced_path1', 'namespace1'),
+            array('namespaced_path2', 'namespace2'),
+            array('namespaced_path3', 'namespace3'),
+            array(__DIR__.'/../Fixtures/templates/Resources/TwigBundle/views', 'Twig'),
+            array(__DIR__.'/Fixtures/templates/bundles/TwigBundle', 'Twig'),
+            array(realpath(__DIR__.'/../..').'/Resources/views', 'Twig'),
+            array(realpath(__DIR__.'/../..').'/Resources/views', '!Twig'),
+            array(__DIR__.'/../Fixtures/templates/Resources/views'),
             array(__DIR__.'/Fixtures/templates'),
         ), $paths);
     }
@@ -271,11 +306,11 @@ class TwigExtensionTest extends TestCase
         $this->assertEquals('foo', $args['FooClass']->getValues()[0]);
     }
 
-    private function createContainer()
+    private function createContainer(string $rootDir = __DIR__.'/Fixtures')
     {
         $container = new ContainerBuilder(new ParameterBag(array(
             'kernel.cache_dir' => __DIR__,
-            'kernel.root_dir' => __DIR__.'/Fixtures',
+            'kernel.root_dir' => $rootDir,
             'kernel.project_dir' => __DIR__,
             'kernel.charset' => 'UTF-8',
             'kernel.debug' => false,

src/Symfony/Bundle/TwigBundle/Tests/Fixtures/templates/Resources/TwigBundle/views/layout.html.twig

@@ -0,0 +1 @@
+{# Twig template #}

src/Symfony/Bundle/TwigBundle/Tests/Functional/NoTemplatingEntryTest.php

@@ -66,8 +66,9 @@ class NoTemplatingEntryKernel extends Kernel
                     'secret' => '$ecret',
                     'form' => array('enabled' => false),
                 ))
-                ->loadFromExtension('twig', array( // to be removed in 5.0 relying on default
-                    'strict_variables' => false,
+                ->loadFromExtension('twig', array(
+                    'strict_variables' => false, // to be removed in 5.0 relying on default
+                    'default_path' => __DIR__.'/templates',
                 ))
             ;
         });

src/Symfony/Bundle/TwigBundle/Tests/Loader/FilesystemLoaderTest.php

@@ -24,10 +24,10 @@ class FilesystemLoaderTest extends TestCase
         $locator
             ->expects($this->once())
             ->method('locate')
-            ->will($this->returnValue(__DIR__.'/../DependencyInjection/Fixtures/Resources/views/layout.html.twig'))
+            ->will($this->returnValue(__DIR__.'/../DependencyInjection/Fixtures/templates/layout.html.twig'))
         ;
         $loader = new FilesystemLoader($locator, $parser);
-        $loader->addPath(__DIR__.'/../DependencyInjection/Fixtures/Resources/views', 'namespace');
+        $loader->addPath(__DIR__.'/../DependencyInjection/Fixtures/templates', 'namespace');
 
         // Twig-style
         $this->assertEquals("This is a layout\n", $loader->getSourceContext('@namespace/layout.html.twig')->getCode());
@@ -44,7 +44,7 @@ class FilesystemLoaderTest extends TestCase
         $locator
             ->expects($this->once())
             ->method('locate')
-            ->will($this->returnValue($template = __DIR__.'/../DependencyInjection/Fixtures/Resources/views/layout.html.twig'))
+            ->will($this->returnValue($template = __DIR__.'/../DependencyInjection/Fixtures/templates/layout.html.twig'))
         ;
         $loader = new FilesystemLoader($locator, $parser);
 
@@ -101,7 +101,7 @@ class FilesystemLoaderTest extends TestCase
 
     /**
      * @expectedException \Twig\Error\LoaderError
-     * @expectedExceptionMessageRegExp /Unable to find template "name\.format\.engine" \(looked into: .*Tests.Loader.\.\..DependencyInjection.Fixtures.Resources.views\)/
+     * @expectedExceptionMessageRegExp /Unable to find template "name\.format\.engine" \(looked into: .*Tests.Loader.\.\..DependencyInjection.Fixtures.templates\)/
      */
     public function testTwigErrorIfTemplateDoesNotExist()
     {
@@ -109,7 +109,7 @@ class FilesystemLoaderTest extends TestCase
         $locator = $this->getMockBuilder('Symfony\Component\Config\FileLocatorInterface')->getMock();
 
         $loader = new FilesystemLoader($locator, $parser);
-        $loader->addPath(__DIR__.'/../DependencyInjection/Fixtures/Resources/views');
+        $loader->addPath(__DIR__.'/../DependencyInjection/Fixtures/templates');
 
         $method = new \ReflectionMethod('Symfony\Bundle\TwigBundle\Loader\FilesystemLoader', 'findTemplate');
         $method->setAccessible(true);
@@ -122,7 +122,7 @@ class FilesystemLoaderTest extends TestCase
         $locator = $this->getMockBuilder('Symfony\Component\Config\FileLocatorInterface')->getMock();
 
         $loader = new FilesystemLoader($locator, $parser);
-        $loader->addPath(__DIR__.'/../DependencyInjection/Fixtures/Resources/views');
+        $loader->addPath(__DIR__.'/../DependencyInjection/Fixtures/templates');
 
         $method = new \ReflectionMethod('Symfony\Bundle\TwigBundle\Loader\FilesystemLoader', 'findTemplate');
         $method->setAccessible(true);