Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added support for first-class callables #7113

Merged
merged 3 commits into from Dec 10, 2021
Merged

Added support for first-class callables #7113

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1f3a37f
Add support for first-class callables
trowski Dec 9, 2021
d5519fc
Use $stmt->args directly instead of creating new VariadicPlaceholder
trowski Dec 9, 2021
3c5e99e
Revert reorder of getting method return type and checking args
trowski Dec 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php
View file
Open in desktop
Expand Up @@ -227,7 +227,7 @@ public static function scrapeAssertions(
);
}

if ($conditional instanceof PhpParser\Node\Expr\FuncCall) {
if ($conditional instanceof PhpParser\Node\Expr\FuncCall && !$conditional->isFirstClassCallable()) {
return self::processFunctionCall(
$conditional,
$this_class_name,
Expand All @@ -237,8 +237,9 @@ public static function scrapeAssertions(
);
}

if ($conditional instanceof PhpParser\Node\Expr\MethodCall
|| $conditional instanceof PhpParser\Node\Expr\StaticCall
if (($conditional instanceof PhpParser\Node\Expr\MethodCall
|| $conditional instanceof PhpParser\Node\Expr\StaticCall)
&& !$conditional->isFirstClassCallable()
) {
$custom_assertions = self::processCustomAssertion($conditional, $this_class_name, $source);

Expand Down
76 changes: 62 additions & 14 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php
View file
Open in desktop
Expand Up @@ -20,6 +20,7 @@
use Psalm\Internal\MethodIdentifier;
use Psalm\Internal\Type\Comparator\CallableTypeComparator;
use Psalm\Internal\Type\TemplateResult;
use Psalm\Internal\Type\TypeCombiner;
use Psalm\Issue\DeprecatedFunction;
use Psalm\Issue\ImpureFunctionCall;
use Psalm\Issue\InvalidFunctionCall;
Expand Down Expand Up @@ -85,6 +86,7 @@ public static function analyze(
$real_stmt = $stmt;

if ($function_name instanceof PhpParser\Node\Name
&& !$stmt->isFirstClassCallable()
&& isset($stmt->getArgs()[0])
&& !$stmt->getArgs()[0]->unpack
) {
Expand Down Expand Up @@ -143,6 +145,7 @@ public static function analyze(
}
}

$is_first_class_callable = $stmt->isFirstClassCallable();
$set_inside_conditional = false;

if ($function_name instanceof PhpParser\Node\Name
Expand All @@ -153,22 +156,27 @@ public static function analyze(
$set_inside_conditional = true;
}

ArgumentsAnalyzer::analyze(
$statements_analyzer,
$stmt->getArgs(),
$function_call_info->function_params,
$function_call_info->function_id,
$function_call_info->allow_named_args,
$context
);
if (!$is_first_class_callable) {
ArgumentsAnalyzer::analyze(
$statements_analyzer,
$stmt->getArgs(),
$function_call_info->function_params,
$function_call_info->function_id,
$function_call_info->allow_named_args,
$context
);
}

if ($set_inside_conditional) {
$context->inside_conditional = false;
}

$function_callable = null;

if ($function_name instanceof PhpParser\Node\Name && $function_call_info->function_id) {
if (!$is_first_class_callable
&& $function_name instanceof PhpParser\Node\Name
&& $function_call_info->function_id
) {
if (!$function_call_info->is_stubbed && $function_call_info->in_call_map) {
$function_callable = InternalCallMapHandler::getCallableFromCallMapById(
$codebase,
Expand All @@ -184,7 +192,7 @@ public static function analyze(
$template_result = new TemplateResult([], []);

// do this here to allow closure param checks
if ($function_call_info->function_params !== null) {
if (!$is_first_class_callable && $function_call_info->function_params !== null) {
ArgumentsAnalyzer::checkArgumentsMatch(
$statements_analyzer,
$stmt->getArgs(),
Expand Down Expand Up @@ -235,6 +243,45 @@ public static function analyze(
);
Copy link
Collaborator

@orklah orklah Dec 9, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the AfterEveryFunctionCallAnalysisEvent is the plugin hook. I feel like this should stay at the bottom of the Analyzer. Or maybe it should not be called at all when dealing with first class callable, not sure

EDIT: well, just saw this is currently not at the bottom of the function, I may be wrong here


$config->eventDispatcher->dispatchAfterEveryFunctionCallAnalysis($event);

if ($is_first_class_callable) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you returning early here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure about this, but it seemed the code below was generally analyzing a call to the given function, which isn't actually happening. Some guidance on what still needs to be called from the code below would be appreciated.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you wouldn't go that far anyway, you have another if ($is_first_class_callable) just below that create the closure. I just found weird to return before that only when this condition if ($function_name instanceof PhpParser\Node\Name && $function_call_info->function_id) { is met and not otherwise

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The path taken by if ($function_name instanceof PhpParser\Node\Name && $function_call_info->function_id) invokes FunctionCallReturnTypeFetcher::fetch, which returns a closure for first-class callables from named calls (e.g. strlen(...) or var_dump(...)). The if-statement below creates a closure for a first-class callable created from another expression, such as $closure(...) (see the test labelled FirstClassCallable:FromClosure).

Perhaps there is a more elegant way of doing this, but that's why the separation exists here.

return true;
}
}

if ($is_first_class_callable) {
$type_provider = $statements_analyzer->getNodeTypeProvider();
$closure_types = [];

if ($input_type = $type_provider->getType($function_name)) {
foreach ($input_type->getAtomicTypes() as $atomic_type) {
$candidate_callable = CallableTypeComparator::getCallableFromAtomic(
$codebase,
$atomic_type,
null,
$statements_analyzer
);

if ($candidate_callable) {
$closure_types[] = new Type\Atomic\TClosure(
'Closure',
$candidate_callable->params,
$candidate_callable->return_type,
$candidate_callable->is_pure
);
}
}
}

if ($closure_types) {
$stmt_type = TypeCombiner::combine($closure_types, $codebase);
} else {
$stmt_type = Type::getClosure();
}

$statements_analyzer->node_data->setType($real_stmt, $stmt_type);

return true;
}

foreach ($function_call_info->defined_constants as $const_name => $const_type) {
Expand Down Expand Up @@ -457,13 +504,14 @@ private static function handleNamedFunction(
$function_call_info->function_params = null;
$function_call_info->defined_constants = [];
$function_call_info->global_variables = [];
$args = $stmt->isFirstClassCallable() ? [] : $stmt->getArgs();

if ($function_call_info->function_exists) {
if ($codebase->functions->params_provider->has($function_call_info->function_id)) {
$function_call_info->function_params = $codebase->functions->params_provider->getFunctionParams(
$statements_analyzer,
$function_call_info->function_id,
$stmt->getArgs(),
$args,
null,
$code_location
);
Expand Down Expand Up @@ -496,7 +544,7 @@ private static function handleNamedFunction(
$function_callable = InternalCallMapHandler::getCallableFromCallMapById(
$codebase,
$function_call_info->function_id,
$stmt->getArgs(),
$args,
$statements_analyzer->node_data
);

Expand Down Expand Up @@ -789,7 +837,7 @@ private static function analyzeInvokeCall(
$fake_method_call = new VirtualMethodCall(
$function_name,
new VirtualIdentifier('__invoke', $function_name->getAttributes()),
$stmt->getArgs()
$stmt->isFirstClassCallable() ? [new PhpParser\Node\VariadicPlaceholder()] : $stmt->getArgs()
trowski marked this conversation as resolved.
Show resolved Hide resolved
);

$suppressed_issues = $statements_analyzer->getSuppressedIssues();
Expand Down Expand Up @@ -948,7 +996,7 @@ private static function checkFunctionCallPurity(
$codebase,
$statements_analyzer->node_data,
$function_call_info->function_id,
$stmt->getArgs(),
$stmt->isFirstClassCallable() ? [] : $stmt->getArgs(),
$must_use
)
: null;
Expand Down
22 changes: 21 additions & 1 deletion src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallReturnTypeFetcher.php
View file
Open in desktop
Expand Up @@ -13,6 +13,7 @@
use Psalm\Internal\DataFlow\DataFlowNode;
use Psalm\Internal\DataFlow\TaintSource;
use Psalm\Internal\FileManipulation\FileManipulationBuffer;
use Psalm\Internal\Type\Comparator\CallableTypeComparator;
use Psalm\Internal\Type\TemplateBound;
use Psalm\Internal\Type\TemplateInferredTypeReplacer;
use Psalm\Internal\Type\TemplateResult;
Expand Down Expand Up @@ -58,7 +59,26 @@ public static function fetch(
$stmt_type = null;
$config = $codebase->config;

if ($codebase->functions->return_type_provider->has($function_id)) {
if ($stmt->isFirstClassCallable()) {
orklah marked this conversation as resolved.
Show resolved Hide resolved
$candidate_callable = CallableTypeComparator::getCallableFromAtomic(
$codebase,
new Type\Atomic\TLiteralString($function_id),
null,
$statements_analyzer,
true
);

if ($candidate_callable) {
$stmt_type = new Type\Union([new Type\Atomic\TClosure(
'Closure',
$candidate_callable->params,
$candidate_callable->return_type,
$candidate_callable->is_pure
)]);
} else {
$stmt_type = Type::getClosure();
}
} elseif ($codebase->functions->return_type_provider->has($function_id)) {
$stmt_type = $codebase->functions->return_type_provider->getReturnType(
$statements_analyzer,
$function_id,
Expand Down
2 changes: 1 addition & 1 deletion src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php
View file
Open in desktop
Expand Up @@ -193,7 +193,7 @@ public static function analyze(

$method_id = new MethodIdentifier($fq_class_name, $method_name_lc);

$args = $stmt->getArgs();
$args = $stmt->isFirstClassCallable() ? [] : $stmt->getArgs();

$naive_method_id = $method_id;

Expand Down
26 changes: 15 additions & 11 deletions .../Internal/Analyzer/Statements/Expression/Call/Method/ExistingAtomicMethodCallAnalyzer.php
View file
Open in desktop
Expand Up @@ -196,17 +196,6 @@ public static function analyze(
);
}

if (self::checkMethodArgs(
$method_id,
$args,
$template_result,
$context,
new CodeLocation($source, $stmt_name),
$statements_analyzer
) === false) {
return Type::getMixed();
}

$declaring_method_id = $codebase->methods->getDeclaringMethodId($method_id);

$return_type_candidate = MethodCallReturnTypeFetcher::fetch(
Expand All @@ -225,6 +214,21 @@ public static function analyze(
$template_result
);

if ($stmt->isFirstClassCallable()) {
return $return_type_candidate;
}

if (self::checkMethodArgs(
$method_id,
$args,
$template_result,
$context,
new CodeLocation($source, $stmt_name),
$statements_analyzer
) === false) {
return Type::getMixed();
}

$in_call_map = InternalCallMapHandler::inCallMap((string) ($declaring_method_id ?? $method_id));

if (!$in_call_map) {
Expand Down