Skip to content

Commit ba47d74

Browse files
jrfnlgsherwood
andcommitted
File::getMemberProperties(): removed parse error warning
The `File::getMemberProperties()` method used to be inconsistent in how it handled variable tokens which were not property declarations. * In most cases ("normal" variable, function parameter), it would throw a `RuntimeException` _"'$stackPtr is not a class member var'"_. * However, for "properties" declared in an `enum` or `interface` construct, it would register a warning about a possible parse error and return an empty array. This parse error warning has now been removed. As, as of PHP 8.4, declaring (hooked) properties in an interface is no longer a parse error (see the [Property Hooks RFC](https://wiki.php.net/rfc/property-hooks)), properties declared in interfaces will now be analyzed by the function, like any other property, and will return an array of information about the property. For "properties" declared in enums, which is still not allowed in PHP, the method will throw the `RuntimeException` _"'$stackPtr is not a class member var'"_. Includes updated unit tests for the `File::getMemberProperties()` method. Includes a review of all uses of the `File::getMemberProperties()` method in PHPCS native sniffs and updating the code where necessary. Includes adding/updating tests with properties in interfaces and enums for each of those sniffs. Closes squizlabs/PHP_CodeSniffer 2455 Also related to 734 Co-authored-by: Greg Sherwood <[email protected]>
1 parent 8a464d9 commit ba47d74

28 files changed

+174
-128
lines changed

src/Files/File.php

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1791,34 +1791,15 @@ public function getMemberProperties($stackPtr)
17911791
throw new RuntimeException('$stackPtr must be of type T_VARIABLE');
17921792
}
17931793

1794-
$conditions = array_keys($this->tokens[$stackPtr]['conditions']);
1794+
$conditions = $this->tokens[$stackPtr]['conditions'];
1795+
$conditions = array_keys($conditions);
17951796
$ptr = array_pop($conditions);
17961797
if (isset($this->tokens[$ptr]) === false
1797-
|| ($this->tokens[$ptr]['code'] !== T_CLASS
1798-
&& $this->tokens[$ptr]['code'] !== T_ANON_CLASS
1799-
&& $this->tokens[$ptr]['code'] !== T_TRAIT)
1798+
|| isset(Tokens::$ooScopeTokens[$this->tokens[$ptr]['code']]) === false
1799+
|| $this->tokens[$ptr]['code'] === T_ENUM
18001800
) {
1801-
if (isset($this->tokens[$ptr]) === true
1802-
&& ($this->tokens[$ptr]['code'] === T_INTERFACE
1803-
|| $this->tokens[$ptr]['code'] === T_ENUM)
1804-
) {
1805-
// T_VARIABLEs in interfaces/enums can actually be method arguments
1806-
// but they won't be seen as being inside the method because there
1807-
// are no scope openers and closers for abstract methods. If it is in
1808-
// parentheses, we can be pretty sure it is a method argument.
1809-
if (isset($this->tokens[$stackPtr]['nested_parenthesis']) === false
1810-
|| empty($this->tokens[$stackPtr]['nested_parenthesis']) === true
1811-
) {
1812-
$error = 'Possible parse error: %ss may not include member vars';
1813-
$code = sprintf('Internal.ParseError.%sHasMemberVar', ucfirst($this->tokens[$ptr]['content']));
1814-
$data = [strtolower($this->tokens[$ptr]['content'])];
1815-
$this->addWarning($error, $stackPtr, $code, $data);
1816-
return [];
1817-
}
1818-
} else {
1819-
throw new RuntimeException('$stackPtr is not a class member var');
1820-
}
1821-
}//end if
1801+
throw new RuntimeException('$stackPtr is not a class member var');
1802+
}
18221803

18231804
// Make sure it's not a method parameter.
18241805
if (empty($this->tokens[$stackPtr]['nested_parenthesis']) === false) {

src/Standards/Generic/Sniffs/PHP/LowerCaseTypeSniff.php

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -162,15 +162,10 @@ public function process(File $phpcsFile, $stackPtr)
162162
try {
163163
$props = $phpcsFile->getMemberProperties($i);
164164
} catch (RuntimeException $e) {
165-
// Not an OO property.
165+
// Parse error: property in enum. Ignore.
166166
continue;
167167
}
168168

169-
if (empty($props) === true) {
170-
// Parse error - property in interface or enum. Ignore.
171-
return;
172-
}
173-
174169
// Strip off potential nullable indication.
175170
$type = ltrim($props['type'], '?');
176171

src/Standards/Generic/Tests/PHP/LowerCaseTypeUnitTest.1.inc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,6 @@ class DNFTypes {
139139
function DNFReturnTypes ($var): object|(Self&\Package\Other_Class)|sTRINg|false {}
140140
}
141141

142-
// Intentional error, should be ignored by the sniff.
143-
interface PropertiesNotAllowed {
144-
public $notAllowed;
142+
interface PHP84HookedProperty {
143+
public String $readable { get; }
145144
}

src/Standards/Generic/Tests/PHP/LowerCaseTypeUnitTest.1.inc.fixed

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,6 @@ class DNFTypes {
139139
function DNFReturnTypes ($var): object|(self&\Package\Other_Class)|string|false {}
140140
}
141141

142-
// Intentional error, should be ignored by the sniff.
143-
interface PropertiesNotAllowed {
144-
public $notAllowed;
142+
interface PHP84HookedProperty {
143+
public string $readable { get; }
145144
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
// Intentional parse error, should be ignored by the sniff.
4+
// This should be the only test in this test case file.
5+
6+
enum PropertiesNotAllowed {
7+
public STRING $notAllowed;
8+
}

src/Standards/Generic/Tests/PHP/LowerCaseTypeUnitTest.php

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ public function getErrorList($testFile='')
9797
135 => 1,
9898
136 => 1,
9999
139 => 2,
100+
143 => 1,
100101
];
101102

102103
default:
@@ -112,20 +113,11 @@ public function getErrorList($testFile='')
112113
* The key of the array should represent the line number and the value
113114
* should represent the number of warnings that should occur on that line.
114115
*
115-
* @param string $testFile The name of the file being tested.
116-
*
117116
* @return array<int, int>
118117
*/
119-
public function getWarningList($testFile='')
118+
public function getWarningList()
120119
{
121-
switch ($testFile) {
122-
case 'LowerCaseTypeUnitTest.1.inc':
123-
// Warning from getMemberProperties() about parse error.
124-
return [144 => 1];
125-
126-
default:
127-
return [];
128-
}//end switch
120+
return [];
129121

130122
}//end getWarningList()
131123

src/Standards/PEAR/Sniffs/NamingConventions/ValidVariableNameSniff.php

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
namespace PHP_CodeSniffer\Standards\PEAR\Sniffs\NamingConventions;
1111

12+
use PHP_CodeSniffer\Exceptions\RuntimeException;
1213
use PHP_CodeSniffer\Files\File;
1314
use PHP_CodeSniffer\Sniffs\AbstractVariableSniff;
1415

@@ -27,13 +28,14 @@ class ValidVariableNameSniff extends AbstractVariableSniff
2728
*/
2829
protected function processMemberVar(File $phpcsFile, $stackPtr)
2930
{
30-
$tokens = $phpcsFile->getTokens();
31-
32-
$memberProps = $phpcsFile->getMemberProperties($stackPtr);
33-
if (empty($memberProps) === true) {
31+
try {
32+
$memberProps = $phpcsFile->getMemberProperties($stackPtr);
33+
} catch (RuntimeException $e) {
34+
// Parse error: property in enum. Ignore.
3435
return;
3536
}
3637

38+
$tokens = $phpcsFile->getTokens();
3739
$memberName = ltrim($tokens[$stackPtr]['content'], '$');
3840
$scope = $memberProps['scope'];
3941
$scopeSpecified = $memberProps['scope_specified'];

src/Standards/PEAR/Tests/NamingConventions/ValidVariableNameUnitTest.1.inc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,3 +99,8 @@ $util->setLogger(
9999
private $varName = 'hello';
100100
private $_varName = 'hello';
101101
});
102+
103+
interface PHP84HookedProperty {
104+
public $thisisfine { get; }
105+
public $_underscore { get; }
106+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
// Intentional parse error, should be ignored by the sniff.
4+
// This should be the only test in this test case file.
5+
6+
enum PropertiesNotAllowed {
7+
public $_notAllowed;
8+
}

src/Standards/PEAR/Tests/NamingConventions/ValidVariableNameUnitTest.php

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,14 @@ public function getErrorList($testFile='')
3535
switch ($testFile) {
3636
case 'ValidVariableNameUnitTest.1.inc':
3737
return [
38-
12 => 1,
39-
17 => 1,
40-
22 => 1,
41-
92 => 1,
42-
93 => 1,
43-
94 => 1,
44-
99 => 1,
38+
12 => 1,
39+
17 => 1,
40+
22 => 1,
41+
92 => 1,
42+
93 => 1,
43+
94 => 1,
44+
99 => 1,
45+
105 => 1,
4546
];
4647

4748
default:

0 commit comments

Comments
 (0)