From 4102b38b6b5106887018199a9c14b183d875964c Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 18 Apr 2023 18:02:25 +0200 Subject: [PATCH 01/18] Composer: remove the explicit PHPUnit requirement ... in favour of letting the PHPUnit Polyfills handle the supported PHPUnit versions. --- composer.json | 1 - 1 file changed, 1 deletion(-) diff --git a/composer.json b/composer.json index 3cb7167b..2fcdb955 100644 --- a/composer.json +++ b/composer.json @@ -31,7 +31,6 @@ "phpcsstandards/phpcsdevcs": "^1.1.3", "php-parallel-lint/php-parallel-lint": "^1.3.2", "php-parallel-lint/php-console-highlighter": "^1.0", - "phpunit/phpunit": "^4.8.36 || ^5.7.21 || ^6.0 || ^7.0 || ^8.0 || ^9.3", "yoast/phpunit-polyfills": "^1.0.1" }, "minimum-stability": "dev", From d7215c1095c526b7820c35ccdb4ad059f7deaba2 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 21 Apr 2023 15:50:32 +0200 Subject: [PATCH 02/18] RemarkLint: switch plugin The [`remark-lint-are-links-valid-alive`](https://github.com/wemake-services/remark-lint-are-links-valid) has been abandoned and doesn't allow to skip false positives. The [`remark-lint-no-dead-urls`](https://github.com/remarkjs/remark-lint-no-dead-urls) plugin is still maintained and does allow adding some configuration to skip false positives. This commit switches the one plugin out for the other. --- .github/workflows/basics.yml | 2 +- .remarkrc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/basics.yml b/.github/workflows/basics.yml index 3fa103ae..9e58809f 100644 --- a/.github/workflows/basics.yml +++ b/.github/workflows/basics.yml @@ -143,6 +143,7 @@ jobs: remark-preset-lint-markdown-style-guide remark-lint-checkbox-content-indent remark-lint-linebreak-style + remark-lint-no-dead-urls remark-lint-no-duplicate-defined-urls remark-lint-no-empty-url remark-lint-no-heading-like-paragraph @@ -154,7 +155,6 @@ jobs: remark-lint-list-item-punctuation remark-lint-match-punctuation remark-lint-no-hr-after-heading - remark-lint-are-links-valid-alive remark-lint-are-links-valid-duplicate remark-validate-links diff --git a/.remarkrc b/.remarkrc index 83ab108b..95c4c322 100644 --- a/.remarkrc +++ b/.remarkrc @@ -8,6 +8,7 @@ ["remark-lint-linebreak-style", "unix"], ["remark-lint-link-title-style", "\""], ["remark-lint-ordered-list-marker-style", "."], + "remark-lint-no-dead-urls", "remark-lint-no-duplicate-defined-urls", "remark-lint-no-duplicate-definitions", "remark-lint-no-empty-url", @@ -29,7 +30,6 @@ "remark-lint-list-item-punctuation", "remark-lint-match-punctuation", "remark-lint-no-hr-after-heading", - "remark-lint-are-links-valid-alive", "remark-lint-are-links-valid-duplicate", "remark-validate-links" ] From 80492c66a14a746037220115f1b13e087a75bce4 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 29 Apr 2023 02:17:13 +0200 Subject: [PATCH 03/18] FunctionDeclarations::getProperties(): add caching Follow up on 332 While parsing the properties of function declarations was reasonably straight forward, with the introduction of union type in PHP 8.0, intersection types in PHP 8.1 and disjunctive normal form types in PHP 8.2, the retrieving of the return type has become more token walking intensive. With that in mind, caching the results of the function seems prudent. Includes a dedicated test to verify the cache is used and working. --- PHPCSUtils/Utils/FunctionDeclarations.php | 9 +++- .../GetPropertiesDiffTest.php | 45 +++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/PHPCSUtils/Utils/FunctionDeclarations.php b/PHPCSUtils/Utils/FunctionDeclarations.php index c6944fc9..85986a64 100644 --- a/PHPCSUtils/Utils/FunctionDeclarations.php +++ b/PHPCSUtils/Utils/FunctionDeclarations.php @@ -193,6 +193,10 @@ public static function getProperties(File $phpcsFile, $stackPtr) throw new RuntimeException('$stackPtr must be of type T_FUNCTION or T_CLOSURE or an arrow function'); } + if (Cache::isCached($phpcsFile, __METHOD__, $stackPtr) === true) { + return Cache::get($phpcsFile, __METHOD__, $stackPtr); + } + if ($tokens[$stackPtr]['code'] === \T_FUNCTION) { $valid = Tokens::$methodPrefixes; } else { @@ -292,7 +296,7 @@ public static function getProperties(File $phpcsFile, $stackPtr) $returnType = '?' . $returnType; } - return [ + $returnValue = [ 'scope' => $scope, 'scope_specified' => $scopeSpecified, 'return_type' => $returnType, @@ -304,6 +308,9 @@ public static function getProperties(File $phpcsFile, $stackPtr) 'is_static' => $isStatic, 'has_body' => $hasBody, ]; + + Cache::set($phpcsFile, __METHOD__, $stackPtr, $returnValue); + return $returnValue; } /** diff --git a/Tests/Utils/FunctionDeclarations/GetPropertiesDiffTest.php b/Tests/Utils/FunctionDeclarations/GetPropertiesDiffTest.php index 1a0755a6..03259499 100644 --- a/Tests/Utils/FunctionDeclarations/GetPropertiesDiffTest.php +++ b/Tests/Utils/FunctionDeclarations/GetPropertiesDiffTest.php @@ -10,7 +10,9 @@ namespace PHPCSUtils\Tests\Utils\FunctionDeclarations; +use PHPCSUtils\Internal\Cache; use PHPCSUtils\TestUtils\UtilityMethodTestCase; +use PHPCSUtils\Tokens\Collections; use PHPCSUtils\Utils\FunctionDeclarations; /** @@ -159,4 +161,47 @@ protected function getPropertiesTestHelper( $this->assertSame($expected, $found); } + + /** + * Verify that the build-in caching is used when caching is enabled. + * + * @return void + */ + public function testResultIsCached() + { + // The test case used is specifically selected to be one which will always reach the cache check. + $methodName = 'PHPCSUtils\\Utils\\FunctionDeclarations::getProperties'; + $testMarker = '/* testMessyPhpcsAnnotationsStaticClosure */'; + $expected = [ + 'scope' => 'public', + 'scope_specified' => false, + 'return_type' => '', + 'return_type_token' => false, + 'return_type_end_token' => false, + 'nullable_return_type' => false, + 'is_abstract' => false, + 'is_final' => false, + 'is_static' => true, + 'has_body' => true, + ]; + + $stackPtr = $this->getTargetToken($testMarker, Collections::functionDeclarationTokens()); + + // Verify the caching works. + $origStatus = Cache::$enabled; + Cache::$enabled = true; + + $resultFirstRun = FunctionDeclarations::getProperties(self::$phpcsFile, $stackPtr); + $isCached = Cache::isCached(self::$phpcsFile, $methodName, $stackPtr); + $resultSecondRun = FunctionDeclarations::getProperties(self::$phpcsFile, $stackPtr); + + if ($origStatus === false) { + Cache::clear(); + } + Cache::$enabled = $origStatus; + + $this->assertSame($expected, $resultFirstRun, 'First result did not match expectation'); + $this->assertTrue($isCached, 'Cache::isCached() could not find the cached value'); + $this->assertSame($resultFirstRun, $resultSecondRun, 'Second result did not match first'); + } } From d3a71d1595ea887af8dfdca1421bf0730bedee06 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 29 Apr 2023 02:23:24 +0200 Subject: [PATCH 04/18] Composer: update dev dependencies --- composer.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 2fcdb955..0a7a3d4f 100644 --- a/composer.json +++ b/composer.json @@ -28,10 +28,10 @@ }, "require-dev" : { "ext-filter": "*", - "phpcsstandards/phpcsdevcs": "^1.1.3", + "phpcsstandards/phpcsdevcs": "^1.1.6", "php-parallel-lint/php-parallel-lint": "^1.3.2", "php-parallel-lint/php-console-highlighter": "^1.0", - "yoast/phpunit-polyfills": "^1.0.1" + "yoast/phpunit-polyfills": "^1.0.5" }, "minimum-stability": "dev", "prefer-stable": true, From 419f5922cc4da471b9b3364d16a7b1975d6e99d6 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 29 Apr 2023 02:53:25 +0200 Subject: [PATCH 05/18] Variables::getMemberProperties(): add caching Follow up on 332 While parsing the properties of property declarations was reasonably straight forward, with the introduction of union type in PHP 8.0, intersection types in PHP 8.1 and disjunctive normal form types in PHP 8.2, the retrieving of the property type has become more token walking intensive. With that in mind, caching the results of the function seems prudent. Includes a dedicated test to verify the cache is used and working. --- PHPCSUtils/Utils/Variables.php | 11 +++- .../Variables/GetMemberPropertiesDiffTest.php | 56 ++++++++++++++++--- 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/PHPCSUtils/Utils/Variables.php b/PHPCSUtils/Utils/Variables.php index 28cf466f..4c92e1ed 100644 --- a/PHPCSUtils/Utils/Variables.php +++ b/PHPCSUtils/Utils/Variables.php @@ -13,6 +13,7 @@ use PHP_CodeSniffer\Exceptions\RuntimeException; use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Util\Tokens; +use PHPCSUtils\Internal\Cache; use PHPCSUtils\Tokens\Collections; use PHPCSUtils\Utils\Scopes; use PHPCSUtils\Utils\TextStrings; @@ -82,6 +83,7 @@ final class Variables * - Defensive coding against incorrect calls to this method. * - Support PHP 8.0 identifier name tokens in property types, cross-version PHP & PHPCS. * - Support for the PHP 8.2 `true` type. + * - The results of this function call are cached during a PHPCS run for faster response times. * * @see \PHP_CodeSniffer\Files\File::getMemberProperties() Original source. * @see \PHPCSUtils\BackCompat\BCFile::getMemberProperties() Cross-version compatible version of the original. @@ -127,6 +129,10 @@ public static function getMemberProperties(File $phpcsFile, $stackPtr) throw new RuntimeException('$stackPtr is not a class member var'); } + if (Cache::isCached($phpcsFile, __METHOD__, $stackPtr) === true) { + return Cache::get($phpcsFile, __METHOD__, $stackPtr); + } + $valid = Collections::propertyModifierKeywords() + Tokens::$emptyTokens; $scope = 'public'; @@ -210,7 +216,7 @@ public static function getMemberProperties(File $phpcsFile, $stackPtr) } } - return [ + $returnValue = [ 'scope' => $scope, 'scope_specified' => $scopeSpecified, 'is_static' => $isStatic, @@ -220,6 +226,9 @@ public static function getMemberProperties(File $phpcsFile, $stackPtr) 'type_end_token' => $typeEndToken, 'nullable_type' => $nullableType, ]; + + Cache::set($phpcsFile, __METHOD__, $stackPtr, $returnValue); + return $returnValue; } /** diff --git a/Tests/Utils/Variables/GetMemberPropertiesDiffTest.php b/Tests/Utils/Variables/GetMemberPropertiesDiffTest.php index 544d6707..3e5c189d 100644 --- a/Tests/Utils/Variables/GetMemberPropertiesDiffTest.php +++ b/Tests/Utils/Variables/GetMemberPropertiesDiffTest.php @@ -10,6 +10,7 @@ namespace PHPCSUtils\Tests\Utils\Variables; +use PHPCSUtils\Internal\Cache; use PHPCSUtils\TestUtils\UtilityMethodTestCase; use PHPCSUtils\Utils\Variables; @@ -108,8 +109,8 @@ public function dataGetMemberProperties() { return [ 'php8.2-pseudo-type-true' => [ - '/* testPHP82PseudoTypeTrue */', - [ + 'identifier' => '/* testPHP82PseudoTypeTrue */', + 'expected' => [ 'scope' => 'public', 'scope_specified' => true, 'is_static' => false, @@ -121,8 +122,8 @@ public function dataGetMemberProperties() ], ], 'php8.2-pseudo-type-true-nullable' => [ - '/* testPHP82NullablePseudoTypeTrue */', - [ + 'identifier' => '/* testPHP82NullablePseudoTypeTrue */', + 'expected' => [ 'scope' => 'protected', 'scope_specified' => true, 'is_static' => true, @@ -134,8 +135,8 @@ public function dataGetMemberProperties() ], ], 'php8.2-pseudo-type-true-in-union' => [ - '/* testPHP82PseudoTypeTrueInUnion */', - [ + 'identifier' => '/* testPHP82PseudoTypeTrueInUnion */', + 'expected' => [ 'scope' => 'private', 'scope_specified' => true, 'is_static' => false, @@ -147,8 +148,8 @@ public function dataGetMemberProperties() ], ], 'php8.2-pseudo-type-invalid-true-false-union' => [ - '/* testPHP82PseudoTypeFalseAndTrue */', - [ + 'identifier' => '/* testPHP82PseudoTypeFalseAndTrue */', + 'expected' => [ 'scope' => 'public', 'scope_specified' => false, 'is_static' => false, @@ -161,4 +162,43 @@ public function dataGetMemberProperties() ], ]; } + + /** + * Verify that the build-in caching is used when caching is enabled. + * + * @return void + */ + public function testResultIsCached() + { + $methodName = 'PHPCSUtils\\Utils\\Variables::getMemberProperties'; + $cases = $this->dataGetMemberProperties(); + $identifier = $cases['php8.2-pseudo-type-true-in-union']['identifier']; + $expected = $cases['php8.2-pseudo-type-true-in-union']['expected']; + + $variable = $this->getTargetToken($identifier, \T_VARIABLE); + + if (isset($expected['type_token']) && $expected['type_token'] !== false) { + $expected['type_token'] += $variable; + } + if (isset($expected['type_end_token']) && $expected['type_end_token'] !== false) { + $expected['type_end_token'] += $variable; + } + + // Verify the caching works. + $origStatus = Cache::$enabled; + Cache::$enabled = true; + + $resultFirstRun = Variables::getMemberProperties(self::$phpcsFile, $variable); + $isCached = Cache::isCached(self::$phpcsFile, $methodName, $variable); + $resultSecondRun = Variables::getMemberProperties(self::$phpcsFile, $variable); + + if ($origStatus === false) { + Cache::clear(); + } + Cache::$enabled = $origStatus; + + $this->assertSame($expected, $resultFirstRun, 'First result did not match expectation'); + $this->assertTrue($isCached, 'Cache::isCached() could not find the cached value'); + $this->assertSame($resultFirstRun, $resultSecondRun, 'Second result did not match first'); + } } From b9ab8ee6f1baa7fbdfd1fcc1e138f32fc2948919 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 29 Apr 2023 02:43:10 +0200 Subject: [PATCH 06/18] UseStatements::splitImportUseStatement(): cache more often and check cache earlier Follow up on 332. Includes additional test. --- PHPCSUtils/Utils/UseStatements.php | 9 +++--- .../SplitImportUseStatementTest.php | 32 +++++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/PHPCSUtils/Utils/UseStatements.php b/PHPCSUtils/Utils/UseStatements.php index 4d0e6d6d..4e917322 100644 --- a/PHPCSUtils/Utils/UseStatements.php +++ b/PHPCSUtils/Utils/UseStatements.php @@ -199,6 +199,10 @@ public static function splitImportUseStatement(File $phpcsFile, $stackPtr) throw new RuntimeException('$stackPtr must be an import use statement'); } + if (Cache::isCached($phpcsFile, __METHOD__, $stackPtr) === true) { + return Cache::get($phpcsFile, __METHOD__, $stackPtr); + } + $statements = [ 'name' => [], 'function' => [], @@ -208,13 +212,10 @@ public static function splitImportUseStatement(File $phpcsFile, $stackPtr) $endOfStatement = $phpcsFile->findNext([\T_SEMICOLON, \T_CLOSE_TAG], ($stackPtr + 1)); if ($endOfStatement === false) { // Live coding or parse error. + Cache::set($phpcsFile, __METHOD__, $stackPtr, $statements); return $statements; } - if (Cache::isCached($phpcsFile, __METHOD__, $stackPtr) === true) { - return Cache::get($phpcsFile, __METHOD__, $stackPtr); - } - ++$endOfStatement; $start = true; diff --git a/Tests/Utils/UseStatements/SplitImportUseStatementTest.php b/Tests/Utils/UseStatements/SplitImportUseStatementTest.php index bbc99642..83b40647 100644 --- a/Tests/Utils/UseStatements/SplitImportUseStatementTest.php +++ b/Tests/Utils/UseStatements/SplitImportUseStatementTest.php @@ -333,4 +333,36 @@ public function testResultIsCached() $this->assertTrue($isCached, 'Cache::isCached() could not find the cached value'); $this->assertSame($resultFirstRun, $resultSecondRun, 'Second result did not match first'); } + + /** + * Verify that the build-in caching is used when caching is enabled and a parse error is encountered. + * + * @return void + */ + public function testResultIsCachedForParseError() + { + $methodName = 'PHPCSUtils\\Utils\\UseStatements::splitImportUseStatement'; + $cases = $this->dataSplitImportUseStatement(); + $testMarker = $cases['parse-error']['testMarker']; + $expected = $cases['parse-error']['expected']; + + $stackPtr = $this->getTargetToken($testMarker, \T_USE); + + // Verify the caching works. + $origStatus = Cache::$enabled; + Cache::$enabled = true; + + $resultFirstRun = UseStatements::splitImportUseStatement(self::$phpcsFile, $stackPtr); + $isCached = Cache::isCached(self::$phpcsFile, $methodName, $stackPtr); + $resultSecondRun = UseStatements::splitImportUseStatement(self::$phpcsFile, $stackPtr); + + if ($origStatus === false) { + Cache::clear(); + } + Cache::$enabled = $origStatus; + + $this->assertSame($expected, $resultFirstRun, 'First result did not match expectation'); + $this->assertTrue($isCached, 'Cache::isCached() could not find the cached value'); + $this->assertSame($resultFirstRun, $resultSecondRun, 'Second result did not match first'); + } } From f55fc8f3d1e9b36d5e1551bbe73eaa0bb09d41ec Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 29 Apr 2023 02:53:40 +0200 Subject: [PATCH 07/18] Docs: minor update --- PHPCSUtils/Utils/FunctionDeclarations.php | 1 + 1 file changed, 1 insertion(+) diff --git a/PHPCSUtils/Utils/FunctionDeclarations.php b/PHPCSUtils/Utils/FunctionDeclarations.php index 85986a64..efe921b2 100644 --- a/PHPCSUtils/Utils/FunctionDeclarations.php +++ b/PHPCSUtils/Utils/FunctionDeclarations.php @@ -150,6 +150,7 @@ public static function getName(File $phpcsFile, $stackPtr) * - Support for PHP 8.0 identifier name tokens in return types, cross-version PHP & PHPCS. * - Support for constructor property promotion with the PHP 8.1 readonly keyword without explicit visibility. * - Support for the PHP 8.2 `true` type. + * - The results of this function call are cached during a PHPCS run for faster response times. * * @see \PHP_CodeSniffer\Files\File::getMethodProperties() Original source. * @see \PHPCSUtils\BackCompat\BCFile::getMethodProperties() Cross-version compatible version of the original. From 13d50f0e10b614049b91aadd5c48190ef3be8b17 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 29 Apr 2023 14:47:18 +0200 Subject: [PATCH 08/18] Docs: minor fix --- PHPCSUtils/Utils/Namespaces.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PHPCSUtils/Utils/Namespaces.php b/PHPCSUtils/Utils/Namespaces.php index 4056e037..0d249926 100644 --- a/PHPCSUtils/Utils/Namespaces.php +++ b/PHPCSUtils/Utils/Namespaces.php @@ -208,7 +208,7 @@ public static function getDeclaredName(File $phpcsFile, $stackPtr, $clean = true } /** - * Determine the namespace an arbitrary token lives in. + * Find the stack pointer to the namespace declaration applicable for an arbitrary token. * * Take note: * 1. When a namespace declaration token or a token which is part of the namespace From f2f2e4b369a68b8c3a2523a9ea4d9803aa6d14e1 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 3 May 2023 11:43:03 +0200 Subject: [PATCH 09/18] Coveralls: use Coveralls specific token Coveralls prefers for repos to be identified with the Coveralls specific token they provide. While the GitHub secret token still works, the intention is to drop support for it at some point in the future. This commit anticipates on that by switching the token. Notes: * The `COVERALLS_TOKEN` has been added to the repo secrets. * People with admin access to the GH repo automatically also have access to the admin settings in Coveralls. If ever needed, the Coveralls token can be found (and regenerated) in the Coveralls admin for a repo. * After regeneration, the token as stored in the GH repo Settings -> Secrets and Variables -> Actions -> Repository secrets should be updated. :point_right: This does mean that forks which have turned Coveralls on for their own fork of this repo will also need to add a `COVERALLS_TOKEN` secret to their GitHub fork (with the token as can be found in the Coveralls settings for their fork). This is a one-time only action. --- .github/workflows/test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5977e75b..dbcc316f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -347,7 +347,7 @@ jobs: - name: Upload coverage results to Coveralls if: ${{ success() }} env: - COVERALLS_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }} + 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 @@ -362,5 +362,5 @@ jobs: - name: Coveralls Finished uses: coverallsapp/github-action@v2 with: - github-token: ${{ secrets.GITHUB_TOKEN }} + github-token: ${{ secrets.COVERALLS_TOKEN }} parallel-finished: true From 36b24d73cc0be1ce8d2b53714a3462acbd112185 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 15 May 2023 14:21:13 +0200 Subject: [PATCH 10/18] GH Actions: fix failing build Somewhere in the Remark toolchain something has changed which means that it now throws warnings about the cell padding for aligned tables. As those aligned tables are by design (for readability during maintenance of the markdown), this commit is set up to removes those warnings and to enforce aligned tables. Failing build: https://github.com/PHPCSStandards/PHPCSUtils/actions/runs/4980020304/jobs/8912378342 Refs: * https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-table-pipe-alignment * https://github.com/remarkjs/remark-lint/tree/main/packages/remark-lint-table-cell-padding --- .remarkrc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.remarkrc b/.remarkrc index 95c4c322..035ad2d9 100644 --- a/.remarkrc +++ b/.remarkrc @@ -25,7 +25,7 @@ "remark-lint-no-unneeded-full-reference-link", "remark-lint-no-unused-definitions", ["remark-lint-strikethrough-marker", "~~"], - ["remark-lint-table-cell-padding", "consistent"], + "remark-lint-table-pipe-alignment", "remark-lint-heading-whitespace", "remark-lint-list-item-punctuation", "remark-lint-match-punctuation", From 45f4964c2c7615ca5342b3f0a40244ebb044d1de Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 30 Oct 2022 22:40:49 +0100 Subject: [PATCH 11/18] BCFile::getClassProperties(): sync with upstream / add support for readonly classes > PHP 8.2 introduces `readonly` classes. The `readonly` keyword can be combined with the `abstract` or `final` keyword. See: https://3v4l.org/VIXgD Includes unit tests. Support for readonly classes was previously already added to the `ObjectDeclarations::getClassProperties()` in PR 367. Refs: * https://wiki.php.net/rfc/readonly_classes * squizlabs/PHP_CodeSniffer 3686. --- PHPCSUtils/BackCompat/BCFile.php | 49 ++++++++++++++- PHPCSUtils/Utils/ObjectDeclarations.php | 1 - .../BCFile/GetClassPropertiesTest.inc | 17 +++++ .../BCFile/GetClassPropertiesTest.php | 62 +++++++++++++++++-- .../GetClassPropertiesDiffTest.inc | 17 ----- .../GetClassPropertiesDiffTest.php | 55 ---------------- 6 files changed, 120 insertions(+), 81 deletions(-) diff --git a/PHPCSUtils/BackCompat/BCFile.php b/PHPCSUtils/BackCompat/BCFile.php index bd8a4614..d7d8b2b8 100644 --- a/PHPCSUtils/BackCompat/BCFile.php +++ b/PHPCSUtils/BackCompat/BCFile.php @@ -527,6 +527,7 @@ public static function getMemberProperties(File $phpcsFile, $stackPtr) * array( * 'is_abstract' => boolean, // TRUE if the abstract keyword was found. * 'is_final' => boolean, // TRUE if the final keyword was found. + * 'is_readonly' => false, // TRUE if the readonly keyword was found. * ); * ``` * @@ -534,12 +535,13 @@ public static function getMemberProperties(File $phpcsFile, $stackPtr) * * Changelog for the PHPCS native function: * - Introduced in PHPCS 1.3.0. - * - The upstream method has received no significant updates since PHPCS 3.7.1. + * - PHPCS 3.8.0: Added support for PHP 8.2 `readonly` classes. * * @see \PHP_CodeSniffer\Files\File::getClassProperties() Original source. * @see \PHPCSUtils\Utils\ObjectDeclarations::getClassProperties() PHPCSUtils native improved version. * * @since 1.0.0 + * @since 1.0.6 Sync with PHPCS 3.8.0, support for readonly classes. PHPCS#3686. * * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned. * @param int $stackPtr The position in the stack of the `T_CLASS` @@ -552,7 +554,50 @@ public static function getMemberProperties(File $phpcsFile, $stackPtr) */ public static function getClassProperties(File $phpcsFile, $stackPtr) { - return $phpcsFile->getClassProperties($stackPtr); + $tokens = $phpcsFile->getTokens(); + + if ($tokens[$stackPtr]['code'] !== T_CLASS) { + throw new RuntimeException('$stackPtr must be of type T_CLASS'); + } + + $valid = [ + T_FINAL => T_FINAL, + T_ABSTRACT => T_ABSTRACT, + T_READONLY => T_READONLY, + T_WHITESPACE => T_WHITESPACE, + T_COMMENT => T_COMMENT, + T_DOC_COMMENT => T_DOC_COMMENT, + ]; + + $isAbstract = false; + $isFinal = false; + $isReadonly = false; + + for ($i = ($stackPtr - 1); $i > 0; $i--) { + if (isset($valid[$tokens[$i]['code']]) === false) { + break; + } + + switch ($tokens[$i]['code']) { + case T_ABSTRACT: + $isAbstract = true; + break; + + case T_FINAL: + $isFinal = true; + break; + + case T_READONLY: + $isReadonly = true; + break; + } + } + + return [ + 'is_abstract' => $isAbstract, + 'is_final' => $isFinal, + 'is_readonly' => $isReadonly, + ]; } /** diff --git a/PHPCSUtils/Utils/ObjectDeclarations.php b/PHPCSUtils/Utils/ObjectDeclarations.php index 4b4c05ca..e284224c 100644 --- a/PHPCSUtils/Utils/ObjectDeclarations.php +++ b/PHPCSUtils/Utils/ObjectDeclarations.php @@ -136,7 +136,6 @@ public static function getName(File $phpcsFile, $stackPtr) * - Handling of PHPCS annotations. * - Handling of unorthodox docblock placement. * - Defensive coding against incorrect calls to this method. - * - Support for PHP 8.2 readonly classes. * - Additional `'abstract_token'`, `'final_token'`, and `'readonly_token'` indexes in the return array. * * @see \PHP_CodeSniffer\Files\File::getClassProperties() Original source. diff --git a/Tests/BackCompat/BCFile/GetClassPropertiesTest.inc b/Tests/BackCompat/BCFile/GetClassPropertiesTest.inc index 5ec030f6..2490a096 100644 --- a/Tests/BackCompat/BCFile/GetClassPropertiesTest.inc +++ b/Tests/BackCompat/BCFile/GetClassPropertiesTest.inc @@ -18,6 +18,23 @@ abstract class AbstractClass {} /* testFinalClass */ final class FinalClass {} +/* testReadonlyClass */ +readonly class ReadOnlyClass {} + +/* testFinalReadonlyClass */ +final readonly class FinalReadOnlyClass extends Foo {} + +/* testReadonlyFinalClass */ +readonly /*comment*/ final class ReadOnlyFinalClass {} + +/* testAbstractReadonlyClass */ +abstract readonly class AbstractReadOnlyClass {} + +/* testReadonlyAbstractClass */ +readonly +abstract +class ReadOnlyAbstractClass {} + /* testWithCommentsAndNewLines */ abstract /* comment */ diff --git a/Tests/BackCompat/BCFile/GetClassPropertiesTest.php b/Tests/BackCompat/BCFile/GetClassPropertiesTest.php index 89a563e2..9121c5b4 100644 --- a/Tests/BackCompat/BCFile/GetClassPropertiesTest.php +++ b/Tests/BackCompat/BCFile/GetClassPropertiesTest.php @@ -92,12 +92,7 @@ public function dataNotAClassException() public function testGetClassProperties($testMarker, $expected) { // Remove keys which will only exist in the PHPCSUtils version of this method. - unset( - $expected['abstract_token'], - $expected['final_token'], - $expected['is_readonly'], - $expected['readonly_token'] - ); + unset($expected['abstract_token'], $expected['final_token'], $expected['readonly_token']); $class = $this->getTargetToken($testMarker, \T_CLASS); $result = BCFile::getClassProperties(self::$phpcsFile, $class); @@ -147,6 +142,61 @@ public function dataGetClassProperties() 'readonly_token' => false, ], ], + 'readonly' => [ + '/* testReadonlyClass */', + [ + 'is_abstract' => false, + 'abstract_token' => false, + 'is_final' => false, + 'final_token' => false, + 'is_readonly' => true, + 'readonly_token' => -2, + ], + ], + 'final-readonly' => [ + '/* testFinalReadonlyClass */', + [ + 'is_abstract' => false, + 'abstract_token' => false, + 'is_final' => true, + 'final_token' => -4, + 'is_readonly' => true, + 'readonly_token' => -2, + ], + ], + 'readonly-final' => [ + '/* testReadonlyFinalClass */', + [ + 'is_abstract' => false, + 'abstract_token' => false, + 'is_final' => true, + 'final_token' => -2, + 'is_readonly' => true, + 'readonly_token' => -6, + ], + ], + 'abstract-readonly' => [ + '/* testAbstractReadonlyClass */', + [ + 'is_abstract' => true, + 'abstract_token' => -4, + 'is_final' => false, + 'final_token' => false, + 'is_readonly' => true, + 'readonly_token' => -2, + ], + ], + 'readonly-abstract' => [ + '/* testReadonlyAbstractClass */', + [ + 'is_abstract' => true, + 'abstract_token' => -2, + 'is_final' => false, + 'final_token' => false, + 'is_readonly' => true, + 'readonly_token' => -4, + ], + ], 'comments-and-new-lines' => [ '/* testWithCommentsAndNewLines */', [ diff --git a/Tests/Utils/ObjectDeclarations/GetClassPropertiesDiffTest.inc b/Tests/Utils/ObjectDeclarations/GetClassPropertiesDiffTest.inc index 2abe6a38..ccd0ba5e 100644 --- a/Tests/Utils/ObjectDeclarations/GetClassPropertiesDiffTest.inc +++ b/Tests/Utils/ObjectDeclarations/GetClassPropertiesDiffTest.inc @@ -16,20 +16,3 @@ final * @phpcs:disable Standard.Cat.SniffName -- Just because. */ class ClassWithModifierBeforeDocblock {} - -/* testReadonlyClass */ -readonly class ReadOnlyClass {} - -/* testFinalReadonlyClass */ -final readonly class FinalReadOnlyClass extends Foo {} - -/* testReadonlyFinalClass */ -readonly /*comment*/ final class ReadOnlyFinalClass {} - -/* testAbstractReadonlyClass */ -abstract readonly class AbstractReadOnlyClass {} - -/* testReadonlyAbstractClass */ -readonly -abstract -class ReadOnlyAbstractClass {} diff --git a/Tests/Utils/ObjectDeclarations/GetClassPropertiesDiffTest.php b/Tests/Utils/ObjectDeclarations/GetClassPropertiesDiffTest.php index b802e6a6..8a2b8a48 100644 --- a/Tests/Utils/ObjectDeclarations/GetClassPropertiesDiffTest.php +++ b/Tests/Utils/ObjectDeclarations/GetClassPropertiesDiffTest.php @@ -101,61 +101,6 @@ public function dataGetClassProperties() 'readonly_token' => false, ], ], - 'readonly' => [ - '/* testReadonlyClass */', - [ - 'is_abstract' => false, - 'abstract_token' => false, - 'is_final' => false, - 'final_token' => false, - 'is_readonly' => true, - 'readonly_token' => -2, - ], - ], - 'final-readonly' => [ - '/* testFinalReadonlyClass */', - [ - 'is_abstract' => false, - 'abstract_token' => false, - 'is_final' => true, - 'final_token' => -4, - 'is_readonly' => true, - 'readonly_token' => -2, - ], - ], - 'readonly-final' => [ - '/* testReadonlyFinalClass */', - [ - 'is_abstract' => false, - 'abstract_token' => false, - 'is_final' => true, - 'final_token' => -2, - 'is_readonly' => true, - 'readonly_token' => -6, - ], - ], - 'abstract-readonly' => [ - '/* testAbstractReadonlyClass */', - [ - 'is_abstract' => true, - 'abstract_token' => -4, - 'is_final' => false, - 'final_token' => false, - 'is_readonly' => true, - 'readonly_token' => -2, - ], - ], - 'readonly-abstract' => [ - '/* testReadonlyAbstractClass */', - [ - 'is_abstract' => true, - 'abstract_token' => -2, - 'is_final' => false, - 'final_token' => false, - 'is_readonly' => true, - 'readonly_token' => -4, - ], - ], ]; } } From ab80915de169292db7c1bb0373275bcd90ea6f82 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 19 May 2023 07:34:45 +0200 Subject: [PATCH 12/18] GH Actions: tweaks for the markdown QA check Looks like the `remark-lint` project has released a new version fixing the bug causing the failing build earlier this week. This commit reverts PR 469. Refs: * https://github.com/remarkjs/remark-lint/releases/tag/9.1.2 * https://github.com/remarkjs/remark-lint/commit/639271aed95fa579623f385bade4939a9c70e959 --- .remarkrc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.remarkrc b/.remarkrc index 035ad2d9..95c4c322 100644 --- a/.remarkrc +++ b/.remarkrc @@ -25,7 +25,7 @@ "remark-lint-no-unneeded-full-reference-link", "remark-lint-no-unused-definitions", ["remark-lint-strikethrough-marker", "~~"], - "remark-lint-table-pipe-alignment", + ["remark-lint-table-cell-padding", "consistent"], "remark-lint-heading-whitespace", "remark-lint-list-item-punctuation", "remark-lint-match-punctuation", From ae529754bab5ee83824433c6c4b231d6245c0e43 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 19 May 2023 07:41:16 +0200 Subject: [PATCH 13/18] Add `security.md` file ... containing information about how to report security issues and what versions of PHPCompatibility are supported from a security point of view. The file is placed in the `.github` directory. This will allow for it to be recognized correctly by GitHub, while not cluttering up the project root directory. Ref: https://docs.github.com/en/code-security/getting-started/adding-a-security-policy-to-your-repository --- .github/SECURITY.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 .github/SECURITY.md diff --git a/.github/SECURITY.md b/.github/SECURITY.md new file mode 100644 index 00000000..383da734 --- /dev/null +++ b/.github/SECURITY.md @@ -0,0 +1,22 @@ +# Security Policy + +## Supported Versions + +The latest patch version of the `1.x` release series is supported for security updates. + +## Reporting a Vulnerability + +PHPCSUtils is a developer tool and should generally not be used in a production (web accessible) environment. + +Having said that, responsible disclosure of security issues is highly appreciated. + +**Please do not report or discuss security vulnerabilities through public GitHub issues, discussions, or pull requests.** + +Issues can be reported privately to the maintainers by opening a [Security vulnerability report](https://github.com/PHPCSStandards/PHPCSUtils/security/advisories/new). + +### Preferences + +* Please provide detailed reports with reproducible steps and a clearly defined impact. +* Include the version number of the vulnerable package in your report. +* Fixes are most welcome. + A private PR can be created from the security report to work on and discuss the patch. From a72c5e7039fd0a235ae053dfde564772a46ff123 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 15 Apr 2023 10:35:54 +0200 Subject: [PATCH 14/18] BCFile::getMethodParameters(): sync with upstream / readonly without visibility PHPCS upstream has now also added support for constructor property promotion with `readonly` properties without explicit visibility. This commit updates the `BCFile::getMethodParameters()` method to reflect this. Support for readonly constructor properties without visibility was previously already added to the `FunctionDeclarations::getParameter()` in PR 456. The associated "diff" test has now been moved to the base test file for these methods. Refs: * squizlabs/PHP_CodeSniffer 3801. --- PHPCSUtils/BackCompat/BCFile.php | 33 +++++++----- PHPCSUtils/Utils/FunctionDeclarations.php | 1 - .../BCFile/GetMethodParametersTest.inc | 5 ++ .../BCFile/GetMethodParametersTest.php | 53 ++++++++++++++++++- .../GetParametersDiffTest.inc | 5 -- .../GetParametersDiffTest.php | 51 ------------------ .../GetParametersTest.php | 2 +- 7 files changed, 78 insertions(+), 72 deletions(-) diff --git a/PHPCSUtils/BackCompat/BCFile.php b/PHPCSUtils/BackCompat/BCFile.php index d7d8b2b8..0491138f 100644 --- a/PHPCSUtils/BackCompat/BCFile.php +++ b/PHPCSUtils/BackCompat/BCFile.php @@ -138,23 +138,25 @@ public static function getDeclarationName(File $phpcsFile, $stackPtr) * * Parameters declared using PHP 8 constructor property promotion, have these additional array indexes: * ```php - * 'property_visibility' => string, // The property visibility as declared. - * 'visibility_token' => integer, // The stack pointer to the visibility modifier token. - * 'property_readonly' => bool, // TRUE if the readonly keyword was found. - * 'readonly_token' => integer, // The stack pointer to the readonly modifier token. - * // This index will only be set if the property is readonly. + * 'property_visibility' => string, // The property visibility as declared. + * 'visibility_token' => integer,|false // The stack pointer to the visibility modifier token. + * // or FALSE if the visibility is not explicitly declared. + * 'property_readonly' => bool, // TRUE if the readonly keyword was found. + * 'readonly_token' => integer, // The stack pointer to the readonly modifier token. + * // This index will only be set if the property is readonly. * ``` * * PHPCS cross-version compatible version of the `File::getMethodParameters()` method. * * 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: Added support for constructor property promotion with readonly without explicit visibility. * * @see \PHP_CodeSniffer\Files\File::getMethodParameters() Original source. * @see \PHPCSUtils\Utils\FunctionDeclarations::getParameters() PHPCSUtils native improved version. * * @since 1.0.0 + * @since 1.0.6 Sync with PHPCS 3.8.0, support for readonly properties without explicit visibility. PHPCS#3801. * * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned. * @param int $stackPtr The position in the stack of the function token @@ -380,15 +382,20 @@ public static function getMethodParameters(File $phpcsFile, $stackPtr) $vars[$paramCount]['type_hint_end_token'] = $typeHintEndToken; $vars[$paramCount]['nullable_type'] = $nullableType; - if ($visibilityToken !== null) { - $vars[$paramCount]['property_visibility'] = $tokens[$visibilityToken]['content']; - $vars[$paramCount]['visibility_token'] = $visibilityToken; + if ($visibilityToken !== null || $readonlyToken !== null) { + $vars[$paramCount]['property_visibility'] = 'public'; + $vars[$paramCount]['visibility_token'] = false; $vars[$paramCount]['property_readonly'] = false; - } - if ($readonlyToken !== null) { - $vars[$paramCount]['property_readonly'] = true; - $vars[$paramCount]['readonly_token'] = $readonlyToken; + if ($visibilityToken !== null) { + $vars[$paramCount]['property_visibility'] = $tokens[$visibilityToken]['content']; + $vars[$paramCount]['visibility_token'] = $visibilityToken; + } + + if ($readonlyToken !== null) { + $vars[$paramCount]['property_readonly'] = true; + $vars[$paramCount]['readonly_token'] = $readonlyToken; + } } if ($tokens[$i]['code'] === T_COMMA) { diff --git a/PHPCSUtils/Utils/FunctionDeclarations.php b/PHPCSUtils/Utils/FunctionDeclarations.php index efe921b2..153bce45 100644 --- a/PHPCSUtils/Utils/FunctionDeclarations.php +++ b/PHPCSUtils/Utils/FunctionDeclarations.php @@ -148,7 +148,6 @@ public static function getName(File $phpcsFile, $stackPtr) * - Defensive coding against incorrect calls to this method. * - More efficient checking whether a function has a body. * - Support for PHP 8.0 identifier name tokens in return types, cross-version PHP & PHPCS. - * - Support for constructor property promotion with the PHP 8.1 readonly keyword without explicit visibility. * - Support for the PHP 8.2 `true` type. * - The results of this function call are cached during a PHPCS run for faster response times. * diff --git a/Tests/BackCompat/BCFile/GetMethodParametersTest.inc b/Tests/BackCompat/BCFile/GetMethodParametersTest.inc index 34263a77..17ba4a98 100644 --- a/Tests/BackCompat/BCFile/GetMethodParametersTest.inc +++ b/Tests/BackCompat/BCFile/GetMethodParametersTest.inc @@ -212,6 +212,11 @@ class ConstructorPropertyPromotionWithReadOnlyNoTypeDeclaration { public function __construct(public readonly $promotedProp, ReadOnly private &$promotedToo) {} } +class ConstructorPropertyPromotionWithOnlyReadOnly { + /* testPHP81ConstructorPropertyPromotionWithOnlyReadOnly */ + public function __construct(readonly Foo&Bar $promotedProp, readonly ?bool $promotedToo,) {} +} + /* testPHP8ConstructorPropertyPromotionGlobalFunction */ // Intentional fatal error. Property promotion not allowed in non-constructor, but that's not the concern of this method. function globalFunction(private $x) {} diff --git a/Tests/BackCompat/BCFile/GetMethodParametersTest.php b/Tests/BackCompat/BCFile/GetMethodParametersTest.php index fbad04c7..9ea1faa3 100644 --- a/Tests/BackCompat/BCFile/GetMethodParametersTest.php +++ b/Tests/BackCompat/BCFile/GetMethodParametersTest.php @@ -2012,6 +2012,57 @@ public function testPHP81ConstructorPropertyPromotionWithReadOnlyNoTypeDeclarati $this->getMethodParametersTestHelper('/* ' . __FUNCTION__ . ' */', $expected); } + /** + * Verify recognition of PHP8 constructor with property promotion using PHP 8.1 readonly + * keyword without explicit visibility. + * + * @return void + */ + public function testPHP81ConstructorPropertyPromotionWithOnlyReadOnly() + { + $expected = []; + $expected[0] = [ + 'token' => 10, // Offset from the T_FUNCTION token. + 'name' => '$promotedProp', + 'content' => 'readonly Foo&Bar $promotedProp', + 'has_attributes' => false, + 'pass_by_reference' => false, + 'reference_token' => false, + 'variable_length' => false, + 'variadic_token' => false, + 'type_hint' => 'Foo&Bar', + 'type_hint_token' => 6, // Offset from the T_FUNCTION token. + 'type_hint_end_token' => 8, // Offset from the T_FUNCTION token. + 'nullable_type' => false, + 'property_visibility' => 'public', + 'visibility_token' => false, + 'property_readonly' => true, + 'readonly_token' => 4, // Offset from the T_FUNCTION token. + 'comma_token' => 11, + ]; + $expected[1] = [ + 'token' => 18, // Offset from the T_FUNCTION token. + 'name' => '$promotedToo', + 'content' => 'readonly ?bool $promotedToo', + 'has_attributes' => false, + 'pass_by_reference' => false, + 'reference_token' => false, + 'variable_length' => false, + 'variadic_token' => false, + 'type_hint' => '?bool', + 'type_hint_token' => 16, // Offset from the T_FUNCTION token. + 'type_hint_end_token' => 16, // Offset from the T_FUNCTION token. + 'nullable_type' => true, + 'property_visibility' => 'public', + 'visibility_token' => false, + 'property_readonly' => true, + 'readonly_token' => 13, // Offset from the T_FUNCTION token. + 'comma_token' => 19, + ]; + + $this->getMethodParametersTestHelper('/* ' . __FUNCTION__ . ' */', $expected); + } + /** * Verify behaviour when a non-constructor function uses PHP 8 property promotion syntax. * @@ -2717,7 +2768,7 @@ protected function getMethodParametersTestHelper($marker, $expected, $targetType if (isset($param['default_equal_token'])) { $expected[$key]['default_equal_token'] += $target; } - if (isset($param['visibility_token'])) { + if (isset($param['visibility_token']) && $param['visibility_token'] !== false) { $expected[$key]['visibility_token'] += $target; } if (isset($param['readonly_token'])) { diff --git a/Tests/Utils/FunctionDeclarations/GetParametersDiffTest.inc b/Tests/Utils/FunctionDeclarations/GetParametersDiffTest.inc index dd99cbed..d9f8c995 100644 --- a/Tests/Utils/FunctionDeclarations/GetParametersDiffTest.inc +++ b/Tests/Utils/FunctionDeclarations/GetParametersDiffTest.inc @@ -6,8 +6,3 @@ function pseudoTypeTrue(?true $var = true) {} /* testPHP82PseudoTypeFalseAndTrue */ // Intentional fatal error - Type contains both true and false, bool should be used instead, but that's not the concern of the method. function pseudoTypeFalseAndTrue(true|false $var = true) {} - -class ConstructorPropertyPromotionWithOnlyReadOnly { - /* testPHP81ConstructorPropertyPromotionWithOnlyReadOnly */ - public function __construct(readonly Foo&Bar $promotedProp, readonly ?bool $promotedToo,) {} -} diff --git a/Tests/Utils/FunctionDeclarations/GetParametersDiffTest.php b/Tests/Utils/FunctionDeclarations/GetParametersDiffTest.php index 6975cc28..51303ffc 100644 --- a/Tests/Utils/FunctionDeclarations/GetParametersDiffTest.php +++ b/Tests/Utils/FunctionDeclarations/GetParametersDiffTest.php @@ -102,57 +102,6 @@ public function testPHP82PseudoTypeFalseAndTrue() $this->getMethodParametersTestHelper('/* ' . __FUNCTION__ . ' */', $expected); } - /** - * Verify recognition of PHP8 constructor with property promotion using PHP 8.1 readonly - * keyword without explicit visibility. - * - * @return void - */ - public function testPHP81ConstructorPropertyPromotionWithOnlyReadOnly() - { - $expected = []; - $expected[0] = [ - 'token' => 10, // Offset from the T_FUNCTION token. - 'name' => '$promotedProp', - 'content' => 'readonly Foo&Bar $promotedProp', - 'has_attributes' => false, - 'pass_by_reference' => false, - 'reference_token' => false, - 'variable_length' => false, - 'variadic_token' => false, - 'type_hint' => 'Foo&Bar', - 'type_hint_token' => 6, // Offset from the T_FUNCTION token. - 'type_hint_end_token' => 8, // Offset from the T_FUNCTION token. - 'nullable_type' => false, - 'property_visibility' => 'public', - 'visibility_token' => false, - 'property_readonly' => true, - 'readonly_token' => 4, // Offset from the T_FUNCTION token. - 'comma_token' => 11, - ]; - $expected[1] = [ - 'token' => 18, // Offset from the T_FUNCTION token. - 'name' => '$promotedToo', - 'content' => 'readonly ?bool $promotedToo', - 'has_attributes' => false, - 'pass_by_reference' => false, - 'reference_token' => false, - 'variable_length' => false, - 'variadic_token' => false, - 'type_hint' => '?bool', - 'type_hint_token' => 16, // Offset from the T_FUNCTION token. - 'type_hint_end_token' => 16, // Offset from the T_FUNCTION token. - 'nullable_type' => true, - 'property_visibility' => 'public', - 'visibility_token' => false, - 'property_readonly' => true, - 'readonly_token' => 13, // Offset from the T_FUNCTION token. - 'comma_token' => 19, - ]; - - $this->getMethodParametersTestHelper('/* ' . __FUNCTION__ . ' */', $expected); - } - /** * Test helper. * diff --git a/Tests/Utils/FunctionDeclarations/GetParametersTest.php b/Tests/Utils/FunctionDeclarations/GetParametersTest.php index 43e779f1..995501a0 100644 --- a/Tests/Utils/FunctionDeclarations/GetParametersTest.php +++ b/Tests/Utils/FunctionDeclarations/GetParametersTest.php @@ -140,7 +140,7 @@ protected function getMethodParametersTestHelper($marker, $expected, $targetType if (isset($param['default_equal_token'])) { $expected[$key]['default_equal_token'] += $target; } - if (isset($param['visibility_token'])) { + if (isset($param['visibility_token']) && $param['visibility_token'] !== false) { $expected[$key]['visibility_token'] += $target; } if (isset($param['readonly_token'])) { From e8b37eb8bbffe32c229872c1ab92921505750dc7 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 21 May 2023 13:46:39 +0200 Subject: [PATCH 15/18] ControlStructures: more defensive coding against parse errors Only add an exception to the array if one was found. Includes unit tests. --- PHPCSUtils/Utils/ControlStructures.php | 15 +++++++++------ .../ControlStructures/GetCaughtExceptionsTest.inc | 6 ++++++ .../ControlStructures/GetCaughtExceptionsTest.php | 8 ++++++++ 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/PHPCSUtils/Utils/ControlStructures.php b/PHPCSUtils/Utils/ControlStructures.php index 0e8180ad..44bc053d 100644 --- a/PHPCSUtils/Utils/ControlStructures.php +++ b/PHPCSUtils/Utils/ControlStructures.php @@ -209,6 +209,7 @@ public static function isElseIf(File $phpcsFile, $stackPtr) * 'type_end_token' => integer, // The stack pointer to the end of the type declaration. * ) * ``` + * In case of an invalid catch structure, the array may be empty. * * @throws \PHP_CodeSniffer\Exceptions\RuntimeException If the specified `$stackPtr` is not of * type `T_CATCH` or doesn't exist. @@ -243,12 +244,14 @@ public static function getCaughtExceptions(File $phpcsFile, $stackPtr) } if (isset(Collections::namespacedNameTokens()[$tokens[$i]['code']]) === false) { - // Add the current exception to the result array. - $exceptions[] = [ - 'type' => $foundName, - 'type_token' => $firstToken, - 'type_end_token' => $lastToken, - ]; + // Add the current exception to the result array if one was found. + if ($foundName !== '') { + $exceptions[] = [ + 'type' => $foundName, + 'type_token' => $firstToken, + 'type_end_token' => $lastToken, + ]; + } if ($tokens[$i]['code'] === \T_BITWISE_OR) { // Multi-catch. Reset and continue. diff --git a/Tests/Utils/ControlStructures/GetCaughtExceptionsTest.inc b/Tests/Utils/ControlStructures/GetCaughtExceptionsTest.inc index aee219ad..9072c1cb 100644 --- a/Tests/Utils/ControlStructures/GetCaughtExceptionsTest.inc +++ b/Tests/Utils/ControlStructures/GetCaughtExceptionsTest.inc @@ -30,6 +30,12 @@ try { /* testPHP8NonCapturingCatch */ } catch (RuntimeException | AnotherException) { +/* testMissingExceptionName */ +} catch ($e) { + +/* testMultiMissingExceptionNames */ +} catch ( | $e) { + /* testLiveCoding */ // Intentional parse error. } catch ( diff --git a/Tests/Utils/ControlStructures/GetCaughtExceptionsTest.php b/Tests/Utils/ControlStructures/GetCaughtExceptionsTest.php index 0b0b3702..32d4dcba 100644 --- a/Tests/Utils/ControlStructures/GetCaughtExceptionsTest.php +++ b/Tests/Utils/ControlStructures/GetCaughtExceptionsTest.php @@ -214,6 +214,14 @@ public function dataGetCaughtExceptions() ], ], ], + 'catch-without-named-exception' => [ + 'testMarker' => '/* testMissingExceptionName */', + 'expected' => [], + ], + 'multi-catch-without-named-exceptions' => [ + 'testMarker' => '/* testMultiMissingExceptionNames */', + 'expected' => [], + ], ]; } } From 2db153402ec6361dce82d35d091c10e30a242e35 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 27 May 2023 14:49:03 +0200 Subject: [PATCH 16/18] AbstractArrayDeclarationSniff: extra defensive coding for live coding/parse errors In case of live coding/an unfinished array, the `PassedParameters::getParameters()` will return an empty array (no items), while in reality, it couldn't be determined. To prevent incorrect sniff results, it is better for the sniff to bow out early in those situations. This prevents a "Trying to access array offset on value of type bool" PHP notice. Fixed now. Includes additional test. --- .../AbstractArrayDeclarationSniff.php | 7 +++- .../AbstractArrayDeclarationSniffTest.inc | 4 +++ .../AbstractArrayDeclarationSniffTest.php | 34 +++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/PHPCSUtils/AbstractSniffs/AbstractArrayDeclarationSniff.php b/PHPCSUtils/AbstractSniffs/AbstractArrayDeclarationSniff.php index 271c9af7..b8616693 100644 --- a/PHPCSUtils/AbstractSniffs/AbstractArrayDeclarationSniff.php +++ b/PHPCSUtils/AbstractSniffs/AbstractArrayDeclarationSniff.php @@ -184,9 +184,14 @@ final public function process(File $phpcsFile, $stackPtr) return; } + $openClose = Arrays::getOpenClose($phpcsFile, $stackPtr, true); + if ($openClose === false) { + // Parse error or live coding. + return; + } + $this->stackPtr = $stackPtr; $this->tokens = $phpcsFile->getTokens(); - $openClose = Arrays::getOpenClose($phpcsFile, $stackPtr, true); $this->arrayOpener = $openClose['opener']; $this->arrayCloser = $openClose['closer']; $this->itemCount = \count($this->arrayItems); diff --git a/Tests/AbstractSniffs/AbstractArrayDeclaration/AbstractArrayDeclarationSniffTest.inc b/Tests/AbstractSniffs/AbstractArrayDeclaration/AbstractArrayDeclarationSniffTest.inc index f5bb0edb..1edce6b3 100644 --- a/Tests/AbstractSniffs/AbstractArrayDeclaration/AbstractArrayDeclarationSniffTest.inc +++ b/Tests/AbstractSniffs/AbstractArrayDeclaration/AbstractArrayDeclarationSniffTest.inc @@ -30,3 +30,7 @@ $array = [1, 'a' => 2, ]; /* testShortList */ [$a, $b] = $array; + +/* testLiveCoding */ +// This must be the last test in the file!! +$array = array( diff --git a/Tests/AbstractSniffs/AbstractArrayDeclaration/AbstractArrayDeclarationSniffTest.php b/Tests/AbstractSniffs/AbstractArrayDeclaration/AbstractArrayDeclarationSniffTest.php index 300e1173..fbdf04f9 100644 --- a/Tests/AbstractSniffs/AbstractArrayDeclaration/AbstractArrayDeclarationSniffTest.php +++ b/Tests/AbstractSniffs/AbstractArrayDeclaration/AbstractArrayDeclarationSniffTest.php @@ -611,4 +611,38 @@ public function testShortCircuitOnProcessComma() $mockObj->process(self::$phpcsFile, $target); } + + /** + * Test that the abstract sniff correctly bows out when presented with an unfinished array. + * + * @return void + */ + public function testBowOutOnUnfinishedArray() + { + $target = $this->getTargetToken('/* testLiveCoding */', Collections::arrayOpenTokensBC()); + + $mockObj = $this->getMockBuilder('\PHPCSUtils\AbstractSniffs\AbstractArrayDeclarationSniff') + ->setMethods($this->methodsToMock) + ->getMockForAbstractClass(); + + $mockObj->expects($this->never()) + ->method('processOpenClose'); + + $mockObj->expects($this->never()) + ->method('processKey'); + + $mockObj->expects($this->never()) + ->method('processNoKey'); + + $mockObj->expects($this->never()) + ->method('processArrow'); + + $mockObj->expects($this->never()) + ->method('processValue'); + + $mockObj->expects($this->never()) + ->method('processComma'); + + $mockObj->process(self::$phpcsFile, $target); + } } From f327dc3fbb359cdc7e47ed42447be29c442ef504 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 27 May 2023 14:53:36 +0200 Subject: [PATCH 17/18] AbstractArrayDeclarationSniff: improve the property reset Unsetting a property leaves a property in a different PHP internal state then (re)setting it to an empty value. Most notably, an unset property triggers the magic `__set()`/`__get()` etc methods if available, and while this abstract doesn't declare those methods, a sniff implementing the abstract _may_. This change maintains the target behaviour (resetting the property values to save memory), while preventing side-effects from using `unset()` on these properties. --- PHPCSUtils/AbstractSniffs/AbstractArrayDeclarationSniff.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/PHPCSUtils/AbstractSniffs/AbstractArrayDeclarationSniff.php b/PHPCSUtils/AbstractSniffs/AbstractArrayDeclarationSniff.php index b8616693..21420a8e 100644 --- a/PHPCSUtils/AbstractSniffs/AbstractArrayDeclarationSniff.php +++ b/PHPCSUtils/AbstractSniffs/AbstractArrayDeclarationSniff.php @@ -204,7 +204,8 @@ final public function process(File $phpcsFile, $stackPtr) $this->processArray($phpcsFile); // Reset select properties between calls to this sniff to lower memory usage. - unset($this->tokens, $this->arrayItems); + $this->tokens = []; + $this->arrayItems = []; } /** From 8bb2504502c6b8b6f83dd00bde9060a0a14e6399 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 21 May 2023 14:06:40 +0200 Subject: [PATCH 18/18] Changelog for PHPCSUtils 1.0.6 --- CHANGELOG.md | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a804693..fd8bab48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,45 @@ This projects adheres to [Keep a CHANGELOG](https://keepachangelog.com/) and use _Nothing yet._ + +## [1.0.6] - 2023-05-27 + +### Changed + +#### PHPCS BackCompat + +* `BCFile::getClassProperties()`: sync with PHPCS 3.8.0 - support for PHP 8.2 `readonly` classes. [#470] +* `BCFile::getMethodParameters()`: sync with PHPCS 3.8.0 - support for constructor property promotion with `readonly` properties without explicit visibility. [#472] + +#### Utils + +* The results of the following methods will now (also) be cached for improved performance when multiple sniffs call these functions for the same token during a PHPCS run. [#464], [#466] + - `FunctionDeclarations::getProperties()` + - `Variables::getMemberProperties()` +* Additionally, the results of the `UseStatements::splitImportUseStatement()` method will be cached more often and the cache checked earlier. [#467] +* The return value of the `ControlStructures::getCaughtExceptions()` method will no longer contain "empty" entries for catch statements without a named exception. It will return an empty array instead. [#474] + +#### Other + +* Various small housekeeping and maintenance updates. + +### Fixed + +### Abstract Sniffs + +* `AbstractArrayDeclarationSniff`: fixed a potential "Trying to access array offset on value of type bool" PHP notice. [#476] +* `AbstractArrayDeclarationSniff`: the abstract will no longer trigger potentially available magic `__get()`/`__set()` etc methods. [#477] + +[#464]: https://github.com/PHPCSStandards/PHPCSUtils/pull/464 +[#466]: https://github.com/PHPCSStandards/PHPCSUtils/pull/466 +[#467]: https://github.com/PHPCSStandards/PHPCSUtils/pull/467 +[#470]: https://github.com/PHPCSStandards/PHPCSUtils/pull/470 +[#472]: https://github.com/PHPCSStandards/PHPCSUtils/pull/472 +[#474]: https://github.com/PHPCSStandards/PHPCSUtils/pull/474 +[#476]: https://github.com/PHPCSStandards/PHPCSUtils/pull/476 +[#477]: https://github.com/PHPCSStandards/PHPCSUtils/pull/477 + + ## [1.0.5] - 2023-04-17 ### Fixed @@ -850,6 +889,7 @@ This initial alpha release contains the following utility classes: [Unreleased]: https://github.com/PHPCSStandards/PHPCSUtils/compare/stable...HEAD +[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 [1.0.4]: https://github.com/PHPCSStandards/PHPCSUtils/compare/1.0.3...1.0.4 [1.0.3]: https://github.com/PHPCSStandards/PHPCSUtils/compare/1.0.2...1.0.3