mirror of
https://github.com/twigphp/Twig.git
synced 2026-10-02 18:07:35 +00:00
feature #4951 Allow skipping the directory check when adding paths to FilesystemLoader (nicolas-grekas)
This PR was merged into the 3.x branch.
Discussion
----------
Allow skipping the directory check when adding paths to `FilesystemLoader`
`FilesystemLoader::addPath()` and `prependPath()` call `is_dir()` on each path, so an app built from a compiled container pays for it on each request, even though the paths were checked when the container was built. This adds a third argument to skip the check:
```php
$loader->addPath($templateDir, 'admin', false);
```
It's read with `func_get_arg()` because Drupal and Contao override both methods with two arguments. On the Symfony Demo, where TwigBundle registers 14 paths, building the loader goes from 64µs to 21µs per request once TwigBundle passes `false` (symfony/symfony#66334).
<details>
<summary>Merge-up notes</summary>
To 4.x: declare the argument for real, `bool $check = true`, instead of reading it with `func_get_arg()`; the tests apply as is. The CHANGELOG and `doc/deprecated.rst` entries don't go to 4.x, and the `versionadded` block has to be removed from `doc/api.rst` since the 4.x docs have none.
</details>
Commits
-------
9e1e2e2cea Allow skipping the directory check when adding paths to `FilesystemLoader`
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
# 3.30.1 (2026-XX-XX)
|
||||
# 3.31.0 (2026-XX-XX)
|
||||
|
||||
* Fix the `loop` variable being undefined when used in the sequence of a nested `for` tag
|
||||
* Reject a deprecated `Template` instance created by another environment in `Environment::resolveTemplate()`
|
||||
@@ -8,6 +8,7 @@
|
||||
* Fix the sandbox not reporting the line of a rejected `guard` tag
|
||||
* Fix the spread operator compiling to invalid PHP outside sequences, mappings, and call arguments
|
||||
* Fix the `deprecated` tag generating invalid PHP for an integer message
|
||||
* Add a third argument to `FilesystemLoader::addPath()` and `FilesystemLoader::prependPath()` to skip checking that the directory exists
|
||||
|
||||
# 3.30.0 (2026-09-25)
|
||||
|
||||
|
||||
+12
@@ -296,6 +296,18 @@ Namespaced templates can be accessed via the special
|
||||
|
||||
$twig->render('@admin/index.html.twig', []);
|
||||
|
||||
``addPath()`` and ``prependPath()`` throw an exception when the directory does
|
||||
not exist. If you already know that it does, for instance because a build step
|
||||
checked it, pass ``false`` as the third argument to skip the check and save a
|
||||
filesystem call::
|
||||
|
||||
$loader->addPath($templateDir, 'admin', false);
|
||||
|
||||
.. versionadded:: 3.31
|
||||
|
||||
The third argument of ``addPath()`` and ``prependPath()`` was added in
|
||||
Twig 3.31.
|
||||
|
||||
``\Twig\Loader\FilesystemLoader`` supports absolute and relative paths. Using relative
|
||||
paths is preferred as it makes the cache keys independent of the project root
|
||||
directory (for instance, it allows warming the cache from a build server where
|
||||
|
||||
@@ -472,6 +472,13 @@ Testing Utilities
|
||||
* The data providers ``getTests()`` and ``getLegacyTests()`` on
|
||||
``Twig\Test\IntegrationTestCase`` are considered final as of Twig 3.13.
|
||||
|
||||
Loaders
|
||||
-------
|
||||
|
||||
* Overriding ``Twig\Loader\FilesystemLoader::addPath()`` or ``prependPath()``
|
||||
without declaring their ``bool $check = true`` third argument is deprecated as
|
||||
of Twig 3.31; the argument will be part of their signature in Twig 4.0.
|
||||
|
||||
Environment
|
||||
-----------
|
||||
|
||||
|
||||
@@ -7,6 +7,12 @@ parameters:
|
||||
path: src/Sandbox/SecurityPolicyInterface.php
|
||||
|
||||
|
||||
- # The "$check" parameter is documented but not declared, as overrides of these methods with 2 parameters would break
|
||||
message: '#^PHPDoc tag @param references unknown parameter\: \$check$#'
|
||||
identifier: parameter.notFound
|
||||
count: 2
|
||||
path: src/Loader/FilesystemLoader.php
|
||||
|
||||
- # The "$tests" argument will be part of the signature in 4.0; 3-parameter implementations are detected and called with 3 arguments
|
||||
message: '#^Method Twig\\Sandbox\\SecurityPolicyInterface\:\:checkSecurity\(\) invoked with 4 parameters, 3 required\.$#'
|
||||
identifier: arguments.count
|
||||
|
||||
@@ -87,32 +87,40 @@ class FilesystemLoader implements LoaderInterface
|
||||
}
|
||||
|
||||
/**
|
||||
* @param bool $check Whether to check that the directory exists; pass false when the caller already checked it
|
||||
*
|
||||
* @throws LoaderError
|
||||
*/
|
||||
public function addPath(string $path, string $namespace = self::MAIN_NAMESPACE): void
|
||||
public function addPath(string $path, string $namespace = self::MAIN_NAMESPACE/* , bool $check = true */): void
|
||||
{
|
||||
// invalidate the cache
|
||||
$this->cache = $this->errorCache = [];
|
||||
|
||||
$checkPath = $this->isAbsolutePath($path) ? $path : $this->rootPath.$path;
|
||||
if (!is_dir($checkPath)) {
|
||||
throw new LoaderError(\sprintf('The "%s" directory does not exist ("%s").', $path, $checkPath));
|
||||
if (\func_num_args() < 3 || func_get_arg(2)) {
|
||||
$checkPath = $this->isAbsolutePath($path) ? $path : $this->rootPath.$path;
|
||||
if (!is_dir($checkPath)) {
|
||||
throw new LoaderError(\sprintf('The "%s" directory does not exist ("%s").', $path, $checkPath));
|
||||
}
|
||||
}
|
||||
|
||||
$this->paths[$namespace][] = rtrim($path, '/\\');
|
||||
}
|
||||
|
||||
/**
|
||||
* @param bool $check Whether to check that the directory exists; pass false when the caller already checked it
|
||||
*
|
||||
* @throws LoaderError
|
||||
*/
|
||||
public function prependPath(string $path, string $namespace = self::MAIN_NAMESPACE): void
|
||||
public function prependPath(string $path, string $namespace = self::MAIN_NAMESPACE/* , bool $check = true */): void
|
||||
{
|
||||
// invalidate the cache
|
||||
$this->cache = $this->errorCache = [];
|
||||
|
||||
$checkPath = $this->isAbsolutePath($path) ? $path : $this->rootPath.$path;
|
||||
if (!is_dir($checkPath)) {
|
||||
throw new LoaderError(\sprintf('The "%s" directory does not exist ("%s").', $path, $checkPath));
|
||||
if (\func_num_args() < 3 || func_get_arg(2)) {
|
||||
$checkPath = $this->isAbsolutePath($path) ? $path : $this->rootPath.$path;
|
||||
if (!is_dir($checkPath)) {
|
||||
throw new LoaderError(\sprintf('The "%s" directory does not exist ("%s").', $path, $checkPath));
|
||||
}
|
||||
}
|
||||
|
||||
$path = rtrim($path, '/\\');
|
||||
|
||||
@@ -163,6 +163,75 @@ class FilesystemTest extends TestCase
|
||||
$this->assertEquals([FilesystemLoader::MAIN_NAMESPACE, 'named'], $loader->getNamespaces());
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider getPathsThatAreNotDirectories
|
||||
*/
|
||||
#[DataProvider('getPathsThatAreNotDirectories')]
|
||||
public function testAddingAPathThatIsNotADirectoryThrows(string $method, string $path, array $extraArgs): void
|
||||
{
|
||||
$loader = new FilesystemLoader([], __DIR__);
|
||||
|
||||
try {
|
||||
$loader->$method($path, 'named', ...$extraArgs);
|
||||
$this->fail('A LoaderError should have been thrown.');
|
||||
} catch (LoaderError $e) {
|
||||
$this->assertSame(\sprintf('The "%s" directory does not exist ("%s").', $path, realpath(__DIR__).\DIRECTORY_SEPARATOR.$path), $e->getMessage());
|
||||
}
|
||||
|
||||
$this->assertSame([], $loader->getPaths('named'));
|
||||
}
|
||||
|
||||
public static function getPathsThatAreNotDirectories(): iterable
|
||||
{
|
||||
foreach (['addPath', 'prependPath'] as $method) {
|
||||
foreach (['Fixtures/missing', 'Fixtures/normal/index.html'] as $path) {
|
||||
yield [$method, $path, []];
|
||||
yield [$method, $path, [true]];
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider getPathMethods
|
||||
*/
|
||||
#[DataProvider('getPathMethods')]
|
||||
public function testAddingAPathWithoutCheckingItAcceptsAMissingDirectory(string $method): void
|
||||
{
|
||||
$loader = new FilesystemLoader([], __DIR__);
|
||||
$loader->addPath('Fixtures/named', 'named', false);
|
||||
$loader->$method('Fixtures/missing/', 'named', false);
|
||||
|
||||
$expected = 'addPath' === $method ? ['Fixtures/named', 'Fixtures/missing'] : ['Fixtures/missing', 'Fixtures/named'];
|
||||
$this->assertSame($expected, $loader->getPaths('named'));
|
||||
$this->assertSame("named path\n", $loader->getSourceContext('@named/index.html')->getCode());
|
||||
|
||||
$this->expectException(LoaderError::class);
|
||||
$this->expectExceptionMessage(\sprintf('Unable to find template "@named/nowhere.html" (looked into: %s).', implode(', ', $expected)));
|
||||
|
||||
$loader->getSourceContext('@named/nowhere.html');
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider getPathMethods
|
||||
*/
|
||||
#[DataProvider('getPathMethods')]
|
||||
public function testAddingAPathWithoutCheckingItInvalidatesTheCache(string $method): void
|
||||
{
|
||||
$loader = new FilesystemLoader([], __DIR__);
|
||||
$loader->addPath('Fixtures/normal', 'named');
|
||||
$this->assertFalse($loader->exists('@named/named_absolute.html'));
|
||||
|
||||
$loader->$method('Fixtures/named_quater', 'named', false);
|
||||
|
||||
$this->assertSame("named path (quater)\n", $loader->getSourceContext('@named/named_absolute.html')->getCode());
|
||||
}
|
||||
|
||||
public static function getPathMethods(): iterable
|
||||
{
|
||||
yield ['addPath'];
|
||||
yield ['prependPath'];
|
||||
}
|
||||
|
||||
public function testFindTemplateExceptionNamespace(): void
|
||||
{
|
||||
$basePath = __DIR__.'/Fixtures';
|
||||
|
||||
Reference in New Issue
Block a user