Add support for checking closure return types

config.xsd

@@ -86,6 +86,7 @@
             <xs:element name="InvalidClass" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="InvalidParamDefault" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="InvalidDocblock" type="IssueHandlerType" minOccurs="0" />
+            <xs:element name="InvalidFunctionCall" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="InvalidGlobal" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="InvalidIterator" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="InvalidNamespace" type="IssueHandlerType" minOccurs="0" />

src/Psalm/Checker/FunctionLikeChecker.php

@@ -335,7 +335,7 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
             $this->function->inferredType = new Type\Union([
                 new Type\Fn(
                     'Closure',
-                    array_values($function_param_names),
+                    $function_params,
                     $closure_return_type ?: Type::getMixed()
                 )
             ]);
@@ -1070,10 +1070,9 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
     /**
      * @param  string                           $method_id
      * @param  array<int, PhpParser\Node\Arg>   $args
-     * @param  string                           $file_path
+     * @return array<int, FunctionLikeParameter>
-     * @return array<int,FunctionLikeParameter>
      */
-    public static function getParamsById($method_id, array $args, $file_path)
+    public static function getMethodParamsById($method_id, array $args)
     {
         $fq_class_name = strpos($method_id, '::') !== false ? explode('::', $method_id)[0] : null;
 
@@ -1085,13 +1084,8 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
             }
 
             return $method_params;
-        } elseif (!$fq_class_name && FunctionChecker::inCallMap($method_id)) {
-            $function_param_options = FunctionChecker::getParamsFromCallMap($method_id);
-
-            if ($function_param_options === null) {
-                throw new \UnexpectedValueException('Not expecting $function_param_options to be null');
         }
-        } elseif ($fq_class_name) {
+
         $declaring_method_id = MethodChecker::getDeclaringMethodId($method_id);
 
         if (FunctionChecker::inCallMap($declaring_method_id ?: $method_id)) {
@@ -1100,16 +1094,46 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo
             if ($function_param_options === null) {
                 throw new \UnexpectedValueException('Not expecting $function_param_options to be null');
             }
-            } elseif ($method_params = MethodChecker::getMethodParams($method_id)) {
+
+            return self::getMatchingParamsFromCallMapOptions($function_param_options, $args);
+        }
+
+        if ($method_params = MethodChecker::getMethodParams($method_id)) {
             // fall back to using reflected params anyway
             return $method_params;
-            } else {
+        }
+
         throw new \InvalidArgumentException('Cannot get params for ' . $method_id);
     }
-        } else {
+
+    /**
+     * @param  string                           $method_id
+     * @param  array<int, PhpParser\Node\Arg>   $args
+     * @param  string                           $file_path
+     * @return array<int, FunctionLikeParameter>
+     */
+    public static function getFunctionParamsById($method_id, array $args, $file_path)
+    {
+        if (FunctionChecker::inCallMap($method_id)) {
+            $function_param_options = FunctionChecker::getParamsFromCallMap($method_id);
+
+            if ($function_param_options === null) {
+                throw new \UnexpectedValueException('Not expecting $function_param_options to be null');
+            }
+
+            return self::getMatchingParamsFromCallMapOptions($function_param_options, $args);
+        }
+
         return FunctionChecker::getParams(strtolower($method_id), $file_path);
     }
 
+     /**
+     * @param  array<int, array<int, FunctionLikeParameter>>  $function_param_options
+     * @param  array<int, PhpParser\Node\Arg>                 $args
+     * @return array<int, FunctionLikeParameter>
+     */
+    protected static function getMatchingParamsFromCallMapOptions(array $function_param_options, array $args)
+    {
         $function_params = null;
 
         if (count($function_param_options) === 1) {

src/Psalm/Checker/Statements/Expression/CallChecker.php

@@ -13,9 +13,11 @@ use Psalm\Checker\TraitChecker;
 use Psalm\Checker\TypeChecker;
 use Psalm\CodeLocation;
 use Psalm\Context;
+use Psalm\FunctionLikeParameter;
 use Psalm\Issue\ForbiddenCode;
 use Psalm\Issue\ImplicitToStringCast;
 use Psalm\Issue\InvalidArgument;
+use Psalm\Issue\InvalidFunctionCall;
 use Psalm\Issue\InvalidScalarArgument;
 use Psalm\Issue\InvalidScope;
 use Psalm\Issue\MixedArgument;
@@ -94,10 +96,58 @@ class CallChecker
         $method_id = null;
 
         if ($context->check_functions) {
-            if (!($stmt->name instanceof PhpParser\Node\Name)) {
+            $in_call_map = false;
-                return null;
+
+            $function_params = null;
+
+            $code_location = new CodeLocation($statements_checker->getSource(), $stmt);
+
+            if ($stmt->name instanceof PhpParser\Node\Expr) {
+                if (ExpressionChecker::check($statements_checker, $stmt->name, $context) === false) {
+                    return false;
                 }
 
+                if (isset($stmt->name->inferredType)) {
+                    foreach ($stmt->name->inferredType->types as $var_type_part) {
+                        if ($var_type_part instanceof Type\Fn) {
+                            $function_params = $var_type_part->parameters;
+
+                            if ($var_type_part->return_type) {
+                                if (isset($stmt->inferredType)) {
+                                    $stmt->inferredType = Type::combineUnionTypes(
+                                        $stmt->inferredType,
+                                        $var_type_part->return_type
+                                    );
+                                } else {
+                                    $stmt->inferredType = $var_type_part->return_type;
+                                }
+                            }
+                        } elseif (!$var_type_part->isMixed() && $var_type_part->value !== 'Closure') {
+                            $var_id = ExpressionChecker::getVarId(
+                                $stmt->name,
+                                $statements_checker->getFQCLN(),
+                                $statements_checker->getNamespace(),
+                                $statements_checker->getAliasedClasses()
+                            );
+
+                            if (IssueBuffer::accepts(
+                                new InvalidFunctionCall(
+                                    'Cannot treat ' . $var_id . ' of type ' . $var_type_part . ' as function',
+                                    new CodeLocation($statements_checker->getSource(), $stmt)
+                                ),
+                                $statements_checker->getSuppressedIssues()
+                            )) {
+                                return false;
+                            }
+                        }
+                    }
+                }
+
+                if (!isset($stmt->inferredType)) {
+                    $stmt->inferredType = Type::getMixed();
+                }
+
+            } else {
                 $method_id = implode('\\', $stmt->name->parts);
 
                 $aliased_functions = $statements_checker->getAliasedFunctions();
@@ -112,33 +162,48 @@ class CallChecker
 
                 $in_call_map = FunctionChecker::inCallMap($method_id);
 
-            $code_location = new CodeLocation($statements_checker->getSource(), $stmt);
-
                 if (!$in_call_map &&
                     self::checkFunctionExists($statements_checker, $method_id, $context, $code_location) === false
                 ) {
                     return false;
                 }
 
+                $function_params = FunctionLikeChecker::getFunctionParamsById(
+                    $method_id,
+                    $stmt->args,
+                    $statements_checker->getFilePath()
+                );
+            }
+
             if (self::checkFunctionArguments(
                 $statements_checker,
                 $stmt->args,
-                $method_id,
+                $function_params,
                 $context
             ) === false) {
                 // fall through
             }
 
+            if ($stmt->name instanceof PhpParser\Node\Name && $method_id) {
+                $function_params = FunctionLikeChecker::getFunctionParamsById(
+                    $method_id,
+                    $stmt->args,
+                    $statements_checker->getFilePath()
+                );
+            }
+
             if (self::checkFunctionArgumentsMatch(
                 $statements_checker,
                 $stmt->args,
                 $method_id,
+                $function_params,
                 $context,
                 $code_location
             ) === false) {
                 // fall through
             }
 
+            if ($method_id) {
                 if ($in_call_map) {
                     $stmt->inferredType = FunctionChecker::getReturnTypeFromCallMapWithArgs(
                         $method_id,
@@ -158,6 +223,7 @@ class CallChecker
                     }
                 }
             }
+        }
 
         if ($stmt->name instanceof PhpParser\Node\Name &&
             ($stmt->name->parts === ['get_class'] || $stmt->name->parts === ['gettype']) &&
@@ -236,10 +302,12 @@ class CallChecker
             if (MethodChecker::methodExists($fq_class_name . '::__construct')) {
                 $method_id = $fq_class_name . '::__construct';
 
+                $method_params = FunctionLikeChecker::getMethodParamsById($method_id, $stmt->args);
+
                 if (self::checkFunctionArguments(
                     $statements_checker,
                     $stmt->args,
-                    $method_id,
+                    $method_params,
                     $context
                 ) === false) {
                     return false;
@@ -249,6 +317,7 @@ class CallChecker
                     $statements_checker,
                     $stmt->args,
                     $method_id,
+                    $method_params,
                     $context,
                     new CodeLocation($statements_checker->getSource(), $stmt)
                 ) === false) {
@@ -527,10 +596,12 @@ class CallChecker
             $stmt->inferredType = $return_type;
         }
 
+        $method_params = $method_id ? FunctionLikeChecker::getMethodParamsById($method_id, $stmt->args) : [];
+
         if (self::checkFunctionArguments(
             $statements_checker,
             $stmt->args,
-            $method_id,
+            $method_params,
             $context
         ) === false) {
             return false;
@@ -540,6 +611,7 @@ class CallChecker
             $statements_checker,
             $stmt->args,
             $method_id,
+            $method_params,
             $context,
             new CodeLocation($statements_checker->getSource(), $stmt),
             $has_mock
@@ -660,6 +732,8 @@ class CallChecker
 
             $has_mock = $has_mock || $is_mock;
 
+            $method_id = null;
+
             if (is_string($stmt->name) &&
                 !MethodChecker::methodExists($fq_class_name . '::__callStatic') &&
                 !$is_mock
@@ -732,10 +806,12 @@ class CallChecker
                 }
             }
 
+            $method_params = $method_id ? FunctionLikeChecker::getMethodParamsById($method_id, $stmt->args) : [];
+
             if (self::checkFunctionArguments(
                 $statements_checker,
                 $stmt->args,
-                $method_id,
+                $method_params,
                 $context
             ) === false) {
                 return false;
@@ -745,6 +821,7 @@ class CallChecker
                 $statements_checker,
                 $stmt->args,
                 $method_id,
+                $method_params,
                 $context,
                 new CodeLocation($statements_checker->getSource(), $stmt),
                 $has_mock
@@ -759,31 +836,19 @@ class CallChecker
     /**
      * @param   StatementsChecker                       $statements_checker
      * @param   array<int, PhpParser\Node\Arg>          $args
-     * @param   string|null                     $method_id
+     * @param   array<int,FunctionLikeParameter>|null   $function_params
      * @param   Context                                 $context
      * @return  false|null
      */
     protected static function checkFunctionArguments(
         StatementsChecker $statements_checker,
         array $args,
-        $method_id,
+        array $function_params = null,
         Context $context
     ) {
-        $function_params = null;
-
-        $in_call_map = $method_id ? FunctionChecker::inCallMap($method_id) : false;
-
-        if ($method_id) {
-            $function_params = FunctionLikeChecker::getParamsById(
-                $method_id,
-                $args,
-                $statements_checker->getFilePath()
-            );
-        }
-
         foreach ($args as $argument_offset => $arg) {
             if ($arg->value instanceof PhpParser\Node\Expr\PropertyFetch) {
-                if ($method_id) {
+                if ($function_params !== null) {
                     $by_ref = false;
                     $by_ref_type = null;
 
@@ -821,7 +886,7 @@ class CallChecker
                     }
                 }
             } elseif ($arg->value instanceof PhpParser\Node\Expr\Variable) {
-                if ($method_id) {
+                if ($function_params !== null) {
                     $by_ref = false;
                     $by_ref_type = null;
 
@@ -868,6 +933,7 @@ class CallChecker
      * @param   StatementsChecker                       $statements_checker
      * @param   array<int, PhpParser\Node\Arg>          $args
      * @param   string|null                             $method_id
+     * @param   array<int,FunctionLikeParameter>|null   $function_params
      * @param   Context                                 $context
      * @param   CodeLocation                            $code_location
      * @param   boolean                                 $is_mock
@@ -877,15 +943,11 @@ class CallChecker
         StatementsChecker $statements_checker,
         array $args,
         $method_id,
+        array $function_params = null,
         Context $context,
         CodeLocation $code_location,
         $is_mock = false
     ) {
-        // we need to do this calculation after the above vars have already processed
-        $function_params = $method_id
-            ? FunctionLikeChecker::getParamsById($method_id, $args, $statements_checker->getFilePath())
-            : [];
-
         $in_call_map = $method_id ? FunctionChecker::inCallMap($method_id) : false;
 
         $cased_method_id = $method_id;
@@ -920,7 +982,7 @@ class CallChecker
         }
 
         foreach ($args as $argument_offset => $arg) {
-            if ($method_id && $cased_method_id && isset($arg->value->inferredType)) {
+            if ($function_params !== null && $cased_method_id && isset($arg->value->inferredType)) {
                 if (count($function_params) > $argument_offset) {
                     $param_type = $function_params[$argument_offset]->type;
 
@@ -948,7 +1010,77 @@ class CallChecker
             }
         }
 
-        if ($method_id === 'array_map' || $method_id === 'array_filter') {
+        if ($function_params !== null && ($method_id === 'array_map' || $method_id === 'array_filter')) {
+            if (self::checkArrayFunctionArgumentsMatch(
+                $statements_checker,
+                $args,
+                $method_id,
+                $function_params,
+                $context,
+                $code_location
+            ) === false
+            ) {
+                return false;
+            }
+        }
+
+        if ($function_params !== null) {
+            if (!$is_variadic
+                && count($args) > count($function_params)
+                && (!count($function_params) || $function_params[count($function_params) - 1]->name !== '...=')
+            ) {
+                if (IssueBuffer::accepts(
+                    new TooManyArguments(
+                        'Too many arguments for method ' . ($cased_method_id ?: $method_id),
+                        $code_location
+                    ),
+                    $statements_checker->getSuppressedIssues()
+                )) {
+                    return false;
+                }
+
+                return null;
+            }
+
+            if (!$has_packed_var && count($args) < count($function_params)) {
+                for ($i = count($args); $i < count($function_params); $i++) {
+                    $param = $function_params[$i];
+
+                    if (!$param->is_optional && !$param->is_variadic) {
+                        if (IssueBuffer::accepts(
+                            new TooFewArguments(
+                                'Too few arguments for method ' . $cased_method_id,
+                                $code_location
+                            ),
+                            $statements_checker->getSuppressedIssues()
+                        )) {
+                            return false;
+                        }
+
+                        break;
+                    }
+                }
+            }
+        }
+    }
+
+    /**
+     * @param   StatementsChecker                       $statements_checker
+     * @param   array<int, PhpParser\Node\Arg>          $args
+     * @param   string|null                             $method_id
+     * @param   array<int,FunctionLikeParameter>        $function_params
+     * @param   Context                                 $context
+     * @param   CodeLocation                            $code_location
+     * @return  false|null
+     */
+    protected static function checkArrayFunctionArgumentsMatch(
+        StatementsChecker $statements_checker,
+        array $args,
+        $method_id,
+        array $function_params,
+        Context $context,
+        CodeLocation $code_location
+    ) {
         $closure_index = $method_id === 'array_map' ? 0 : 1;
 
         $array_arg_types = [];
@@ -991,7 +1123,7 @@ class CallChecker
                 if (count($closure_type->parameters) > $expected_closure_param_count) {
                     if (IssueBuffer::accepts(
                         new TooManyArguments(
-                                'Too many arguments in closure for ' . ($cased_method_id ?: $method_id),
+                            'Too many arguments in closure for ' . $method_id,
                             new CodeLocation($statements_checker->getSource(), $closure_arg)
                         ),
                         $statements_checker->getSuppressedIssues()
@@ -1001,7 +1133,7 @@ class CallChecker
                 } elseif (count($closure_type->parameters) < $expected_closure_param_count) {
                     if (IssueBuffer::accepts(
                         new TooFewArguments(
-                                'You must supply a param in the closure for ' . ($cased_method_id ?: $method_id),
+                            'You must supply a param in the closure for ' . $method_id,
                             new CodeLocation($statements_checker->getSource(), $closure_arg)
                         ),
                         $statements_checker->getSuppressedIssues()
@@ -1014,7 +1146,7 @@ class CallChecker
                 $closure_params = $closure_type->parameters;
                 $closure_return_type = $closure_type->return_type;
 
-                    foreach ($closure_params as $i => $closure_param_type) {
+                foreach ($closure_params as $i => $closure_param) {
                     if (!$array_arg_types[$i]) {
                         continue;
                     }
@@ -1028,6 +1160,8 @@ class CallChecker
                         continue;
                     }
 
+                    $closure_param_type = $closure_param->type;
+
                     $type_match_found = TypeChecker::isContainedBy(
                         $input_type,
                         $closure_param_type,
@@ -1039,7 +1173,7 @@ class CallChecker
                     if ($coerced_type) {
                         if (IssueBuffer::accepts(
                             new TypeCoercion(
-                                    'First parameter of closure passed to function ' . $cased_method_id . ' expects ' .
+                                'First parameter of closure passed to function ' . $method_id . ' expects ' .
                                     $closure_param_type . ', parent type ' . $input_type . ' provided',
                                 new CodeLocation($statements_checker->getSource(), $closure_arg)
                             ),
@@ -1053,7 +1187,7 @@ class CallChecker
                         if ($scalar_type_match_found) {
                             if (IssueBuffer::accepts(
                                 new InvalidScalarArgument(
-                                        'First parameter of closure passed to function ' . $cased_method_id . ' expects ' .
+                                    'First parameter of closure passed to function ' . $method_id . ' expects ' .
                                         $closure_param_type . ', ' . $input_type . ' provided',
                                     new CodeLocation($statements_checker->getSource(), $closure_arg)
                                 ),
@@ -1063,7 +1197,7 @@ class CallChecker
                             }
                         } elseif (IssueBuffer::accepts(
                             new InvalidArgument(
-                                    'First parameter of closure passed to function ' . $cased_method_id . ' expects ' .
+                                'First parameter of closure passed to function ' . $method_id . ' expects ' .
                                     $closure_param_type . ', ' . $input_type . ' provided',
                                 new CodeLocation($statements_checker->getSource(), $closure_arg)
                             ),
@@ -1077,46 +1211,6 @@ class CallChecker
         }
     }
 
-        if ($method_id) {
-            if (!$is_variadic
-                && count($args) > count($function_params)
-                && (!count($function_params) || $function_params[count($function_params) - 1]->name !== '...=')
-            ) {
-                if (IssueBuffer::accepts(
-                    new TooManyArguments(
-                        'Too many arguments for method ' . ($cased_method_id ?: $method_id),
-                        $code_location
-                    ),
-                    $statements_checker->getSuppressedIssues()
-                )) {
-                    return false;
-                }
-
-                return null;
-            }
-
-            if (!$has_packed_var && count($args) < count($function_params)) {
-                for ($i = count($args); $i < count($function_params); $i++) {
-                    $param = $function_params[$i];
-
-                    if (!$param->is_optional && !$param->is_variadic) {
-                        if (IssueBuffer::accepts(
-                            new TooFewArguments(
-                                'Too few arguments for method ' . $cased_method_id,
-                                $code_location
-                            ),
-                            $statements_checker->getSuppressedIssues()
-                        )) {
-                            return false;
-                        }
-
-                        break;
-                    }
-                }
-            }
-        }
-    }
-
     /**
      * @param   StatementsChecker   $statements_checker
      * @param   Type\Union          $input_type

src/Psalm/Issue/InvalidFunctionCall.php

@@ -0,0 +1,6 @@
+<?php
+namespace Psalm\Issue;
+
+class InvalidFunctionCall extends CodeIssue
+{
+}

src/Psalm/Type/Fn.php

@@ -1,6 +1,8 @@
 <?php
 namespace Psalm\Type;
 
+use Psalm\FunctionLikeParameter;
+
 class Fn extends Atomic
 {
     /**
@@ -9,7 +11,7 @@ class Fn extends Atomic
     public $value = 'Closure';
 
     /**
-     * @var array<int, Union>
+     * @var array<int, FunctionLikeParameter>
      */
     public $parameters = [];
 
@@ -22,7 +24,7 @@ class Fn extends Atomic
      * Constructs a new instance of a generic type
      *
      * @param string                            $value
-     * @param array<int, Union> $parameters
+     * @param array<int, FunctionLikeParameter> $parameters
      * @param Union                             $return_type
      */
     public function __construct($value, array $parameters, Union $return_type)

tests/ClosureTest.php

@@ -121,4 +121,38 @@ class ClosureTest extends PHPUnit_Framework_TestCase
         $context = new Context('somefile.php');
         $file_checker->check(true, true, $context);
     }
+
+    public function testVarReturnType()
+    {
+        $stmts = self::$parser->parse('<?php
+
+        $add_one = function(int $a) : int {
+            return $a + 1;
+        };
+
+        $a = $add_one(1);
+        ');
+
+        $file_checker = new FileChecker('somefile.php', $stmts);
+        $context = new Context('somefile.php');
+        $file_checker->check(true, true, $context);
+        $this->assertEquals('int', (string) $context->vars_in_scope['$a']);
+    }
+
+    /**
+     * @expectedException \Psalm\Exception\CodeException
+     * @expectedExceptionMessage InvalidFunctionCall
+     */
+    public function testStringFunctionCall()
+    {
+        $stmts = self::$parser->parse('<?php
+        $bad_one = "hello";
+        $a = $bad_one(1);
+        ');
+
+        $file_checker = new FileChecker('somefile.php', $stmts);
+        $context = new Context('somefile.php');
+        $file_checker->check(true, true, $context);
+        $this->assertEquals('int', (string) $context->vars_in_scope['$a']);
+    }
 }