Skip to content

Commit e9c99cd

Browse files
jrfnljrfnl
authored
PHPCS: switch to phpcsdevcs (sirbrillig#259)
* Composer: require PHPCSDevCS * 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 * 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. * 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 * 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. Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
1 parent 8f68af0 commit e9c99cd

18 files changed

+114
-34
lines changed

‎.github/workflows/test.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,8 @@ jobs:
147147

148148
- name: 'Composer: adjust dependencies'
149149
run: |
150-
# Remove dev dependencies which are not compatible with all supported PHP versions.
151-
composer remove --dev --no-update sirbrillig/phpcs-import-detection phpstan/phpstan
150+
# Remove dev dependencies which are not compatible with all supported PHP/PHPCS versions.
151+
composer remove --dev --no-update phpcsstandards/phpcsdevcs sirbrillig/phpcs-import-detection phpstan/phpstan
152152
composer require --no-update squizlabs/php_codesniffer:"${{ matrix.phpcs_version }}"
153153
154154
- name: Install Composer dependencies

‎Tests/BaseTestCase.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
namespace VariableAnalysis\Tests;
34

45
use PHPUnit\Framework\TestCase;

‎Tests/VariableAnalysisSniff/ArrayAssignmentShortcutTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
namespace VariableAnalysis\Tests\VariableAnalysisSniff;
34

45
use VariableAnalysis\Tests\BaseTestCase;

‎Tests/VariableAnalysisSniff/ArrowFunctionTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
namespace VariableAnalysis\Tests\VariableAnalysisSniff;
34

45
use VariableAnalysis\Tests\BaseTestCase;

‎Tests/VariableAnalysisSniff/ClosingPhpTagsTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
namespace VariableAnalysis\Tests\VariableAnalysisSniff;
34

45
use VariableAnalysis\Tests\BaseTestCase;

‎Tests/VariableAnalysisSniff/GlobalScopeTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
namespace VariableAnalysis\Tests\VariableAnalysisSniff;
34

45
use VariableAnalysis\Tests\BaseTestCase;

‎Tests/VariableAnalysisSniff/IfConditionTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
namespace VariableAnalysis\Tests\VariableAnalysisSniff;
34

45
use VariableAnalysis\Tests\BaseTestCase;

‎Tests/VariableAnalysisSniff/IssetTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
namespace VariableAnalysis\Tests\VariableAnalysisSniff;
34

45
use VariableAnalysis\Tests\BaseTestCase;

‎Tests/VariableAnalysisSniff/UnsetTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
namespace VariableAnalysis\Tests\VariableAnalysisSniff;
34

45
use VariableAnalysis\Tests\BaseTestCase;

‎Tests/VariableAnalysisSniff/UnusedFollowedByRequireTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
namespace VariableAnalysis\Tests\VariableAnalysisSniff;
34

45
use VariableAnalysis\Tests\BaseTestCase;

‎Tests/VariableAnalysisSniff/VariableAnalysisTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
namespace VariableAnalysis\Tests\VariableAnalysisSniff;
34

45
use VariableAnalysis\Tests\BaseTestCase;

‎Tests/VariableAnalysisSniff/VariableArgumentListTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
namespace VariableAnalysis\Tests\VariableAnalysisSniff;
34

45
use VariableAnalysis\Tests\BaseTestCase;

‎Tests/VariableAnalysisSniff/WhileLoopTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
namespace VariableAnalysis\Tests\VariableAnalysisSniff;
34

45
use VariableAnalysis\Tests\BaseTestCase;

‎Tests/bootstrap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
<?php
2+
23
require_once __DIR__ . '/../vendor/squizlabs/php_codesniffer/tests/bootstrap.php';
34
require_once __DIR__ . '/BaseTestCase.php';

‎VariableAnalysis/Lib/Helpers.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public static function getPossibleEndOfFileTokens()
3131
*/
3232
public static function getIntOrNull($value)
3333
{
34-
return is_int($value) ? $value: null;
34+
return is_int($value) ? $value : null;
3535
}
3636

3737
/**

‎VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php

Lines changed: 20 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,7 @@ protected function getOrCreateScopeInfo($currScope)
378378
protected function getVariableInfo($varName, $currScope)
379379
{
380380
$scopeInfo = $this->getScopeInfo($currScope);
381-
return ( $scopeInfo && isset($scopeInfo->variables[$varName]) ) ? $scopeInfo->variables[$varName] : null;
381+
return ($scopeInfo && isset($scopeInfo->variables[$varName])) ? $scopeInfo->variables[$varName] : null;
382382
}
383383

384384
/**
@@ -838,8 +838,7 @@ protected function processVariableAsThisWithinClass(File $phpcsFile, $stackPtr,
838838
*/
839839
protected function processVariableAsSuperGlobal($varName)
840840
{
841-
// Are we a superglobal variable?
842-
if (in_array($varName, [
841+
$superglobals = [
843842
'GLOBALS',
844843
'_SERVER',
845844
'_GET',
@@ -854,11 +853,9 @@ protected function processVariableAsSuperGlobal($varName)
854853
'php_errormsg',
855854
'http_response_header',
856855
'HTTP_RAW_POST_DATA',
857-
])) {
858-
return true;
859-
}
860-
861-
return false;
856+
];
857+
// Are we a superglobal variable?
858+
return (in_array($varName, $superglobals, true));
862859
}
863860

864861
/**
@@ -1140,22 +1137,21 @@ protected function processVariableAsStaticDeclaration(File $phpcsFile, $stackPtr
11401137
// class constant T_STRING T_DOUBLE_COLON T_STRING
11411138
// Search backwards for first token that isn't whitespace, comma, variable,
11421139
// equals, or on the list of assignable constant values above.
1143-
$staticPtr = $phpcsFile->findPrevious(
1144-
[
1145-
T_WHITESPACE, T_VARIABLE, T_COMMA, T_EQUAL,
1146-
T_MINUS, T_LNUMBER, T_DNUMBER,
1147-
T_CONSTANT_ENCAPSED_STRING,
1148-
T_STRING,
1149-
T_DOUBLE_COLON,
1150-
T_START_HEREDOC, T_HEREDOC, T_END_HEREDOC,
1151-
T_START_NOWDOC, T_NOWDOC, T_END_NOWDOC,
1152-
],
1153-
$stackPtr - 1,
1154-
null,
1155-
true,
1156-
null,
1157-
true
1158-
);
1140+
$find = [
1141+
T_WHITESPACE => T_WHITESPACE,
1142+
T_VARIABLE => T_VARIABLE,
1143+
T_COMMA => T_COMMA,
1144+
T_EQUAL => T_EQUAL,
1145+
T_MINUS => T_MINUS,
1146+
T_LNUMBER => T_LNUMBER,
1147+
T_DNUMBER => T_DNUMBER,
1148+
T_CONSTANT_ENCAPSED_STRING => T_CONSTANT_ENCAPSED_STRING,
1149+
T_STRING => T_STRING,
1150+
T_DOUBLE_COLON => T_DOUBLE_COLON,
1151+
];
1152+
$find += Tokens::$heredocTokens;
1153+
1154+
$staticPtr = $phpcsFile->findPrevious($find, $stackPtr - 1, null, true, null, true);
11591155
if (($staticPtr === false) || ($tokens[$staticPtr]['code'] !== T_STATIC)) {
11601156
return false;
11611157
}

‎composer.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
"require-dev": {
5050
"phpunit/phpunit": "^4.8.36 || ^5.7.21 || ^6.5 || ^7.0 || ^8.0 || ^9.0",
5151
"sirbrillig/phpcs-import-detection": "^1.1",
52+
"phpcsstandards/phpcsdevcs": "^1.1",
5253
"phpstan/phpstan": "^1.7",
5354
"dealerdirect/phpcodesniffer-composer-installer": "^0.7.0"
5455
}

‎phpcs.xml.dist

Lines changed: 77 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,15 @@
11
<?xml version="1.0"?>
22
<ruleset name="PaytonsStandard">
3-
<file>./VariableAnalysis/</file>
4-
<file>./Tests/</file>
3+
4+
<!--
5+
#############################################################################
6+
COMMAND LINE ARGUMENTS
7+
https://github.com/squizlabs/PHP_CodeSniffer/wiki/Annotated-ruleset.xml
8+
#############################################################################
9+
-->
10+
11+
<file>.</file>
12+
513
<exclude-pattern>./Tests/VariableAnalysisSniff/fixtures/</exclude-pattern>
614
<exclude-pattern>./vendor/</exclude-pattern>
715

@@ -17,21 +25,83 @@
1725
<!-- Check up to 8 files simultaneously. -->
1826
<arg name="parallel" value="8"/>
1927

28+
<!-- One tab = 4 spaces. This is needed to properly support tab indentation. -->
2029
<arg name="tab-width" value="4"/>
2130

22-
<!-- Rules -->
23-
<rule ref="PSR2">
31+
<!--
32+
#############################################################################
33+
USE THE PHPCSDev, ImportDetection and VariableAnalysis RULESETS
34+
#############################################################################
35+
-->
36+
37+
<!-- Set minimum PHP version supported to PHP 5.4. -->
38+
<config name="testVersion" value="5.4-"/>
39+
40+
<rule ref="PHPCSDev">
41+
<!-- This code base uses tab indentation instead of spaces. -->
2442
<exclude name="Generic.WhiteSpace.DisallowTabIndent"/>
43+
44+
<!-- Don't enforce lining up the assignment operators in assignment blocks. -->
45+
<exclude name="Generic.Formatting.MultipleStatementAlignment.NotSameWarning"/>
46+
47+
<!-- Don't enforce lining up the double arrows in array declarations.
48+
Possibly enforce this later once the sniff has been replaced by a better, more configurable version. -->
49+
<exclude name="Squiz.Arrays.ArrayDeclaration.DoubleArrowNotAligned"/>
50+
51+
<!-- Don't enforce documentation (yet). -->
52+
<exclude name="Generic.Commenting.DocComment"/>
53+
<exclude name="PEAR.Commenting.ClassComment"/>
54+
<exclude name="PEAR.Commenting.FileComment"/>
55+
<exclude name="PEAR.Commenting.InlineComment"/>
56+
57+
<!-- WIP: This is part of PSR12 and should probably be enforced,
58+
but the codebase needs work before it can be enabled. -->
2559
<exclude name="Generic.Files.LineLength.TooLong" />
2660
</rule>
2761

28-
<rule ref="Generic.WhiteSpace.DisallowSpaceIndent"/>
62+
<rule ref="ImportDetection" />
63+
<rule ref="VariableAnalysis"/>
64+
65+
66+
<!--
67+
#############################################################################
68+
SNIFF SPECIFIC CONFIGURATION AND SELECTIVELY DEVIATE FROM THE STANDARD
69+
#############################################################################
70+
-->
71+
72+
<!-- Enforce the use of tab indentation instead of spaces. -->
73+
<rule name="Generic.WhiteSpace.DisallowSpaceIndent"/>
2974
<rule ref="Generic.WhiteSpace.ScopeIndent">
3075
<properties>
3176
<property name="tabIndent" value="true"/>
3277
</properties>
3378
</rule>
3479

35-
<rule ref="ImportDetection" />
36-
<rule ref="VariableAnalysis"/>
80+
<!-- Disallow a blank line at the start of a class.
81+
This is a minor deviation from PHPCSDev to stay closer to PSR12. -->
82+
<rule ref="PSR12.Classes.OpeningBraceSpace.Found">
83+
<severity>5</severity>
84+
</rule>
85+
<rule ref="Squiz.WhiteSpace.FunctionSpacing">
86+
<properties>
87+
<property name="spacingBeforeFirst" value="0"/>
88+
</properties>
89+
</rule>
90+
<rule ref="Squiz.WhiteSpace.MemberVarSpacing">
91+
<properties>
92+
<property name="spacingBeforeFirst" value="0"/>
93+
</properties>
94+
</rule>
95+
96+
<!-- While (most of) the documentation sniffs included in PHPCSDev are disabled,
97+
do enforce some basic checking on the tags in function docblocks, like
98+
enforcing columnization of the information in @param tags. -->
99+
<rule ref="PEAR.Commenting.FunctionComment">
100+
<exclude name="PEAR.Commenting.FunctionComment.MissingParamTag"/>
101+
<exclude name="PEAR.Commenting.FunctionComment.MissingParamComment"/>
102+
<exclude name="PEAR.Commenting.FunctionComment.InvalidThrows"/>
103+
<exclude name="PEAR.Commenting.FunctionComment.MissingReturn"/>
104+
<exclude name="PEAR.Commenting.FunctionComment.Missing"/>
105+
</rule>
106+
37107
</ruleset>

0 commit comments

Comments
 (0)