Merge pull request #51385 from nextcloud/fix/cleanup-template-functions

fix: Remove multiple require_once calls for template functions

8 files changed, 53 insertions, 73 deletions

diff --git a/build/psalm-baseline-ocp.xml b/build/psalm-baseline-ocp.xml
index ce406cfde75..ed932f8d487 100644
--- a/build/psalm-baseline-ocp.xml
+++ b/build/psalm-baseline-ocp.xml
@@ -53,17 +53,6 @@
       <code><![CDATA[\Iterator]]></code>
     </MissingTemplateParam>
   </file>
-  <file src="lib/public/Template.php">
-    <UndefinedFunction>
-      <code><![CDATA[\html_select_options($options, $selected, $params)]]></code>
-      <code><![CDATA[\human_file_size($bytes)]]></code>
-      <code><![CDATA[\image_path($app, $image)]]></code>
-      <code><![CDATA[\mimetype_icon($mimetype)]]></code>
-      <code><![CDATA[\preview_icon($path)]]></code>
-      <code><![CDATA[\publicPreview_icon($path, $token)]]></code>
-      <code><![CDATA[\relative_modified_date($timestamp, null, $dateOnly)]]></code>
-    </UndefinedFunction>
-  </file>
   <file src="lib/public/Util.php">
     <UndefinedClass>
       <code><![CDATA[\OC]]></code>
diff --git a/lib/private/Template/Base.php b/lib/private/Template/Base.php
index 0dd7d92d6be..a13e6703960 100644
--- a/lib/private/Template/Base.php
+++ b/lib/private/Template/Base.php
@@ -136,6 +136,7 @@ class Base {
 		// Include
 		ob_start();
 		try {
+			require_once __DIR__ . '/functions.php';
 			include $file;
 			$data = ob_get_contents();
 		} catch (\Exception $e) {
diff --git a/lib/private/Template/Template.php b/lib/private/Template/Template.php
index b69d68b944f..ee85562091f 100644
--- a/lib/private/Template/Template.php
+++ b/lib/private/Template/Template.php
@@ -21,8 +21,6 @@ use OCP\Template\ITemplate;
 use OCP\Template\TemplateNotFoundException;
 use OCP\Util;
 
-require_once __DIR__ . '/../legacy/template/functions.php';
-
 class Template extends Base implements ITemplate {
 	private string $path;
 	private array $headers = [];
diff --git a/lib/private/legacy/template/functions.php b/lib/private/Template/functions.php
index 37df801c3c4..4354d0d8fce 100644
--- a/lib/private/legacy/template/functions.php
+++ b/lib/private/Template/functions.php
@@ -6,20 +6,23 @@
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 
+use OC\Security\CSP\ContentSecurityPolicyNonceManager;
+use OCP\Files\IMimeTypeDetector;
 use OCP\IDateTimeFormatter;
+use OCP\IURLGenerator;
+use OCP\Server;
 use OCP\Util;
 
-function p($string) {
-	print(\OCP\Util::sanitizeHTML($string));
+function p(string $string): void {
+	print(Util::sanitizeHTML($string));
 }
 
-
 /**
  * Prints a <link> tag for loading css
  * @param string $href the source URL, ignored when empty
  * @param string $opts, additional optional options
  */
-function emit_css_tag($href, $opts = '') {
+function emit_css_tag($href, $opts = ''): void {
 	$s = '<link rel="stylesheet"';
 	if (!empty($href)) {
 		$s .= ' href="' . $href . '"';
@@ -34,7 +37,7 @@ function emit_css_tag($href, $opts = '') {
  * Prints all tags for CSS loading
  * @param array $obj all the script information from template
  */
-function emit_css_loading_tags($obj) {
+function emit_css_loading_tags($obj): void {
 	foreach ($obj['cssfiles'] as $css) {
 		emit_css_tag($css);
 	}
@@ -49,8 +52,8 @@ function emit_css_loading_tags($obj) {
  * @param string $script_content the inline script content, ignored when empty
  * @param string $content_type the type of the source (e.g. 'module')
  */
-function emit_script_tag(string $src, string $script_content = '', string $content_type = '') {
-	$nonceManager = \OC::$server->get(\OC\Security\CSP\ContentSecurityPolicyNonceManager::class);
+function emit_script_tag(string $src, string $script_content = '', string $content_type = ''): void {
+	$nonceManager = Server::get(ContentSecurityPolicyNonceManager::class);
 
 	$defer_str = ' defer';
 	$type = $content_type !== '' ? ' type="' . $content_type . '"' : '';
@@ -74,7 +77,7 @@ function emit_script_tag(string $src, string $script_content = '', string $conte
  * Print all <script> tags for loading JS
  * @param array $obj all the script information from template
  */
-function emit_script_loading_tags($obj) {
+function emit_script_loading_tags($obj): void {
 	foreach ($obj['jsfiles'] as $jsfile) {
 		$fileName = explode('?', $jsfile, 2)[0];
 		$type = str_ends_with($fileName, '.mjs') ? 'module' : '';
@@ -88,9 +91,9 @@ function emit_script_loading_tags($obj) {
 /**
  * Prints an unsanitized string - usage of this function may result into XSS.
  * Consider using p() instead.
- * @param string|array $string the string which will be printed as it is
+ * @param string $string the string which will be printed as it is
  */
-function print_unescaped($string) {
+function print_unescaped($string): void {
 	print($string);
 }
 
@@ -106,7 +109,7 @@ function print_unescaped($string) {
  * @param string|string[] $file the filename,
  *                              if an array is given it will add all scripts
  */
-function script($app, $file = null) {
+function script($app, $file = null): void {
 	if (is_array($file)) {
 		foreach ($file as $script) {
 			Util::addScript($app, $script, 'core');
@@ -122,7 +125,7 @@ function script($app, $file = null) {
  * @param string|string[] $file the filename,
  *                              if an array is given it will add all scripts
  */
-function vendor_script($app, $file = null) {
+function vendor_script($app, $file = null): void {
 	if (is_array($file)) {
 		foreach ($file as $f) {
 			OC_Util::addVendorScript($app, $f);
@@ -138,7 +141,7 @@ function vendor_script($app, $file = null) {
  * @param string|string[] $file the filename,
  *                              if an array is given it will add all styles
  */
-function style($app, $file = null) {
+function style($app, $file = null): void {
 	if (is_array($file)) {
 		foreach ($file as $f) {
 			OC_Util::addStyle($app, $f);
@@ -154,7 +157,7 @@ function style($app, $file = null) {
  * @param string|string[] $file the filename,
  *                              if an array is given it will add all styles
  */
-function vendor_style($app, $file = null) {
+function vendor_style($app, $file = null): void {
 	if (is_array($file)) {
 		foreach ($file as $f) {
 			OC_Util::addVendorStyle($app, $f);
@@ -169,29 +172,11 @@ function vendor_style($app, $file = null) {
  * @param string $app the appname
  *                    if an array is given it will add all styles
  */
-function translation($app) {
+function translation($app): void {
 	OC_Util::addTranslations($app);
 }
 
 /**
- * Shortcut for HTML imports
- * @param string $app the appname
- * @param string|string[] $file the path relative to the app's component folder,
- *                              if an array is given it will add all components
- */
-function component($app, $file) {
-	if (is_array($file)) {
-		foreach ($file as $f) {
-			$url = link_to($app, 'component/' . $f . '.html');
-			OC_Util::addHeader('link', ['rel' => 'import', 'href' => $url]);
-		}
-	} else {
-		$url = link_to($app, 'component/' . $file . '.html');
-		OC_Util::addHeader('link', ['rel' => 'import', 'href' => $url]);
-	}
-}
-
-/**
  * make \OCP\IURLGenerator::linkTo available as a simple function
  * @param string $app app
  * @param string $file file
@@ -201,15 +186,15 @@ function component($app, $file) {
  * For further information have a look at \OCP\IURLGenerator::linkTo
  */
 function link_to($app, $file, $args = []) {
-	return \OC::$server->getURLGenerator()->linkTo($app, $file, $args);
+	return Server::get(IURLGenerator::class)->linkTo($app, $file, $args);
 }
 
 /**
- * @param $key
+ * @param string $key
  * @return string url to the online documentation
  */
 function link_to_docs($key) {
-	return \OC::$server->getURLGenerator()->linkToDocs($key);
+	return Server::get(IURLGenerator::class)->linkToDocs($key);
 }
 
 /**
@@ -221,16 +206,16 @@ function link_to_docs($key) {
  * For further information have a look at \OCP\IURLGenerator::imagePath
  */
 function image_path($app, $image) {
-	return \OC::$server->getURLGenerator()->imagePath($app, $image);
+	return Server::get(IURLGenerator::class)->imagePath($app, $image);
 }
 
 /**
- * make OC_Helper::mimetypeIcon available as a simple function
+ * make mimetypeIcon available as a simple function
  * @param string $mimetype mimetype
  * @return string link to the image
  */
 function mimetype_icon($mimetype) {
-	return \OC::$server->getMimeTypeDetector()->mimeTypeIcon($mimetype);
+	return Server::get(IMimeTypeDetector::class)->mimeTypeIcon($mimetype);
 }
 
 /**
@@ -240,7 +225,7 @@ function mimetype_icon($mimetype) {
  * @return string link to the preview
  */
 function preview_icon($path) {
-	return \OC::$server->getURLGenerator()->linkToRoute('core.Preview.getPreview', ['x' => 32, 'y' => 32, 'file' => $path]);
+	return Server::get(IURLGenerator::class)->linkToRoute('core.Preview.getPreview', ['x' => 32, 'y' => 32, 'file' => $path]);
 }
 
 /**
@@ -249,18 +234,19 @@ function preview_icon($path) {
  * @return string
  */
 function publicPreview_icon($path, $token) {
-	return \OC::$server->getURLGenerator()->linkToRoute('files_sharing.PublicPreview.getPreview', ['x' => 32, 'y' => 32, 'file' => $path, 'token' => $token]);
+	return Server::get(IURLGenerator::class)->linkToRoute('files_sharing.PublicPreview.getPreview', ['x' => 32, 'y' => 32, 'file' => $path, 'token' => $token]);
 }
 
 /**
- * make OC_Helper::humanFileSize available as a simple function
+ * make Util::humanFileSize available as a simple function
  * @param int $bytes size in bytes
  * @return string size as string
+ * @deprecated use Util::humanFileSize instead
  *
- * For further information have a look at OC_Helper::humanFileSize
+ * For further information have a look at Util::humanFileSize
  */
 function human_file_size($bytes) {
-	return OC_Helper::humanFileSize($bytes);
+	return Util::humanFileSize($bytes);
 }
 
 /**
@@ -283,7 +269,7 @@ function strip_time($timestamp) {
  * @return string timestamp
  */
 function relative_modified_date($timestamp, $fromTime = null, $dateOnly = false): string {
-	$formatter = \OCP\Server::get(IDateTimeFormatter::class);
+	$formatter = Server::get(IDateTimeFormatter::class);
 
 	if ($dateOnly) {
 		return $formatter->formatDateSpan($timestamp, $fromTime);
@@ -291,7 +277,12 @@ function relative_modified_date($timestamp, $fromTime = null, $dateOnly = false)
 	return $formatter->formatTimeSpan($timestamp, $fromTime);
 }
 
-function html_select_options($options, $selected, $params = []) {
+/**
+ * @param array $options
+ * @param string[]|string $selected
+ * @param array $params
+ */
+function html_select_options($options, $selected, $params = []): string {
 	if (!is_array($selected)) {
 		$selected = [$selected];
 	}
@@ -314,7 +305,7 @@ function html_select_options($options, $selected, $params = []) {
 			$label = $label[$label_name];
 		}
 		$select = in_array($value, $selected) ? ' selected="selected"' : '';
-		$html .= '<option value="' . \OCP\Util::sanitizeHTML($value) . '"' . $select . '>' . \OCP\Util::sanitizeHTML($label) . '</option>' . "\n";
+		$html .= '<option value="' . Util::sanitizeHTML($value) . '"' . $select . '>' . Util::sanitizeHTML($label) . '</option>' . "\n";
 	}
 	return $html;
 }
diff --git a/lib/private/legacy/OC_Template.php b/lib/private/legacy/OC_Template.php
index 77b25477d31..bccf99af65e 100644
--- a/lib/private/legacy/OC_Template.php
+++ b/lib/private/legacy/OC_Template.php
@@ -8,8 +8,6 @@
 use OCP\Server;
 use OCP\Template\ITemplateManager;
 
-require_once __DIR__ . '/template/functions.php';
-
 /**
  * This class provides the templates for ownCloud.
  * @deprecated 32.0.0 Use \OCP\Template\ITemplateManager instead
diff --git a/lib/public/Template.php b/lib/public/Template.php
index d9b3083319b..20783d31cb6 100644
--- a/lib/public/Template.php
+++ b/lib/public/Template.php
@@ -7,6 +7,11 @@
  */
 namespace OCP;
 
+/*
+ * We have to require the functions file because this class contains aliases to the functions
+ */
+require_once __DIR__ . '/../private/Template/functions.php';
+
 /**
  * This class provides the template system for owncloud. You can use it to load
  * specific templates, add data and generate the html code
@@ -24,7 +29,7 @@ class Template extends \OC_Template {
 	 * @param string $image
 	 * @return string to the image
 	 * @since 8.0.0
-	 * @suppress PhanDeprecatedFunction
+	 * @deprecated 32.0.0 Use the function directly instead
 	 */
 	public static function image_path($app, $image) {
 		return \image_path($app, $image);
@@ -37,7 +42,7 @@ class Template extends \OC_Template {
 	 * @param string $mimetype
 	 * @return string to the image of this file type.
 	 * @since 8.0.0
-	 * @suppress PhanDeprecatedFunction
+	 * @deprecated 32.0.0 Use the function directly instead
 	 */
 	public static function mimetype_icon($mimetype) {
 		return \mimetype_icon($mimetype);
@@ -49,7 +54,7 @@ class Template extends \OC_Template {
 	 * @param string $path path to file
 	 * @return string to the preview of the image
 	 * @since 8.0.0
-	 * @suppress PhanDeprecatedFunction
+	 * @deprecated 32.0.0 Use the function directly instead
 	 */
 	public static function preview_icon($path) {
 		return \preview_icon($path);
@@ -63,7 +68,7 @@ class Template extends \OC_Template {
 	 * @param string $token
 	 * @return string link to the preview
 	 * @since 8.0.0
-	 * @suppress PhanDeprecatedFunction
+	 * @deprecated 32.0.0 Use the function directly instead
 	 */
 	public static function publicPreview_icon($path, $token) {
 		return \publicPreview_icon($path, $token);
@@ -76,10 +81,10 @@ class Template extends \OC_Template {
 	 * @param int $bytes in bytes
 	 * @return string size as string
 	 * @since 8.0.0
-	 * @suppress PhanDeprecatedFunction
+	 * @deprecated 32.0.0 Use \OCP\Util::humanFileSize instead
 	 */
 	public static function human_file_size($bytes) {
-		return \human_file_size($bytes);
+		return Util::humanFileSize($bytes);
 	}
 
 	/**
@@ -89,8 +94,8 @@ class Template extends \OC_Template {
 	 * @param boolean $dateOnly
 	 * @return string human readable interpretation of the timestamp
 	 * @since 8.0.0
-	 * @suppress PhanDeprecatedFunction
 	 * @suppress PhanTypeMismatchArgument
+	 * @deprecated 32.0.0 Use the function directly instead
 	 */
 	public static function relative_modified_date($timestamp, $dateOnly = false) {
 		return \relative_modified_date($timestamp, null, $dateOnly);
@@ -104,7 +109,7 @@ class Template extends \OC_Template {
 	 * @param array $params the parameters
 	 * @return string html options
 	 * @since 8.0.0
-	 * @suppress PhanDeprecatedFunction
+	 * @deprecated 32.0.0 Use the function directly instead
 	 */
 	public static function html_select_options($options, $selected, $params = []) {
 		return \html_select_options($options, $selected, $params);
diff --git a/tests/lib/TemplateFunctionsTest.php b/tests/lib/TemplateFunctionsTest.php
index a693a39c580..5b23ef434c2 100644
--- a/tests/lib/TemplateFunctionsTest.php
+++ b/tests/lib/TemplateFunctionsTest.php
@@ -11,7 +11,7 @@ class TemplateFunctionsTest extends \Test\TestCase {
 	protected function setUp(): void {
 		parent::setUp();
 
-		require_once \OC::$SERVERROOT . '/lib/private/legacy/OC_Template.php';
+		require_once \OC::$SERVERROOT . '/lib/private/Template/functions.php';
 	}
 
 	public function testPJavaScript(): void {
diff --git a/tests/lib/TestCase.php b/tests/lib/TestCase.php
index cfed2b06c61..918edf7150e 100644
--- a/tests/lib/TestCase.php
+++ b/tests/lib/TestCase.php
@@ -549,8 +549,6 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase {
 	 * @param array $vars
 	 */
 	protected function assertTemplate($expectedHtml, $template, $vars = []) {
-		require_once __DIR__ . '/../../lib/private/legacy/template/functions.php';
-
 		$requestToken = 12345;
 		/** @var Defaults|\PHPUnit\Framework\MockObject\MockObject $l10n */
 		$theme = $this->getMockBuilder('\OCP\Defaults')