Skip to content

Commit 92047de

Browse files
authored
Improve check for static var defs inside functions (#80)
* Tests: Try to add test for #79 * Tests: Add failing test for static inside control * Improve check for static var defs inside functions When trying to determine if a static keyword is being used to define a static class property or a static variable, we were checking only to see if the enclosing block was a function. However, if the enclosing block is (for example) an if statement inside a function, then this check would fail when it should pass. This change instead checks for any enclosing function, so long as the enclosing function comes before the enclosing class (to prevent false positives when a function returns an anonymous class).
1 parent 879da2d commit 92047de

File tree

3 files changed

+60
-13
lines changed

3 files changed

+60
-13
lines changed

VariableAnalysis/Lib/Helpers.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,21 @@ public static function areAnyConditionsAClass(array $conditions) {
6060
return false;
6161
}
6262

63+
public static function areConditionsWithinFunctionBeforeClass(array $conditions) {
64+
// Return true if the token conditions are within a function before
65+
// they are within a class.
66+
$classTypes = [T_CLASS, T_ANON_CLASS, T_TRAIT];
67+
foreach (array_reverse($conditions, true) as $scopePtr => $scopeCode) {
68+
if (in_array($scopeCode, $classTypes)) {
69+
return false;
70+
}
71+
if ($scopeCode === T_FUNCTION) {
72+
return true;
73+
}
74+
}
75+
return false;
76+
}
77+
6378
public static function findPreviousFunctionPtr(File $phpcsFile, $openPtr) {
6479
// Function names are T_STRING, and return-by-reference is T_BITWISE_AND,
6580
// so we look backwards from the opening bracket for the first thing that

VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ protected function checkForClassProperty(File $phpcsFile, $stackPtr, $varName, $
367367
// assuming it's a property.
368368
$tokens = $phpcsFile->getTokens();
369369
$token = $tokens[$stackPtr];
370-
if ($token && !empty($token['conditions']) && end($token['conditions']) !== T_FUNCTION) {
370+
if ($token && !empty($token['conditions']) && !Helpers::areConditionsWithinFunctionBeforeClass($token['conditions'])) {
371371
return Helpers::areAnyConditionsAClass($token['conditions']);
372372
}
373373
return false;

VariableAnalysis/Tests/CodeAnalysis/fixtures/ClassWithMembersFixture.php

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,21 @@
22

33
class ClassWithoutMembers {
44
function method_without_param() {
5-
echo $var;
6-
echo "xxx $var xxx";
7-
echo "xxx {$var} xxx";
8-
echo "xxx $var $var2 xxx";
9-
echo "xxx {$var} {$var2} xxx";
10-
func($var);
11-
func(12, $var);
12-
func($var, 12);
13-
func(12, $var, 12);
5+
echo $var; // this should be a warning
6+
echo "xxx $var xxx"; // this should be a warning
7+
echo "xxx {$var} xxx"; // this should be a warning
8+
echo "xxx $var $var2 xxx"; // this should be a warning
9+
echo "xxx {$var} {$var2} xxx"; // this should be a warning
10+
func($var); // this should be a warning
11+
func(12, $var); // this should be a warning
12+
func($var, 12); // this should be a warning
13+
func(12, $var, 12); // this should be a warning
1414
$var = 'set the var';
1515
echo $var;
1616
echo "xxx $var xxx";
1717
echo "xxx {$var} xxx";
18-
echo "xxx $var $var2 xxx";
19-
echo "xxx {$var} {$var2} xxx";
18+
echo "xxx $var $var2 xxx"; // this should be a warning
19+
echo "xxx {$var} {$var2} xxx"; // this should be a warning
2020
func($var);
2121
func(12, $var);
2222
func($var, 12);
@@ -63,7 +63,7 @@ class ClassWithLateStaticBinding {
6363

6464
static function method_with_late_static_binding($param) {
6565
static::some_method($param);
66-
static::some_method($var); // should report a warning
66+
static::some_method($var); // should report a warning // this should be a warning
6767
static::some_method(static::CONSTANT, $param);
6868
$called_class = get_called_class();
6969
echo $called_class::$static_member_var;
@@ -76,3 +76,35 @@ function method_with_parent_reference() {
7676
echo parent::$no_such_static_member_var;
7777
}
7878
}
79+
80+
class ClassWithAssignedMembers {
81+
public $member_var = 'hello';
82+
private $private_member_var = 'hi';
83+
protected $protected_member_var = 'foo';
84+
static $static_member_var = 'bar';
85+
86+
function method_with_member_var() {
87+
echo $this->member_var;
88+
echo $this->no_such_member_var;
89+
echo self::$static_member_var;
90+
echo self::$no_such_static_member_var;
91+
echo SomeOtherClass::$external_static_member_var;
92+
}
93+
94+
function method_with_static_assigned_member_var() {
95+
static $new_var = null;
96+
if ($new_var === null) {
97+
echo 'it is null';
98+
}
99+
}
100+
101+
function method_with_static_assigned_var_inside_block() {
102+
$bool = true;
103+
if ($bool === true) {
104+
static $new_var = null;
105+
if ($new_var === null) {
106+
echo 'it is null';
107+
}
108+
}
109+
}
110+
}

0 commit comments

Comments
 (0)