Skip to content

Commit

Permalink
stub-generator: Set function stub return type when needed (#36883)
Browse files Browse the repository at this point in the history
If a function stub lacks any declared return type or phpdoc `@return`,
Phan will assume the return type is "void" and will therefore generate
issues like PhanTypeVoidAssignment if the return value is used.

To avoid this situation, document the return type as "mixed" if the
function has any non-empty `return` statement.
  • Loading branch information
anomiex authored Apr 15, 2024
1 parent 29f4dc1 commit 3c0ef60
Show file tree
Hide file tree
Showing 4 changed files with 176 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: added

Document function stub return type as "mixed" if it has a non-empty `return` but lacks any declaration or phpdoc, so Phan doesn't assume "void".
56 changes: 56 additions & 0 deletions projects/packages/stub-generator/src/PhpParser/StubNodeVisitor.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
PHAN;

use PhpParser\BuilderFactory;
use PhpParser\Comment\Doc as DocComment;
use PhpParser\Node;
use PhpParser\Node\Expr\BinaryOp\Concat as BinaryOp_Concat;
use PhpParser\Node\Expr\FuncCall;
Expand All @@ -28,8 +29,10 @@
use PhpParser\Node\Stmt\Interface_;
use PhpParser\Node\Stmt\Namespace_;
use PhpParser\Node\Stmt\Property;
use PhpParser\Node\Stmt\Return_;
use PhpParser\Node\Stmt\Trait_;
use PhpParser\NodeFinder;
use PhpParser\NodeTraverser;
use PhpParser\NodeVisitorAbstract;
use PhpParser\PrettyPrinter\Standard as PrettyPrinter_Standard;
use RuntimeException;
Expand Down Expand Up @@ -223,6 +226,7 @@ public function enterNode( Node $node ) {
if ( $this->defs['function'] === '*' || in_array( $node->namespacedName->toString(), $this->defs['function'], true ) ) {
// Ignore anything inside the function.
if ( $node->stmts ) {
$this->addFunctionReturnType( $node );
$this->mutateForFuncGetArgs( $node );
$node->stmts = array();
}
Expand Down Expand Up @@ -406,4 +410,56 @@ function ( Node $n ) {
$node->params[] = ( new BuilderFactory() )->param( 'func_get_args' )->makeVariadic()->getNode();
}
}

/**
* Add function return type.
*
* If a function stub has no declared return type and no phpdoc, Phan seems
* to assume "void". If there are any non-empty `return` statements in the
* function body, document it as "mixed" so Phan won't give bogus PhanTypeVoidAssignment
* and the like.
*
* This doesn't seem to apply to methods though. 🤷
*
* @param Function_ $node Node.
*/
private function addFunctionReturnType( Function_ $node ): void {
// First, see if the function already has a return type, either declared or phpdoc.
if ( $node->getReturnType() !== null ||
preg_match( '/@(phan-|phan-real-)?return /', (string) $node->getDocComment() )
) {
return;
}

$visitor = new class() extends NodeVisitorAbstract {
/**
* Whether a return was found.
*
* @var bool
*/
public $found = false;

// phpcs:ignore Squiz.Commenting.FunctionComment.Missing -- Inherited.
public function enterNode( Node $n ) {
if ( $n instanceof Return_ && $n->expr ) {
$this->found = true;
return self::STOP_TRAVERSAL;
}

if ( $n instanceof \PhpParser\Node\Expr\Closure || $n instanceof ClassMethod || $n instanceof Function_ ) {
return self::DONT_TRAVERSE_CHILDREN;
}

return null;
}
};
$traverser = new NodeTraverser( $visitor );
$traverser->traverse( $node->stmts );

if ( $visitor->found ) {
$docComment = $node->getDocComment() ? $node->getDocComment()->getText() : '/** */';
$docComment = rtrim( substr( $docComment, 0, -2 ), " \t" ) . "\n * @phan-return mixed Dummy doc for stub.\n */";
$node->setDocComment( new DocComment( $docComment ) );
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ function bar( string $p ): bool {
'*',
<<<'PHP'
namespace {
/**
* @phan-return mixed Dummy doc for stub.
*/
function foo()
{
}
Expand All @@ -107,6 +110,9 @@ function bar( string $p ): bool {
array( 'function' => '*' ),
<<<'PHP'
namespace {
/**
* @phan-return mixed Dummy doc for stub.
*/
function foo()
{
}
Expand Down Expand Up @@ -186,7 +192,9 @@ function bar( string $p ): bool {
array( 'function' => array( 'foo', 'Some\NS\bar' ) ),
<<<'PHP'
namespace {
/** Non-namespaced */
/** Non-namespaced
* @phan-return mixed Dummy doc for stub.
*/
function foo()
{
}
Expand Down Expand Up @@ -1501,6 +1509,112 @@ function uses_func_num_args(...$func_get_args)
}
PHP,
),
'Function return type inference' => array(
<<<'PHP'
namespace X;
function no_return() {
}
function empty_return() {
return;
}
function has_return() {
if ( foo() ) {
return;
} else {
return 42;
}
}
function return_only_in_subfunctions() {
function xxx() {
return 42;
}
class Huh {
public function xxx() {
return 42;
}
}
$x = function () {
return 42;
};
$x = new class() {
function xxx() {
return 42;
}
};
}
function has_return_and_decl(): array {
return array();
}
/** @return array */
function has_return_and_phpdoc() {
return array();
}
/** @phan-return array */
function has_return_and_phan_phpdoc() {
return array();
}
/** @phan-real-return array */
function has_return_and_phan_phpdoc_real() {
return array();
}
class Foo {
function has_return() {
return 42;
}
}
PHP,
'*',
<<<'PHP'
namespace X;
function no_return()
{
}
function empty_return()
{
}
/**
* @phan-return mixed Dummy doc for stub.
*/
function has_return()
{
}
function return_only_in_subfunctions()
{
}
function has_return_and_decl(): array
{
}
/** @return array */
function has_return_and_phpdoc()
{
}
/** @phan-return array */
function has_return_and_phan_phpdoc()
{
}
/** @phan-real-return array */
function has_return_and_phan_phpdoc_real()
{
}
class Foo
{
function has_return()
{
}
}
PHP,
),
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,5 @@ function another_function() {

// This function has no docs.
function undocumented_function( $args ) {
return $args;
var_dump( $args );
}

0 comments on commit 3c0ef60

Please sign in to comment.