Skip to content

Commit

Permalink
Make assignment of a reference variable be considered a use (#128)
Browse files Browse the repository at this point in the history
* Add a test

* Mark reference variables are used when they are

* Fix coding standards
  • Loading branch information
alexpott authored Feb 11, 2020
1 parent 4f8e440 commit 984a0ae
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 0 deletions.
5 changes: 5 additions & 0 deletions VariableAnalysis/Lib/VariableInfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ class VariableInfo {
*/
public $passByReference = false;

/**
* @var bool
*/
public $isReference = false;

/**
* Stack pointer of first declaration
*
Expand Down
13 changes: 13 additions & 0 deletions VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -756,6 +756,19 @@ protected function checkForAssignment(File $phpcsFile, $stackPtr, $varName, $cur
// Plain ol' assignment. Simpl(ish).
$writtenPtr = Helpers::findWhereAssignExecuted($phpcsFile, $assignPtr);
$this->markVariableAssignment($varName, $writtenPtr, $currScope);

// Are we are reference variable?
$tokens = $tokens = $phpcsFile->getTokens();
$referencePtr = $phpcsFile->findNext(T_WHITESPACE, $assignPtr + 1, null, true, null, true);
$varInfo = $this->getOrCreateVariableInfo($varName, $currScope);
if (($referencePtr !== false) && ($tokens[$referencePtr]['code'] === T_BITWISE_AND)) {
$varInfo->isReference = true;
} elseif ($varInfo->isReference) {
// If this is an assigment to a reference variable then that variable is
// used.
$this->markVariableRead($varName, $stackPtr, $currScope);
}

return true;
}

Expand Down
12 changes: 12 additions & 0 deletions VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -936,4 +936,16 @@ public function testUnusedVarWithValueChange() {
];
$this->assertEquals($expectedWarnings, $lines);
}

public function testAssignmentByReference() {
$fixtureFile = $this->getFixture('AssignmentByReferenceFixture.php');
$phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
$phpcsFile->process();
$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
$expectedWarnings = [
26,
33,
];
$this->assertEquals($expectedWarnings, $lines);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php

class A {
protected $prop = [];

public function __construct() {
$this->prop[] = 'foo';
}

public function &getProp() {
return $this->prop;
}
}

function usedAssignmentByReference() {
$a = new A();

$var = &$a->getProp();
$var = ['bar'];
return $a;
}

function unusedAssignmentByReference() {
$a = new A();

$var = &$a->getProp();
return $a;
}

function doubleUnusedAssignmentByReference() {
$a = new A();

$var = &$a->getProp();
$var = &$a->getProp();
return $a;
}

function doubleUnusedThenUsedAssignmentByReference() {
$a = new A();

// @todo the first one should be marked as unused.
$var = &$a->getProp();
$var = &$a->getProp();
return $var;
}

0 comments on commit 984a0ae

Please sign in to comment.