From 0fdc11f5c322e0e69d371dca924974185dffba47 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 31 Mar 2024 15:08:07 +0200 Subject: [PATCH 01/15] IsReferenceTest: fix invalid test case Sister-PR to PHPCSStandards/PHP_CodeSniffer 426 Closures use clauses only take plain variables, not complex variables, like properties or array keys. In other words, this test case as-is, was a parse error. For the purposes of this test, it makes no difference what type of variable is passed, so let's fix the test. --- Tests/BackCompat/BCFile/IsReferenceTest.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/BackCompat/BCFile/IsReferenceTest.inc b/Tests/BackCompat/BCFile/IsReferenceTest.inc index 93c7acc6..d371d6ef 100644 --- a/Tests/BackCompat/BCFile/IsReferenceTest.inc +++ b/Tests/BackCompat/BCFile/IsReferenceTest.inc @@ -169,7 +169,7 @@ functionCall( $something , &new Foobar() ); $closure = function() use (&$var){}; /* testUseByReferenceWithCommentFirstParam */ -$closure = function() use /*comment*/ (&$this->value){}; +$closure = function() use /*comment*/ (&$value){}; /* testUseByReferenceWithCommentSecondParam */ $closure = function() use /*comment*/ ($varA, &$varB){}; From 58c7c13733f726f47c44c5e183f31f00d2e54184 Mon Sep 17 00:00:00 2001 From: jrfnl <663378+jrfnl@users.noreply.github.com> Date: Mon, 1 Apr 2024 03:41:14 +0000 Subject: [PATCH 02/15] GetVersionTest: update for release of PHPCS 3.9.1 --- Tests/BackCompat/Helper/GetVersionTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/BackCompat/Helper/GetVersionTest.php b/Tests/BackCompat/Helper/GetVersionTest.php index f27c5d1c..52357a5d 100644 --- a/Tests/BackCompat/Helper/GetVersionTest.php +++ b/Tests/BackCompat/Helper/GetVersionTest.php @@ -30,7 +30,7 @@ final class GetVersionTest extends TestCase * * @var string */ - const DEVMASTER = '3.9.0'; + const DEVMASTER = '3.9.1'; /** * Test the method. From fa297333bc88b132902f13c0811f6723e68bbc9c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 1 Apr 2024 07:02:42 +0000 Subject: [PATCH 03/15] GH Actions: bump actions/configure-pages from 4 to 5 Bumps [actions/configure-pages](https://github.com/actions/configure-pages) from 4 to 5. - [Release notes](https://github.com/actions/configure-pages/releases) - [Commits](https://github.com/actions/configure-pages/compare/v4...v5) --- updated-dependencies: - dependency-name: actions/configure-pages 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 65bc564e..40bd015d 100644 --- a/.github/workflows/update-docs.yml +++ b/.github/workflows/update-docs.yml @@ -120,7 +120,7 @@ jobs: retention-days: 5 - name: Setup GH Pages - uses: actions/configure-pages@v4 + uses: actions/configure-pages@v5 - name: Build the GH Pages site with Jekyll uses: actions/jekyll-build-pages@v1 From 7e013859aa53d255f1ac2380701004ef421dd853 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 29 Mar 2024 04:50:56 +0100 Subject: [PATCH 04/15] BCFile/FunctionDeclarations::get[Method]Properties(): bug fix - skip over closure use statements Sister-PR to upstream PR PHPCSStandards/PHP_CodeSniffer 421 This PR improves performance of the `BCFile::getMethodProperties()` and the `FunctionDeclarations::getProperties()` methods and prevents incorrect return type information for closure `use` clauses containing invalid variable imports in the `use` clause (defensive coding). Closure `use` statements can only import plain variables, not properties or other more complex variables. As things were, when such "illegal" variables were imported in a closure `use`, the information for the return type could get mangled. While this would be a parse error, for the purposes of static analysis, the `BCFile::getMethodProperties()` method and the `FunctionDeclarations::getProperties()` method should still handle this correctly. This commit updates both methods to always skip over the complete `use` clause, which prevents the issue and improves performance as the same time (less token walking). Includes unit tests. --- PHPCSUtils/BackCompat/BCFile.php | 144 +++++++++++++++++- PHPCSUtils/Utils/FunctionDeclarations.php | 19 +++ .../GetMethodPropertiesParseError1Test.inc | 5 + .../GetMethodPropertiesParseError1Test.php | 54 +++++++ .../BCFile/GetMethodPropertiesTest.inc | 12 ++ .../BCFile/GetMethodPropertiesTest.php | 97 ++++++++++++ .../GetPropertiesParseError1Test.php | 76 +++++++++ 7 files changed, 405 insertions(+), 2 deletions(-) create mode 100644 Tests/BackCompat/BCFile/GetMethodPropertiesParseError1Test.inc create mode 100644 Tests/BackCompat/BCFile/GetMethodPropertiesParseError1Test.php create mode 100644 Tests/Utils/FunctionDeclarations/GetPropertiesParseError1Test.php diff --git a/PHPCSUtils/BackCompat/BCFile.php b/PHPCSUtils/BackCompat/BCFile.php index 14c6adf6..dc49bdca 100644 --- a/PHPCSUtils/BackCompat/BCFile.php +++ b/PHPCSUtils/BackCompat/BCFile.php @@ -462,7 +462,7 @@ public static function getMethodParameters(File $phpcsFile, $stackPtr) * * Changelog for the PHPCS native function: * - Introduced in PHPCS 0.0.5. - * - The upstream method has received no significant updates since PHPCS 3.9.0. + * - PHPCS 3.9.1: skip over closure use statements. PHPCS #421. * * @see \PHP_CodeSniffer\Files\File::getMethodProperties() Original source. * @see \PHPCSUtils\Utils\FunctionDeclarations::getProperties() PHPCSUtils native improved version. @@ -480,7 +480,147 @@ public static function getMethodParameters(File $phpcsFile, $stackPtr) */ public static function getMethodProperties(File $phpcsFile, $stackPtr) { - return $phpcsFile->getMethodProperties($stackPtr); + $tokens = $phpcsFile->getTokens(); + + if ($tokens[$stackPtr]['code'] !== T_FUNCTION + && $tokens[$stackPtr]['code'] !== T_CLOSURE + && $tokens[$stackPtr]['code'] !== T_FN + ) { + throw new RuntimeException('$stackPtr must be of type T_FUNCTION or T_CLOSURE or T_FN'); + } + + if ($tokens[$stackPtr]['code'] === T_FUNCTION) { + $valid = [ + T_PUBLIC => T_PUBLIC, + T_PRIVATE => T_PRIVATE, + T_PROTECTED => T_PROTECTED, + T_STATIC => T_STATIC, + T_FINAL => T_FINAL, + T_ABSTRACT => T_ABSTRACT, + T_WHITESPACE => T_WHITESPACE, + T_COMMENT => T_COMMENT, + T_DOC_COMMENT => T_DOC_COMMENT, + ]; + } else { + $valid = [ + T_STATIC => T_STATIC, + T_WHITESPACE => T_WHITESPACE, + T_COMMENT => T_COMMENT, + T_DOC_COMMENT => T_DOC_COMMENT, + ]; + } + + $scope = 'public'; + $scopeSpecified = false; + $isAbstract = false; + $isFinal = false; + $isStatic = false; + + for ($i = ($stackPtr - 1); $i > 0; $i--) { + if (isset($valid[$tokens[$i]['code']]) === false) { + break; + } + + switch ($tokens[$i]['code']) { + case T_PUBLIC: + $scope = 'public'; + $scopeSpecified = true; + break; + case T_PRIVATE: + $scope = 'private'; + $scopeSpecified = true; + break; + case T_PROTECTED: + $scope = 'protected'; + $scopeSpecified = true; + break; + case T_ABSTRACT: + $isAbstract = true; + break; + case T_FINAL: + $isFinal = true; + break; + case T_STATIC: + $isStatic = true; + break; + } + } + + $returnType = ''; + $returnTypeToken = false; + $returnTypeEndToken = false; + $nullableReturnType = false; + $hasBody = true; + $returnTypeTokens = Collections::returnTypeTokens(); + + if (isset($tokens[$stackPtr]['parenthesis_closer']) === true) { + $scopeOpener = null; + if (isset($tokens[$stackPtr]['scope_opener']) === true) { + $scopeOpener = $tokens[$stackPtr]['scope_opener']; + } + + for ($i = $tokens[$stackPtr]['parenthesis_closer']; $i < $phpcsFile->numTokens; $i++) { + if (($scopeOpener === null && $tokens[$i]['code'] === T_SEMICOLON) + || ($scopeOpener !== null && $i === $scopeOpener) + ) { + // End of function definition. + break; + } + + if ($tokens[$i]['code'] === T_USE) { + // Skip over closure use statements. + for ($j = ($i + 1); $j < $phpcsFile->numTokens && isset(Tokens::$emptyTokens[$tokens[$j]['code']]) === true; $j++); + if ($tokens[$j]['code'] === T_OPEN_PARENTHESIS) { + if (isset($tokens[$j]['parenthesis_closer']) === false) { + // Live coding/parse error, stop parsing. + break; + } + + $i = $tokens[$j]['parenthesis_closer']; + continue; + } + } + + if ($tokens[$i]['code'] === T_NULLABLE) { + $nullableReturnType = true; + } + + if (isset($returnTypeTokens[$tokens[$i]['code']]) === true) { + if ($returnTypeToken === false) { + $returnTypeToken = $i; + } + + $returnType .= $tokens[$i]['content']; + $returnTypeEndToken = $i; + } + } + + if ($tokens[$stackPtr]['code'] === T_FN) { + $bodyToken = T_FN_ARROW; + } else { + $bodyToken = T_OPEN_CURLY_BRACKET; + } + + $end = $phpcsFile->findNext([$bodyToken, T_SEMICOLON], $tokens[$stackPtr]['parenthesis_closer']); + $hasBody = ($end !== false && $tokens[$end]['code'] === $bodyToken); + } + + if ($returnType !== '' && $nullableReturnType === true) { + $returnType = '?' . $returnType; + } + + return [ + 'scope' => $scope, + 'scope_specified' => $scopeSpecified, + 'return_type' => $returnType, + 'return_type_token' => $returnTypeToken, + 'return_type_end_token' => $returnTypeEndToken, + 'nullable_return_type' => $nullableReturnType, + 'is_abstract' => $isAbstract, + 'is_final' => $isFinal, + 'is_static' => $isStatic, + 'has_body' => $hasBody, + ]; } /** diff --git a/PHPCSUtils/Utils/FunctionDeclarations.php b/PHPCSUtils/Utils/FunctionDeclarations.php index e24bcf27..353ffdb1 100644 --- a/PHPCSUtils/Utils/FunctionDeclarations.php +++ b/PHPCSUtils/Utils/FunctionDeclarations.php @@ -270,6 +270,25 @@ public static function getProperties(File $phpcsFile, $stackPtr) break; } + if ($tokens[$i]['code'] === \T_USE) { + // Skip over closure use statements. + for ( + $j = ($i + 1); + $j < $phpcsFile->numTokens && isset(Tokens::$emptyTokens[$tokens[$j]['code']]) === true; + $j++ + ); + + if ($tokens[$j]['code'] === \T_OPEN_PARENTHESIS) { + if (isset($tokens[$j]['parenthesis_closer']) === false) { + // Live coding/parse error, stop parsing. + break; + } + + $i = $tokens[$j]['parenthesis_closer']; + continue; + } + } + if ($tokens[$i]['code'] === \T_NULLABLE) { $nullableReturnType = true; } diff --git a/Tests/BackCompat/BCFile/GetMethodPropertiesParseError1Test.inc b/Tests/BackCompat/BCFile/GetMethodPropertiesParseError1Test.inc new file mode 100644 index 00000000..6a2f3065 --- /dev/null +++ b/Tests/BackCompat/BCFile/GetMethodPropertiesParseError1Test.inc @@ -0,0 +1,5 @@ +getTargetToken('/* testParseError */', Collections::functionDeclarationTokens()); + $result = BCFile::getMethodProperties(self::$phpcsFile, $target); + + $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' => false, + 'has_body' => false, + ]; + + $this->assertSame($expected, $result); + } +} diff --git a/Tests/BackCompat/BCFile/GetMethodPropertiesTest.inc b/Tests/BackCompat/BCFile/GetMethodPropertiesTest.inc index 94ffdd1e..736a4ee0 100644 --- a/Tests/BackCompat/BCFile/GetMethodPropertiesTest.inc +++ b/Tests/BackCompat/BCFile/GetMethodPropertiesTest.inc @@ -185,6 +185,18 @@ $value = $obj->fn(true); /* testFunctionDeclarationNestedInTernaryPHPCS2975 */ return (!$a ? [ new class { public function b(): c {} } ] : []); +/* testClosureWithUseNoReturnType */ +$closure = function () use($a) /*comment*/ {}; + +/* testClosureWithUseNoReturnTypeIllegalUseProp */ +$closure = function () use ($this->prop){}; + +/* testClosureWithUseWithReturnType */ +$closure = function () use /*comment*/ ($a): Type {}; + +/* testClosureWithUseMultiParamWithReturnType */ +$closure = function () use ($a, &$b, $c, $d, $e, $f, $g): ?array {}; + /* testArrowFunctionLiveCoding */ // Intentional parse error. This has to be the last test in the file. $fn = fn diff --git a/Tests/BackCompat/BCFile/GetMethodPropertiesTest.php b/Tests/BackCompat/BCFile/GetMethodPropertiesTest.php index 93a61fa8..15c7dfcc 100644 --- a/Tests/BackCompat/BCFile/GetMethodPropertiesTest.php +++ b/Tests/BackCompat/BCFile/GetMethodPropertiesTest.php @@ -1198,6 +1198,103 @@ public function testFunctionDeclarationNestedInTernaryPHPCS2975() $this->getMethodPropertiesTestHelper('/* ' . __FUNCTION__ . ' */', $expected); } + /** + * Test handling of closure declarations with a use variable import without a return type declaration. + * + * @return void + */ + public function testClosureWithUseNoReturnType() + { + // Offsets are relative to the T_CLOSURE token. + $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' => false, + 'has_body' => true, + ]; + + $this->getMethodPropertiesTestHelper('/* ' . __FUNCTION__ . ' */', $expected); + } + + /** + * Test handling of closure declarations with an illegal use variable for a property import (not allowed in PHP) + * without a return type declaration. + * + * @return void + */ + public function testClosureWithUseNoReturnTypeIllegalUseProp() + { + // Offsets are relative to the T_CLOSURE token. + $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' => false, + 'has_body' => true, + ]; + + $this->getMethodPropertiesTestHelper('/* ' . __FUNCTION__ . ' */', $expected); + } + + /** + * Test handling of closure declarations with a use variable import with a return type declaration. + * + * @return void + */ + public function testClosureWithUseWithReturnType() + { + // Offsets are relative to the T_CLOSURE token. + $expected = [ + 'scope' => 'public', + 'scope_specified' => false, + 'return_type' => 'Type', + 'return_type_token' => 14, + 'return_type_end_token' => 14, + 'nullable_return_type' => false, + 'is_abstract' => false, + 'is_final' => false, + 'is_static' => false, + 'has_body' => true, + ]; + + $this->getMethodPropertiesTestHelper('/* ' . __FUNCTION__ . ' */', $expected); + } + + /** + * Test handling of closure declarations with a use variable import with a return type declaration. + * + * @return void + */ + public function testClosureWithUseMultiParamWithReturnType() + { + // Offsets are relative to the T_CLOSURE token. + $expected = [ + 'scope' => 'public', + 'scope_specified' => false, + 'return_type' => '?array', + 'return_type_token' => 32, + 'return_type_end_token' => 32, + 'nullable_return_type' => true, + 'is_abstract' => false, + 'is_final' => false, + 'is_static' => false, + 'has_body' => true, + ]; + + $this->getMethodPropertiesTestHelper('/* ' . __FUNCTION__ . ' */', $expected); + } + /** * Test helper. * diff --git a/Tests/Utils/FunctionDeclarations/GetPropertiesParseError1Test.php b/Tests/Utils/FunctionDeclarations/GetPropertiesParseError1Test.php new file mode 100644 index 00000000..3650a78f --- /dev/null +++ b/Tests/Utils/FunctionDeclarations/GetPropertiesParseError1Test.php @@ -0,0 +1,76 @@ +getTargetToken('/* testParseError */', Collections::functionDeclarationTokens()); + $result = FunctionDeclarations::getProperties(self::$phpcsFile, $target); + + $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' => false, + 'has_body' => false, + ]; + + $this->assertSame($expected, $result); + } +} From 12fb8a47c5b5b93264df8ba6ad5109b3c596e24b Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 4 Apr 2024 03:16:02 +0200 Subject: [PATCH 05/15] GH Actions: update markdown lint workflow The author of the `markdownlint-cli2` NPM package has published a GH Action runner for the package. Advantages of switching to using that package: * No need to monitor which Node version is supported by the NPM package, nor to manually update that. This is especially useful as the `markdownlint-cli2` doesn't use the GH release feature, which means that one can't subscribe to automatically get informed of releases (and the changelog of those). Ref: * https://github.com/marketplace/actions/markdownlint-cli2-action --- .github/workflows/basics.yml | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/.github/workflows/basics.yml b/.github/workflows/basics.yml index a3f48115..4e817867 100644 --- a/.github/workflows/basics.yml +++ b/.github/workflows/basics.yml @@ -118,24 +118,13 @@ jobs: - name: Checkout code uses: actions/checkout@v4 - # This action also handles the caching of the dependencies. - # https://github.com/actions/setup-node - - name: Set up node and enable caching of dependencies - uses: actions/setup-node@v4 - with: - node-version: '16' - - # @link https://github.com/DavidAnson/markdownlint-cli2 - # @link https://github.com/DavidAnson/markdownlint - - name: Install Markdownlint CLI2 - run: npm install -g markdownlint-cli2 - # @link https://github.com/marketplace/actions/problem-matcher-for-markdownlint-cli - name: Enable showing issue in PRs uses: xt0rted/markdownlint-problem-matcher@v3 + # @link https://github.com/marketplace/actions/markdownlint-cli2-action - name: Check markdown with CLI2 - run: markdownlint-cli2 + uses: DavidAnson/markdownlint-cli2-action@v15 remark: name: 'QA Markdown' From b9e51fe05620407546bf1691bd325e36fb6a6d53 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 8 Apr 2024 07:06:10 +0000 Subject: [PATCH 06/15] GH Actions: bump DavidAnson/markdownlint-cli2-action from 15 to 16 Bumps [DavidAnson/markdownlint-cli2-action](https://github.com/davidanson/markdownlint-cli2-action) from 15 to 16. - [Release notes](https://github.com/davidanson/markdownlint-cli2-action/releases) - [Commits](https://github.com/davidanson/markdownlint-cli2-action/compare/v15...v16) --- updated-dependencies: - dependency-name: DavidAnson/markdownlint-cli2-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/basics.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/basics.yml b/.github/workflows/basics.yml index 4e817867..c24a5ee1 100644 --- a/.github/workflows/basics.yml +++ b/.github/workflows/basics.yml @@ -124,7 +124,7 @@ jobs: # @link https://github.com/marketplace/actions/markdownlint-cli2-action - name: Check markdown with CLI2 - uses: DavidAnson/markdownlint-cli2-action@v15 + uses: DavidAnson/markdownlint-cli2-action@v16 remark: name: 'QA Markdown' From 194b3e66824a3a5072d01e938a3e6286542fab1f Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 19 Apr 2024 14:52:20 +0200 Subject: [PATCH 07/15] Changelog: tweak markdown ... to comply with changes in Remark. _sigh_ --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12cfc3e2..2a30b62f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -477,7 +477,7 @@ Please report any bugs/oversights you encounter! All properties have a replacement which should be used instead, in most cases this will be a method with the same name as the previously used property, | Deprecated | Replacement | PR | Remarks | -|---------------------------------------------------------------|------------------------------------------------------------------------------------------------------|----------------|------------------------------------------| +| ------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------- | -------------- | ---------------------------------------- | | `Collections::$alternativeControlStructureSyntaxTokens` | `Collections::alternativeControlStructureSyntaxes()` | [#311] | Mind the change in the name! | | `Collections::$alternativeControlStructureSyntaxCloserTokens` | `Collections::alternativeControlStructureSyntaxClosers()` | [#311] | Mind the change in the name! | | `Collections::$arrayTokens` | `Collections::arrayTokens()` | [#311] | | @@ -510,7 +510,7 @@ All properties have a replacement which should be used instead, in most cases th Additionally, the following methods in the `Collections` class have been deprecated: | Deprecated | Replacement | PR | -|----------------------------------------------|--------------------------------------------|--------| +| -------------------------------------------- | ------------------------------------------ | ------ | | `Collections::arrowFunctionTokensBC()` | Use the `T_FN` token instead. | [#347] | | `Collections::functionDeclarationTokensBC()` | `Collections::functionDeclarationTokens()` | [#347] | | `Collections::parameterTypeTokensBC()` | `Collections::parameterTypeTokens()` | [#347] | From b46e5f963ed6fb0b080cca282b8f6ecb5b6a7111 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 19 Apr 2024 13:29:47 +0200 Subject: [PATCH 08/15] NumberTypesTest: split off the "not a string" tests --- Tests/Utils/Numbers/NumberTypesTest.php | 120 +++++++++++++++++++----- 1 file changed, 98 insertions(+), 22 deletions(-) diff --git a/Tests/Utils/Numbers/NumberTypesTest.php b/Tests/Utils/Numbers/NumberTypesTest.php index becf2f91..8a6bb8e2 100644 --- a/Tests/Utils/Numbers/NumberTypesTest.php +++ b/Tests/Utils/Numbers/NumberTypesTest.php @@ -27,6 +27,104 @@ final class NumberTypesTest extends TestCase { + /** + * Test correctly rejecting non-string input. + * + * @dataProvider dataNotAString + * @covers ::isDecimalInt + * + * @param mixed $input The input data. + * + * @return void + */ + public function testIsDecimalIntInvalidInput($input) + { + $this->assertFalse(Numbers::isDecimalInt($input)); + } + + /** + * Test correctly rejecting non-string input. + * + * @dataProvider dataNotAString + * @covers ::isHexidecimalInt + * + * @param mixed $input The input data. + * + * @return void + */ + public function testIsHexidecimalIntInvalidInput($input) + { + $this->assertFalse(Numbers::isHexidecimalInt($input)); + } + + /** + * Test correctly rejecting non-string input. + * + * @dataProvider dataNotAString + * @covers ::isBinaryInt + * + * @param mixed $input The input data. + * + * @return void + */ + public function testIsBinaryIntInvalidInput($input) + { + $this->assertFalse(Numbers::isBinaryInt($input)); + } + + /** + * Test correctly rejecting non-string input. + * + * @dataProvider dataNotAString + * @covers ::isOctalInt + * + * @param mixed $input The input data. + * + * @return void + */ + public function testIsOctalIntInvalidInput($input) + { + $this->assertFalse(Numbers::isOctalInt($input)); + } + + /** + * Test correctly rejecting non-string input. + * + * @dataProvider dataNotAString + * @covers ::isFloat + * + * @param mixed $input The input data. + * + * @return void + */ + public function testIsFloatInvalidInput($input) + { + $this->assertFalse(Numbers::isFloat($input)); + } + + /** + * Data Provider. + * + * @see testIsDecimalInt() For the array format. + * @see testIsHexidecimalInt() For the array format. + * @see testIsBinaryInt() For the array format. + * @see testIsOctalInt() For the array format. + * @see testIsDecimalFloat() For the array format. + * + * @return array> + */ + public static function dataNotAString() + { + return [ + 'not-a-string-bool' => [ + 'input' => true, + ], + 'not-a-string-int' => [ + 'input' => 10, + ], + ]; + } + /** * Test correctly recognizing an arbitrary string representing a decimal integer. * @@ -121,28 +219,6 @@ public function testIsFloat($input, $expected) public static function dataNumbers() { return [ - // Not strings. - 'not-a-string-bool' => [ - 'input' => true, - 'expected' => [ - 'decimal' => false, - 'hex' => false, - 'binary' => false, - 'octal' => false, - 'float' => false, - ], - ], - 'not-a-string-int' => [ - 'input' => 10, - 'expected' => [ - 'decimal' => false, - 'hex' => false, - 'binary' => false, - 'octal' => false, - 'float' => false, - ], - ], - // Not numeric strings. 'empty-string' => [ 'input' => '', From c0baf890060f2f86d3f468248ed52425f7601ed4 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 28 Dec 2023 07:47:56 +0100 Subject: [PATCH 09/15] IsValidIdentifierNameTest: move invalid input test to separate test method --- .../IsValidIdentifierNameTest.php | 14 ++++++++++---- phpstan.neon.dist | 1 + 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/Tests/Utils/NamingConventions/IsValidIdentifierNameTest.php b/Tests/Utils/NamingConventions/IsValidIdentifierNameTest.php index fa5d924e..7c1e3d0c 100644 --- a/Tests/Utils/NamingConventions/IsValidIdentifierNameTest.php +++ b/Tests/Utils/NamingConventions/IsValidIdentifierNameTest.php @@ -23,6 +23,16 @@ final class IsValidIdentifierNameTest extends TestCase { + /** + * Test that non-string input is rejected as invalid for a PHP identifier name. + * + * @return void + */ + public function testIsValidIdentifierNameReturnsFalseOnInvalidType() + { + $this->assertFalse(NamingConventions::isValidIdentifierName(12345)); + } + /** * Test correctly detecting whether an arbitrary string can be a valid PHP identifier name. * @@ -95,10 +105,6 @@ public static function dataIsValidIdentifierName() ], // Invalid names. - 'not-a-string' => [ - 'input' => 12345, - 'expected' => false, - ], 'empty-string' => [ 'input' => '', 'expected' => false, diff --git a/phpstan.neon.dist b/phpstan.neon.dist index bdcaef3b..8d42ecc5 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -79,6 +79,7 @@ parameters: - Tests/Exceptions/TestTargetNotFound/TestTargetNotFoundTest.php - Tests/Fixers/SpacesFixer/SpacesFixerExceptionsTest.php - Tests/Utils/GetTokensAsString/GetTokensAsStringTest.php + - Tests/Utils/NamingConventions/IsValidIdentifierNameTest.php # Ignoring as this is fine. - From 1f74f80b8bdde3737c62bd1474a31de6b35d8f0d Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Sun, 21 Apr 2024 19:36:31 +0100 Subject: [PATCH 10/15] Composer: avoid writing a lock file (#579) * Composer: avoid writing a lock file * Allow use of Composer lock file in some tests --- .github/workflows/quicktest.yml | 4 ++++ .github/workflows/test.yml | 8 ++++++++ composer.json | 3 ++- 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/.github/workflows/quicktest.yml b/.github/workflows/quicktest.yml index 687acbcd..c32176cf 100644 --- a/.github/workflows/quicktest.yml +++ b/.github/workflows/quicktest.yml @@ -64,6 +64,10 @@ jobs: if: ${{ matrix.phpcs_version != 'lowest' }} run: composer require squizlabs/php_codesniffer:"${{ matrix.phpcs_version }}" --no-update --no-scripts --no-interaction + - name: "Composer: use lock file when necessary" + if: ${{ matrix.phpcs_version == 'lowest' }} + run: composer config --unset lock + # Install dependencies and handle caching in one go. # @link https://github.com/marketplace/actions/install-php-dependencies-with-composer - name: Install Composer dependencies diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ab9c4530..324897f1 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -180,6 +180,10 @@ jobs: - name: 'Composer: remove PHPCSDevCS' run: composer remove --dev --no-update phpcsstandards/phpcsdevcs + - name: "Composer: use lock file when necessary" + if: ${{ matrix.phpcs_version == 'lowest' }} + run: composer config --unset lock + # Install dependencies and handle caching in one go. # @link https://github.com/marketplace/actions/install-php-dependencies-with-composer - name: Install Composer dependencies - normal @@ -308,6 +312,10 @@ jobs: if: ${{ matrix.phpcs_version != 'lowest' }} run: composer require squizlabs/php_codesniffer:"${{ matrix.phpcs_version }}" --no-update --no-scripts --no-interaction + - name: "Composer: use lock file when necessary" + if: ${{ matrix.phpcs_version == 'lowest' }} + run: composer config --unset lock + - name: Install Composer dependencies uses: "ramsey/composer-install@v3" with: diff --git a/composer.json b/composer.json index 0fe11b88..09405e9f 100644 --- a/composer.json +++ b/composer.json @@ -86,6 +86,7 @@ "config": { "allow-plugins": { "dealerdirect/phpcodesniffer-composer-installer": true - } + }, + "lock": false } } From 791abd86c741e3dd955183934a9de9130f420848 Mon Sep 17 00:00:00 2001 From: jrfnl <663378+jrfnl@users.noreply.github.com> Date: Wed, 24 Apr 2024 03:40:58 +0000 Subject: [PATCH 11/15] GetVersionTest: update for release of PHPCS 3.9.2 --- Tests/BackCompat/Helper/GetVersionTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/BackCompat/Helper/GetVersionTest.php b/Tests/BackCompat/Helper/GetVersionTest.php index 52357a5d..25d7b402 100644 --- a/Tests/BackCompat/Helper/GetVersionTest.php +++ b/Tests/BackCompat/Helper/GetVersionTest.php @@ -30,7 +30,7 @@ final class GetVersionTest extends TestCase * * @var string */ - const DEVMASTER = '3.9.1'; + const DEVMASTER = '3.9.2'; /** * Test the method. From 325e574a86dd6653f63a48526abba822fa17a920 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 24 Apr 2024 12:29:27 +0200 Subject: [PATCH 12/15] GH Actions: work around intermittent apt-get errors Okay, so apparently, there is a long-standing bug in the Microsoft package deploy process which caused `apt-get update` to fail in the first half hour after Microsoft has deployed a package. The failure looks like this: ``` E: Failed to fetch https://packages.microsoft.com/ubuntu/22.04/prod/dists/jammy/InRelease Clearsigned file isn't valid, got 'NOSPLIT' (does the network require authentication?) ``` As this only happens intermittently (after a MS package deploy), the chance of running into this bug are slim, but guess what: today I ran into it. This change to the workflow is intended to prevent the next person running into this issue from having to waste time on figuring this out. By splitting the "Install xmllint" step into two steps: one doing the `apt-get update` and one doing the actual install and making the first step one which is allowed to `continue-on-error`, this issue should hopefully not crop up anymore. Any errors in the `apt-get update` step will now be ignored and as most errors which could potentially come from that step are irrelevant for the rest of the job anyway, this is fine. If a relevant error would be surfaced, the next step (the xmllint install), will fail the job anyway. Refs: * https://github.com/actions/runner-images/issues/3410 * https://github.com/dotnet/core/issues/4167 --- .github/workflows/basics.yml | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/.github/workflows/basics.yml b/.github/workflows/basics.yml index c24a5ee1..f1a7e15c 100644 --- a/.github/workflows/basics.yml +++ b/.github/workflows/basics.yml @@ -55,10 +55,15 @@ jobs: # Bust the cache at least once a month - output format: YYYY-MM. custom-cache-suffix: $(date -u "+%Y-%m") + # Updating the lists can fail intermittently, typically after Microsoft has released a new package. + # This should not be blocking for this job, so ignore any errors from this step. + # Ref: https://github.com/dotnet/core/issues/4167 + - name: Update the available packages list + continue-on-error: true + run: sudo apt-get update + - name: Install xmllint - run: | - sudo apt-get update - sudo apt-get install --no-install-recommends -y libxml2-utils + run: sudo apt-get install --no-install-recommends -y libxml2-utils # Show XML violations inline in the file diff. # @link https://github.com/marketplace/actions/xmllint-problem-matcher From 4abf6331939174cec7fe1c7a1d36f0fb979a940d Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 19 Apr 2024 13:18:32 +0200 Subject: [PATCH 13/15] CS: minor fixes --- PHPCSUtils/Utils/NamingConventions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PHPCSUtils/Utils/NamingConventions.php b/PHPCSUtils/Utils/NamingConventions.php index 49554580..cec7a27f 100644 --- a/PHPCSUtils/Utils/NamingConventions.php +++ b/PHPCSUtils/Utils/NamingConventions.php @@ -111,6 +111,6 @@ public static function isEqual($nameA, $nameB) } // Comparing via strcasecmp will only compare ASCII letters case-insensitively. - return (strcasecmp($nameA, $nameB) === 0); + return (\strcasecmp($nameA, $nameB) === 0); } } From 7d5498ba1295de332a60410374f75a7c2cc981be Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 19 Apr 2024 13:30:22 +0200 Subject: [PATCH 14/15] Docs: improve type specificity --- Tests/Internal/Cache/GetClearTest.php | 2 +- Tests/Internal/NoFileCache/GetClearTest.php | 2 +- Tests/Utils/Numbers/NumberTypesTest.php | 22 ++++++++++----------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/Tests/Internal/Cache/GetClearTest.php b/Tests/Internal/Cache/GetClearTest.php index 652aa62b..e329af4a 100644 --- a/Tests/Internal/Cache/GetClearTest.php +++ b/Tests/Internal/Cache/GetClearTest.php @@ -422,7 +422,7 @@ public function testClearCache($id) * * @see testClearCache() For the array format. * - * @return array + * @return array> */ public static function dataClearCache() { diff --git a/Tests/Internal/NoFileCache/GetClearTest.php b/Tests/Internal/NoFileCache/GetClearTest.php index ff5ef8d4..840bb9d4 100644 --- a/Tests/Internal/NoFileCache/GetClearTest.php +++ b/Tests/Internal/NoFileCache/GetClearTest.php @@ -286,7 +286,7 @@ public function testClearCache($id) * * @see testClearCache() For the array format. * - * @return array + * @return array> */ public static function dataClearCache() { diff --git a/Tests/Utils/Numbers/NumberTypesTest.php b/Tests/Utils/Numbers/NumberTypesTest.php index 8a6bb8e2..21e61f8d 100644 --- a/Tests/Utils/Numbers/NumberTypesTest.php +++ b/Tests/Utils/Numbers/NumberTypesTest.php @@ -131,8 +131,8 @@ public static function dataNotAString() * @dataProvider dataNumbers * @covers ::isDecimalInt * - * @param string $input The input string. - * @param array $expected The expected output for the various functions. + * @param string $input The input string. + * @param array $expected The expected output for the various functions. * * @return void */ @@ -147,8 +147,8 @@ public function testIsDecimalInt($input, $expected) * @dataProvider dataNumbers * @covers ::isHexidecimalInt * - * @param string $input The input string. - * @param array $expected The expected output for the various functions. + * @param string $input The input string. + * @param array $expected The expected output for the various functions. * * @return void */ @@ -163,8 +163,8 @@ public function testIsHexidecimalInt($input, $expected) * @dataProvider dataNumbers * @covers ::isBinaryInt * - * @param string $input The input string. - * @param array $expected The expected output for the various functions. + * @param string $input The input string. + * @param array $expected The expected output for the various functions. * * @return void */ @@ -179,8 +179,8 @@ public function testIsBinaryInt($input, $expected) * @dataProvider dataNumbers * @covers ::isOctalInt * - * @param string $input The input string. - * @param array $expected The expected output for the various functions. + * @param string $input The input string. + * @param array $expected The expected output for the various functions. * * @return void */ @@ -195,8 +195,8 @@ public function testIsOctalInt($input, $expected) * @dataProvider dataNumbers * @covers ::isFloat * - * @param string $input The input string. - * @param array $expected The expected output for the various functions. + * @param string $input The input string. + * @param array $expected The expected output for the various functions. * * @return void */ @@ -214,7 +214,7 @@ public function testIsFloat($input, $expected) * @see testIsOctalInt() For the array format. * @see testIsDecimalFloat() For the array format. * - * @return array + * @return array>> */ public static function dataNumbers() { From 859a67b87a4b9c787b4b5b2e334873393c55a257 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 29 Mar 2024 05:46:35 +0100 Subject: [PATCH 15/15] Changelog for PHPCSUtils 1.0.11 --- CHANGELOG.md | 22 ++++++++++++++++++++++ PHPCSUtils/BackCompat/BCFile.php | 2 +- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a30b62f..1599c220 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,27 @@ This projects adheres to [Keep a CHANGELOG](https://keepachangelog.com/) and use _Nothing yet._ +## [1.0.11] - 2024-04-24 + +### Changed + +#### Other + +* Various housekeeping and documentation improvements. Includes a contribution from [@fredden]. + +### Fixed + +#### PHPCS BackCompat + +* `BCFile::getMethodProperties()`: small performance improvement & more defensive coding, in line with same fix in PHPCS 3.9.2. [#573] + +#### Utils + +* `FunctionDeclarations::getProperties()`: small performance improvement & more defensive coding, in line with same fix in PHPCS 3.9.2. [#573] + +[#573]: https://github.com/PHPCSStandards/PHPCSUtils/pull/573 + + ## [1.0.10] - 2024-03-18 ### Changed @@ -984,6 +1005,7 @@ This initial alpha release contains the following utility classes: [Unreleased]: https://github.com/PHPCSStandards/PHPCSUtils/compare/stable...HEAD +[1.0.11]: https://github.com/PHPCSStandards/PHPCSUtils/compare/1.0.10...1.0.11 [1.0.10]: https://github.com/PHPCSStandards/PHPCSUtils/compare/1.0.9...1.0.10 [1.0.9]: https://github.com/PHPCSStandards/PHPCSUtils/compare/1.0.8...1.0.9 [1.0.8]: https://github.com/PHPCSStandards/PHPCSUtils/compare/1.0.7...1.0.8 diff --git a/PHPCSUtils/BackCompat/BCFile.php b/PHPCSUtils/BackCompat/BCFile.php index dc49bdca..f64d7da0 100644 --- a/PHPCSUtils/BackCompat/BCFile.php +++ b/PHPCSUtils/BackCompat/BCFile.php @@ -462,7 +462,7 @@ public static function getMethodParameters(File $phpcsFile, $stackPtr) * * Changelog for the PHPCS native function: * - Introduced in PHPCS 0.0.5. - * - PHPCS 3.9.1: skip over closure use statements. PHPCS #421. + * - PHPCS 3.9.2: skip over closure use statements. PHPCS #421. * * @see \PHP_CodeSniffer\Files\File::getMethodProperties() Original source. * @see \PHPCSUtils\Utils\FunctionDeclarations::getProperties() PHPCSUtils native improved version.