From 510e83853b7afe9ccde2422a9edaabbcf74326ce Mon Sep 17 00:00:00 2001 From: jdarwood007 Date: Sat, 3 Feb 2024 09:49:22 -0800 Subject: [PATCH 1/8] Setup action for CS Fixer This will now run PHP-CS-Fixer as a dedicated GitHub Action. When it finds errors with the changed files, it will fail the check. This is almost a copy and paste from: https://github.com/OskarStark/php-cs-fixer-ga --- .github/workflows/php-cs-fixer.yml | 31 ++++++++++++++++++++++++++++++ .github/workflows/php.yml | 4 +--- 2 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/php-cs-fixer.yml diff --git a/.github/workflows/php-cs-fixer.yml b/.github/workflows/php-cs-fixer.yml new file mode 100644 index 0000000000..98dc440aaf --- /dev/null +++ b/.github/workflows/php-cs-fixer.yml @@ -0,0 +1,31 @@ +on: + push: + branches: + - release-2.1 + pull_request: + +name: Main +jobs: + php-cs-fixer: + name: PHP-CS-Fixer + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Get changed files + id: changed-files + uses: tj-actions/changed-files@v42 + + - name: Get extra arguments for PHP-CS-Fixer + id: phpcs-intersection + run: | + CHANGED_FILES=$(echo "${{ steps.changed-files.outputs.all_changed_and_modified_files }}" | tr ' ' '\n') + if ! echo "${CHANGED_FILES}" | grep -qE "^(\\.php-cs-fixer(\\.dist)?\\.php|composer\\.lock)$"; then EXTRA_ARGS=$(printf -- '--path-mode=intersection\n--\n%s' "${CHANGED_FILES}"); else EXTRA_ARGS=''; fi + echo "PHPCS_EXTRA_ARGS<> $GITHUB_ENV + echo "$EXTRA_ARGS" >> $GITHUB_ENV + echo "EOF" >> $GITHUB_ENV + + - name: PHP-CS-Fixer + uses: docker://oskarstark/php-cs-fixer-ga + with: + args: --config=.php-cs-fixer.dist.php -v --dry-run --stop-on-violation --using-cache=no ${{ env.PHPCS_EXTRA_ARGS }}" \ No newline at end of file diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index 6b531a7597..0c9a84134a 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -27,8 +27,6 @@ jobs: - run: php -v - - run: composer lint - - name: Checking for sign off (GPG also accepted) run: php ./vendor/simplemachines/build-tools/check-signed-off.php @@ -44,7 +42,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - php: [ 8.0, 8.1 ] + php: [ 8.0, 8.1, 8.2, 8.3 ] name: PHP ${{ matrix.php }} Syntax Check steps: From 6f1400a3177a859d9d1d9896ca86d4b8904ed52e Mon Sep 17 00:00:00 2001 From: jdarwood007 Date: Sat, 3 Feb 2024 09:51:57 -0800 Subject: [PATCH 2/8] Change the name --- .github/workflows/php-cs-fixer.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/php-cs-fixer.yml b/.github/workflows/php-cs-fixer.yml index 98dc440aaf..cf0b21f782 100644 --- a/.github/workflows/php-cs-fixer.yml +++ b/.github/workflows/php-cs-fixer.yml @@ -4,7 +4,7 @@ on: - release-2.1 pull_request: -name: Main +name: PHP Check jobs: php-cs-fixer: name: PHP-CS-Fixer From 3a7430e870fde9d73a716a00da89a9b514e858a7 Mon Sep 17 00:00:00 2001 From: jdarwood007 Date: Sun, 4 Feb 2024 09:38:08 -0800 Subject: [PATCH 3/8] Add a custom PHP CS Fixer rule for our end of file fixes This should fix the following cases Case 1: ``` } ``` Case 2: ``` }?> ``` Case 3: ``` } ?> ``` Case 4: ``` ;?> ``` --- .github/phpcs/SMFClosingTag.php | 116 ++++++++++++++++++++++++++++++++ .github/workflows/php.yml | 1 - .php-cs-fixer.dist.php | 9 ++- 3 files changed, 124 insertions(+), 2 deletions(-) create mode 100644 .github/phpcs/SMFClosingTag.php diff --git a/.github/phpcs/SMFClosingTag.php b/.github/phpcs/SMFClosingTag.php new file mode 100644 index 0000000000..a5a89f7e4b --- /dev/null +++ b/.github/phpcs/SMFClosingTag.php @@ -0,0 +1,116 @@ + + * Dariusz Rumiński + * + * This source file is subject to the MIT license that is bundled + * with this source code in the file LICENSE. + */ + +namespace SMF\Fixer\Whitespace; + +use PhpCsFixer\AbstractFixer; +use PhpCsFixer\Fixer\WhitespacesAwareFixerInterface; +use PhpCsFixer\Fixer\Whitespace; +use PhpCsFixer\FixerDefinition\CodeSample; +use PhpCsFixer\FixerDefinition\FixerDefinition; +use PhpCsFixer\FixerDefinition\FixerDefinitionInterface; +use PhpCsFixer\Tokenizer\Tokens; +use PhpCsFixer\Tokenizer\Token; + +/** + * Ensure line endings match SMF standards. + * + * + * @author Fabien Potencier + * @author Dariusz Rumiński + * @author Jeremy Darwood + */ +final class closing_tag_fixer extends AbstractFixer implements WhitespacesAwareFixerInterface +{ + public function getName(): string + { + return 'SMF/closing_tag_fixer'; + } + + public function getDefinition(): FixerDefinitionInterface + { + return new FixerDefinition( + 'A PHP file must end with a closing tag.', + [ + new CodeSample(""), + new CodeSample(""), + ] + ); + } + + public function getPriority(): int + { + return -110; + } + + public function isCandidate(Tokens $tokens): bool + { + $count = $tokens->count(); + + // No tokens, not a canidate. + if ($count == 0) { + return false; + } + + // Last character is a white space, needs fixed. + if ($tokens[$count - 1]->isGivenKind(T_WHITESPACE)) { + return true; + } + + // We have closing bracket then closing barcket, single white space and then closing tag. + if ($tokens[$count - 1]->isGivenKind(T_CLOSE_TAG) && $tokens[$count - 2]->isGivenKind(T_WHITESPACE) && $tokens[$count - 3]->getContent() !== ';') { + return true; + } + + // We have a closing bracket, and then closing tag, no white space. + if ($tokens[$count - 1]->isGivenKind(T_CLOSE_TAG) && $tokens[$count - 2]->getContent() === '}') { + return true; + } + + return false; + } + + protected function applyFix(\SplFileInfo $file, Tokens $tokens): void + { + $count = $tokens->count(); + + // Last character is a white space. Adds a closing tag. + if ($count > 0 && $tokens[$count - 1]->isGivenKind(T_WHITESPACE)) { + $tokens[$count - 1] = new Token([ + T_WHITESPACE, + $this->whitespacesConfig->getLineEnding() . $this->whitespacesConfig->getLineEnding() . '?' . '>' + ]); + } + + // We have closing bracket then closing barcket, single white space and then closing tag. Add one more return. + if ($count > 0 && $tokens[$count - 1]->isGivenKind(T_CLOSE_TAG) && $tokens[$count - 2]->isGivenKind(T_WHITESPACE) && $tokens[$count - 3]->getContent() !== ';') { + $tokens[$count - 2] = new Token([ + T_WHITESPACE, + $this->whitespacesConfig->getLineEnding() . $this->whitespacesConfig->getLineEnding() + ]); + } + + // We have a closing bracket, and then closing tag, no white space, add returns. + // There is no ID/Name for closing curely bracket or semi-colon. + if ($count > 0 && $tokens[$count - 1]->isGivenKind(T_CLOSE_TAG) && $tokens[$count - 2]->getContent() === '}') { + + $tokens[$count - 1] = new Token([ + T_WHITESPACE, + $this->whitespacesConfig->getLineEnding() . $this->whitespacesConfig->getLineEnding() . '?' . '>' + ]); + } + } +} diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index 0c9a84134a..e2a156e665 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -32,7 +32,6 @@ jobs: - name: Checking file integrity run: | - php ./vendor/simplemachines/build-tools/check-eof.php php ./vendor/simplemachines/build-tools/check-smf-license.php php ./vendor/simplemachines/build-tools/check-smf-languages.php php ./vendor/simplemachines/build-tools/check-smf-index.php diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php index 55903ec773..6fd874e2b7 100644 --- a/.php-cs-fixer.dist.php +++ b/.php-cs-fixer.dist.php @@ -10,7 +10,6 @@ * * @version 3.0 Alpha 1 */ - $finder = (new PhpCsFixer\Finder()) ->in(__DIR__) // Don't touch libraries. @@ -29,10 +28,18 @@ // Skip anything being ignored in .gitignore. ->ignoreVCSIgnored(true); +require_once('.github/phpcs/SMFClosingTag.php'); + return (new PhpCsFixer\Config()) + ->registerCustomFixers([ + new \SMF\Fixer\Whitespace\closing_tag_fixer(), + ]) ->setRules([ '@PSR12' => true, + // A custom fixer for us to apply our line endings. + 'SMF/closing_tag_fixer' => true, + // PSR12 overrides. 'no_closing_tag' => false, 'no_break_comment' => false, // A bit buggy with comments. From dd676e08e89755701e5fef069316869a8b0df33b Mon Sep 17 00:00:00 2001 From: jdarwood007 Date: Sun, 4 Feb 2024 09:41:22 -0800 Subject: [PATCH 4/8] Licensing, not certain it needs to be this way. --- .github/phpcs/SMFClosingTag.php | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/.github/phpcs/SMFClosingTag.php b/.github/phpcs/SMFClosingTag.php index a5a89f7e4b..bd55f0cefa 100644 --- a/.github/phpcs/SMFClosingTag.php +++ b/.github/phpcs/SMFClosingTag.php @@ -1,16 +1,17 @@ - * Dariusz Rumiński + * Simple Machines Forum (SMF) * - * This source file is subject to the MIT license that is bundled - * with this source code in the file LICENSE. + * @package SMF + * @author Simple Machines https://www.simplemachines.org + * @copyright 2024 Simple Machines and individual contributors + * @license https://www.simplemachines.org/about/smf/license.php BSD + * + * @version 3.0 */ +declare(strict_types=1); namespace SMF\Fixer\Whitespace; @@ -26,9 +27,6 @@ /** * Ensure line endings match SMF standards. * - * - * @author Fabien Potencier - * @author Dariusz Rumiński * @author Jeremy Darwood */ final class closing_tag_fixer extends AbstractFixer implements WhitespacesAwareFixerInterface From 9ec4d4d1db33ffba07821fe69ac6300aa0f6c36a Mon Sep 17 00:00:00 2001 From: Bugo Date: Mon, 5 Feb 2024 00:16:43 +0500 Subject: [PATCH 5/8] Update some links in credits (http => https) Signed-off-by: Bugo --- Sources/Actions/Credits.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Sources/Actions/Credits.php b/Sources/Actions/Credits.php index 57a51005a8..dc742809aa 100644 --- a/Sources/Actions/Credits.php +++ b/Sources/Actions/Credits.php @@ -322,22 +322,22 @@ public function execute(): void // Give credit to any graphic library's, software library's, plugins etc Utils::$context['credits_software_graphics'] = [ 'graphics' => [ - 'Fugue Icons | © 2012 Yusuke Kamiyamane | These icons are licensed under a Creative Commons Attribution 3.0 License', - 'Oxygen Icons | These icons are licensed under GNU LGPLv3', + 'Fugue Icons | © 2012 Yusuke Kamiyamane | These icons are licensed under a Creative Commons Attribution 3.0 License', + 'Oxygen Icons | These icons are licensed under GNU LGPLv3', ], 'software' => [ 'JQuery | © John Resig | Licensed under The MIT License (MIT)', 'hoverIntent | © Brian Cherne | Licensed under The MIT License (MIT)', 'SCEditor | © Sam Clarke | Licensed under The MIT License (MIT)', - 'animaDrag | © Abel Mohler | Licensed under The MIT License (MIT)', - 'jQuery Custom Scrollbar | © Maciej Zubala | Licensed under The MIT License (MIT)', - 'jQuery Responsive Slider | © booncon ROCKETS | Licensed under The MIT License (MIT)', + 'animaDrag | © Abel Mohler | Licensed under The MIT License (MIT)', + 'jQuery Custom Scrollbar | © Maciej Zubala | Licensed under The MIT License (MIT)', + 'jQuery Responsive Slider | © booncon ROCKETS | Licensed under The MIT License (MIT)', 'At.js | © chord.luo@gmail.com | Licensed under The MIT License (MIT)', 'HTML5 Desktop Notifications | © Tsvetan Tsvetkov | Licensed under The Apache License Version 2.0', 'GAuth Code Generator/Validator | © Chris Cornutt | Licensed under The MIT License (MIT)', - 'Dropzone.js | © Matias Meno | Licensed under The MIT License (MIT)', - 'Minify | © Matthias Mullie | Licensed under The MIT License (MIT)', - 'PHP-Punycode | © True B.V. | Licensed under The MIT License (MIT)', + 'Dropzone.js | © Matias Meno | Licensed under The MIT License (MIT)', + 'Minify | © Matthias Mullie | Licensed under The MIT License (MIT)', + 'PHP-Punycode | © True B.V. | Licensed under The MIT License (MIT)', ], 'fonts' => [ ' Anonymous Pro | © 2009 | This font is licensed under the SIL Open Font License, Version 1.1', From 4d3f18907cda658d967ab1fe550923788c65ce90 Mon Sep 17 00:00:00 2001 From: Jon Stovell Date: Tue, 6 Feb 2024 13:41:33 -0700 Subject: [PATCH 6/8] Fixes some typecasting issues in SMF\Actions\Admin\Subscriptions Signed-off-by: Jon Stovell --- Sources/Actions/Admin/Subscriptions.php | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Sources/Actions/Admin/Subscriptions.php b/Sources/Actions/Admin/Subscriptions.php index 131373e5e2..6bf8458a83 100644 --- a/Sources/Actions/Admin/Subscriptions.php +++ b/Sources/Actions/Admin/Subscriptions.php @@ -1176,19 +1176,19 @@ public function modifyUser(): void Utils::$context['sub'] = [ 'id' => 0, 'start' => [ - 'year' => (int) Time::strftime('%Y', $row['start_time']), - 'month' => (int) Time::strftime('%m', $row['start_time']), - 'day' => (int) Time::strftime('%d', $row['start_time']), - 'hour' => (int) Time::strftime('%H', $row['start_time']), - 'min' => (int) Time::strftime('%M', $row['start_time']) < 10 ? '0' . (int) Time::strftime('%M', $row['start_time']) : (int) Time::strftime('%M', $row['start_time']), + 'year' => (int) Time::strftime('%Y', (int) $row['start_time']), + 'month' => (int) Time::strftime('%m', (int) $row['start_time']), + 'day' => (int) Time::strftime('%d', (int) $row['start_time']), + 'hour' => (int) Time::strftime('%H', (int) $row['start_time']), + 'min' => (int) Time::strftime('%M', (int) $row['start_time']), 'last_day' => 0, ], 'end' => [ - 'year' => (int) Time::strftime('%Y', $row['end_time']), - 'month' => (int) Time::strftime('%m', $row['end_time']), - 'day' => (int) Time::strftime('%d', $row['end_time']), - 'hour' => (int) Time::strftime('%H', $row['end_time']), - 'min' => (int) Time::strftime('%M', $row['end_time']) < 10 ? '0' . (int) Time::strftime('%M', $row['end_time']) : (int) Time::strftime('%M', $row['end_time']), + 'year' => (int) Time::strftime('%Y', (int) $row['end_time']), + 'month' => (int) Time::strftime('%m', (int) $row['end_time']), + 'day' => (int) Time::strftime('%d', (int) $row['end_time']), + 'hour' => (int) Time::strftime('%H', (int) $row['end_time']), + 'min' => (int) Time::strftime('%M', (int) $row['end_time']), 'last_day' => 0, ], 'status' => $row['status'], From 31aba214b6c76f6779dfa06b0bda02ad7a18b17b Mon Sep 17 00:00:00 2001 From: Bugo Date: Wed, 7 Feb 2024 02:23:21 +0500 Subject: [PATCH 7/8] Fix various type errors Signed-off-by: Bugo --- Sources/Actions/TopicRestore.php | 4 ++-- Sources/Actions/TopicSplit.php | 10 +++++----- Sources/Msg.php | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Sources/Actions/TopicRestore.php b/Sources/Actions/TopicRestore.php index 9642174ea0..34560b8bd1 100644 --- a/Sources/Actions/TopicRestore.php +++ b/Sources/Actions/TopicRestore.php @@ -60,7 +60,7 @@ public function execute(): void } // Can we be in here? - User::$me->isAllowedTo('move_any', Config::$modSettings['recycle_board']); + User::$me->isAllowedTo('move_any', [Config::$modSettings['recycle_board']]); $unfound_messages = []; $topics_to_restore = []; @@ -215,7 +215,7 @@ public function execute(): void } // Ok we got here so me move them from here to there. - Topic::move($row['id_topic'], $row['id_previous_board']); + Topic::move([$row['id_topic']], (int) $row['id_previous_board']); // Lets see if the board that we are returning to has post count enabled. $request2 = Db::$db->query( diff --git a/Sources/Actions/TopicSplit.php b/Sources/Actions/TopicSplit.php index dac2761d3b..c2dd871c7b 100644 --- a/Sources/Actions/TopicSplit.php +++ b/Sources/Actions/TopicSplit.php @@ -668,11 +668,11 @@ public static function splitTopic(int $split1_ID_TOPIC, array $splitMessages, st while ($row = Db::$db->fetch_assoc($request)) { // Get the right first and last message dependent on approved state... if (empty($split1_first_msg) || $row['myid_first_msg'] < $split1_first_msg) { - $split1_first_msg = $row['myid_first_msg']; + $split1_first_msg = (int) $row['myid_first_msg']; } if (empty($split1_last_msg) || $row['approved']) { - $split1_last_msg = $row['myid_last_msg']; + $split1_last_msg = (int) $row['myid_last_msg']; } // Get the counts correct... @@ -715,11 +715,11 @@ public static function splitTopic(int $split1_ID_TOPIC, array $splitMessages, st while ($row = Db::$db->fetch_assoc($request)) { // As before get the right first and last message dependent on approved state... if (empty($split2_first_msg) || $row['myid_first_msg'] < $split2_first_msg) { - $split2_first_msg = $row['myid_first_msg']; + $split2_first_msg = (int) $row['myid_first_msg']; } if (empty($split2_last_msg) || $row['approved']) { - $split2_last_msg = $row['myid_last_msg']; + $split2_last_msg = (int) $row['myid_last_msg']; } // Then do the counts again... @@ -935,7 +935,7 @@ public static function splitTopic(int $split1_ID_TOPIC, array $splitMessages, st // Housekeeping. Logging::updateStats('topic'); - Msg::updateLastMessages($id_board); + Msg::updateLastMessages([$id_board]); Logging::logAction('split', ['topic' => $split1_ID_TOPIC, 'new_topic' => $split2_ID_TOPIC, 'board' => $id_board]); diff --git a/Sources/Msg.php b/Sources/Msg.php index e825213797..d129beaf4f 100644 --- a/Sources/Msg.php +++ b/Sources/Msg.php @@ -2758,9 +2758,9 @@ public static function remove(int $message, bool $decreasePostCount = true): boo // And now to update the last message of each board we messed with. if ($recycle) { - Msg::updateLastMessages([$row['id_board'], Config::$modSettings['recycle_board']]); + self::updateLastMessages([$row['id_board'], Config::$modSettings['recycle_board']]); } else { - Msg::updateLastMessages($row['id_board']); + self::updateLastMessages([$row['id_board']]); } // Close any moderation reports for this message. From 5ca00681c72083d1c492db2eb302d89d0eebcab5 Mon Sep 17 00:00:00 2001 From: Jon Stovell Date: Tue, 6 Feb 2024 14:43:49 -0700 Subject: [PATCH 8/8] Fixes another typecasting bug Signed-off-by: Jon Stovell --- Sources/Actions/Admin/Tasks.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Actions/Admin/Tasks.php b/Sources/Actions/Admin/Tasks.php index d34913c31c..ab7f5521e4 100644 --- a/Sources/Actions/Admin/Tasks.php +++ b/Sources/Actions/Admin/Tasks.php @@ -374,7 +374,7 @@ public function edit(): void 'disabled' => $row['disabled'], 'offset' => $row['time_offset'], 'regularity' => $row['time_regularity'], - 'offset_formatted' => date('H:i', $row['time_offset']), + 'offset_formatted' => date('H:i', (int) $row['time_offset']), 'unit' => $row['time_unit'], ]; }