diff --git a/check_upgrade_savepoints/check_upgrade_savepoints.php b/check_upgrade_savepoints/check_upgrade_savepoints.php index 09d25ec9..a1fb4acd 100644 --- a/check_upgrade_savepoints/check_upgrade_savepoints.php +++ b/check_upgrade_savepoints/check_upgrade_savepoints.php @@ -1,5 +1,4 @@ dependencies in version.php files $plugin = new stdClass(); $module = new stdClass(); -// Detect if we are in 2.3 and up by looking for $branch -if (file_exists('version.php')) { - require_once('version.php'); -} -$moodle23andup = isset($branch) ? true : false; - $dir = dirname(__FILE__); - $files = files_to_check($dir); foreach ($files as $file) { @@ -60,48 +45,36 @@ $contents = file_get_contents($file); - $function_regexp = '\s*function\s+xmldb_[a-zA-Z0-9_]+?_upgrade\s*\(.*?version.*?\)(?::\sbool)?\s*(?=\{)'; - $return_regexp = '\s*return true;'; - $anyfunction_regexp = '\s*function\s*[a-z0-9_]+?\s*\(.*?\)\s*{'; // MDL-34103 + $functionregexp = '\s*function\s+xmldb_[a-zA-Z0-9_]+?_upgrade\s*\(.*?version.*?\)(?::\sbool)?\s*(?=\{)'; + $returnregexp = '\s*return true;'; + $anyfunctionregexp = '\s*function\s*[a-z0-9_]+?\s*\(.*?\)\s*{'; // MDL-34103 -/// Find we have some xmldb_xxxx_function in code - if (! $countxmldb = preg_match_all('@' . $function_regexp . '@is', $contents, $matches)) { + // Find we have some xmldb_xxxx_function in code + if (! $countxmldb = preg_match_all('@' . $functionregexp . '@is', $contents, $matches)) { echo " + ERROR: upgrade function not found" . LINEFEED; continue; } -/// Verify there is only one upgrade function + // Verify there is only one upgrade function if ($countxmldb !== 1) { echo " + ERROR: multiple upgrade functions detected" . LINEFEED; continue; } - // These checks are only performed for 23_STABLE and up - if ($moodle23andup) { - - // Find we have some return true; in code - if (! $countreturn = preg_match_all('@' . $return_regexp . '@is', $contents, $matches)) { - echo " + ERROR: 'return true;' not found" . LINEFEED; - continue; - } - // Verify there is only one return true; - if ($countreturn !== 1) { - echo " + ERROR: multiple 'return true;' detected" . LINEFEED; - continue; - } - - /** Commented till MDL-34103 is fixed to avoid getting fails - // Find we have some function in code - if (! $countfunction = preg_match_all('@' . $anyfunction_regexp . '@is', $contents, $matches)) { - echo " + ERROR: functions not found" . LINEFEED; - continue; - } - // Verify there is only one function - if ($countfunction !== 1) { - echo " + ERROR: multiple functions detected (use upgradelib, plz)" . LINEFEED; - continue; - } - */ + // Find we have some return true; in code + if (! $countreturn = preg_match_all('@' . $returnregexp . '@is', $contents, $matches)) { + echo " + ERROR: 'return true;' not found" . LINEFEED; + continue; + } + // Verify there is only one return true; + if ($countreturn !== 1) { + echo " + ERROR: multiple 'return true;' detected" . LINEFEED; + continue; + } + // Verify there is not more than one function + if (preg_match_all('@' . $anyfunctionregexp . '@is', $contents, $matches) > 1) { + echo " + ERROR: multiple functions detected (use upgradelib, plz)" . LINEFEED; + continue; } // Extract all string literals in upgrade code, we are not interested on them and can lead to @@ -114,7 +87,7 @@ // In any case, all the replacements performed are stored in $discardedliterals just in case // something needs to be recovered back. $regexp = '(["\'])(?:\\\\\1|.)*?\1'; // Match all quoted literals in a text, ignoring escaped ones. - $discardedliterals = array(); + $discardedliterals = []; // Look for all quoted strings. preg_match_all('@' . $regexp . '@', $contents, $matches); // Iterate them, keeping safe ones and replacing by placeholder conflictive ones. @@ -123,7 +96,7 @@ $unsaferegexp = '[\[\(\{\<\>\}\)\]]'; // Consider everything but [({<>})] safe. if (preg_match('@' . $unsaferegexp . '@', $string)) { // The string is not safe, replace it by placeholder and annotate the replacement. - $replacement = "'<%&%" . (string)(count($discardedliterals) +1) . "%&%>'"; + $replacement = "'<%&%" . (string)(count($discardedliterals) + 1) . "%&%>'"; $discardedliterals[$replacement] = $string; } else { // The string is safe, keep it as is, no need to replace it by placeholder. @@ -134,44 +107,44 @@ $contents = str_replace($discardedliterals, array_keys($discardedliterals), $contents); } -/// Arrived here, extract function contents - if (! preg_match_all('@' . $function_regexp . '.*?(\{(?>(?>[^{}]+)|(?1))*\})@is', $contents, $matches)) { + // Arrived here, extract function contents + if (! preg_match_all('@' . $functionregexp . '.*?(\{(?>(?>[^{}]+)|(?1))*\})@is', $contents, $matches)) { echo " + NOTE: cannot find upgrade function contents" . LINEFEED; continue; } -/// Calculate function contents (must be a group of "if" blocks) + // Calculate function contents (must be a group of "if" blocks) $contents = trim(trim($matches[1][0], '}{')); - $if_regexp = 'if\s+?\(\s*?\$oldversion\s*?<\s*?([0-9.]{8,13}).*?\)\s*?'; - $sp_regexp = 'upgrade_(main|mod|block|plugin)_savepoint\s*?\(\s*?true\s*?,\s*?([0-9.]{8,13})\s*?.*?\);'; + $ifregexp = 'if\s+?\(\s*?\$oldversion\s*?<\s*?([0-9.]{8,13}).*?\)\s*?'; + $spregexp = 'upgrade_(main|mod|block|plugin)_savepoint\s*?\(\s*?true\s*?,\s*?([0-9.]{8,13})\s*?.*?\);'; -/// Count ifs and savepoints. Must match - $count_if = preg_match_all('@' . $if_regexp . '@is', $contents, $matches1); - $count_sp = preg_match_all('@' . $sp_regexp . '@is', $contents, $matches2); - if ($count_if > 0 || $count_sp > 0) { - if ($count_if !== $count_sp) { - if ($count_if < $count_sp) { - echo " + WARN: Detected fewer 'if' blocks ($count_if) than 'savepoint' calls ($count_sp). Repeated savepoints?" . LINEFEED; + // Count ifs and savepoints. Must match + $countif = preg_match_all('@' . $ifregexp . '@is', $contents, $matches1); + $countsp = preg_match_all('@' . $spregexp . '@is', $contents, $matches2); + if ($countif > 0 || $countsp > 0) { + if ($countif !== $countsp) { + if ($countif < $countsp) { + echo " + WARN: Detected fewer 'if' blocks ($countif) than 'savepoint' calls ($countsp). Repeated savepoints?" . LINEFEED; } else { - echo " + ERROR: Detected more 'if' blocks ($count_if) than 'savepoint' calls ($count_sp)" . LINEFEED; + echo " + ERROR: Detected more 'if' blocks ($countif) than 'savepoint' calls ($countsp)" . LINEFEED; } } else { - echo " + found $count_if matching 'if' blocks and 'savepoint' calls" . LINEFEED; + echo " + found $countif matching 'if' blocks and 'savepoint' calls" . LINEFEED; } } -/// Let's ensure there are no duplicate calls to a save point with the same version. - if ($count_sp > 0) { + // Let's ensure there are no duplicate calls to a save point with the same version. + if ($countsp > 0) { foreach (array_count_values($matches2[2]) as $version => $count) { if ($count > 1) { - echo " + ERROR: Detected multiple 'savepoint' calls for version $version".LINEFEED; + echo " + ERROR: Detected multiple 'savepoint' calls for version $version" . LINEFEED; } } } -/// Let's split them - if (!preg_match_all('@(' . $if_regexp . '(\{(?>(?>[^{}]+)|(?3))*\}))@is', $contents, $matches)) { + // Let's split them + if (!preg_match_all('@(' . $ifregexp . '(\{(?>(?>[^{}]+)|(?3))*\}))@is', $contents, $matches)) { echo " + NOTE: cannot find 'if' blocks within the upgrade function" . LINEFEED; continue; } @@ -179,52 +152,52 @@ $versions = $matches[2]; $blocks = $matches[3]; -/// Foreach version, check order - $version_p = 0; - $has_version_error = false; - foreach($versions as $version) { - if (!$version_p) { - $version_p = $version; + // Foreach version, check order + $versionp = 0; + $hasversionerror = false; + foreach ($versions as $version) { + if (!$versionp) { + $versionp = $version; continue; } - if (((float)$version * 100) < ((float)$version_p * 100)) { - echo " + ERROR: Wrong order in versions: $version_p and $version" . LINEFEED; - $has_version_error = true; + if (((float)$version * 100) < ((float)$versionp * 100)) { + echo " + ERROR: Wrong order in versions: $versionp and $version" . LINEFEED; + $hasversionerror = true; } - $version_p = $version; + $versionp = $version; } - if (!$has_version_error) { + if (!$hasversionerror) { echo " + versions in upgrade blocks properly ordered" . LINEFEED; } -/// Foreach version, look for corresponding savepoint - $has_version_mismatch = false; + // Foreach version, look for corresponding savepoint + $hasversionmismatch = false; foreach ($versions as $key => $version) { - $count_spv = preg_match_all('@' .$sp_regexp . '@is', $blocks[$key], $matches); - if ($count_spv == 0) { + $countspv = preg_match_all('@' . $spregexp . '@is', $blocks[$key], $matches); + if ($countspv == 0) { echo " + ERROR: version $version is missing corresponding savepoint call" . LINEFEED; - $has_version_mismatch = true; - } else if ($count_spv > 1) { + $hasversionmismatch = true; + } else if ($countspv > 1) { echo " + ERROR: version $version has more than one savepoint call" . LINEFEED; - $has_version_mismatch = true; + $hasversionmismatch = true; } else { if ($version !== $matches[2][0]) { echo " + ERROR: version $version has wrong savepoint call with version {$matches[2][0]}" . LINEFEED; - $has_version_mismatch = true; + $hasversionmismatch = true; } } } - if (!$has_version_mismatch) { + if (!$hasversionmismatch) { echo " + versions in savepoint calls properly matching upgrade blocks" . LINEFEED; } -/// Ensure a plugin does not upgrade past its defined version. - $versionfile = dirname(dirname($file)).'/version.php'; + // Ensure a plugin does not upgrade past its defined version. + $versionfile = dirname(dirname($file)) . '/version.php'; if (file_exists($versionfile)) { if (preg_match('/^\s*\$(module|plugin)->version\s*=\s*([\d.]+)/m', file_get_contents($versionfile), $versionmatches) === 1) { foreach ($versions as $version) { if (((float) $versionmatches[2] * 100) < ((float) $version * 100)) { - echo " + ERROR: version $version is higher than that defined in $versionfile file".LINEFEED; + echo " + ERROR: version $version is higher than that defined in $versionfile file" . LINEFEED; } } } @@ -234,43 +207,41 @@ /** * Given one full path, return one array with all the files to check */ - function files_to_check($path) { - - $results = array(); - $pending = array(); +function files_to_check($path) { - $dir = opendir($path); - while (false !== ($file=readdir($dir))) { + $results = []; + $pending = []; - $fullpath = $path . '/' . $file; + $dir = opendir($path); + while (false !== ($file = readdir($dir))) { + $fullpath = $path . '/' . $file; - if (substr($file, 0, 1)=='.' || $file=='CVS' || $file=='.git') { /// Exclude some dirs - continue; - } - - if (is_dir($fullpath)) { /// Process dirs later - $pending[] = $fullpath; - continue; - } + if (substr($file, 0, 1) == '.' || $file == 'CVS' || $file == '.git') { // Exclude some dirs + continue; + } - if (is_file($fullpath) && strpos($file, basename(__FILE__))!==false) { /// Exclude me - continue; - } + if (is_dir($fullpath)) { // Process dirs later + $pending[] = $fullpath; + continue; + } - if (is_file($fullpath) && strpos($fullpath, 'db/upgrade.php')===false) { /// Exclude non upgrade.php files - continue; - } + if (is_file($fullpath) && strpos($file, basename(__FILE__)) !== false) { // Exclude me + continue; + } - if (!in_array($fullpath, $results)) { /// Add file if doesn't exists - $results[$fullpath] = $fullpath; - } + if (is_file($fullpath) && strpos($fullpath, 'db/upgrade.php') === false) { // Exclude non upgrade.php files + continue; } - closedir($dir); - foreach ($pending as $pend) { - $results = array_merge($results, files_to_check($pend)); + if (!in_array($fullpath, $results)) { // Add file if doesn't exists + $results[$fullpath] = $fullpath; } + } + closedir($dir); - return $results; + foreach ($pending as $pend) { + $results = array_merge($results, files_to_check($pend)); } -?> + + return $results; +}