From 18e02fc729668f525153b4fde4004d72d26f85fb Mon Sep 17 00:00:00 2001 From: Alexander Schnitzler <git@alexanderschnitzler.de> Date: Mon, 11 May 2020 17:33:41 +0200 Subject: [PATCH] [TASK] Fix phpstan checkFunctionArgumentTypes errors in ext:core TypoScript This patch fixes incompatible type usage in function arguments and is preparatory work for introducing native type hints and strict mode in all core files. Releases: master, 10.4 Resolves: #92275 Change-Id: I3745b51d93a5337bf3b2ad3a124f59bc43343002 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/65658 Tested-by: Daniel Goerz <daniel.goerz@posteo.de> Tested-by: TYPO3com <noreply@typo3.com> Tested-by: Benni Mack <benni@typo3.org> Reviewed-by: Daniel Goerz <daniel.goerz@posteo.de> Reviewed-by: Oliver Klee <typo3-coding@oliverklee.de> Reviewed-by: Benni Mack <benni@typo3.org> --- .../TypoScript/ExtendedTemplateService.php | 30 ++++----- .../Parser/ConstantConfigurationParser.php | 2 +- .../TypoScript/Parser/TypoScriptParser.php | 64 +++++++++++-------- .../Classes/TypoScript/TemplateService.php | 21 +++++- 4 files changed, 70 insertions(+), 47 deletions(-) diff --git a/typo3/sysext/core/Classes/TypoScript/ExtendedTemplateService.php b/typo3/sysext/core/Classes/TypoScript/ExtendedTemplateService.php index 2ae4df23189c..e003a41fdcdc 100644 --- a/typo3/sysext/core/Classes/TypoScript/ExtendedTemplateService.php +++ b/typo3/sysext/core/Classes/TypoScript/ExtendedTemplateService.php @@ -351,7 +351,7 @@ class ExtendedTemplateService extends TemplateService foreach ($arr as $key => $value) { // Don't do anything with comments / linenumber registrations... if (substr($key, -2) !== '..') { - $key = preg_replace('/\\.$/', '', $key); + $key = preg_replace('/\\.$/', '', $key) ?? ''; if (substr($key, -1) !== '.') { if (MathUtility::canBeInterpretedAsInteger($key)) { $keyArr_num[$key] = $arr[$key]; @@ -421,15 +421,15 @@ class ExtendedTemplateService extends TemplateService $HTML .= ' = <span class="list-tree-value">' . htmlspecialchars($theValue) . '</span>'; } if ($this->ext_regComments && isset($arr[$key . '..'])) { - $comment = $arr[$key . '..']; + $comment = (string)$arr[$key . '..']; // Skip INCLUDE_TYPOSCRIPT comments, they are almost useless if (!preg_match('/### <INCLUDE_TYPOSCRIPT:.*/', $comment)) { // Remove linebreaks, replace with ' ' - $comment = preg_replace('/[\\r\\n]/', ' ', $comment); + $comment = preg_replace('/[\\r\\n]/', ' ', $comment) ?? ''; // Remove # and * if more than twice in a row - $comment = preg_replace('/[#\\*]{2,}/', '', $comment); + $comment = preg_replace('/[#\\*]{2,}/', '', $comment) ?? ''; // Replace leading # (just if it exists) and add it again. Result: Every comment should be prefixed by a '#'. - $comment = preg_replace('/^[#\\*\\s]+/', '# ', $comment); + $comment = preg_replace('/^[#\\*\\s]+/', '# ', $comment) ?? ''; // Masking HTML Tags: Replace < with < and > with > $comment = htmlspecialchars($comment); $HTML .= ' <i class="text-muted">' . trim($comment) . '</i>'; @@ -501,7 +501,7 @@ class ExtendedTemplateService extends TemplateService { $keyArr = []; foreach ($arr as $key => $value) { - $key = preg_replace('/\\.$/', '', $key); + $key = preg_replace('/\\.$/', '', $key) ?? ''; if (substr($key, -1) !== '.') { $keyArr[$key] = 1; } @@ -585,7 +585,7 @@ class ExtendedTemplateService extends TemplateService { $keyArr = []; foreach ($arr as $key => $value) { - $key = preg_replace('/\\.$/', '', $key); + $key = preg_replace('/\\.$/', '', $key) ?? ''; if (substr($key, -1) !== '.') { $keyArr[$key] = 1; } @@ -1333,7 +1333,7 @@ class ExtendedTemplateService extends TemplateService switch ($typeDat['type']) { case 'int': if ($typeDat['paramstr']) { - $var = MathUtility::forceIntegerInRange($var, $typeDat['params'][0], $typeDat['params'][1]); + $var = MathUtility::forceIntegerInRange((int)$var, $typeDat['params'][0], $typeDat['params'][1]); } else { $var = (int)$var; } @@ -1344,15 +1344,15 @@ class ExtendedTemplateService extends TemplateService case 'color': $col = []; if ($var) { - $var = preg_replace('/[^A-Fa-f0-9]*/', '', $var); + $var = preg_replace('/[^A-Fa-f0-9]*/', '', $var) ?? ''; $useFulHex = strlen($var) > 3; - $col[] = hexdec($var[0]); - $col[] = hexdec($var[1]); - $col[] = hexdec($var[2]); + $col[] = (int)hexdec($var[0]); + $col[] = (int)hexdec($var[1]); + $col[] = (int)hexdec($var[2]); if ($useFulHex) { - $col[] = hexdec($var[3]); - $col[] = hexdec($var[4]); - $col[] = hexdec($var[5]); + $col[] = (int)hexdec($var[3]); + $col[] = (int)hexdec($var[4]); + $col[] = (int)hexdec($var[5]); } $var = substr('0' . dechex($col[0]), -1) . substr('0' . dechex($col[1]), -1) . substr('0' . dechex($col[2]), -1); if ($useFulHex) { diff --git a/typo3/sysext/core/Classes/TypoScript/Parser/ConstantConfigurationParser.php b/typo3/sysext/core/Classes/TypoScript/Parser/ConstantConfigurationParser.php index 3cd58ce6b697..56144743e56a 100644 --- a/typo3/sysext/core/Classes/TypoScript/Parser/ConstantConfigurationParser.php +++ b/typo3/sysext/core/Classes/TypoScript/Parser/ConstantConfigurationParser.php @@ -234,7 +234,7 @@ class ConstantConfigurationParser $counter++; $comment = trim($flatSetup[$key]); foreach (explode(LF, $comment) as $k => $v) { - $line = trim(preg_replace('/^[#\\/]*/', '', $v)); + $line = trim(preg_replace('/^[#\\/]*/', '', $v) ?? ''); if (!$line) { continue; } diff --git a/typo3/sysext/core/Classes/TypoScript/Parser/TypoScriptParser.php b/typo3/sysext/core/Classes/TypoScript/Parser/TypoScriptParser.php index 0b112b98e8f8..bf3114270a3e 100644 --- a/typo3/sysext/core/Classes/TypoScript/Parser/TypoScriptParser.php +++ b/typo3/sysext/core/Classes/TypoScript/Parser/TypoScriptParser.php @@ -45,7 +45,7 @@ class TypoScriptParser /** * Raw data, the input string exploded by LF * - * @var array + * @var string[] */ protected $raw; @@ -305,7 +305,7 @@ class TypoScriptParser * Parsing the $this->raw TypoScript lines from pointer, $this->rawP * * @param array $setup Reference to the setup array in which to accumulate the values. - * @return string|null Returns the string of the condition found, the exit signal or possible nothing (if it completed parsing with no interruptions) + * @return string Returns the string of the condition found, the exit signal or possible nothing (if it completed parsing with no interruptions) */ protected function parseSub(array &$setup) { @@ -477,7 +477,7 @@ class TypoScriptParser } // unserialize(serialize(...)) may look stupid but is needed because of some reference issues. // See forge issue #76919 and functional test hasFlakyReferences() - $this->setVal($objStrName, $setup, unserialize(serialize($res), ['allowed_classes' => false]), 1); + $this->setVal($objStrName, $setup, unserialize(serialize($res), ['allowed_classes' => false]), true); break; case '>': if ($this->syntaxHighLight) { @@ -528,7 +528,7 @@ class TypoScriptParser } } } - return null; + return ''; } /** @@ -542,44 +542,46 @@ class TypoScriptParser */ protected function executeValueModifier($modifierName, $modifierArgument = null, $currentValue = null) { + $modifierArgumentAsString = (string)$modifierArgument; + $currentValueAsString = (string)$currentValue; $newValue = null; switch ($modifierName) { case 'prependString': - $newValue = $modifierArgument . $currentValue; + $newValue = $modifierArgumentAsString . $currentValueAsString; break; case 'appendString': - $newValue = $currentValue . $modifierArgument; + $newValue = $currentValueAsString . $modifierArgumentAsString; break; case 'removeString': - $newValue = str_replace($modifierArgument, '', $currentValue); + $newValue = str_replace($modifierArgumentAsString, '', $currentValueAsString); break; case 'replaceString': - $modifierArgumentArray = explode('|', $modifierArgument, 2); + $modifierArgumentArray = explode('|', $modifierArgumentAsString, 2); $fromStr = $modifierArgumentArray[0] ?? ''; $toStr = $modifierArgumentArray[1] ?? ''; - $newValue = str_replace($fromStr, $toStr, $currentValue); + $newValue = str_replace($fromStr, $toStr, $currentValueAsString); break; case 'addToList': - $newValue = ((string)$currentValue !== '' ? $currentValue . ',' : '') . $modifierArgument; + $newValue = ($currentValueAsString !== '' ? $currentValueAsString . ',' : '') . $modifierArgumentAsString; break; case 'removeFromList': - $existingElements = GeneralUtility::trimExplode(',', $currentValue); - $removeElements = GeneralUtility::trimExplode(',', $modifierArgument); + $existingElements = GeneralUtility::trimExplode(',', $currentValueAsString); + $removeElements = GeneralUtility::trimExplode(',', $modifierArgumentAsString); if (!empty($removeElements)) { $newValue = implode(',', array_diff($existingElements, $removeElements)); } break; case 'uniqueList': - $elements = GeneralUtility::trimExplode(',', $currentValue); + $elements = GeneralUtility::trimExplode(',', $currentValueAsString); $newValue = implode(',', array_unique($elements)); break; case 'reverseList': - $elements = GeneralUtility::trimExplode(',', $currentValue); + $elements = GeneralUtility::trimExplode(',', $currentValueAsString); $newValue = implode(',', array_reverse($elements)); break; case 'sortList': - $elements = GeneralUtility::trimExplode(',', $currentValue); - $arguments = GeneralUtility::trimExplode(',', $modifierArgument); + $elements = GeneralUtility::trimExplode(',', $currentValueAsString); + $arguments = GeneralUtility::trimExplode(',', $modifierArgumentAsString); $arguments = array_map('strtolower', $arguments); $sort_flags = SORT_REGULAR; if (in_array('numeric', $arguments)) { @@ -590,7 +592,7 @@ class TypoScriptParser // See also the warning on http://us.php.net/manual/en/function.sort.php foreach ($elements as $element) { if (!is_numeric($element)) { - throw new \InvalidArgumentException('The list "' . $currentValue . '" should be sorted numerically but contains a non-numeric value', 1438191758); + throw new \InvalidArgumentException('The list "' . $currentValueAsString . '" should be sorted numerically but contains a non-numeric value', 1438191758); } } } @@ -601,7 +603,7 @@ class TypoScriptParser $newValue = implode(',', $elements); break; case 'getEnv': - $environmentValue = getenv(trim($modifierArgument)); + $environmentValue = getenv(trim($modifierArgumentAsString)); if ($environmentValue !== false) { $newValue = $environmentValue; } @@ -840,8 +842,10 @@ class TypoScriptParser if (strpos($string, '<INCLUDE_TYPOSCRIPT:') !== false) { $splitRegEx = '/\r?\n\s*<INCLUDE_TYPOSCRIPT:\s*(?i)source\s*=\s*"((?i)file|dir):\s*([^"]*)"(.*)>[\ \t]*/'; $parts = preg_split($splitRegEx, LF . $string . LF, -1, PREG_SPLIT_DELIM_CAPTURE); + $parts = is_array($parts) ? $parts : []; + // First text part goes through - $newString = $parts[0] . LF; + $newString = ($parts[0] ?? '') . LF; $partCount = count($parts); for ($i = 1; $i + 3 < $partCount; $i += 4) { // $parts[$i] contains 'FILE' or 'DIR' @@ -856,6 +860,8 @@ class TypoScriptParser // Check condition $matches = preg_split('#(?i)condition\\s*=\\s*"((?:\\\\\\\\|\\\\"|[^\\"])*)"(\\s*|>)#', $optionalProperties, 2, PREG_SPLIT_DELIM_CAPTURE); + $matches = is_array($matches) ? $matches : []; + // If there was a condition if (count($matches) > 1) { // Unescape the condition @@ -913,7 +919,7 @@ class TypoScriptParser $filePointerPathParts = explode('/', substr($filename, 4)); // remove file part, determine whether to load setup or constants - [$includeType, ] = explode('.', array_pop($filePointerPathParts)); + [$includeType, ] = explode('.', (string)array_pop($filePointerPathParts)); if (in_array($includeType, ['setup', 'constants'])) { // adapt extension key to required format (no underscores) @@ -957,6 +963,7 @@ class TypoScriptParser if (strpos($typoScript, '@import \'') !== false || strpos($typoScript, '@import "') !== false) { $splitRegEx = '/\r?\n\s*@import\s[\'"]([^\'"]*)[\'"][\ \t]?/'; $parts = preg_split($splitRegEx, LF . $typoScript . LF, -1, PREG_SPLIT_DELIM_CAPTURE); + $parts = is_array($parts) ? $parts : []; // First text part goes through $newString = $parts[0] . LF; $partCount = count($parts); @@ -1058,7 +1065,7 @@ class TypoScriptParser if (strpos(strtoupper($filename), 'EXT:') === 0) { $filePointerPathParts = explode('/', substr($filename, 4)); // remove file part, determine whether to load setup or constants - [$includeType] = explode('.', array_pop($filePointerPathParts)); + [$includeType] = explode('.', (string)array_pop($filePointerPathParts)); if (in_array($includeType, ['setup', 'constants'], true)) { // adapt extension key to required format (no underscores) @@ -1118,7 +1125,7 @@ class TypoScriptParser if ($fileExists) { $includedFiles[] = $absfilename; // check for includes in included text - $included_text = self::checkIncludeLines(file_get_contents($absfilename), $cycle_counter + 1, $returnFiles, $absfilename); + $included_text = self::checkIncludeLines((string)file_get_contents($absfilename), $cycle_counter + 1, $returnFiles, $absfilename); // If the method also has to return all included files, merge currently included // files with files included by recursively calling itself if ($returnFiles && is_array($included_text)) { @@ -1153,6 +1160,7 @@ class TypoScriptParser { // Extract the value of the property extensions="..." $matches = preg_split('#(?i)extensions\s*=\s*"([^"]*)"(\s*|>)#', $optionalProperties, 2, PREG_SPLIT_DELIM_CAPTURE); + $matches = is_array($matches) ? $matches : []; if (count($matches) > 1) { $includedFileExtensions = $matches[1]; } else { @@ -1271,7 +1279,7 @@ class TypoScriptParser $expectedEndTag = '### <INCLUDE_TYPOSCRIPT: source="' . $inIncludePart . ':' . $fileName . '"' . $optionalProperties . '> END'; // Strip all whitespace characters to make comparison safer - $expectedEndTag = strtolower(preg_replace('/\s/', '', $expectedEndTag)); + $expectedEndTag = strtolower(preg_replace('/\s/', '', $expectedEndTag) ?? ''); } else { // If this is not a beginning commented include statement this line goes into the rest content $restContent[] = $line; @@ -1446,7 +1454,7 @@ class TypoScriptParser $lineC = ''; if (is_array($this->highLightData[$rawP])) { foreach ($this->highLightData[$rawP] as $set) { - $len = $strlen - $start - $set[1]; + $len = $strlen - $start - (int)$set[1]; if ($len > 0) { $part = substr($value, $start, $len); $start += $len; @@ -1468,7 +1476,7 @@ class TypoScriptParser $lineC .= $this->highLightStyles['error'][0] . '<strong> - ERROR:</strong> ' . htmlspecialchars(implode(';', $errA[$rawP])) . $this->highLightStyles['error'][1]; } if ($highlightBlockMode && $this->highLightData_bracelevel[$rawP]) { - $lineC = str_pad('', $this->highLightData_bracelevel[$rawP] * 2, ' ', STR_PAD_LEFT) . '<span style="' . $this->highLightBlockStyles . ($this->highLightBlockStyles_basecolor ? 'background-color: ' . $this->modifyHTMLColorAll($this->highLightBlockStyles_basecolor, -$this->highLightData_bracelevel[$rawP] * 16) : '') . '">' . ($lineC !== '' ? $lineC : ' ') . '</span>'; + $lineC = str_pad('', $this->highLightData_bracelevel[$rawP] * 2, ' ', STR_PAD_LEFT) . '<span style="' . $this->highLightBlockStyles . ($this->highLightBlockStyles_basecolor ? 'background-color: ' . $this->modifyHTMLColorAll($this->highLightBlockStyles_basecolor, -(int)($this->highLightData_bracelevel[$rawP] * 16)) : '') . '">' . ($lineC !== '' ? $lineC : ' ') . '</span>'; } if (is_array($lineNumDat)) { $lineNum = $rawP + $lineNumDat[0]; @@ -1503,9 +1511,9 @@ class TypoScriptParser protected function modifyHTMLColor($color, $R, $G, $B) { // This takes a hex-color (# included!) and adds $R, $G and $B to the HTML-color (format: #xxxxxx) and returns the new color - $nR = MathUtility::forceIntegerInRange(hexdec(substr($color, 1, 2)) + $R, 0, 255); - $nG = MathUtility::forceIntegerInRange(hexdec(substr($color, 3, 2)) + $G, 0, 255); - $nB = MathUtility::forceIntegerInRange(hexdec(substr($color, 5, 2)) + $B, 0, 255); + $nR = MathUtility::forceIntegerInRange((int)hexdec(substr($color, 1, 2)) + $R, 0, 255); + $nG = MathUtility::forceIntegerInRange((int)hexdec(substr($color, 3, 2)) + $G, 0, 255); + $nB = MathUtility::forceIntegerInRange((int)hexdec(substr($color, 5, 2)) + $B, 0, 255); return '#' . substr('0' . dechex($nR), -2) . substr('0' . dechex($nG), -2) . substr('0' . dechex($nB), -2); } diff --git a/typo3/sysext/core/Classes/TypoScript/TemplateService.php b/typo3/sysext/core/Classes/TypoScript/TemplateService.php index fce5a247b41f..89cb0017c58e 100644 --- a/typo3/sysext/core/Classes/TypoScript/TemplateService.php +++ b/typo3/sysext/core/Classes/TypoScript/TemplateService.php @@ -770,11 +770,26 @@ class TemplateService $ISF_filePath = ExtensionManagementUtility::extPath($ISF_extKey) . $ISF_localPath; if (@is_dir($ISF_filePath)) { $mExtKey = str_replace('_', '', $ISF_extKey . '/' . $ISF_localPath); + + $includeStaticTxtPath = $ISF_filePath . 'include_static.txt'; + $includeStaticTxtContents = ''; + if (@file_exists($includeStaticTxtPath)) { + $includeStaticTxtContents = (string)file_get_contents($includeStaticTxtPath); + $includeStaticTxtContents = implode(',', array_unique(GeneralUtility::intExplode(',', $includeStaticTxtContents))); + } + + $includeStaticFileTxtPath = $ISF_filePath . 'include_static_file.txt'; + $includeStaticFileTxtContents = ''; + if (@file_exists($includeStaticFileTxtPath)) { + $includeStaticFileTxtContents = (string)file_get_contents($includeStaticFileTxtPath); + $includeStaticFileTxtContents = implode(',', array_unique(GeneralUtility::trimExplode(',', $includeStaticFileTxtContents))); + } + $subrow = [ 'constants' => $this->getTypoScriptSourceFileContent($ISF_filePath, 'constants'), 'config' => $this->getTypoScriptSourceFileContent($ISF_filePath, 'setup'), - 'include_static' => @file_exists($ISF_filePath . 'include_static.txt') ? implode(',', array_unique(GeneralUtility::intExplode(',', file_get_contents($ISF_filePath . 'include_static.txt')))) : '', - 'include_static_file' => @file_exists($ISF_filePath . 'include_static_file.txt') ? implode(',', array_unique(explode(',', file_get_contents($ISF_filePath . 'include_static_file.txt')))) : '', + 'include_static' => $includeStaticTxtContents, + 'include_static_file' => $includeStaticFileTxtContents, 'title' => $ISF_file, 'uid' => $mExtKey ]; @@ -1097,7 +1112,7 @@ class TemplateService // Recursive substitution of constants (up to 10 nested levels) for ($i = 0; $i < 10 && !$noChange; $i++) { $old_all = $all; - $all = preg_replace_callback('/\\{\\$(.[^}]*)\\}/', [$this, 'substituteConstantsCallBack'], $all); + $all = preg_replace_callback('/\\{\\$(.[^}]*)\\}/', [$this, 'substituteConstantsCallBack'], $all) ?? ''; if ($old_all == $all) { $noChange = true; } -- GitLab