From 7fa541e39bce2423e517bd60b861cf73de522db7 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Thu, 11 Jan 2018 23:18:13 -0500 Subject: [PATCH] Allow reference-returning functions to be passed as arguments of byref functions --- .../Statements/Expression/CallChecker.php | 22 +++++++++++++++---- .../Checker/Statements/ExpressionChecker.php | 1 + src/Psalm/Storage/FunctionLikeStorage.php | 5 +++++ src/Psalm/Type/Union.php | 7 ++++++ src/Psalm/Visitor/DependencyFinderVisitor.php | 8 +++++++ tests/ReferenceConstraintTest.php | 16 ++++++++++++++ 6 files changed, 55 insertions(+), 4 deletions(-) diff --git a/src/Psalm/Checker/Statements/Expression/CallChecker.php b/src/Psalm/Checker/Statements/Expression/CallChecker.php index f3924e13c..a43eecba4 100644 --- a/src/Psalm/Checker/Statements/Expression/CallChecker.php +++ b/src/Psalm/Checker/Statements/Expression/CallChecker.php @@ -1767,7 +1767,14 @@ class CallChecker $by_ref_type = $by_ref_type ? clone $by_ref_type : Type::getMixed(); } - if ($by_ref && $by_ref_type) { + if ($by_ref + && $by_ref_type + && !( + $arg->value instanceof PhpParser\Node\Expr\ConstFetch + || $arg->value instanceof PhpParser\Node\Expr\FuncCall + || $arg->value instanceof PhpParser\Node\Expr\MethodCall + ) + ) { // special handling for array sort if ($argument_offset === 0 && $method_id @@ -1991,9 +1998,16 @@ class CallChecker if ($arg->value instanceof PhpParser\Node\Scalar || $arg->value instanceof PhpParser\Node\Expr\Array_ || $arg->value instanceof PhpParser\Node\Expr\ClassConstFetch - || $arg->value instanceof PhpParser\Node\Expr\ConstFetch - || $arg->value instanceof PhpParser\Node\Expr\FuncCall - || $arg->value instanceof PhpParser\Node\Expr\MethodCall + || ( + ( + $arg->value instanceof PhpParser\Node\Expr\ConstFetch + || $arg->value instanceof PhpParser\Node\Expr\FuncCall + || $arg->value instanceof PhpParser\Node\Expr\MethodCall + ) && ( + !isset($arg->value->inferredType) + || !$arg->value->inferredType->by_ref + ) + ) ) { if (IssueBuffer::accepts( new InvalidPassByReference( diff --git a/src/Psalm/Checker/Statements/ExpressionChecker.php b/src/Psalm/Checker/Statements/ExpressionChecker.php index c203dfe73..e6d34960b 100644 --- a/src/Psalm/Checker/Statements/ExpressionChecker.php +++ b/src/Psalm/Checker/Statements/ExpressionChecker.php @@ -1850,6 +1850,7 @@ class ExpressionChecker $fleshed_out_type->from_docblock = $return_type->from_docblock; $fleshed_out_type->ignore_nullable_issues = $return_type->ignore_nullable_issues; + $fleshed_out_type->by_ref = $return_type->by_ref; return $fleshed_out_type; } diff --git a/src/Psalm/Storage/FunctionLikeStorage.php b/src/Psalm/Storage/FunctionLikeStorage.php index 9a3817e16..c90c60636 100644 --- a/src/Psalm/Storage/FunctionLikeStorage.php +++ b/src/Psalm/Storage/FunctionLikeStorage.php @@ -62,6 +62,11 @@ class FunctionLikeStorage */ public $variadic; + /** + * @var bool + */ + public $returns_by_ref; + /** * @var bool */ diff --git a/src/Psalm/Type/Union.php b/src/Psalm/Type/Union.php index a2462cd46..5ff74e525 100644 --- a/src/Psalm/Type/Union.php +++ b/src/Psalm/Type/Union.php @@ -46,6 +46,13 @@ class Union */ public $ignore_nullable_issues = false; + /** + * Whether or not the type was passed by reference + * + * @var bool + */ + public $by_ref = false; + /** @var ?string */ private $id; diff --git a/src/Psalm/Visitor/DependencyFinderVisitor.php b/src/Psalm/Visitor/DependencyFinderVisitor.php index 39bc76cde..8a4483ca7 100644 --- a/src/Psalm/Visitor/DependencyFinderVisitor.php +++ b/src/Psalm/Visitor/DependencyFinderVisitor.php @@ -742,6 +742,10 @@ class DependencyFinderVisitor extends PhpParser\NodeVisitorAbstract implements P CodeLocation::FUNCTION_RETURN_TYPE ); + if ($stmt->returnsByRef()) { + $storage->return_type->by_ref = true; + } + $storage->signature_return_type = $storage->return_type; $storage->signature_return_type_location = $storage->return_type_location; } @@ -902,6 +906,10 @@ class DependencyFinderVisitor extends PhpParser\NodeVisitorAbstract implements P $storage->return_type->ignore_nullable_issues = true; } + if ($storage->return_type && $stmt->returnsByRef()) { + $storage->return_type->by_ref = true; + } + if ($docblock_info->return_type_line_number) { $storage->return_type_location->setCommentLine($docblock_info->return_type_line_number); } diff --git a/tests/ReferenceConstraintTest.php b/tests/ReferenceConstraintTest.php index a087f9b1d..6a5fd287f 100644 --- a/tests/ReferenceConstraintTest.php +++ b/tests/ReferenceConstraintTest.php @@ -31,6 +31,22 @@ class ReferenceConstraintTest extends TestCase } }', ], + 'trackFunctionReturnRefs' => [ + 'foo; + } + } + + function useString(string &$s) : void {} + $a = new A(); + + useString($a->getString());', + ], ]; }