Skip to content

Commit ff194d9

Browse files
nicolas-grekasnicolas-grekas
committed
minor #33792 Sync Twig templateExists behaviors (fancyweb)
This PR was merged into the 3.4 branch. Discussion ---------- Sync Twig templateExists behaviors | Q | A | ------------- | --- | Branch? | 4.3 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - There are 5 places in the code that does the same check, but they are not consistent. The difficulty here is to support Twig 1, 2 & 3. I think relying on `Environment::MAJOR_VERSION` is OK. `method_exists` are useless IMO. The proof is that they are currenty missing in some of them. Basically if Twig >= 2 -> the method `exists` exists on the loader so just call it. For Twig 1, check `ExistsLoaderInterface`, then check for `SourceContextLoaderInterface`, then call `getSource()`. Commits ------- d7682fe Sync Twig templateExists behaviors
2 parents d2b66ff + d7682fe commit ff194d9

File tree

6 files changed

+79
-51
lines changed

6 files changed

+79
-51
lines changed

‎src/Symfony/Bridge/Twig/TwigEngine.php

+15-9
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use Twig\Error\Error;
2020
use Twig\Error\LoaderError;
2121
use Twig\Loader\ExistsLoaderInterface;
22+
use Twig\Loader\SourceContextLoaderInterface;
2223
use Twig\Template;
2324

2425
/**
@@ -74,19 +75,24 @@ public function exists($name)
7475

7576
$loader = $this->environment->getLoader();
7677

77-
if ($loader instanceof ExistsLoaderInterface || method_exists($loader, 'exists')) {
78-
return $loader->exists((string) $name);
79-
}
78+
if (1 === Environment::MAJOR_VERSION && !$loader instanceof ExistsLoaderInterface) {
79+
try {
80+
// cast possible TemplateReferenceInterface to string because the
81+
// EngineInterface supports them but LoaderInterface does not
82+
if ($loader instanceof SourceContextLoaderInterface) {
83+
$loader->getSourceContext((string) $name);
84+
} else {
85+
$loader->getSource((string) $name);
86+
}
87+
88+
return true;
89+
} catch (LoaderError $e) {
90+
}
8091

81-
try {
82-
// cast possible TemplateReferenceInterface to string because the
83-
// EngineInterface supports them but LoaderInterface does not
84-
$loader->getSourceContext((string) $name)->getCode();
85-
} catch (LoaderError $e) {
8692
return false;
8793
}
8894

89-
return true;
95+
return $loader->exists((string) $name);
9096
}
9197

9298
/**

‎src/Symfony/Bundle/TwigBundle/Controller/ExceptionController.php

+15-9
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Twig\Environment;
1919
use Twig\Error\LoaderError;
2020
use Twig\Loader\ExistsLoaderInterface;
21+
use Twig\Loader\SourceContextLoaderInterface;
2122

2223
/**
2324
* ExceptionController renders error or exception pages for a given
@@ -120,23 +121,28 @@ protected function findTemplate(Request $request, $format, $code, $showException
120121
return sprintf('@Twig/Exception/%s.html.twig', $showException ? 'exception_full' : $name);
121122
}
122123

123-
// to be removed when the minimum required version of Twig is >= 3.0
124+
// to be removed when the minimum required version of Twig is >= 2.0
124125
protected function templateExists($template)
125126
{
126127
$template = (string) $template;
127128

128129
$loader = $this->twig->getLoader();
129-
if ($loader instanceof ExistsLoaderInterface || method_exists($loader, 'exists')) {
130-
return $loader->exists($template);
131-
}
132130

133-
try {
134-
$loader->getSourceContext($template)->getCode();
131+
if (1 === Environment::MAJOR_VERSION && !$loader instanceof ExistsLoaderInterface) {
132+
try {
133+
if ($loader instanceof SourceContextLoaderInterface) {
134+
$loader->getSourceContext($template);
135+
} else {
136+
$loader->getSource($template);
137+
}
138+
139+
return true;
140+
} catch (LoaderError $e) {
141+
}
135142

136-
return true;
137-
} catch (LoaderError $e) {
143+
return false;
138144
}
139145

140-
return false;
146+
return $loader->exists($template);
141147
}
142148
}

‎src/Symfony/Bundle/WebProfilerBundle/Controller/ExceptionController.php

+14-8
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use Twig\Environment;
2020
use Twig\Error\LoaderError;
2121
use Twig\Loader\ExistsLoaderInterface;
22+
use Twig\Loader\SourceContextLoaderInterface;
2223

2324
/**
2425
* ExceptionController.
@@ -118,17 +119,22 @@ protected function getTemplate()
118119
protected function templateExists($template)
119120
{
120121
$loader = $this->twig->getLoader();
121-
if ($loader instanceof ExistsLoaderInterface) {
122-
return $loader->exists($template);
123-
}
124122

125-
try {
126-
$loader->getSource($template);
123+
if (1 === Environment::MAJOR_VERSION && !$loader instanceof ExistsLoaderInterface) {
124+
try {
125+
if ($loader instanceof SourceContextLoaderInterface) {
126+
$loader->getSourceContext($template);
127+
} else {
128+
$loader->getSource($template);
129+
}
130+
131+
return true;
132+
} catch (LoaderError $e) {
133+
}
127134

128-
return true;
129-
} catch (LoaderError $e) {
135+
return false;
130136
}
131137

132-
return false;
138+
return $loader->exists($template);
133139
}
134140
}

‎src/Symfony/Bundle/WebProfilerBundle/Profiler/TemplateManager.php

+12-11
Original file line numberDiff line numberDiff line change
@@ -117,21 +117,22 @@ public function getNames(Profile $profile)
117117
protected function templateExists($template)
118118
{
119119
$loader = $this->twig->getLoader();
120-
if ($loader instanceof ExistsLoaderInterface) {
121-
return $loader->exists($template);
122-
}
123120

124-
try {
125-
if ($loader instanceof SourceContextLoaderInterface || method_exists($loader, 'getSourceContext')) {
126-
$loader->getSourceContext($template);
127-
} else {
128-
$loader->getSource($template);
121+
if (1 === Environment::MAJOR_VERSION && !$loader instanceof ExistsLoaderInterface) {
122+
try {
123+
if ($loader instanceof SourceContextLoaderInterface) {
124+
$loader->getSourceContext($template);
125+
} else {
126+
$loader->getSource($template);
127+
}
128+
129+
return true;
130+
} catch (LoaderError $e) {
129131
}
130132

131-
return true;
132-
} catch (LoaderError $e) {
133+
return false;
133134
}
134135

135-
return false;
136+
return $loader->exists($template);
136137
}
137138
}

‎src/Symfony/Bundle/WebProfilerBundle/Tests/Profiler/TemplateManagerTest.php

+10-3
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
use Symfony\Bundle\WebProfilerBundle\Tests\TestCase;
1616
use Symfony\Component\HttpKernel\Profiler\Profile;
1717
use Twig\Environment;
18+
use Twig\Loader\LoaderInterface;
19+
use Twig\Loader\SourceContextLoaderInterface;
1820

1921
/**
2022
* Test for TemplateManager class.
@@ -124,11 +126,16 @@ protected function mockTwigEnvironment()
124126
->method('loadTemplate')
125127
->willReturn('loadedTemplate');
126128

127-
if (interface_exists('Twig\Loader\SourceContextLoaderInterface')) {
128-
$loader = $this->getMockBuilder('Twig\Loader\SourceContextLoaderInterface')->getMock();
129+
if (Environment::MAJOR_VERSION > 1) {
130+
$loader = $this->createMock(LoaderInterface::class);
131+
$loader
132+
->expects($this->any())
133+
->method('exists')
134+
->willReturn(true);
129135
} else {
130-
$loader = $this->getMockBuilder('Twig\Loader\LoaderInterface')->getMock();
136+
$loader = $this->createMock(SourceContextLoaderInterface::class);
131137
}
138+
132139
$this->twigEnvironment->expects($this->any())->method('getLoader')->willReturn($loader);
133140

134141
return $this->twigEnvironment;

‎src/Symfony/Component/HttpKernel/Fragment/HIncludeFragmentRenderer.php

+13-11
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use Twig\Environment;
2020
use Twig\Error\LoaderError;
2121
use Twig\Loader\ExistsLoaderInterface;
22+
use Twig\Loader\SourceContextLoaderInterface;
2223

2324
/**
2425
* Implements the Hinclude rendering strategy.
@@ -137,22 +138,23 @@ private function templateExists($template)
137138
}
138139

139140
$loader = $this->templating->getLoader();
140-
if ($loader instanceof ExistsLoaderInterface || method_exists($loader, 'exists')) {
141-
return $loader->exists($template);
142-
}
143141

144-
try {
145-
if (method_exists($loader, 'getSourceContext')) {
146-
$loader->getSourceContext($template);
147-
} else {
148-
$loader->getSource($template);
142+
if (1 === Environment::MAJOR_VERSION && !$loader instanceof ExistsLoaderInterface) {
143+
try {
144+
if ($loader instanceof SourceContextLoaderInterface) {
145+
$loader->getSourceContext($template);
146+
} else {
147+
$loader->getSource($template);
148+
}
149+
150+
return true;
151+
} catch (LoaderError $e) {
149152
}
150153

151-
return true;
152-
} catch (LoaderError $e) {
154+
return false;
153155
}
154156

155-
return false;
157+
return $loader->exists($template);
156158
}
157159

158160
/**

0 commit comments

Comments
 (0)