Check closure/function/method returning nullsafe by ref

Author: ondrejmirtes

conf/config.level0.neon

@@ -37,6 +37,7 @@ rules:
 	- PHPStan\Rules\Functions\ExistingClassesInTypehintsRule
 	- PHPStan\Rules\Functions\InnerFunctionRule
 	- PHPStan\Rules\Functions\PrintfParametersRule
+	- PHPStan\Rules\Functions\ReturnNullsafeByRefRule
 	- PHPStan\Rules\Methods\AbstractMethodInNonAbstractClassRule
 	- PHPStan\Rules\Methods\ExistingClassesInTypehintsRule
 	- PHPStan\Rules\Methods\MissingMethodImplementationRule

src/Node/ClosureReturnStatementsNode.php

@@ -6,7 +6,7 @@
 use PhpParser\NodeAbstract;
 use PHPStan\Analyser\StatementResult;
 
-class ClosureReturnStatementsNode extends NodeAbstract implements VirtualNode
+class ClosureReturnStatementsNode extends NodeAbstract implements ReturnStatementsNode
 {
 
 	private \PhpParser\Node\Expr\Closure $closureExpr;
@@ -51,6 +51,11 @@ public function getStatementResult(): StatementResult
 		return $this->statementResult;
 	}
 
+	public function returnsByRef(): bool
+	{
+		return $this->closureExpr->byRef;
+	}
+
 	public function getType(): string
 	{
 		return 'PHPStan_Node_ClosureReturnStatementsNode';

src/Node/FunctionReturnStatementsNode.php

@@ -6,9 +6,11 @@
 use PhpParser\NodeAbstract;
 use PHPStan\Analyser\StatementResult;
 
-class FunctionReturnStatementsNode extends NodeAbstract implements VirtualNode
+class FunctionReturnStatementsNode extends NodeAbstract implements ReturnStatementsNode
 {
 
+	private Function_ $function;
+
 	/** @var \PHPStan\Node\ReturnStatement[] */
 	private array $returnStatements;
 
@@ -26,6 +28,7 @@ public function __construct(
 	)
 	{
 		parent::__construct($function->getAttributes());
+		$this->function = $function;
 		$this->returnStatements = $returnStatements;
 		$this->statementResult = $statementResult;
 	}
@@ -43,6 +46,11 @@ public function getStatementResult(): StatementResult
 		return $this->statementResult;
 	}
 
+	public function returnsByRef(): bool
+	{
+		return $this->function->byRef;
+	}
+
 	public function getType(): string
 	{
 		return 'PHPStan_Node_FunctionReturnStatementsNode';

src/Node/MethodReturnStatementsNode.php

@@ -6,9 +6,11 @@
 use PhpParser\NodeAbstract;
 use PHPStan\Analyser\StatementResult;
 
-class MethodReturnStatementsNode extends NodeAbstract implements VirtualNode
+class MethodReturnStatementsNode extends NodeAbstract implements ReturnStatementsNode
 {
 
+	private ClassMethod $classMethod;
+
 	/** @var \PHPStan\Node\ReturnStatement[] */
 	private array $returnStatements;
 
@@ -26,6 +28,7 @@ public function __construct(
 	)
 	{
 		parent::__construct($method->getAttributes());
+		$this->classMethod = $method;
 		$this->returnStatements = $returnStatements;
 		$this->statementResult = $statementResult;
 	}
@@ -43,6 +46,11 @@ public function getStatementResult(): StatementResult
 		return $this->statementResult;
 	}
 
+	public function returnsByRef(): bool
+	{
+		return $this->classMethod->byRef;
+	}
+
 	public function getType(): string
 	{
 		return 'PHPStan_Node_FunctionReturnStatementsNode';

src/Node/ReturnStatementsNode.php

@@ -0,0 +1,19 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Node;
+
+use PHPStan\Analyser\StatementResult;
+
+interface ReturnStatementsNode extends VirtualNode
+{
+
+	/**
+	 * @return \PHPStan\Node\ReturnStatement[]
+	 */
+	public function getReturnStatements(): array;
+
+	public function getStatementResult(): StatementResult;
+
+	public function returnsByRef(): bool;
+
+}

src/Rules/Functions/ReturnNullsafeByRefRule.php

@@ -0,0 +1,53 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Functions;
+
+use PhpParser\Node;
+use PHPStan\Analyser\Scope;
+use PHPStan\Node\ReturnStatementsNode;
+use PHPStan\Rules\NullsafeCheck;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+
+/**
+ * @implements Rule<ReturnStatementsNode>
+ */
+class ReturnNullsafeByRefRule implements Rule
+{
+
+	private NullsafeCheck $nullsafeCheck;
+
+	public function __construct(NullsafeCheck $nullsafeCheck)
+	{
+		$this->nullsafeCheck = $nullsafeCheck;
+	}
+
+	public function getNodeType(): string
+	{
+		return ReturnStatementsNode::class;
+	}
+
+	public function processNode(Node $node, Scope $scope): array
+	{
+		if (!$node->returnsByRef()) {
+			return [];
+		}
+
+		$errors = [];
+		foreach ($node->getReturnStatements() as $returnStatement) {
+			$returnNode = $returnStatement->getReturnNode();
+			if ($returnNode->expr === null) {
+				continue;
+			}
+
+			if (!$this->nullsafeCheck->containsNullSafe($returnNode->expr)) {
+				continue;
+			}
+
+			$errors[] = RuleErrorBuilder::message('Nullsafe cannot be returned by reference.')->line($returnNode->getLine())->nonIgnorable()->build();
+		}
+
+		return $errors;
+	}
+
+}

tests/PHPStan/Rules/Functions/ReturnNullsafeByRefRuleTest.php

@@ -0,0 +1,42 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Functions;
+
+use PHPStan\Rules\NullsafeCheck;
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+
+/**
+ * @extends RuleTestCase<ReturnNullsafeByRefRule>
+ */
+class ReturnNullsafeByRefRuleTest extends RuleTestCase
+{
+
+	protected function getRule(): Rule
+	{
+		return new ReturnNullsafeByRefRule(new NullsafeCheck());
+	}
+
+	public function testRule(): void
+	{
+		if (!self::$useStaticReflectionProvider) {
+			$this->markTestSkipped('Test requires static reflection.');
+		}
+
+		$this->analyse([__DIR__ . '/data/return-null-safe-by-ref.php'], [
+			[
+				'Nullsafe cannot be returned by reference.',
+				15,
+			],
+			[
+				'Nullsafe cannot be returned by reference.',
+				25,
+			],
+			[
+				'Nullsafe cannot be returned by reference.',
+				36,
+			],
+		]);
+	}
+
+}

tests/PHPStan/Rules/Functions/data/return-null-safe-by-ref.php

@@ -0,0 +1,37 @@
+<?php
+
+namespace ReturnNullsafeByRef;
+
+class Foo
+{
+
+	public function doFoo(?\stdClass $foo)
+	{
+		function &() use ($foo) {
+			if (rand(0, 1)) {
+				return $foo;
+			}
+
+			return $foo?->bar->foo;
+		};
+	}
+
+	public function &doBar()
+	{
+		if (rand(0, 1)) {
+			return $foo;
+		}
+
+		return $foo?->bar->foo;
+	}
+
+}
+
+function &foo(): void
+{
+	if (rand(0, 1)) {
+		return $foo;
+	}
+
+	return $foo?->bar->foo;
+}