UselessVariableSniff: Fixed false positive

Author: kukulich

SlevomatCodingStandard/Sniffs/Variables/UselessVariableSniff.php

@@ -15,6 +15,7 @@
 use function preg_quote;
 use function sprintf;
 use const T_AND_EQUAL;
+use const T_BITWISE_AND;
 use const T_CONCAT_EQUAL;
 use const T_DIV_EQUAL;
 use const T_DO;
@@ -78,7 +79,13 @@ public function process(File $phpcsFile, $returnPointer): void
 
 		$variableName = $tokens[$variablePointer]['content'];
 
-		if ($this->isStaticVariable($phpcsFile, $variablePointer, $variableName)) {
+		$functionPointer = $this->findFunctionPointer($phpcsFile, $variablePointer);
+
+		if ($this->isReturnedByReference($phpcsFile, $functionPointer)) {
+			return;
+		}
+
+		if ($this->isStaticVariable($phpcsFile, $functionPointer, $variablePointer, $variableName)) {
 			return;
 		}
 
@@ -118,7 +125,10 @@ public function process(File $phpcsFile, $returnPointer): void
 			return;
 		}
 
-		if (!in_array($tokens[$pointerBeforePreviousVariable]['code'], [T_SEMICOLON, T_OPEN_CURLY_BRACKET], true)) {
+		if (
+			!in_array($tokens[$pointerBeforePreviousVariable]['code'], [T_SEMICOLON, T_OPEN_CURLY_BRACKET], true)
+			&& TokenHelper::findNextEffective($phpcsFile, $returnSemicolonPointer + 1) !== null
+		) {
 			$phpcsFile->addError(...$errorParameters);
 			return;
 		}
@@ -223,22 +233,27 @@ private function isAssigmentToVariable(File $phpcsFile, int $pointer): bool
 		], true);
 	}
 
-	private function isStaticVariable(File $phpcsFile, int $variablePointer, string $variableName): bool
+	private function findFunctionPointer(File $phpcsFile, int $pointer): ?int
 	{
 		$tokens = $phpcsFile->getTokens();
 
-		$functionPointer = null;
-		foreach (array_reverse($tokens[$variablePointer]['conditions'], true) as $conditionPointer => $conditionTokenCode) {
+		foreach (array_reverse($tokens[$pointer]['conditions'], true) as $conditionPointer => $conditionTokenCode) {
 			if (in_array($conditionTokenCode, TokenHelper::$functionTokenCodes, true)) {
-				$functionPointer = $conditionPointer;
-				break;
+				return $conditionPointer;
 			}
 		}
 
+		return null;
+	}
+
+	private function isStaticVariable(File $phpcsFile, ?int $functionPointer, int $variablePointer, string $variableName): bool
+	{
 		if ($functionPointer === null) {
 			return false;
 		}
 
+		$tokens = $phpcsFile->getTokens();
+
 		for ($i = $tokens[$functionPointer]['scope_opener'] + 1; $i < $variablePointer; $i++) {
 			if ($tokens[$i]['code'] !== T_VARIABLE) {
 				continue;
@@ -256,6 +271,18 @@ private function isStaticVariable(File $phpcsFile, int $variablePointer, string
 		return false;
 	}
 
+	private function isReturnedByReference(File $phpcsFile, ?int $functionPointer): bool
+	{
+		if ($functionPointer === null) {
+			return false;
+		}
+
+		$tokens = $phpcsFile->getTokens();
+
+		$referencePointer = TokenHelper::findNextEffective($phpcsFile, $functionPointer + 1);
+		return $tokens[$referencePointer]['code'] === T_BITWISE_AND;
+	}
+
 	private function hasVariableVarAnnotation(File $phpcsFile, int $variablePointer): bool
 	{
 		$tokens = $phpcsFile->getTokens();

tests/Sniffs/Variables/UselessVariableSniffTest.php

@@ -17,7 +17,7 @@ public function testErrors(): void
 	{
 		$report = self::checkFile(__DIR__ . '/data/uselessVariableErrors.php');
 
-		self::assertSame(17, $report->getErrorCount());
+		self::assertSame(18, $report->getErrorCount());
 
 		self::assertSniffError($report, 4, UselessVariableSniff::CODE_USELESS_VARIABLE, 'Useless variable $a.');
 		self::assertSniffError($report, 9, UselessVariableSniff::CODE_USELESS_VARIABLE, 'Useless variable $b.');
@@ -36,6 +36,7 @@ public function testErrors(): void
 		self::assertSniffError($report, 78, UselessVariableSniff::CODE_USELESS_VARIABLE, 'Useless variable $o.');
 		self::assertSniffError($report, 84, UselessVariableSniff::CODE_USELESS_VARIABLE, 'Useless variable $p.');
 		self::assertSniffError($report, 89, UselessVariableSniff::CODE_USELESS_VARIABLE, 'Useless variable $q.');
+		self::assertSniffError($report, 93, UselessVariableSniff::CODE_USELESS_VARIABLE, 'Useless variable $z.');
 
 		self::assertAllFixedInFile($report);
 	}

tests/Sniffs/Variables/data/uselessVariableErrors.fixed.php

@@ -72,3 +72,5 @@ function assigmentAfterAssignment() {
 	doSomething($qq = $q = 0);
 	return $q;
 }
+
+return null;

tests/Sniffs/Variables/data/uselessVariableErrors.php

@@ -89,3 +89,6 @@ function assigmentAfterAssignment() {
 	doSomething($qq = $q = 0);
 	return $q;
 }
+
+$z = null;
+return $z;

tests/Sniffs/Variables/data/uselessVariableNoErrors.php

@@ -148,3 +148,9 @@ public static function popBar() : array
 	}
 
 }
+
+function &returnsReference($result)
+{
+	$value = $result->getValue();
+	return $value;
+}