Issues in TemplateManifest.php - Fixed Issues - Inspection of "API Theme stacking" - silverstripe/silverstripe-framework - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#5804)

by Hamish

created 2016-07-14 21:52 UTC

core/manifest/TemplateManifest.php (4 issues)

Labels

Severity

Upgrade to new PHP Analysis Engine

These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more

<?php
/**
 * A class which builds a manifest of all templates present in a directory,
 * in both modules and themes.
 *
 * @package framework
 * @subpackage manifest
 */
class SS_TemplateManifest {

	const TEMPLATES_DIR = 'templates';

	protected $base;
	protected $tests;
	protected $cache;
	protected $cacheKey;
	protected $project;
	protected $inited;
	protected $templates = array();

	/**
	 * Constructs a new template manifest. The manifest is not actually built
	 * or loaded from cache until needed.
	 *
	 * @param string $base The base path.
	 * @param string $project Path to application code
	 *
	 * @param bool $includeTests Include tests in the manifest.
	 * @param bool $forceRegen Force the manifest to be regenerated.
	 */
	public function __construct($base, $project, $includeTests = false, $forceRegen = false) {
		$this->base  = $base;
		$this->tests = $includeTests;

		$this->project = $project;

		$cacheClass = defined('SS_MANIFESTCACHE') ? SS_MANIFESTCACHE : 'ManifestCache_File';

		$this->cache = new $cacheClass('templatemanifest'.($includeTests ? '_tests' : ''));
		$this->cacheKey = $this->getCacheKey($includeTests);

		if ($forceRegen) {
			$this->regenerate();
		}
	}

	/**
	 * @return string
	 */
	public function getBase() {
		return $this->base;
	}

	/**
	 * Generate a unique cache key to avoid manifest cache collisions.
	 * We compartmentalise based on the base path, the given project, and whether
	 * or not we intend to include tests.
	 * @param boolean $includeTests
	 * @return string
	 */
	public function getCacheKey($includeTests = false) {
		return sha1(sprintf(
			"manifest-%s-%s-%s",
				$this->base,
				$this->project,
				(int) $includeTests // cast true to 1, false to 0
			)
		);
	}

	/**
	 * Returns a map of all template information. The map is in the following
	 * format:
	 *
	 * <code>
	 *   array(
	 *     'moduletemplate' => array(
	 *       'main' => '/path/to/module/templates/Main.ss'
	 *     ),
	 *     'include' => array(
	 *       'include' => '/path/to/module/templates/Includes/Include.ss'
	 *     ),
	 *     'page' => array(
	 *       'themes' => array(
	 *         'simple' => array(
	 *           'main'   => '/path/to/theme/Page.ss'
	 *           'Layout' => '/path/to/theme/Layout/Page.ss'
	 *         )
	 *       )
	 *     )
	 *   )
	 * </code>
	 *
	 * @return array
	 */
	public function getTemplates() {
		if (!$this->inited) {
			$this->init();
		}

		return $this->templates;
	}

	/**
	 * Returns a set of possible candidate templates that match a certain
	 * template name.
	 *
	 * This is the same as extracting an individual array element from
	 * {@link SS_TemplateManifest::getTemplates()}.
	 *
	 * @param  string $name
	 * @return array
	 */
	public function getTemplate($name) {
		if (!$this->inited) {
			$this->init();
		}

		$name = strtolower($name);

		if (array_key_exists($name, $this->templates)) {
			return $this->templates[$name];
		} else {
			return array();
		}
	}

	/**
	 * Returns the correct candidate template. In order of importance, application
	 * project code, current theme and finally modules.
	 *
	 * @param string $name
	 * @param string $theme - theme name
	 *
	 * @return array
	 */
	public function getCandidateTemplate($name, $theme = null) {
		$found = array();
		$candidates = $this->getTemplate($name);

		// theme overrides modules
		if ($theme && isset($candidates['themes'][$theme])) {
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
			$found = array_merge($candidates, $candidates['themes'][$theme]);
		}
		// project overrides theme
		if ($this->project && isset($candidates[$this->project])) {
			$found = array_merge($found, $candidates[$this->project]);
		}

		$found = ($found) ? $found : $candidates;

		if (isset($found['themes'])) unset($found['themes']);
		if (isset($found[$this->project])) unset($found[$this->project]);

		return $found;
	}

	/**
	 * Regenerates the manifest by scanning the base path.
	 *
	 * @param bool $cache
	 */
	public function regenerate($cache = true) {
		$finder = new ManifestFileFinder();
		$finder->setOptions(array(
			'name_regex'     => '/\.ss$/',
			'include_themes' => true,
			'ignore_tests'  => !$this->tests,
			'file_callback'  => array($this, 'handleFile')
		));

		$finder->find($this->base);

		if ($cache) {
			$this->cache->save($this->templates, $this->cacheKey);
		}

		$this->inited = true;
	}

	public function handleFile($basename, $pathname, $depth)

	{
		$projectFile = false;
		$theme = null;

		// Template in theme
		if (preg_match(
			'#'.preg_quote($this->base.'/'.THEMES_DIR).'/([^/_]+)(_[^/]+)?/(.*)$#',
			$pathname,
			$matches
		)) {
			$theme = $matches[1];
			$relPath = $matches[3];

		// Template in project
		} elseif (preg_match(
			'#'.preg_quote($this->base.'/'.$this->project).'/(.*)$#',
			$pathname,
			$matches
		)) {
			$projectFile = true;
			$relPath = $matches[1];

		// Template in module
		} elseif (preg_match(
			'#'.preg_quote($this->base).'/([^/]+)/(.*)$#',
			$pathname,
			$matches
		)) {
			$relPath = $matches[2];

		} else {
			throw new \LogicException("Can't determine meaning of path: $pathname");
		}

		// If a templates subfolder is used, ignore that
		if (preg_match('#'.preg_quote(self::TEMPLATES_DIR).'/(.*)$#', $relPath, $matches)) {
			$relPath = $matches[1];
		}

		// Layout and Content folders have special meaning
		if (preg_match('#^(.*/)?(Layout|Content|Includes)/([^/]+)$#', $relPath, $matches)) {
			$type = $matches[2];
			$relPath = "$matches[1]$matches[3]";
		} else {
			$type = "main";
		}

		$name = strtolower(substr($relPath, 0, -3));
		$name = str_replace('/', '\\', $name);

		if ($theme) {
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
			$this->templates[$name]['themes'][$theme][$type] = $pathname;
		} else if ($projectFile) {
			$this->templates[$name][$this->project][$type] = $pathname;
		} else {
			$this->templates[$name][$type] = $pathname;
		}

		// If we've found a template in a subdirectory, then allow its use for a non-namespaced class
		// as well. This was a common SilverStripe 3 approach, where templates were placed into
		// subfolders to suit the whim of the developer.
		if (strpos($name, '\\') !== false) {
			$name2 = substr($name, strrpos($name, '\\') + 1);
			// In of these cases, the template will only be provided if it isn't already set. This
			// matches SilverStripe 3 prioritisation.
			if ($theme) {
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
				if (!isset($this->templates[$name2]['themes'][$theme][$type])) {
					$this->templates[$name2]['themes'][$theme][$type] = $pathname;
				}
			} else if ($projectFile) {
				if (!isset($this->templates[$name2][$this->project][$type])) {
					$this->templates[$name2][$this->project][$type] = $pathname;
				}
			} else {
				if (!isset($this->templates[$name2][$type])) {
					$this->templates[$name2][$type] = $pathname;
				}
			}
		}
	}

	protected function init() {
		if ($data = $this->cache->load($this->cacheKey)) {
			$this->templates = $data;
			$this->inited    = true;
		} else {
			$this->regenerate();
		}
	}
}


1			<?php
2			/**
3			* A class which builds a manifest of all templates present in a directory,
4			* in both modules and themes.
5			*
6			* @package framework
7			* @subpackage manifest
8			*/
9			class SS_TemplateManifest {
10
11			const TEMPLATES_DIR = 'templates';
12
13			protected $base;
14			protected $tests;
15			protected $cache;
16			protected $cacheKey;
17			protected $project;
18			protected $inited;
19			protected $templates = array();
20
21			/**
22			* Constructs a new template manifest. The manifest is not actually built
23			* or loaded from cache until needed.
24			*
25			* @param string $base The base path.
26			* @param string $project Path to application code
27			*
28			* @param bool $includeTests Include tests in the manifest.
29			* @param bool $forceRegen Force the manifest to be regenerated.
30			*/
31			public function __construct($base, $project, $includeTests = false, $forceRegen = false) {
32			$this->base = $base;
33			$this->tests = $includeTests;
34
35			$this->project = $project;
36
37			$cacheClass = defined('SS_MANIFESTCACHE') ? SS_MANIFESTCACHE : 'ManifestCache_File';
38
39			$this->cache = new $cacheClass('templatemanifest'.($includeTests ? '_tests' : ''));
40			$this->cacheKey = $this->getCacheKey($includeTests);
41
42			if ($forceRegen) {
43			$this->regenerate();
44			}
45			}
46
47			/**
48			* @return string
49			*/
50			public function getBase() {
51			return $this->base;
52			}
53
54			/**
55			* Generate a unique cache key to avoid manifest cache collisions.
56			* We compartmentalise based on the base path, the given project, and whether
57			* or not we intend to include tests.
58			* @param boolean $includeTests
59			* @return string
60			*/
61			public function getCacheKey($includeTests = false) {
62			return sha1(sprintf(
63			"manifest-%s-%s-%s",
64			$this->base,
65			$this->project,
66			(int) $includeTests // cast true to 1, false to 0
67			)
68			);
69			}
70
71			/**
72			* Returns a map of all template information. The map is in the following
73			* format:
74			*
75			* <code>
76			* array(
77			* 'moduletemplate' => array(
78			* 'main' => '/path/to/module/templates/Main.ss'
79			* ),
80			* 'include' => array(
81			* 'include' => '/path/to/module/templates/Includes/Include.ss'
82			* ),
83			* 'page' => array(
84			* 'themes' => array(
85			* 'simple' => array(
86			* 'main' => '/path/to/theme/Page.ss'
87			* 'Layout' => '/path/to/theme/Layout/Page.ss'
88			* )
89			* )
90			* )
91			* )
92			* </code>
93			*
94			* @return array
95			*/
96			public function getTemplates() {
97			if (!$this->inited) {
98			$this->init();
99			}
100
101			return $this->templates;
102			}
103
104			/**
105			* Returns a set of possible candidate templates that match a certain
106			* template name.
107			*
108			* This is the same as extracting an individual array element from
109			* {@link SS_TemplateManifest::getTemplates()}.
110			*
111			* @param string $name
112			* @return array
113			*/
114			public function getTemplate($name) {
115			if (!$this->inited) {
116			$this->init();
117			}
118
119			$name = strtolower($name);
120
121			if (array_key_exists($name, $this->templates)) {
122			return $this->templates[$name];
123			} else {
124			return array();
125			}
126			}
127
128			/**
129			* Returns the correct candidate template. In order of importance, application
130			* project code, current theme and finally modules.
131			*
132			* @param string $name
133			* @param string $theme - theme name
134			*
135			* @return array
136			*/
137			public function getCandidateTemplate($name, $theme = null) {
138			$found = array();
139			$candidates = $this->getTemplate($name);
140
141			// theme overrides modules
142			if ($theme && isset($candidates['themes'][$theme])) {
			0 ignored issues – show Bug Best Practice introduced 2015-11-25 21:50 UTC by Report Bug Copy Issue Report Show Similar Issues like this The expression `$theme` of type `string\|null` is loosely compared to `true`; this is ambiguous if the string can be empty. You might want to explicitly use `!== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `string` values, the empty string `''` is a special case, in particular the following results might be unexpected: '' == false // true '' == null // true 'ab' == false // false 'ab' == null // false // It is often better to use strict comparison '' === false // false '' === null // false Loading history...
143			$found = array_merge($candidates, $candidates['themes'][$theme]);
144			}
145			// project overrides theme
146			if ($this->project && isset($candidates[$this->project])) {
147			$found = array_merge($found, $candidates[$this->project]);
148			}
149
150			$found = ($found) ? $found : $candidates;
151
152			if (isset($found['themes'])) unset($found['themes']);
153			if (isset($found[$this->project])) unset($found[$this->project]);
154
155			return $found;
156			}
157
158			/**
159			* Regenerates the manifest by scanning the base path.
160			*
161			* @param bool $cache
162			*/
163			public function regenerate($cache = true) {
164			$finder = new ManifestFileFinder();
165			$finder->setOptions(array(
166			'name_regex' => '/\.ss$/',
167			'include_themes' => true,
168			'ignore_tests' => !$this->tests,
169			'file_callback' => array($this, 'handleFile')
170			));
171
172			$finder->find($this->base);
173
174			if ($cache) {
175			$this->cache->save($this->templates, $this->cacheKey);
176			}
177
178			$this->inited = true;
179			}
180
181			public function handleFile($basename, $pathname, $depth)
			0 ignored issues – show Unused Code introduced 2015-11-25 21:50 UTC by Report Bug Copy Issue Report Show Similar Issues like this The parameter `$depth` is not used and could be removed. This check looks from parameters that have been defined for a function or method, but which are not used in the method body. Loading history...
182			{
183			$projectFile = false;
184			$theme = null;
185
186			// Template in theme
187			if (preg_match(
188			'#'.preg_quote($this->base.'/'.THEMES_DIR).'/([^/_]+)(_[^/]+)?/(.*)$#',
189			$pathname,
190			$matches
191			)) {
192			$theme = $matches[1];
193			$relPath = $matches[3];
194
195			// Template in project
196			} elseif (preg_match(
197			'#'.preg_quote($this->base.'/'.$this->project).'/(.*)$#',
198			$pathname,
199			$matches
200			)) {
201			$projectFile = true;
202			$relPath = $matches[1];
203
204			// Template in module
205			} elseif (preg_match(
206			'#'.preg_quote($this->base).'/([^/]+)/(.*)$#',
207			$pathname,
208			$matches
209			)) {
210			$relPath = $matches[2];
211
212			} else {
213			throw new \LogicException("Can't determine meaning of path: $pathname");
214			}
215
216			// If a templates subfolder is used, ignore that
217			if (preg_match('#'.preg_quote(self::TEMPLATES_DIR).'/(.*)$#', $relPath, $matches)) {
218			$relPath = $matches[1];
219			}
220
221			// Layout and Content folders have special meaning
222			if (preg_match('#^(.*/)?(Layout\|Content\|Includes)/([^/]+)$#', $relPath, $matches)) {
223			$type = $matches[2];
224			$relPath = "$matches[1]$matches[3]";
225			} else {
226			$type = "main";
227			}
228
229			$name = strtolower(substr($relPath, 0, -3));
230			$name = str_replace('/', '\\', $name);
231
232			if ($theme) {
			0 ignored issues – show Bug Best Practice introduced 2015-11-25 21:50 UTC by Report Bug Copy Issue Report Show Similar Issues like this The expression `$theme` of type `string\|null` is loosely compared to `true`; this is ambiguous if the string can be empty. You might want to explicitly use `!== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `string` values, the empty string `''` is a special case, in particular the following results might be unexpected: '' == false // true '' == null // true 'ab' == false // false 'ab' == null // false // It is often better to use strict comparison '' === false // false '' === null // false Loading history...
233			$this->templates[$name]['themes'][$theme][$type] = $pathname;
234			} else if ($projectFile) {
235			$this->templates[$name][$this->project][$type] = $pathname;
236			} else {
237			$this->templates[$name][$type] = $pathname;
238			}
239
240			// If we've found a template in a subdirectory, then allow its use for a non-namespaced class
241			// as well. This was a common SilverStripe 3 approach, where templates were placed into
242			// subfolders to suit the whim of the developer.
243			if (strpos($name, '\\') !== false) {
244			$name2 = substr($name, strrpos($name, '\\') + 1);
245			// In of these cases, the template will only be provided if it isn't already set. This
246			// matches SilverStripe 3 prioritisation.
247			if ($theme) {
			0 ignored issues – show Bug Best Practice introduced 2016-05-04 07:30 UTC by Report Bug Copy Issue Report Show Similar Issues like this The expression `$theme` of type `string\|null` is loosely compared to `true`; this is ambiguous if the string can be empty. You might want to explicitly use `!== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `string` values, the empty string `''` is a special case, in particular the following results might be unexpected: '' == false // true '' == null // true 'ab' == false // false 'ab' == null // false // It is often better to use strict comparison '' === false // false '' === null // false Loading history...
248			if (!isset($this->templates[$name2]['themes'][$theme][$type])) {
249			$this->templates[$name2]['themes'][$theme][$type] = $pathname;
250			}
251			} else if ($projectFile) {
252			if (!isset($this->templates[$name2][$this->project][$type])) {
253			$this->templates[$name2][$this->project][$type] = $pathname;
254			}
255			} else {
256			if (!isset($this->templates[$name2][$type])) {
257			$this->templates[$name2][$type] = $pathname;
258			}
259			}
260			}
261			}
262
263			protected function init() {
264			if ($data = $this->cache->load($this->cacheKey)) {
265			$this->templates = $data;
266			$this->inited = true;
267			} else {
268			$this->regenerate();
269			}
270			}
271			}
272

silverstripe / silverstripe-framework

Pull Request — master (#5804)

core/manifest/TemplateManifest.php (4 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Duplication Side-by-Side

Filter issues like