From 24f57489945851ea3400b020e53db910b2de2383 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 1 Oct 2024 12:41:17 +0800 Subject: [PATCH] Coding style fixes --- .github/workflows/ci.yml | 39 +++++++++- .gitignore | 1 + phpcs.xml | 22 ++++++ phpunit.xml | 2 +- src/Helper.php | 74 +++++++++--------- src/VersionInfo.php | 137 ++++++++++++++++++++------------- tests/bootstrap.php | 1 + tests/unit/HelperTest.php | 4 +- tests/unit/VersionInfoTest.php | 71 +++++++++-------- 9 files changed, 224 insertions(+), 127 deletions(-) create mode 100644 phpcs.xml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 405b6c7..621b9c8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,15 +14,48 @@ jobs: - 8.4 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Setup PHP uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php }} - - name: Install dependencies - run: composer install + - name: Cache Composer dependencies + uses: actions/cache@v4 + with: + path: ~/.composer/cache + key: php-composer-locked-${{ hashFiles('composer.lock') }} + restore-keys: php-composer-locked- + + - name: Install PHP dependencies + run: composer install --no-interaction --no-progress - name: Run PHPUnit run: vendor/bin/phpunit + cs: + name: Coding standards + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: 8.3 + tools: cs2pr, phpcs + coverage: none + + - name: Cache Composer dependencies + uses: actions/cache@v4 + with: + path: ~/.composer/cache + key: php-composer-locked-${{ hashFiles('composer.lock') }} + restore-keys: php-composer-locked- + + - name: Install PHP dependencies + run: composer install --no-interaction --no-progress + + - name: PHP CodeSniffer + run: phpcs -q --no-colors --report=checkstyle | cs2pr diff --git a/.gitignore b/.gitignore index bf82344..529323a 100644 --- a/.gitignore +++ b/.gitignore @@ -13,3 +13,4 @@ .phpunit.cache coverage coverage.txt +.phpcs-cache diff --git a/phpcs.xml b/phpcs.xml new file mode 100644 index 0000000..95d6097 --- /dev/null +++ b/phpcs.xml @@ -0,0 +1,22 @@ + + + + + + + + + src + tests + + + + + + + + + + + + diff --git a/phpunit.xml b/phpunit.xml index 45f745e..a900720 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,7 +1,7 @@ + bootstrap="tests/bootstrap.php"> tests/unit diff --git a/src/Helper.php b/src/Helper.php index c8117f9..ee6dfe1 100644 --- a/src/Helper.php +++ b/src/Helper.php @@ -1,4 +1,5 @@ - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @copyright 2024 Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class Helper { /** * Bump the version. * - * @param string $path The path to the versio file - * @param string $branch The branch name to set - * @param string $type The type of release - * @param string $rc If a release candidate, the RC number - * @param string $date The date to use for the version - * @param bool $isdevbranch Whether this is a developmentbranch + * @param string $path The path to the versio file + * @param string $branch The branch name to set + * @param string $type The type of release + * @param string $rc If a release candidate, the RC number + * @param string $date The date to use for the version + * @param bool $isdevbranch Whether this is a developmentbranch * @throws Exception * @return string */ @@ -69,7 +70,7 @@ public static function bumpVersion( * this function will not work as expected. If this happens this function * will need to be extended as appropriate in the circumstances. * - * @param int $branch The branch number in integer form + * @param int $branch The branch number in integer form * @return int The new branch number */ public static function getNextBranchNumber( @@ -79,8 +80,8 @@ public static function getNextBranchNumber( return $branch + 1; } - $releasemajor = (int) substr($branch, 0, 1); - $releaseminor = (int) substr($branch, 2, 1); + $releasemajor = (int) substr((string) $branch, 0, 1); + $releaseminor = (int) substr((string) $branch, 2, 1); if ($releaseminor >= 3) { $releasemajor++; @@ -95,40 +96,41 @@ public static function getNextBranchNumber( /** * Ensure that the buymped version is higher than the current one. * - * @param int $versionint The integer part of the version - * @param int $versiondec The decimal part of the version + * @param int $versionint The integer part of the version + * @param string|int $versiondec The decimal part of the version * @return array */ public static function getValidatedVersionNumber( int $versionint, - int $versiondec, + string|int $versiondec, ): array { $today = date('Ymd'); + $versiondec = (int) $versiondec; + if ($versionint >= $today * 100) { // Integer version is already past today * 100, increment version decimal part instead of integer part. $versiondec = (int) $versiondec + 1; - $versiondec = sprintf("%'02d", $versiondec); } else { $versionint = $today . '00'; - $versiondec = '00'; + $versiondec = 0; } if ($versiondec >= 100) { // Decimal version is already past 99, increment integer part and reset decimal part. $versionint = (int) $versionint + 1; - $versiondec = '00'; + $versiondec = 0; } return [ (int) $versionint, - (string) $versiondec, + sprintf("%'02d", $versiondec), ]; } /** * Check if the branch is a stable branch. * - * @param string $branch The branch name - * @param bool $isdevbranch Whether the branch is a development branch + * @param string $branch The branch name + * @param bool $isdevbranch Whether the branch is a development branch * @return bool */ public static function isBranchStable( @@ -141,7 +143,7 @@ public static function isBranchStable( /** * Check whether a branch name is valid. * - * @param string $branch The branch name + * @param string $branch The branch name * @return bool */ public static function isBranchNameValid( @@ -156,13 +158,13 @@ public static function isBranchNameValid( return true; } - return (preg_match('#^(MOODLE_(\d+)_STABLE)$#', $branch, $matches)); + return !!(preg_match('#^(MOODLE_(\d+)_STABLE)$#', $branch, $matches)); } /** * Ensure the branch name is valid. * - * @param string $branch The branch name + * @param string $branch The branch name * @throws Exception */ public static function requireBranchNameValid( @@ -176,7 +178,7 @@ public static function requireBranchNameValid( /** * Check whether the type is valid. * - * @param string $type The type of the release + * @param string $type The type of the release * @return bool */ public static function isTypeValid( @@ -189,7 +191,7 @@ public static function isTypeValid( /** * Ensure the type is valid. * - * @param string $type The type of the release + * @param string $type The type of the release * @throws Exception */ public static function requireTypeValid( @@ -203,7 +205,7 @@ public static function requireTypeValid( /** * Check whether the path is valid. * - * @param string $path The path to the version file + * @param string $path The path to the version file * @return bool */ public static function isPathValid( @@ -220,7 +222,7 @@ public static function isPathValid( /** * Ensure the path is valid. * - * @param string $path The path to the version file + * @param string $path The path to the version file * @throws Exception */ public static function requirePathValid( @@ -238,7 +240,7 @@ public static function requirePathValid( /** * Validate the version file. * - * @param string $contents The contents of the version file + * @param string $contents The contents of the version file * @return bool */ public static function isVersionFileValid( @@ -259,8 +261,7 @@ public static function isVersionFileValid( /** * Ensure the version file is valid. * - * @param string $contents The contents of the version file - * @param string $branch The branch name + * @param string $contents The contents of the version file * @throws Exception */ public static function requireVersionFileValid( @@ -274,9 +275,9 @@ public static function requireVersionFileValid( /** * Get the value of an option from the options array. * - * @param array $options THe options configuration - * @param string $short The short name of the option - * @param string $long The long name of the option + * @param array $options The options configuration + * @param string $short The short name of the option + * @param string $long The long name of the option * @return mixed */ public static function getOption( @@ -288,13 +289,12 @@ public static function getOption( throw new Exception("Required option -$short|--$long must be provided.", __LINE__); } if ( - (isset($options[$short]) && is_array($options[$short])) || - (isset($options[$long]) && is_array($options[$long])) || - (isset($options[$short]) && isset($options[$long])) + (isset($options[$short]) && is_array($options[$short])) + || (isset($options[$long]) && is_array($options[$long])) + || (isset($options[$short]) && isset($options[$long])) ) { throw new Exception("Option -$short|--$long specified more than once.", __LINE__); } return (isset($options[$short])) ? $options[$short] : $options[$long]; } - } diff --git a/src/VersionInfo.php b/src/VersionInfo.php index 4535ab4..df099ca 100644 --- a/src/VersionInfo.php +++ b/src/VersionInfo.php @@ -1,4 +1,5 @@ - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @copyright 2024 Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class VersionInfo { /** @var int The name of the branch either in integer form, or MOODLE_(\d+)_STABLE form */ public readonly int $branch; + /** @var string The integer version number */ + public readonly string $decimalversion; + /** * Construct a new instance of the VersionInfo class. * - * @param int $integerversion The integer version number. - * @param string $decimalversion Part of the version number after the dot. - * @param string $comment The comment to use for the version. - * @param string $release The release name - * @param string $build The build number - * @param int|string $branch The branch number or name - * @param string $maturity The maturity of the branch - * @param string $branchquote The quote used for the branch - * @param string $releasequote The quote used for the release + * @param int $integerversion The integer version number. + * @param int $decimalversion Part of the version number after the dot. + * @param string $comment The comment to use for the version. + * @param string $release The release name + * @param string $build The build number + * @param int|string $branch The branch number or name + * @param string $maturity The maturity of the branch + * @param string $branchquote The quote used for the branch + * @param string $releasequote The quote used for the release */ public function __construct( public readonly int $integerversion, - public readonly string $decimalversion, + int $decimalversion, public readonly string $comment, public readonly string $release, public readonly string $build, @@ -56,27 +60,32 @@ public function __construct( if (is_string($branch)) { $branch = preg_replace('#^MOODLE_(\d+)_STABLE$#', '$1', $branch); } - $this->branch = $branch; + $this->branch = (int) $branch; + + $this->decimalversion = sprintf("%02d", $decimalversion); } /** * Create a new VersionInfo from the content of a version.php file. * - * @param string $versionfile The content of the file + * @param string $versionfile The content of the file * @throws \Exception * @return self */ - public static function fromVersionContent(string $versionfile): self { + public static function fromVersionContent(string $versionfile): self + { Helper::requireVersionFileValid($versionfile); - if (!preg_match('#^ *\$version *= *(?P\d{10})\.(?P\d{2})\d?[^\/]*(?P/[^\n]*)#m', $versionfile, $matches)) { + $commentregex = '#^ *\$version *= *(?P\d{10})\.(?P\d{2})\d?[^\/]*(?P/[^\n]*)#m'; + if (!preg_match($commentregex, $versionfile, $matches)) { throw new Exception('Could not determine version.', __LINE__); } $integerversion = $matches['integer']; $decimalversion = $matches['decimal']; $comment = $matches['comment']; - if (!preg_match('#^ *\$release *= *(?P\'|")(?P[^ \+]+\+?) *\(Build: (?P\d{8})\)\1#m', $versionfile, $matches)) { + $releaseregex = '#^ *\$release *= *(?P\'|")(?P[^ \+]+\+?) *\(Build: (?P\d{8})\)\1#m'; + if (!preg_match($releaseregex, $versionfile, $matches)) { throw new Exception('Could not determine the release.', __LINE__); } $release = $matches['release']; @@ -94,8 +103,8 @@ public static function fromVersionContent(string $versionfile): self { $maturity = $matches['maturity']; return new self( - integerversion: $integerversion, - decimalversion: $decimalversion, + integerversion: (int) $integerversion, + decimalversion: (int) $decimalversion, comment: $comment, release: $release, build: $buildcurrent, @@ -109,11 +118,12 @@ public static function fromVersionContent(string $versionfile): self { /** * Create a new VersionInfo from the path to a version.php file * - * @param string $path The path to the file + * @param string $path The path to the file * @throws \Exception * @return self */ - public static function fromVersionFile(string $path): self { + public static function fromVersionFile(string $path): self + { Helper::requirePathValid($path); $versionfile = file_get_contents($path); @@ -123,11 +133,11 @@ public static function fromVersionFile(string $path): self { /** * Get the VersionInfo instance for the Moodle version that follows this one. * - * @param string $branch The branch name - * @param string $type The release type - * @param string $rc The release candidate number - * @param bool $isdevbranch Whether this is a development branch - * @param mixed $date The date to use for the version + * @param string $branch The branch name + * @param string $type The release type + * @param string $rc The release candidate number + * @param bool $isdevbranch Whether this is a development branch + * @param string|null $date The date to use for the version * @throws \Exception * @return VersionInfo */ @@ -161,7 +171,7 @@ public function getNextVersion( $decimalversion++; $maturity = 'MATURITY_STABLE'; - } else if ($type === 'minor' || $type === 'major') { + } elseif ($type === 'minor' || $type === 'major') { // If it's minor fine, it's if major then stable gets a minor release. // 2.6+ => 2.6.1 // 2.6.12+ => 2.6.13 @@ -171,7 +181,7 @@ public function getNextVersion( } if (preg_match('#^(?P\d+\.\d+)\.(?P\d+)#', $release, $matches)) { $increment = $matches['increment'] + 1; - $release = $matches['version'].'.'.(string)$increment; + $release = $matches['version'] . '.' . (string)$increment; } else { // First minor release on this stable branch. Yay X.Y.1. $release .= '.1'; @@ -186,13 +196,13 @@ public function getNextVersion( } } } - } else { // Ok it's a development branch. if ($type === 'weekly' || $type === 'minor') { - // If it's weekly, ok, if it's minor the dev branch doesn't get a minor release so really it's a weekly anyway. - // It's a dev branch. We need to bump the version, if the version is already higher than today*100 then we need - // to bump accordingly. + // If it's weekly, ok. + // If it's minor the dev branch doesn't get a minor release so really it's a weekly anyway. + // It's a dev branch. We need to bump the version. + // If the version is already higher than today * 100 then we need to bump accordingly. // If under beta or rc, make weekly behave exactly as on-demand. if (strpos($release, 'beta') !== false or strpos($release, 'rc') !== false) { // Add the + if missing. @@ -200,37 +210,48 @@ public function getNextVersion( // Add the + $release .= '+'; } - list($integerversion, $decimalversion) = Helper::getValidatedVersionNumber($integerversion, $decimalversion); - } else if (strpos($release, 'dev') === false) { - // Must be immediately after a major release. Bump the release version and set maturity to Alpha. - $release = (float)$release + 0.1; - $release = (string)$release.'dev'; - $maturity = 'MATURITY_ALPHA'; + } elseif (strpos($release, 'dev') === false) { + throw new Exception( + 'Weekly releases are only allowed on dev branches. Have you run a back-to-dev release yet?', + __LINE__, + ); } - list($integerversion, $decimalversion) = Helper::getValidatedVersionNumber($integerversion, $decimalversion); - } else if ($type === 'beta') { + [ + $integerversion, + $decimalversion, + ] = Helper::getValidatedVersionNumber($integerversion, $decimalversion); + } elseif ($type === 'beta') { $release = preg_replace('#^(\d+.\d+) *(dev|beta)\+?#', '$1', $release); $branch = $branchcurrent; // Branch doesn't change in beta releases ever. $release .= 'beta'; - list($integerversion, $decimalversion) = Helper::getValidatedVersionNumber($integerversion, $decimalversion); + [ + $integerversion, + $decimalversion, + ] = Helper::getValidatedVersionNumber($integerversion, $decimalversion); $maturity = 'MATURITY_BETA'; - } else if ($type === 'rc') { + } elseif ($type === 'rc') { $release = preg_replace('#^(\d+.\d+) *(dev|beta|rc\d)\+?#', '$1', $release); $branch = $branchcurrent; // Branch doesn't change in rc releases ever. - $release .= 'rc'.$rc; - list($integerversion, $decimalversion) = Helper::getValidatedVersionNumber($integerversion, $decimalversion); + $release .= 'rc' . $rc; + [ + $integerversion, + $decimalversion, + ] = Helper::getValidatedVersionNumber($integerversion, $decimalversion); $maturity = 'MATURITY_RC'; - } else if ($type === 'on-demand') { + } elseif ($type === 'on-demand') { // Add the + if missing (normally applies to post betas & rcs only, // but it's not wrong to generalize it to any on-demand). if (strpos($release, '+') === false) { // Add the + $release .= '+'; } - list($integerversion, $decimalversion) = Helper::getValidatedVersionNumber($integerversion, $decimalversion); - } else if ($type === 'on-sync') { + [ + $integerversion, + $decimalversion, + ] = Helper::getValidatedVersionNumber($integerversion, $decimalversion); + } elseif ($type === 'on-sync') { $decimalversion++; - } else if ($type === 'back-to-dev') { + } elseif ($type === 'back-to-dev') { // We perform back-to-dev on the `main` branch only. if (strpos($release, 'dev') !== false) { // Ensure it's not a "dev" version already. throw new Exception('Back-to-dev is only allowed on non-dev branches.', __LINE__); @@ -245,8 +266,8 @@ public function getNextVersion( $branch = Helper::getNextBranchNumber($this->branch); // This require knowledge of our branching scheme. - $releasemajor = (int) substr($branch, 0, 1); - $releaseminor = (int) substr($branch, 2, 1); + $releasemajor = (int) substr((string) $branch, 0, 1); + $releaseminor = (int) substr((string) $branch, 2, 1); $release = "{$releasemajor}.{$releaseminor}dev"; @@ -256,11 +277,14 @@ public function getNextVersion( $build = date('Ymd', strtotime('next Monday')); } } - } else if ($type === 'major') { + } elseif ($type === 'major') { // Awesome major release! $release = preg_replace('#^(\d+.\d+) *(dev|beta|rc\d+)\+?#', '$1', $release); $branch = $branchcurrent; // Branch doesn't change in major releases ever. - list($integerversion, $decimalversion) = Helper::getValidatedVersionNumber($integerversion, $decimalversion); + [ + $integerversion, + $decimalversion, + ] = Helper::getValidatedVersionNumber($integerversion, $decimalversion); $maturity = 'MATURITY_STABLE'; // Now handle builddate for releases. if (empty($date)) { // If no date has been forced, dev majors always are released on Monday. @@ -286,7 +310,7 @@ public function getNextVersion( // Replace the old version with the new version. if (strlen($decimalversion) === 1) { - $decimalversion = '0'.$decimalversion; + $decimalversion = '0' . $decimalversion; } if ($branch === 'main') { @@ -311,9 +335,14 @@ public function getNextVersion( * * @return string */ - public function generateVersionFile(): string { + public function generateVersionFile(): string + { $versionFile = file_get_contents(__DIR__ . '/templates/version.php.tpl'); + if ($versionFile === false) { + throw new Exception('Could not read the version template file.', __LINE__); + } + $replacements = [ 'INTEGERVERSION' => $this->integerversion, 'DECIMALVERSION' => $this->decimalversion, diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 8a1057e..6b1c25c 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -1,4 +1,5 @@ [ 'content' => <<getNextVersion(...$nextVersionArgs); @@ -48,7 +48,7 @@ public static function nextVersionFromMajorProvider(): array { $majorVersion = [ 'integerversion' => 2024092300, - 'decimalversion' => '00', + 'decimalversion' => 0, 'comment' => '// 20240923 = branching date YYYYMMDD - do not modify!', 'release' => '4.5', 'build' => '20240921', @@ -133,24 +133,6 @@ public static function nextVersionFromMajorProvider(): array 'releasequote' => "'", ], ], - 'Development version from major' => [ - $majorVersion, - [ - 'branch' => 'MOODLE_500_STABLE', - 'type' => 'weekly', - 'rc' => '', - 'date' => '20240923', - 'isdevbranch' => true, - ], - [ - 'integerversion' => date('Ymd') * 100, - 'decimalversion' => '00', - 'release' => '4.6dev', // Note: The tooling has not yet been updated to handle the new versioning scheme. - 'build' => '20240923', - 'branchquote' => "'", - 'releasequote' => "'", - ], - ], ]; } @@ -158,7 +140,7 @@ public static function nextVersionFromMinorProvider(): array { $minorVersion = [ 'integerversion' => 2024092301, - 'decimalversion' => '00', + 'decimalversion' => 0, 'comment' => '// 20240923 = branching date YYYYMMDD - do not modify!', 'release' => '4.5.1', 'build' => '20240921', @@ -229,7 +211,7 @@ public static function nextVersionFromWeeklyProvider(): array { $weeklyVersion = [ 'integerversion' => 2024092301, - 'decimalversion' => '00', + 'decimalversion' => 0, 'comment' => '// 20240923 = branching date YYYYMMDD - do not modify!', 'release' => '4.5.1+', 'build' => '20240921', @@ -300,7 +282,7 @@ public static function nextVersionFromDevelopmentProvider(): array { $version = [ 'integerversion' => 2024092301, - 'decimalversion' => '00', + 'decimalversion' => 0, 'comment' => '// 20240923 = branching date YYYYMMDD - do not modify!', 'release' => '5.0dev', 'build' => '20240921', @@ -453,7 +435,7 @@ public static function nextVersionFromBetaProvider(): array { $version = [ 'integerversion' => 2024092301, - 'decimalversion' => '00', + 'decimalversion' => 0, 'comment' => '// 20240923 = branching date YYYYMMDD - do not modify!', 'release' => '5.0beta', 'build' => '20240921', @@ -483,6 +465,25 @@ public static function nextVersionFromBetaProvider(): array 'maturity' => 'MATURITY_BETA', ], ], + 'Weekly version from beta' => [ + $version, + [ + 'branch' => 'MOODLE_500_STABLE', + 'type' => 'weekly', + 'rc' => '', + 'date' => '20240923', + 'isdevbranch' => true, + ], + [ + 'integerversion' => date('Ymd') * 100, + 'decimalversion' => '00', + 'release' => '5.0beta+', + 'build' => '20240923', + 'branchquote' => "'", + 'releasequote' => "'", + 'maturity' => 'MATURITY_BETA', + ], + ], 'RC version from beta' => [ $version, [ @@ -560,7 +561,7 @@ public static function invalidNextVersionMigrationsProvider(): array { $developmentVersion = [ 'integerversion' => 2024092301, - 'decimalversion' => '00', + 'decimalversion' => 0, 'comment' => '// 20240923 = branching date YYYYMMDD - do not modify!', 'release' => '5.0dev', 'build' => '20240921', @@ -571,7 +572,7 @@ public static function invalidNextVersionMigrationsProvider(): array ]; $majorVersion = [ 'integerversion' => 2024092300, - 'decimalversion' => '00', + 'decimalversion' => 0, 'comment' => '// 20240923 = branching date YYYYMMDD - do not modify!', 'release' => '4.5', 'build' => '20240921', @@ -602,6 +603,16 @@ public static function invalidNextVersionMigrationsProvider(): array 'isdevbranch' => true, ], ], + 'Development version from major' => [ + $majorVersion, + [ + 'branch' => 'MOODLE_500_STABLE', + 'type' => 'weekly', + 'rc' => '', + 'date' => '20240923', + 'isdevbranch' => true, + ], + ], ]; } @@ -609,8 +620,7 @@ public static function invalidNextVersionMigrationsProvider(): array public function testFromVersionContent( string $versionFileName, array $expectations, - ): void - { + ): void { $versionFileContent = file_get_contents($versionFileName); $version = VersionInfo::fromVersionContent($versionFileContent); @@ -623,8 +633,7 @@ public function testFromVersionContent( public function testFromVersionFile( string $versionFileName, array $expectations, - ): void - { + ): void { $version = VersionInfo::fromVersionFile($versionFileName); foreach ($expectations as $property => $expectedValue) {