Skip to content

Commit 2c4f3cc

Browse files
jrfnljrfnl
committed
Generic/[Upper|Lower]CaseConstant: ignore type declarations
This commit adjusts the `Generic.PHP.UpperCaseConstant` and the `Generic.PHP.LowerCaseConstant` sniffs to ignore property, parameter and return type declarations. Type declarations have their own rules and should not be treated the same as other `false`, `true` and `null` uses. Includes unit tests. Fixes 3332
1 parent e40edd8 commit 2c4f3cc

8 files changed

+298
-28
lines changed

‎src/Standards/Generic/Sniffs/PHP/LowerCaseConstantSniff.php

Lines changed: 100 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
use PHP_CodeSniffer\Files\File;
1313
use PHP_CodeSniffer\Sniffs\Sniff;
14+
use PHP_CodeSniffer\Util\Tokens;
1415

1516
class LowerCaseConstantSniff implements Sniff
1617
{
@@ -25,6 +26,17 @@ class LowerCaseConstantSniff implements Sniff
2526
'JS',
2627
];
2728

29+
/**
30+
* The tokens this sniff is targetting.
31+
*
32+
* @var array
33+
*/
34+
private $targets = [
35+
T_TRUE => T_TRUE,
36+
T_FALSE => T_FALSE,
37+
T_NULL => T_NULL,
38+
];
39+
2840

2941
/**
3042
* Returns an array of tokens this test wants to listen for.
@@ -33,11 +45,14 @@ class LowerCaseConstantSniff implements Sniff
3345
*/
3446
public function register()
3547
{
36-
return [
37-
T_TRUE,
38-
T_FALSE,
39-
T_NULL,
40-
];
48+
$targets = $this->targets;
49+
50+
// Register function keywords to filter out type declarations.
51+
$targets[] = T_FUNCTION;
52+
$targets[] = T_CLOSURE;
53+
$targets[] = T_FN;
54+
55+
return $targets;
4156

4257
}//end register()
4358

@@ -52,10 +67,89 @@ public function register()
5267
* @return void
5368
*/
5469
public function process(File $phpcsFile, $stackPtr)
70+
{
71+
$tokens = $phpcsFile->getTokens();
72+
73+
// Handle function declarations separately as they may contain the keywords in type declarations.
74+
if ($tokens[$stackPtr]['code'] === T_FUNCTION
75+
|| $tokens[$stackPtr]['code'] === T_CLOSURE
76+
|| $tokens[$stackPtr]['code'] === T_FN
77+
) {
78+
if (isset($tokens[$stackPtr]['parenthesis_closer']) === false) {
79+
return;
80+
}
81+
82+
$end = $tokens[$stackPtr]['parenthesis_closer'];
83+
if (isset($tokens[$stackPtr]['scope_opener']) === true) {
84+
$end = $tokens[$stackPtr]['scope_opener'];
85+
}
86+
87+
// Do a quick check if any of the targets exist in the declaration.
88+
$found = $phpcsFile->findNext($this->targets, $tokens[$stackPtr]['parenthesis_opener'], $end);
89+
if ($found === false) {
90+
// Skip forward, no need to examine these tokens again.
91+
return $end;
92+
}
93+
94+
// Handle the whole function declaration in one go.
95+
$params = $phpcsFile->getMethodParameters($stackPtr);
96+
foreach ($params as $param) {
97+
if (isset($param['default_token']) === false) {
98+
continue;
99+
}
100+
101+
$paramEnd = $param['comma_token'];
102+
if ($param['comma_token'] === false) {
103+
$paramEnd = $tokens[$stackPtr]['parenthesis_closer'];
104+
}
105+
106+
for ($i = $param['default_token']; $i < $paramEnd; $i++) {
107+
if (isset($this->targets[$tokens[$i]['code']]) === true) {
108+
$this->processConstant($phpcsFile, $i);
109+
}
110+
}
111+
}
112+
113+
// Skip over return type declarations.
114+
return $end;
115+
}//end if
116+
117+
// Handle property declarations separately as they may contain the keywords in type declarations.
118+
if (isset($tokens[$stackPtr]['conditions']) === true) {
119+
$conditions = $tokens[$stackPtr]['conditions'];
120+
$lastCondition = end($conditions);
121+
if (isset(Tokens::$ooScopeTokens[$lastCondition]) === true) {
122+
// This can only be an OO constant or property declaration as methods are handled above.
123+
$equals = $phpcsFile->findPrevious(T_EQUAL, ($stackPtr - 1), null, false, null, true);
124+
if ($equals !== false) {
125+
$this->processConstant($phpcsFile, $stackPtr);
126+
}
127+
128+
return;
129+
}
130+
}
131+
132+
// Handle everything else.
133+
$this->processConstant($phpcsFile, $stackPtr);
134+
135+
}//end process()
136+
137+
138+
/**
139+
* Processes a non-type declaration constant.
140+
*
141+
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
142+
* @param int $stackPtr The position of the current token in the
143+
* stack passed in $tokens.
144+
*
145+
* @return void
146+
*/
147+
protected function processConstant(File $phpcsFile, $stackPtr)
55148
{
56149
$tokens = $phpcsFile->getTokens();
57150
$keyword = $tokens[$stackPtr]['content'];
58151
$expected = strtolower($keyword);
152+
59153
if ($keyword !== $expected) {
60154
if ($keyword === strtoupper($keyword)) {
61155
$phpcsFile->recordMetric($stackPtr, 'PHP constant case', 'upper');
@@ -77,7 +171,7 @@ public function process(File $phpcsFile, $stackPtr)
77171
$phpcsFile->recordMetric($stackPtr, 'PHP constant case', 'lower');
78172
}
79173

80-
}//end process()
174+
}//end processConstant()
81175

82176

83177
}//end class

‎src/Standards/Generic/Sniffs/PHP/UpperCaseConstantSniff.php

Lines changed: 100 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,22 @@
1111

1212
use PHP_CodeSniffer\Files\File;
1313
use PHP_CodeSniffer\Sniffs\Sniff;
14+
use PHP_CodeSniffer\Util\Tokens;
1415

1516
class UpperCaseConstantSniff implements Sniff
1617
{
1718

19+
/**
20+
* The tokens this sniff is targetting.
21+
*
22+
* @var array
23+
*/
24+
private $targets = [
25+
T_TRUE => T_TRUE,
26+
T_FALSE => T_FALSE,
27+
T_NULL => T_NULL,
28+
];
29+
1830

1931
/**
2032
* Returns an array of tokens this test wants to listen for.
@@ -23,11 +35,14 @@ class UpperCaseConstantSniff implements Sniff
2335
*/
2436
public function register()
2537
{
26-
return [
27-
T_TRUE,
28-
T_FALSE,
29-
T_NULL,
30-
];
38+
$targets = $this->targets;
39+
40+
// Register function keywords to filter out type declarations.
41+
$targets[] = T_FUNCTION;
42+
$targets[] = T_CLOSURE;
43+
$targets[] = T_FN;
44+
45+
return $targets;
3146

3247
}//end register()
3348

@@ -42,10 +57,89 @@ public function register()
4257
* @return void
4358
*/
4459
public function process(File $phpcsFile, $stackPtr)
60+
{
61+
$tokens = $phpcsFile->getTokens();
62+
63+
// Handle function declarations separately as they may contain the keywords in type declarations.
64+
if ($tokens[$stackPtr]['code'] === T_FUNCTION
65+
|| $tokens[$stackPtr]['code'] === T_CLOSURE
66+
|| $tokens[$stackPtr]['code'] === T_FN
67+
) {
68+
if (isset($tokens[$stackPtr]['parenthesis_closer']) === false) {
69+
return;
70+
}
71+
72+
$end = $tokens[$stackPtr]['parenthesis_closer'];
73+
if (isset($tokens[$stackPtr]['scope_opener']) === true) {
74+
$end = $tokens[$stackPtr]['scope_opener'];
75+
}
76+
77+
// Do a quick check if any of the targets exist in the declaration.
78+
$found = $phpcsFile->findNext($this->targets, $tokens[$stackPtr]['parenthesis_opener'], $end);
79+
if ($found === false) {
80+
// Skip forward, no need to examine these tokens again.
81+
return $end;
82+
}
83+
84+
// Handle the whole function declaration in one go.
85+
$params = $phpcsFile->getMethodParameters($stackPtr);
86+
foreach ($params as $param) {
87+
if (isset($param['default_token']) === false) {
88+
continue;
89+
}
90+
91+
$paramEnd = $param['comma_token'];
92+
if ($param['comma_token'] === false) {
93+
$paramEnd = $tokens[$stackPtr]['parenthesis_closer'];
94+
}
95+
96+
for ($i = $param['default_token']; $i < $paramEnd; $i++) {
97+
if (isset($this->targets[$tokens[$i]['code']]) === true) {
98+
$this->processConstant($phpcsFile, $i);
99+
}
100+
}
101+
}
102+
103+
// Skip over return type declarations.
104+
return $end;
105+
}//end if
106+
107+
// Handle property declarations separately as they may contain the keywords in type declarations.
108+
if (isset($tokens[$stackPtr]['conditions']) === true) {
109+
$conditions = $tokens[$stackPtr]['conditions'];
110+
$lastCondition = end($conditions);
111+
if (isset(Tokens::$ooScopeTokens[$lastCondition]) === true) {
112+
// This can only be an OO constant or property declaration as methods are handled above.
113+
$equals = $phpcsFile->findPrevious(T_EQUAL, ($stackPtr - 1), null, false, null, true);
114+
if ($equals !== false) {
115+
$this->processConstant($phpcsFile, $stackPtr);
116+
}
117+
118+
return;
119+
}
120+
}
121+
122+
// Handle everything else.
123+
$this->processConstant($phpcsFile, $stackPtr);
124+
125+
}//end process()
126+
127+
128+
/**
129+
* Processes a non-type declaration constant.
130+
*
131+
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
132+
* @param int $stackPtr The position of the current token in the
133+
* stack passed in $tokens.
134+
*
135+
* @return void
136+
*/
137+
protected function processConstant(File $phpcsFile, $stackPtr)
45138
{
46139
$tokens = $phpcsFile->getTokens();
47140
$keyword = $tokens[$stackPtr]['content'];
48141
$expected = strtoupper($keyword);
142+
49143
if ($keyword !== $expected) {
50144
if ($keyword === strtolower($keyword)) {
51145
$phpcsFile->recordMetric($stackPtr, 'PHP constant case', 'lower');
@@ -67,7 +161,7 @@ public function process(File $phpcsFile, $stackPtr)
67161
$phpcsFile->recordMetric($stackPtr, 'PHP constant case', 'upper');
68162
}
69163

70-
}//end process()
164+
}//end processConstant()
71165

72166

73167
}//end class

‎src/Standards/Generic/Tests/PHP/LowerCaseConstantUnitTest.inc

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,3 +81,20 @@ var_dump(MyClass::TRUE);
8181
function tRUE() {}
8282

8383
$input->getFilterChain()->attachByName('Null', ['type' => Null::TYPE_STRING]);
84+
85+
// Issue #3332 - ignore type declarations, but not default values.
86+
class TypedThings {
87+
const MYCONST = FALSE;
88+
89+
public int|FALSE $int = FALSE;
90+
public Type|NULL $int = new MyObj(NULL);
91+
92+
private function typed(int|FALSE $param = NULL, Type|NULL $obj = new MyObj(FALSE)) : string|FALSE|NULL
93+
{
94+
if (TRUE === FALSE) {
95+
return NULL;
96+
}
97+
}
98+
}
99+
100+
$cl = function (int|FALSE $param = NULL, Type|NULL $obj = new MyObj(FALSE)) : string|FALSE|NULL {};

‎src/Standards/Generic/Tests/PHP/LowerCaseConstantUnitTest.inc.fixed

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,3 +81,20 @@ var_dump(MyClass::TRUE);
8181
function tRUE() {}
8282

8383
$input->getFilterChain()->attachByName('Null', ['type' => Null::TYPE_STRING]);
84+
85+
// Issue #3332 - ignore type declarations, but not default values.
86+
class TypedThings {
87+
const MYCONST = false;
88+
89+
public int|FALSE $int = false;
90+
public Type|NULL $int = new MyObj(null);
91+
92+
private function typed(int|FALSE $param = null, Type|NULL $obj = new MyObj(false)) : string|FALSE|NULL
93+
{
94+
if (true === false) {
95+
return null;
96+
}
97+
}
98+
}
99+
100+
$cl = function (int|FALSE $param = null, Type|NULL $obj = new MyObj(false)) : string|FALSE|NULL {};

‎src/Standards/Generic/Tests/PHP/LowerCaseConstantUnitTest.php

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,20 +30,27 @@ public function getErrorList($testFile='LowerCaseConstantUnitTest.inc')
3030
switch ($testFile) {
3131
case 'LowerCaseConstantUnitTest.inc':
3232
return [
33-
7 => 1,
34-
10 => 1,
35-
15 => 1,
36-
16 => 1,
37-
23 => 1,
38-
26 => 1,
39-
31 => 1,
40-
32 => 1,
41-
39 => 1,
42-
42 => 1,
43-
47 => 1,
44-
48 => 1,
45-
70 => 1,
46-
71 => 1,
33+
7 => 1,
34+
10 => 1,
35+
15 => 1,
36+
16 => 1,
37+
23 => 1,
38+
26 => 1,
39+
31 => 1,
40+
32 => 1,
41+
39 => 1,
42+
42 => 1,
43+
47 => 1,
44+
48 => 1,
45+
70 => 1,
46+
71 => 1,
47+
87 => 1,
48+
89 => 1,
49+
90 => 1,
50+
92 => 2,
51+
94 => 2,
52+
95 => 1,
53+
100 => 2,
4754
];
4855
break;
4956
case 'LowerCaseConstantUnitTest.js':

‎src/Standards/Generic/Tests/PHP/UpperCaseConstantUnitTest.inc

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,4 +78,21 @@ class MyClass
7878

7979
var_dump(MyClass::true);
8080

81-
function true() {}
81+
function true() {}
82+
83+
// Issue #3332 - ignore type declarations, but not default values.
84+
class TypedThings {
85+
const MYCONST = false;
86+
87+
public int|false $int = false;
88+
public Type|null $int = new MyObj(null);
89+
90+
private function typed(int|false $param = null, Type|null $obj = new MyObj(false)) : string|false|null
91+
{
92+
if (true === false) {
93+
return null;
94+
}
95+
}
96+
}
97+
98+
$cl = function (int|false $param = null, Type|null $obj = new MyObj(false)) : string|false|null {};

0 commit comments

Comments
 (0)