Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix File::isReference() method #1609

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions package.xml
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ http://pear.php.net/dtd/package-2.0.xsd">
<file baseinstalldir="" name="FindImplementedInterfaceNamesTest.php" role="test" />
<file baseinstalldir="" name="GetMethodParametersTest.inc" role="test" />
<file baseinstalldir="" name="GetMethodParametersTest.php" role="test" />
<file baseinstalldir="" name="IsReference.inc" role="test" />
<file baseinstalldir="" name="IsReferenceTest.php" role="test" />
</dir>
<file baseinstalldir="" name="AllTests.php" role="test" />
<file baseinstalldir="" name="ErrorSuppressionTest.php" role="test" />
Expand Down Expand Up @@ -1541,6 +1543,8 @@ http://pear.php.net/dtd/package-2.0.xsd">
<install as="CodeSniffer/Core/File/FindImplementedInterfaceNamesTest.inc" name="tests/Core/File/FindImplementedInterfaceNamesTest.inc" />
<install as="CodeSniffer/Core/File/GetMethodParametersTest.php" name="tests/Core/File/GetMethodParametersTest.php" />
<install as="CodeSniffer/Core/File/GetMethodParametersTest.inc" name="tests/Core/File/GetMethodParametersTest.inc" />
<install as="CodeSniffer/Core/File/IsReferenceTest.php" name="tests/Core/File/IsReferenceTest.php" />
<install as="CodeSniffer/Core/File/IsReferenceTest.inc" name="tests/Core/File/IsReferenceTest.inc" />
<install as="CodeSniffer/Standards/AllSniffs.php" name="tests/Standards/AllSniffs.php" />
<install as="CodeSniffer/Standards/AbstractSniffUnitTest.php" name="tests/Standards/AbstractSniffUnitTest.php" />
</filelist>
Expand All @@ -1563,6 +1567,8 @@ http://pear.php.net/dtd/package-2.0.xsd">
<install as="CodeSniffer/Core/File/FindImplementedInterfaceNamesTest.inc" name="tests/Core/File/FindImplementedInterfaceNamesTest.inc" />
<install as="CodeSniffer/Core/File/GetMethodParametersTest.php" name="tests/Core/File/GetMethodParametersTest.php" />
<install as="CodeSniffer/Core/File/GetMethodParametersTest.inc" name="tests/Core/File/GetMethodParametersTest.inc" />
<install as="CodeSniffer/Core/File/IsReferenceTest.php" name="tests/Core/File/IsReferenceTest.php" />
<install as="CodeSniffer/Core/File/IsReferenceTest.inc" name="tests/Core/File/IsReferenceTest.inc" />
<install as="CodeSniffer/Standards/AllSniffs.php" name="tests/Standards/AllSniffs.php" />
<install as="CodeSniffer/Standards/AbstractSniffUnitTest.php" name="tests/Standards/AbstractSniffUnitTest.php" />
<ignore name="bin/phpcs.bat" />
Expand Down
87 changes: 64 additions & 23 deletions src/Files/File.php
Original file line number Diff line number Diff line change
Expand Up @@ -1158,6 +1158,7 @@ public function getDeclarationName($stackPtr)
* <code>
* 0 => array(
* 'name' => '$var', // The variable name.
* 'token' => integer, // The stack pointer to the variable name.
* 'content' => string, // The full content of the variable definition.
* 'pass_by_reference' => boolean, // Is the variable passed by reference?
* 'variable_length' => boolean, // Is the param of variable length through use of `...` ?
Expand Down Expand Up @@ -1218,7 +1219,9 @@ public function getMethodParameters($stackPtr)

switch ($this->tokens[$i]['code']) {
case T_BITWISE_AND:
$passByReference = true;
if ($defaultStart === null) {
$passByReference = true;
}
break;
case T_VARIABLE:
$currVar = $i;
Expand Down Expand Up @@ -1609,7 +1612,7 @@ public function isReference($stackPtr)
}

if ($this->tokens[$tokenBefore]['code'] === T_DOUBLE_ARROW) {
// Inside a foreach loop, this is a reference.
// Inside a foreach loop or array assignment, this is a reference.
return true;
}

Expand All @@ -1618,29 +1621,51 @@ public function isReference($stackPtr)
return true;
}

if ($this->tokens[$tokenBefore]['code'] === T_OPEN_SHORT_ARRAY) {
// Inside an array declaration, this is a reference.
return true;
}

if (isset(Util\Tokens::$assignmentTokens[$this->tokens[$tokenBefore]['code']]) === true) {
// This is directly after an assignment. It's a reference. Even if
// it is part of an operation, the other tests will handle it.
return true;
}

$tokenAfter = $this->findNext(
Util\Tokens::$emptyTokens,
($stackPtr + 1),
null,
true
);

if ($this->tokens[$tokenAfter]['code'] === T_NEW) {
return true;
}

if (isset($this->tokens[$stackPtr]['nested_parenthesis']) === true) {
$brackets = $this->tokens[$stackPtr]['nested_parenthesis'];
$lastBracket = array_pop($brackets);
if (isset($this->tokens[$lastBracket]['parenthesis_owner']) === true) {
$owner = $this->tokens[$this->tokens[$lastBracket]['parenthesis_owner']];
if ($owner['code'] === T_FUNCTION
|| $owner['code'] === T_CLOSURE
|| $owner['code'] === T_ARRAY
) {
// Inside a function or array declaration, this is a reference.
return true;
}
$params = $this->getMethodParameters($this->tokens[$lastBracket]['parenthesis_owner']);
foreach ($params as $param) {
$varToken = $tokenAfter;
if ($param['variable_length'] === true) {
$varToken = $this->findNext(
(Util\Tokens::$emptyTokens + array(T_ELLIPSIS)),
($stackPtr + 1),
null,
true
);
}

if ($param['token'] === $varToken
&& $param['pass_by_reference'] === true
) {
// Function parameter declared to be passed by reference.
return true;
}
}
}//end if
} else {
$prev = false;
for ($t = ($this->tokens[$lastBracket]['parenthesis_opener'] - 1); $t >= 0; $t--) {
Expand All @@ -1651,24 +1676,40 @@ public function isReference($stackPtr)
}

if ($prev !== false && $this->tokens[$prev]['code'] === T_USE) {
// Closure use by reference.
return true;
}
}//end if
}//end if

$tokenAfter = $this->findNext(
Util\Tokens::$emptyTokens,
($stackPtr + 1),
null,
true
);

if ($this->tokens[$tokenAfter]['code'] === T_VARIABLE
&& ($this->tokens[$tokenBefore]['code'] === T_OPEN_PARENTHESIS
|| $this->tokens[$tokenBefore]['code'] === T_COMMA)
// Pass by reference in function calls and assign by reference in arrays.
if ($this->tokens[$tokenBefore]['code'] === T_OPEN_PARENTHESIS
|| $this->tokens[$tokenBefore]['code'] === T_COMMA
|| $this->tokens[$tokenBefore]['code'] === T_OPEN_SHORT_ARRAY
) {
return true;
}
if ($this->tokens[$tokenAfter]['code'] === T_VARIABLE) {
return true;
} else {
$skip = Util\Tokens::$emptyTokens;
$skip[] = T_NS_SEPARATOR;
$skip[] = T_SELF;
$skip[] = T_PARENT;
$skip[] = T_STATIC;
$skip[] = T_STRING;
$skip[] = T_NAMESPACE;
$skip[] = T_DOUBLE_COLON;

$nextSignificantAfter = $this->findNext(
$skip,
($stackPtr + 1),
null,
true
);
if ($this->tokens[$nextSignificantAfter]['code'] === T_VARIABLE) {
return true;
}
}//end if
}//end if

return false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,3 +210,10 @@ $text = preg_replace_callback(

if (true || /* test */ -1 == $b) {}
$y = 10 + /* test */ -2;

// Issue #1604.
Hooks::run( 'ParserOptionsRegister', [
&self::$defaults,
&self::$inCacheKey,
&self::$lazyOptions,
] );
Original file line number Diff line number Diff line change
Expand Up @@ -204,3 +204,10 @@ $text = preg_replace_callback(

if (true || /* test */ -1 == $b) {}
$y = 10 + /* test */ -2;

// Issue #1604.
Hooks::run( 'ParserOptionsRegister', [
&self::$defaults,
&self::$inCacheKey,
&self::$lazyOptions,
] );
2 changes: 2 additions & 0 deletions tests/Core/AllTests.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
require_once 'File/GetMethodParametersTest.php';
require_once 'File/FindExtendedClassNameTest.php';
require_once 'File/FindImplementedInterfaceNamesTest.php';
require_once 'File/IsReferenceTest.php';

class AllTests
{
Expand Down Expand Up @@ -66,6 +67,7 @@ public static function suite()
$suite->addTestSuite('PHP_CodeSniffer\Tests\Core\File\GetMethodParametersTest');
$suite->addTestSuite('PHP_CodeSniffer\Tests\Core\File\FindExtendedClassNameTest');
$suite->addTestSuite('PHP_CodeSniffer\Tests\Core\File\FindImplementedInterfaceNamesTest');
$suite->addTestSuite('PHP_CodeSniffer\Tests\Core\File\IsReferenceTest');
return $suite;

}//end suite()
Expand Down
3 changes: 3 additions & 0 deletions tests/Core/File/GetMethodParametersTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,6 @@ class MyClass {

/* testNullableTypeHint */
function nullableTypeHint(?int $var1, ?\bar $var2) {}

/* testBitwiseAndConstantExpressionDefaultValue */
function myFunction($a = 10 & 20) {}
34 changes: 34 additions & 0 deletions tests/Core/File/GetMethodParametersTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -354,4 +354,38 @@ public function testDefaultValues()
}//end testDefaultValues()


/**
* Verify "bitwise and" in default value !== pass-by-reference.
*
* @return void
*/
public function testBitwiseAndConstantExpressionDefaultValue()
{
$expected = array();
$expected[0] = array(
'name' => '$a',
'content' => '$a = 10 & 20',
'default' => '10 & 20',
'pass_by_reference' => false,
'variable_length' => false,
'type_hint' => '',
'nullable_type' => false,
);

$start = ($this->phpcsFile->numTokens - 1);
$function = $this->phpcsFile->findPrevious(
T_COMMENT,
$start,
null,
false,
'/* testBitwiseAndConstantExpressionDefaultValue */'
);

$found = $this->phpcsFile->getMethodParameters(($function + 2));
unset($found[0]['token']);
$this->assertSame($expected, $found);

}//end testBitwiseAndConstantExpressionDefaultValue()


}//end class
139 changes: 139 additions & 0 deletions tests/Core/File/IsReferenceTest.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
<?php
/* @codingStandardsIgnoreFile */

namespace PHP_CodeSniffer\Tests\Core\File;

/* bitwiseAndA */
error_reporting( E_NOTICE & E_STRICT );

/* bitwiseAndB */
$a = [ $something & $somethingElse ];

/* bitwiseAndC */
$a = [ $first, $something & self::$somethingElse ];

/* bitwiseAndD */
$a = array $first, $something & $somethingElse );

/* bitwiseAndE */
$a = [ 'a' => $first, 'b' => $something & $somethingElse ];

/* bitwiseAndF */
$a = array( 'a' => $first, 'b' => $something & \MyClass::$somethingElse );

/* bitwiseAndG */
$a = $something & $somethingElse;

/* bitwiseAndH */
function myFunction($a = 10 & 20) {}

/* bitwiseAndI */
$closure = function ($a = MY_CONSTANT & parent::OTHER_CONSTANT) {};

/* functionReturnByReference */
function &myFunction() {}

/* functionPassByReferenceA */
function myFunction( &$a ) {}

/* functionPassByReferenceB */
function myFunction( $a, &$b ) {}

/* functionPassByReferenceC */
$closure = function ( &$a ) {};

/* functionPassByReferenceD */
$closure = function ( $a, &$b ) {};

/* functionPassByReferenceE */
function myFunction(array &$one) {}

/* functionPassByReferenceF */
$closure = function (\MyClass &$one) {};

/* functionPassByReferenceG */
$closure = function myFunc($param, &...$moreParams) {};

/* foreachValueByReference */
foreach( $array as $key => &$value ) {}

/* foreachKeyByReference */
foreach( $array as &$key => $value ) {}

/* arrayValueByReferenceA */
$a = [ 'a' => &$something ];

/* arrayValueByReferenceB */
$a = [ 'a' => $something, 'b' => &$somethingElse ];

/* arrayValueByReferenceC */
$a = [ &$something ];

/* arrayValueByReferenceD */
$a = [ $something, &$somethingElse ];

/* arrayValueByReferenceE */
$a = array( 'a' => &$something );

/* arrayValueByReferenceF */
$a = array( 'a' => $something, 'b' => &$somethingElse );

/* arrayValueByReferenceG */
$a = array( &$something );

/* arrayValueByReferenceH */
$a = array( $something, &$somethingElse );

/* assignByReferenceA */
$b = &$something;

/* assignByReferenceB */
$b =& $something;

/* assignByReferenceC */
$b .= &$something;

/* assignByReferenceD */
$myValue = &$obj->getValue();

/* assignByReferenceE */
$collection = &collector();

/* passByReferenceA */
functionCall(&$something, $somethingElse);

/* passByReferenceB */
functionCall($something, &$somethingElse);

/* passByReferenceC */
functionCall($something, &$this->somethingElse);

/* passByReferenceD */
functionCall($something, &self::$somethingElse);

/* passByReferenceE */
functionCall($something, &parent::$somethingElse);

/* passByReferenceF */
functionCall($something, &static::$somethingElse);

/* passByReferenceG */
functionCall($something, &SomeClass::$somethingElse);

/* passByReferenceH */
functionCall(&\SomeClass::$somethingElse);

/* passByReferenceI */
functionCall($something, &\SomeNS\SomeClass::$somethingElse);

/* passByReferenceJ */
functionCall($something, &namespace\SomeClass::$somethingElse);

/* newByReferenceA */
$foobar2 = &new Foobar();

/* newByReferenceB */
functionCall( $something , &new Foobar() );

/* useByReference */
$closure = function() use (&$var){};
Loading