From e25811bffabc39162ac7e4acf007e1d61617f20b Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 2 Jun 2022 01:59:54 +0200 Subject: [PATCH 1/5] Composer: require PHPCSDevCS --- .github/workflows/test.yml | 4 ++-- composer.json | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1b1df80..2044501 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -147,8 +147,8 @@ jobs: - name: 'Composer: adjust dependencies' run: | - # Remove dev dependencies which are not compatible with all supported PHP versions. - composer remove --dev --no-update sirbrillig/phpcs-import-detection phpstan/phpstan + # Remove dev dependencies which are not compatible with all supported PHP/PHPCS versions. + composer remove --dev --no-update phpcsstandards/phpcsdevcs sirbrillig/phpcs-import-detection phpstan/phpstan composer require --no-update squizlabs/php_codesniffer:"${{ matrix.phpcs_version }}" - name: Install Composer dependencies diff --git a/composer.json b/composer.json index 7172f63..f91cc7f 100644 --- a/composer.json +++ b/composer.json @@ -49,6 +49,7 @@ "require-dev": { "phpunit/phpunit": "^4.8.36 || ^5.7.21 || ^6.5 || ^7.0 || ^8.0 || ^9.0", "sirbrillig/phpcs-import-detection": "^1.1", + "phpcsstandards/phpcsdevcs": "^1.1", "phpstan/phpstan": "^1.7", "dealerdirect/phpcodesniffer-composer-installer": "^0.7.0" } From ec020db028707e9c70409ec42efe0c8170da6be3 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 10 Jun 2022 22:35:37 +0200 Subject: [PATCH 2/5] PHPCS: switch to using the PHPCSDev standard This commit: * Switches out the `PSR2` ruleset in favour of the `PHPCSDev` ruleset. The PHPCSDev ruleset checks the following: > * Compliance with PSR-12, with a few select exceptions. > * Use of camelCase variable and function names. > * Use of normalized arrays. > * All files, classes, functions and properties are documented with a docblock and contain the minimally needed information. > * A number of arbitrary additional code style and QA checks. > * PHP cross-version compatibility, while allowing for tokens back-filled by PHPCS itself. * For the PHP cross-version compatibility check, the minimum supported PHP version (`testVersion`) is set to PHP 5.4, in line with the `require`ment for this package per the `composer.json` file. * In contrast to `PHPCSDev`/PSR12, the ruleset for the VariableAnalysis package will: - Enforce tabs instead of spaces. - Disallow a blank line at the start of a class. Note: this complies with PSR12. PHPCSDev already had the blank line enforcement in place prior to this being forbidden via the PSR12 standard, which is why it excludes the rule. - Not enforce most documentation checks. - Not enforce assignment operator alignment. * Additionally, for the time being, the PSR12 line length guidelines will not be enforced. Enforcing those needs additional (manual) adjustments to the codebase which can be done at a later point in time. * Adds some extra documention to the ruleset. Refs: * https://github.com/PHPCSStandards/PHPCSDevCS * https://github.com/PHPCSStandards/PHPCSDevCS/blob/main/PHPCSDev/ruleset.xml --- phpcs.xml.dist | 84 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 77 insertions(+), 7 deletions(-) diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 46ba582..4f0cd1b 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -1,7 +1,15 @@ - ./VariableAnalysis/ - ./Tests/ + + + + . + ./Tests/VariableAnalysisSniff/fixtures/ ./vendor/ @@ -17,21 +25,83 @@ + - - + + + + + + + + + + + + + + + + + + + + + - + + + + + + + + - - + + + 5 + + + + + + + + + + + + + + + + + + + + + From 865d8ba670b5d87b81f9d0f2f6058a5257c18fa7 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 10 Jun 2022 02:50:03 +0200 Subject: [PATCH 3/5] CS: various minor whitespace fixes Minimal changes needed to comply with the PSR12/PHPCSDev whitespace rules. Most notably this commit adds a blank line between a PHP open tag and the namespace declaration as per the PSR12 file header rules. --- Tests/BaseTestCase.php | 1 + Tests/VariableAnalysisSniff/ArrayAssignmentShortcutTest.php | 1 + Tests/VariableAnalysisSniff/ArrowFunctionTest.php | 1 + Tests/VariableAnalysisSniff/ClosingPhpTagsTest.php | 1 + Tests/VariableAnalysisSniff/GlobalScopeTest.php | 1 + Tests/VariableAnalysisSniff/IfConditionTest.php | 1 + Tests/VariableAnalysisSniff/IssetTest.php | 1 + Tests/VariableAnalysisSniff/UnsetTest.php | 1 + Tests/VariableAnalysisSniff/UnusedFollowedByRequireTest.php | 1 + Tests/VariableAnalysisSniff/VariableAnalysisTest.php | 1 + Tests/VariableAnalysisSniff/VariableArgumentListTest.php | 1 + Tests/VariableAnalysisSniff/WhileLoopTest.php | 1 + Tests/bootstrap.php | 1 + VariableAnalysis/Lib/Helpers.php | 2 +- .../Sniffs/CodeAnalysis/VariableAnalysisSniff.php | 5 +++-- 15 files changed, 17 insertions(+), 3 deletions(-) diff --git a/Tests/BaseTestCase.php b/Tests/BaseTestCase.php index 19f2d84..f8b56cf 100644 --- a/Tests/BaseTestCase.php +++ b/Tests/BaseTestCase.php @@ -1,4 +1,5 @@ getScopeInfo($currScope); - return ( $scopeInfo && isset($scopeInfo->variables[$varName]) ) ? $scopeInfo->variables[$varName] : null; + return ($scopeInfo && isset($scopeInfo->variables[$varName])) ? $scopeInfo->variables[$varName] : null; } /** @@ -854,7 +854,8 @@ protected function processVariableAsSuperGlobal($varName) 'php_errormsg', 'http_response_header', 'HTTP_RAW_POST_DATA', - ])) { + ]) + ) { return true; } From f6e66fa8b1860565f84e6e6f0ecd8226a0b6294f Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 2 Jun 2022 03:31:23 +0200 Subject: [PATCH 4/5] CS: minor code restructuring [1] The `VariableAnalysisSniff::processVariableAsSuperGlobal()` method checks a variable name against a fixed array of names using `in_array()`. To comply with the updated CS rules, each parameter in the function call would need to be placed on a new line and the function call itself as well, making this a very drawn out condition. By restructuring the code to declare the array prior to the `in_array()` function call, this is no longer needed. Additional notes: * I've also changed the `in_array()` call to a _strict_ comparison by adding the third parameter and setting it to `true`. * I've simplified the `return` by removing the unnecessary condition and double return statements. * The array could/should probably be declared as a property instead of within the function. I've not done so at this time, as it could also be considered to switch over to using the PHPCSUtils `Variables::isSuperglobal()` or `Variables::isSuperglobalName()` methods in the future. Ref: * https://www.php.net/manual/en/function.in-array.php * https://phpcsutils.com/phpdoc/classes/PHPCSUtils-Utils-Variables.html#property_phpReservedVars --- .../Sniffs/CodeAnalysis/VariableAnalysisSniff.php | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php index 6144dba..05a140a 100644 --- a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php +++ b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php @@ -838,8 +838,7 @@ protected function processVariableAsThisWithinClass(File $phpcsFile, $stackPtr, */ protected function processVariableAsSuperGlobal($varName) { - // Are we a superglobal variable? - if (in_array($varName, [ + $superglobals = [ 'GLOBALS', '_SERVER', '_GET', @@ -854,12 +853,9 @@ protected function processVariableAsSuperGlobal($varName) 'php_errormsg', 'http_response_header', 'HTTP_RAW_POST_DATA', - ]) - ) { - return true; - } - - return false; + ]; + // Are we a superglobal variable? + return (in_array($varName, $superglobals, true)); } /** From 1a82c938cb27d9114e450706b076af2076313653 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 2 Jun 2022 03:59:41 +0200 Subject: [PATCH 5/5] CS: minor code restructuring [2] Similar to the previous commit, the `VariableAnalysisSniff::processVariableAsStaticDeclaration()` method declares an array within a function call. This commit moves the array declaration out of the function call and leverages a pre-defined array from the PHPCS native `Tokens` class to retrieve a number of the tokens (heredoc and nowdoc tokens). Note: this commit does **not** fix known shortcomings of this method as reported in 158 and 253. --- .../CodeAnalysis/VariableAnalysisSniff.php | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php index 05a140a..211abb8 100644 --- a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php +++ b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php @@ -1137,22 +1137,21 @@ protected function processVariableAsStaticDeclaration(File $phpcsFile, $stackPtr // class constant T_STRING T_DOUBLE_COLON T_STRING // Search backwards for first token that isn't whitespace, comma, variable, // equals, or on the list of assignable constant values above. - $staticPtr = $phpcsFile->findPrevious( - [ - T_WHITESPACE, T_VARIABLE, T_COMMA, T_EQUAL, - T_MINUS, T_LNUMBER, T_DNUMBER, - T_CONSTANT_ENCAPSED_STRING, - T_STRING, - T_DOUBLE_COLON, - T_START_HEREDOC, T_HEREDOC, T_END_HEREDOC, - T_START_NOWDOC, T_NOWDOC, T_END_NOWDOC, - ], - $stackPtr - 1, - null, - true, - null, - true - ); + $find = [ + T_WHITESPACE => T_WHITESPACE, + T_VARIABLE => T_VARIABLE, + T_COMMA => T_COMMA, + T_EQUAL => T_EQUAL, + T_MINUS => T_MINUS, + T_LNUMBER => T_LNUMBER, + T_DNUMBER => T_DNUMBER, + T_CONSTANT_ENCAPSED_STRING => T_CONSTANT_ENCAPSED_STRING, + T_STRING => T_STRING, + T_DOUBLE_COLON => T_DOUBLE_COLON, + ]; + $find += Tokens::$heredocTokens; + + $staticPtr = $phpcsFile->findPrevious($find, $stackPtr - 1, null, true, null, true); if (($staticPtr === false) || ($tokens[$staticPtr]['code'] !== T_STATIC)) { return false; }