Skip to content

Commit 1080e1f

Browse files
committed
Correctly handle $ob->$variable style symbolic references.
1 parent bfad537 commit 1080e1f

File tree

3 files changed

+86
-20
lines changed

3 files changed

+86
-20
lines changed

Sniffs/CodeAnalysis/VariableAnalysisSniff.php

Lines changed: 54 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,18 @@ function isVariableUndefined($varName, $stackPtr, $currScope) {
513513
return true;
514514
}
515515

516+
function markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $stackPtr, $currScope) {
517+
$this->markVariableRead($varName, $stackPtr, $currScope);
518+
519+
if ($this->isVariableUndefined($varName, $stackPtr, $currScope) === true) {
520+
// We haven't been defined by this point.
521+
$phpcsFile->addWarning("Variable %s is undefined.", $stackPtr,
522+
'UndefinedVariable',
523+
array("\${$varName}"));
524+
}
525+
return true;
526+
}
527+
516528
function findFunctionPrototype(
517529
PHP_CodeSniffer_File $phpcsFile,
518530
$stackPtr
@@ -748,7 +760,7 @@ protected function checkForFunctionPrototype(
748760
}
749761
return false;
750762
}
751-
763+
752764
protected function checkForCatchBlock(
753765
PHP_CodeSniffer_File $phpcsFile,
754766
$stackPtr,
@@ -783,7 +795,7 @@ protected function checkForCatchBlock(
783795
}
784796
return false;
785797
}
786-
798+
787799
protected function checkForThisWithinClass(
788800
PHP_CodeSniffer_File $phpcsFile,
789801
$stackPtr,
@@ -1052,7 +1064,7 @@ protected function checkForPassByReferenceFunctionCall(
10521064
}
10531065

10541066
$refArgs = $this->_passByRefFunctions[$functionName];
1055-
1067+
10561068
if (($argPtrs = $this->findFunctionCallArguments($phpcsFile, $stackPtr)) === false) {
10571069
return false;
10581070
}
@@ -1096,6 +1108,28 @@ protected function checkForPassByReferenceFunctionCall(
10961108
return true;
10971109
}
10981110

1111+
protected function checkForSymbolicObjectProperty(
1112+
PHP_CodeSniffer_File $phpcsFile,
1113+
$stackPtr,
1114+
$varName,
1115+
$currScope
1116+
) {
1117+
$tokens = $phpcsFile->getTokens();
1118+
$token = $tokens[$stackPtr];
1119+
1120+
// Are we a symbolic object property/function derefeference?
1121+
// Search backwards for first token that isn't whitespace, is it a "->" operator?
1122+
$objectOperatorPtr = $phpcsFile->findPrevious(
1123+
T_WHITESPACE,
1124+
$stackPtr - 1, null, true, null, true);
1125+
if (($objectOperatorPtr === false) || ($tokens[$objectOperatorPtr]['code'] !== T_OBJECT_OPERATOR)) {
1126+
return false;
1127+
}
1128+
1129+
$this->markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $stackPtr, $currScope);
1130+
return true;
1131+
}
1132+
10991133
/**
11001134
* Called to process class member vars.
11011135
*
@@ -1135,13 +1169,20 @@ protected function processVariable(
11351169
return;
11361170
}
11371171

1138-
//if ($varName == 'param') {
1139-
//echo "Found variable {$varName} on line {$token['line']} in scope {$currScope}.\n";// . print_r($token, true);
1172+
//static $dump_token = false;
1173+
//if ($varName == 'property') {
1174+
// $dump_token = true;
1175+
//}
1176+
//if ($dump_token) {
1177+
// echo "Found variable {$varName} on line {$token['line']} in scope {$currScope}.\n" . print_r($token, true);
1178+
// echo "Prev:\n" . print_r($tokens[$stackPtr - 1], true);
11401179
//}
1141-
//echo "Prev:\n" . print_r($tokens[$stackPtr - 1], true);
11421180

11431181
// Determine if variable is being assigned or read.
11441182

1183+
// Read methods that preempt assignment:
1184+
// Are we a $object->$property type symbolic reference?
1185+
11451186
// Possible assignment methods:
11461187
// Is a mandatory function/closure parameter
11471188
// Is an optional function/closure parameter with non-null value
@@ -1157,6 +1198,11 @@ protected function processVariable(
11571198
// Assignment via foreach (... as ...) { }
11581199
// Pass-by-reference to known pass-by-reference function
11591200

1201+
// Are we a $object->$property type symbolic reference?
1202+
if ($this->checkForSymbolicObjectProperty($phpcsFile, $stackPtr, $varName, $currScope)) {
1203+
return;
1204+
}
1205+
11601206
// Are we a function or closure parameter?
11611207
if ($this->checkForFunctionPrototype($phpcsFile, $stackPtr, $varName, $currScope)) {
11621208
return;
@@ -1212,15 +1258,8 @@ protected function processVariable(
12121258
return;
12131259
}
12141260

1215-
$this->markVariableRead($varName, $stackPtr, $currScope);
1216-
12171261
// OK, we don't appear to be a write to the var, assume we're a read.
1218-
if ($this->isVariableUndefined($varName, $stackPtr, $currScope) === true) {
1219-
// We haven't been defined by this point.
1220-
$phpcsFile->addWarning("Variable %s is undefined.", $stackPtr,
1221-
'UndefinedVariable',
1222-
array("\${$varName}"));
1223-
}
1262+
$this->markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $stackPtr, $currScope);
12241263
}
12251264

12261265
/**
@@ -1259,12 +1298,7 @@ protected function processVariableInString(
12591298
if ($this->checkForSuperGlobal($phpcsFile, $stackPtr, $varName, $currScope)) {
12601299
continue;
12611300
}
1262-
$this->markVariableRead($varName, $stackPtr, $currScope);
1263-
if ($this->isVariableUndefined($varName, $stackPtr, $currScope) === true) {
1264-
$phpcsFile->addWarning("Variable %s is undefined.", $stackPtr,
1265-
'UndefinedVariable',
1266-
array("\${$varName}"));
1267-
}
1301+
$this->markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $stackPtr, $currScope);
12681302
}
12691303
}
12701304

Tests/CodeAnalysis/VariableAnalysisUnitTest.inc

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,3 +392,27 @@ ${var2}
392392
\\$var2
393393
END_OF_TEXT;
394394
}
395+
396+
class ClassWithSymbolicRefProperty {
397+
public $my_property;
398+
399+
function method_with_symbolic_ref_property() {
400+
$properties = array('my_property');
401+
foreach ($properties as $property) {
402+
$this->$property = 'some value';
403+
$this -> $property = 'some value';
404+
$this->$undefined_property = 'some value';
405+
$this -> $undefined_property = 'some value';
406+
}
407+
}
408+
409+
function method_with_symbolic_ref_method() {
410+
$methods = array('method_with_symbolic_ref_property');
411+
foreach ($methods as $method) {
412+
$this->$method();
413+
$this -> $method();
414+
$this->$undefined_method();
415+
$this -> $undefined_method();
416+
}
417+
}
418+
}

Tests/CodeAnalysis/VariableAnalysisUnitTest.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,14 @@ private function _getWarningAndErrorList() {
207207
($base + 7) => 1, // {$var2}
208208
($base + 8) => 1, // ${var2}
209209
($base + 10) => 1, // \\$var2
210+
// method_with_symbolic_ref_property() line (+17)
211+
($base += 17) => 0,
212+
($base + 5) => 1, // $undefined_property
213+
($base + 6) => 1, // $undefined_property
214+
// method_with_symbolic_ref_method() line (+8)
215+
($base += 10) => 0,
216+
($base + 5) => 1, // $undefined_method
217+
($base + 6) => 1, // $undefined_method
210218
);
211219
}//end _getWarningAndErrorList()
212220

0 commit comments

Comments
 (0)