From ed07675e3a21eaade9d31a7709ee5e03902c5c09 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 10 Jul 2023 16:18:48 +0200 Subject: [PATCH 1/8] Various minor docs tweaks --- CHANGELOG.md | 2 +- PHPCSUtils/BackCompat/BCFile.php | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49289847..52fed072 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,7 @@ _Nothing yet._ #### Utils -* The `Arrays::getDoubleArrowPtr()` method could previously get confused over a double arrow in a keyed lists used as an array value. [#485] +* The `Arrays::getDoubleArrowPtr()` method could previously get confused over a double arrow in a keyed list used as an array value. [#485] [#485]: https://github.com/PHPCSStandards/PHPCSUtils/pull/485 diff --git a/PHPCSUtils/BackCompat/BCFile.php b/PHPCSUtils/BackCompat/BCFile.php index 0491138f..3c9b56a0 100644 --- a/PHPCSUtils/BackCompat/BCFile.php +++ b/PHPCSUtils/BackCompat/BCFile.php @@ -48,7 +48,8 @@ * * Additionally, this class works round the following tokenizer issues for * any affected utility functions: - * - None at this time. + * - `readonly` classes. + * - Constructor property promotion with `readonly` without visibility. * * Most functions in this class will have a related twin-function in the relevant * class in the `PHPCSUtils\Utils` namespace. From 5c56087611440413c233542d39f5333c7fb8b340 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 15 Jul 2023 13:26:59 +0200 Subject: [PATCH 2/8] GH Actions: special case Dependabot PRs for Coveralls Follow up on PR 468. Turns out Dependabot PRs do not have access to secrets with the exception of (read-only) access to the `GITHUB_TOKEN`. As the coverage test runs and the Coveralls status are required builds, this blocks Dependabot PRs from being merged without overruling the required statuses. As I'd like to avoid that situation, I'm special casing Dependabot PRs for the token selection. Unfortunately using a condition like `${{ github.actor != 'dependabot[bot]' || secrets.COVERALLS_TOKEN && secrets.GITHUB_TOKEN }}` won't work when it involves secrets, so we need to use duplicate steps to get round this. Refs: * lemurheavy/coveralls-public 1721 * https://docs.github.com/en/code-security/dependabot/working-with-dependabot/automating-dependabot-with-github-actions#responding-to-events --- .github/workflows/test.yml | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 40422292..de41dbdb 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -363,14 +363,25 @@ jobs: if: ${{ success() }} run: composer global require php-coveralls/php-coveralls:"^2.5.3" --no-interaction - - name: Upload coverage results to Coveralls - if: ${{ success() }} + - name: Upload coverage results to Coveralls (normal) + if: ${{ success() && github.actor != 'dependabot[bot]' }} env: COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_TOKEN }} COVERALLS_PARALLEL: true COVERALLS_FLAG_NAME: php-${{ matrix.php }}-phpcs-${{ matrix.phpcs_version }} run: php-coveralls -v -x build/logs/clover.xml + # Dependabot does not have access to secrets, other than the GH token. + # Ref: https://docs.github.com/en/code-security/dependabot/working-with-dependabot/automating-dependabot-with-github-actions + # Ref: https://github.com/lemurheavy/coveralls-public/issues/1721 + - name: Upload coverage results to Coveralls (Dependabot) + if: ${{ success() && github.actor == 'dependabot[bot]' }} + env: + COVERALLS_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }} + COVERALLS_PARALLEL: true + COVERALLS_FLAG_NAME: php-${{ matrix.php }}-phpcs-${{ matrix.phpcs_version }} + run: php-coveralls -v -x build/logs/clover.xml + coveralls-finish: needs: coverage if: always() && needs.coverage.result == 'success' @@ -378,8 +389,19 @@ jobs: runs-on: ubuntu-latest steps: - - name: Coveralls Finished + - name: Coveralls Finished (normal) + if: ${{ github.actor != 'dependabot[bot]' }} uses: coverallsapp/github-action@v2 with: github-token: ${{ secrets.COVERALLS_TOKEN }} parallel-finished: true + + # Dependabot does not have access to secrets, other than the GH token. + # Ref: https://docs.github.com/en/code-security/dependabot/working-with-dependabot/automating-dependabot-with-github-actions + # Ref: https://github.com/lemurheavy/coveralls-public/issues/1721 + - name: Coveralls Finished (Dependabot) + if: ${{ github.actor == 'dependabot[bot]' }} + uses: coverallsapp/github-action@v2 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + parallel-finished: true From 4d28dcaaec0d9bc4c9f487c92b6caa07c23416f6 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 15 Jul 2023 11:59:39 +0000 Subject: [PATCH 3/8] GH Actions: Bump actions/upload-pages-artifact from 1 to 2 Bumps [actions/upload-pages-artifact](https://github.com/actions/upload-pages-artifact) from 1 to 2. - [Release notes](https://github.com/actions/upload-pages-artifact/releases) - [Commits](https://github.com/actions/upload-pages-artifact/compare/v1...v2) --- updated-dependencies: - dependency-name: actions/upload-pages-artifact dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/update-docs.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/update-docs.yml b/.github/workflows/update-docs.yml index 3253c6cc..c5e4d913 100644 --- a/.github/workflows/update-docs.yml +++ b/.github/workflows/update-docs.yml @@ -129,7 +129,7 @@ jobs: destination: ./docs/_site - name: Upload GH Pages artifact - uses: actions/upload-pages-artifact@v1 + uses: actions/upload-pages-artifact@v2 with: path: ./docs/_site From 872a91a54a5a7749655bf3167670a7353482b96b Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 15 Jul 2023 13:14:59 +0200 Subject: [PATCH 4/8] Dependabot: switch to weekly No need to check on a daily basis. ("daily" was only really used to be able to test the setup) Follow up on 298 in which I explicitly stated: > Once the configuration has been "finalized" (I use that term loosely), this should be changed to run the Dependabot check only once a week. --- .github/dependabot.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index fa4063ba..08a065ec 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -8,7 +8,7 @@ updates: - package-ecosystem: "composer" directory: "/" schedule: - interval: "daily" + interval: "weekly" open-pull-requests-limit: 5 # Set to 0 to (temporarily) disable. versioning-strategy: widen commit-message: @@ -20,7 +20,7 @@ updates: - package-ecosystem: "github-actions" directory: "/" schedule: - interval: "daily" + interval: "weekly" open-pull-requests-limit: 5 commit-message: prefix: "GH Actions:" From ba77223b56e1df09dc78e5133459739437b74bc4 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 16 Jul 2023 11:33:06 +0200 Subject: [PATCH 5/8] GH Actions: update for php-coveralls 2.6.0 PHP-Coveralls 2.6.0 has just been released and includes a fix for the last known PHP 8.x issue. This means that it should now be safe to install php-coveralls on PHP 8.x and upload from there, which means we now only need the work-around for the PHP version when on PHP < 5.5 (as Coveralls v1 does not work with GH Actions). Ref: * https://github.com/php-coveralls/php-coveralls/releases/tag/v2.6.0 --- .github/workflows/test.yml | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index de41dbdb..6368e2b8 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -349,19 +349,18 @@ jobs: PHPCS_VERSION: ${{ matrix.phpcs_version }} PHPCSUTILS_USE_CACHE: false - # Uploading the results with PHP Coveralls v1 won't work from GH Actions, so switch the PHP version. - # Also PHP Coveralls itself (still) isn't fully compatible with PHP 8.0+. - - name: Switch to PHP 7.4 - if: ${{ success() && matrix.php != '7.4' }} + # PHP Coveralls v2 (which supports GH Actions) has a PHP 5.5 minimum, so switch the PHP version. + - name: Switch to PHP latest + if: ${{ success() && matrix.php == '5.4' }} uses: shivammathur/setup-php@v2 with: - php-version: 7.4 + php-version: 'latest' coverage: none - # Global install is used to prevent a conflict with the local composer.lock in PHP 8.0+. + # Global install is used to prevent a conflict with the local composer.lock. - name: Install Coveralls if: ${{ success() }} - run: composer global require php-coveralls/php-coveralls:"^2.5.3" --no-interaction + run: composer global require php-coveralls/php-coveralls:"^2.6.0" --no-interaction - name: Upload coverage results to Coveralls (normal) if: ${{ success() && github.actor != 'dependabot[bot]' }} From 977e71d1d0a704ba96fb21bf4f60bb0d3167b081 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 16 Jul 2023 23:06:19 +0200 Subject: [PATCH 6/8] SpacesFixer: prevent an unwarranted exception ... when the second pointer passed is a comment token and the comment is the last content of the current file. Includes tests. --- PHPCSUtils/Fixers/SpacesFixer.php | 2 +- .../SpacesFixer/SpacesFixerAtEOFTest.inc | 4 + .../SpacesFixerAtEOFTest.inc.fixed | 4 + .../SpacesFixer/SpacesFixerAtEOFTest.php | 139 ++++++++++++++++++ 4 files changed, 148 insertions(+), 1 deletion(-) create mode 100644 Tests/Fixers/SpacesFixer/SpacesFixerAtEOFTest.inc create mode 100644 Tests/Fixers/SpacesFixer/SpacesFixerAtEOFTest.inc.fixed create mode 100644 Tests/Fixers/SpacesFixer/SpacesFixerAtEOFTest.php diff --git a/PHPCSUtils/Fixers/SpacesFixer.php b/PHPCSUtils/Fixers/SpacesFixer.php index 168a2078..ff85ace9 100644 --- a/PHPCSUtils/Fixers/SpacesFixer.php +++ b/PHPCSUtils/Fixers/SpacesFixer.php @@ -133,7 +133,7 @@ public static function checkAndFix( } $nextNonEmpty = $phpcsFile->findNext(Tokens::$emptyTokens, ($ptrA + 1), null, true); - if ($nextNonEmpty < $ptrB) { + if ($nextNonEmpty !== false && $nextNonEmpty < $ptrB) { throw new RuntimeException( 'The $stackPtr and the $secondPtr token must be adjacent tokens separated only' . ' by whitespace and/or comments' diff --git a/Tests/Fixers/SpacesFixer/SpacesFixerAtEOFTest.inc b/Tests/Fixers/SpacesFixer/SpacesFixerAtEOFTest.inc new file mode 100644 index 00000000..4cad7aa2 --- /dev/null +++ b/Tests/Fixers/SpacesFixer/SpacesFixerAtEOFTest.inc @@ -0,0 +1,4 @@ +getTargetToken(self::TESTMARKER, \T_SEMICOLON); + $secondPtr = $this->getTargetToken(self::TESTMARKER, \T_COMMENT); + + SpacesFixer::checkAndFix( + self::$phpcsFile, + $stackPtr, + $secondPtr, + self::SPACES, + self::MSG, + self::CODE + ); + + $result = self::$phpcsFile->getErrors(); + $tokens = self::$phpcsFile->getTokens(); + + if (isset($result[$tokens[$stackPtr]['line']][$tokens[$stackPtr]['column']]) === false) { + $this->fail('Expected 1 violation. None found.'); + } + + $messages = $result[$tokens[$stackPtr]['line']][$tokens[$stackPtr]['column']]; + + // Expect one violation. + $this->assertCount(1, $messages, 'Expected 1 violation, found: ' . \count($messages)); + + /* + * Test the violation details. + */ + $this->assertSame(self::CODE, $messages[0]['source'], 'Error code comparison failed'); + + $this->assertSame(true, $messages[0]['fixable'], 'Fixability comparison failed'); + } + + /** + * Test that the fixes are correctly made when there is no non-empty token after the second stack pointer. + * + * @return void + */ + public function testFixesMade() + { + self::$phpcsFile->fixer->startFile(self::$phpcsFile); + self::$phpcsFile->fixer->enabled = true; + + $stackPtr = $this->getTargetToken(self::TESTMARKER, \T_SEMICOLON); + $secondPtr = $this->getTargetToken(self::TESTMARKER, \T_COMMENT); + + SpacesFixer::checkAndFix( + self::$phpcsFile, + $stackPtr, + $secondPtr, + self::SPACES, + self::MSG, + self::CODE + ); + + $fixedFile = self::$phpcsFile->getFilename() . '.fixed'; + $result = self::$phpcsFile->fixer->getContents(); + + $this->assertStringEqualsFile( + $fixedFile, + $result, + \sprintf( + 'Fixed version of %s does not match expected version in %s', + \basename(self::$caseFile), + \basename($fixedFile) + ) + ); + } +} From d16f825b5b16433e50f29ee538d67554c30f7e70 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 13 Apr 2023 04:10:14 +0200 Subject: [PATCH 7/8] BCFile::getDeclarationName(): sync with upstream Follow up on 451 Upstream PR squizlabs/PHP_CodeSniffer 3797, which will be included in PHPCS 3.8.0, fixes a tokenizer issue which affected the `File::getDeclarationName()` and, by extension, the `BackCompat::getDeclarationName()` method. The function name for functions named `self`, `parent` or `static` and declared to return by reference, could previously not be retrieved. The PHPCSUtils native version of the method `ObjectDeclarations::getName()` already handled things correctly. This commit adds back the BC-layer for the `getDeclarationName()` method and polyfills the fix from PHPCS 3.8.0 to backport it for PHPCS < 3.8.0. As the methods will now handle these functions in the same way, this commit also moves the related tests from the `GetNameDiffTest` to the `GetDeclarationNameTest`. --- PHPCSUtils/BackCompat/BCFile.php | 42 ++++++++++++++++++- .../BCFile/GetDeclarationNameTest.inc | 9 ++++ .../BCFile/GetDeclarationNameTest.php | 12 ++++++ .../ObjectDeclarations/GetNameDiffTest.inc | 9 ---- .../ObjectDeclarations/GetNameDiffTest.php | 12 ------ 5 files changed, 61 insertions(+), 23 deletions(-) diff --git a/PHPCSUtils/BackCompat/BCFile.php b/PHPCSUtils/BackCompat/BCFile.php index 3c9b56a0..030bde4b 100644 --- a/PHPCSUtils/BackCompat/BCFile.php +++ b/PHPCSUtils/BackCompat/BCFile.php @@ -50,6 +50,7 @@ * any affected utility functions: * - `readonly` classes. * - Constructor property promotion with `readonly` without visibility. + * - OO methods called `self`, `parent` or `static`. * * Most functions in this class will have a related twin-function in the relevant * class in the `PHPCSUtils\Utils` namespace. @@ -75,7 +76,7 @@ final class BCFile * * Changelog for the PHPCS native function: * - Introduced in PHPCS 0.0.5. - * - The upstream method has received no significant updates since PHPCS 3.7.1. + * - PHPCS 3.8.0: OO methods called `self`, `parent` or `static` are now correctly recognized. * * @see \PHP_CodeSniffer\Files\File::getDeclarationName() Original source. * @see \PHPCSUtils\Utils\ObjectDeclarations::getName() PHPCSUtils native improved version. @@ -97,7 +98,44 @@ final class BCFile */ public static function getDeclarationName(File $phpcsFile, $stackPtr) { - return $phpcsFile->getDeclarationName($stackPtr); + $tokens = $phpcsFile->getTokens(); + $tokenCode = $tokens[$stackPtr]['code']; + + if ($tokenCode === T_ANON_CLASS || $tokenCode === T_CLOSURE) { + return null; + } + + if ($tokenCode !== T_FUNCTION + && $tokenCode !== T_CLASS + && $tokenCode !== T_INTERFACE + && $tokenCode !== T_TRAIT + && $tokenCode !== T_ENUM + ) { + throw new RuntimeException('Token type "' . $tokens[$stackPtr]['type'] . '" is not T_FUNCTION, T_CLASS, T_INTERFACE, T_TRAIT or T_ENUM'); + } + + if ($tokenCode === T_FUNCTION + && strtolower($tokens[$stackPtr]['content']) !== 'function' + ) { + // This is a function declared without the "function" keyword. + // So this token is the function name. + return $tokens[$stackPtr]['content']; + } + + $content = null; + for ($i = ($stackPtr + 1); $i < $phpcsFile->numTokens; $i++) { + if ($tokens[$i]['code'] === T_STRING + // BC: PHPCS < 3.8.0. + || $tokens[$i]['code'] === T_SELF + || $tokens[$i]['code'] === T_PARENT + || $tokens[$i]['code'] === T_STATIC + ) { + $content = $tokens[$i]['content']; + break; + } + } + + return $content; } /** diff --git a/Tests/BackCompat/BCFile/GetDeclarationNameTest.inc b/Tests/BackCompat/BCFile/GetDeclarationNameTest.inc index eba7b48c..14902245 100644 --- a/Tests/BackCompat/BCFile/GetDeclarationNameTest.inc +++ b/Tests/BackCompat/BCFile/GetDeclarationNameTest.inc @@ -88,6 +88,15 @@ enum Suit: int implements Colorful, CardGame {} /* testFunctionReturnByRefWithReservedKeywordEach */ function &each() {} +/* testFunctionReturnByRefWithReservedKeywordParent */ +function &parent() {} + +/* testFunctionReturnByRefWithReservedKeywordSelf */ +function &self() {} + +/* testFunctionReturnByRefWithReservedKeywordStatic */ +function &static() {} + /* testLiveCoding */ // Intentional parse error. This has to be the last test in the file. function // Comment. diff --git a/Tests/BackCompat/BCFile/GetDeclarationNameTest.php b/Tests/BackCompat/BCFile/GetDeclarationNameTest.php index f4fbe64c..f7c8a358 100644 --- a/Tests/BackCompat/BCFile/GetDeclarationNameTest.php +++ b/Tests/BackCompat/BCFile/GetDeclarationNameTest.php @@ -196,6 +196,18 @@ public static function dataGetDeclarationName() '/* testFunctionReturnByRefWithReservedKeywordEach */', 'each', ], + 'function-return-by-reference-with-reserved-keyword-parent' => [ + '/* testFunctionReturnByRefWithReservedKeywordParent */', + 'parent', + ], + 'function-return-by-reference-with-reserved-keyword-self' => [ + '/* testFunctionReturnByRefWithReservedKeywordSelf */', + 'self', + ], + 'function-return-by-reference-with-reserved-keyword-static' => [ + '/* testFunctionReturnByRefWithReservedKeywordStatic */', + 'static', + ], ]; } } diff --git a/Tests/Utils/ObjectDeclarations/GetNameDiffTest.inc b/Tests/Utils/ObjectDeclarations/GetNameDiffTest.inc index 3e54ed11..7a7ae4e5 100644 --- a/Tests/Utils/ObjectDeclarations/GetNameDiffTest.inc +++ b/Tests/Utils/ObjectDeclarations/GetNameDiffTest.inc @@ -12,15 +12,6 @@ interface switch{ // Intentional parse error. public function someFunction(); } -/* testFunctionReturnByRefWithReservedKeywordParent */ -function &parent() {} - -/* testFunctionReturnByRefWithReservedKeywordSelf */ -function &self() {} - -/* testFunctionReturnByRefWithReservedKeywordStatic */ -function &static() {} - /* testLiveCoding */ // Intentional parse error. Redundancy testing. abstract class diff --git a/Tests/Utils/ObjectDeclarations/GetNameDiffTest.php b/Tests/Utils/ObjectDeclarations/GetNameDiffTest.php index 1bcb6942..f30d9549 100644 --- a/Tests/Utils/ObjectDeclarations/GetNameDiffTest.php +++ b/Tests/Utils/ObjectDeclarations/GetNameDiffTest.php @@ -117,18 +117,6 @@ public static function dataGetName() 'testMarker' => '/* testInvalidInterfaceName */', 'expected' => 'switch', ], - 'function-return-by-reference-with-reserved-keyword-parent' => [ - '/* testFunctionReturnByRefWithReservedKeywordParent */', - 'parent', - ], - 'function-return-by-reference-with-reserved-keyword-self' => [ - '/* testFunctionReturnByRefWithReservedKeywordSelf */', - 'self', - ], - 'function-return-by-reference-with-reserved-keyword-static' => [ - '/* testFunctionReturnByRefWithReservedKeywordStatic */', - 'static', - ], ]; } } From 3bae29522388df5cdac4c994e8802fa4f4e9f7a3 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 16 Jul 2023 23:13:35 +0200 Subject: [PATCH 8/8] Changelog for PHPCSUtils 1.0.8 --- CHANGELOG.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52fed072..4094d4ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,28 @@ This projects adheres to [Keep a CHANGELOG](https://keepachangelog.com/) and use _Nothing yet._ +## [1.0.8] - 2023-07-17 + +### Changed + +#### PHPCS BackCompat + +* `BCFile::getDeclarationName()`: sync with PHPCS 3.8.0 - support for functions called `self`, `parent` or `static` which return by reference. [#494] + +#### Other + +* Various housekeeping and minor documentation improvements. + +### Fixed + +#### Fixers + +* The [`SpacesFixer`] will no longer throw an (incorrect) exception when the second pointer passed is a comment token and this comment token is the last content in a file. [#493] + +[#493]: https://github.com/PHPCSStandards/PHPCSUtils/pull/493 +[#494]: https://github.com/PHPCSStandards/PHPCSUtils/pull/494 + + ## [1.0.7] - 2023-07-10 ### Changed @@ -906,6 +928,7 @@ This initial alpha release contains the following utility classes: [Unreleased]: https://github.com/PHPCSStandards/PHPCSUtils/compare/stable...HEAD +[1.0.8]: https://github.com/PHPCSStandards/PHPCSUtils/compare/1.0.7...1.0.8 [1.0.7]: https://github.com/PHPCSStandards/PHPCSUtils/compare/1.0.6...1.0.7 [1.0.6]: https://github.com/PHPCSStandards/PHPCSUtils/compare/1.0.5...1.0.6 [1.0.5]: https://github.com/PHPCSStandards/PHPCSUtils/compare/1.0.4...1.0.5