Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/Analyser/ArgumentsHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -654,13 +654,18 @@ public function processArgs(
}
$hasYield = $hasYield || $exprResult->hasYield();

if ($exprType->isCallable()->yes()) {
// only callable objects (closures) carry expressions to invalidate - asking
// isCallable() of other arguments reflects the classes named by callable-like
// strings and arrays, so it is skipped when nothing would come of it
$invalidateCallbackExpressions = $this->shouldInvalidateCallbackExpressions($parameter) && !$exprType->isObject()->no();
$callCallbackImmediately = $this->callCallbackImmediately($parameter, $parameterType, $calleeReflection);
if (($invalidateCallbackExpressions || $callCallbackImmediately) && $exprType->isCallable()->yes()) {
$acceptors = $exprType->getCallableParametersAcceptors($scope);
if (count($acceptors) === 1) {
if ($this->shouldInvalidateCallbackExpressions($parameter)) {
if ($invalidateCallbackExpressions) {
$deferredInvalidateExpressions[] = [$acceptors[0]->getInvalidateExpressions(), $acceptors[0]->getUsedVariables()];
}
if ($this->callCallbackImmediately($parameter, $parameterType, $calleeReflection)) {
if ($callCallbackImmediately) {
$callableThrowPoints = array_map(static fn (SimpleThrowPoint $throwPoint) => $throwPoint->isExplicit() ? InternalThrowPoint::createExplicit($scope, $throwPoint->getType(), $arg->value, $throwPoint->canContainAnyThrowable(), $throwPoint->isFromThrowExpr()) : InternalThrowPoint::createImplicit($scope, $arg->value), $acceptors[0]->getThrowPoints());
if (!$this->implicitThrows) {
$callableThrowPoints = array_values(array_filter($callableThrowPoints, static fn (InternalThrowPoint $throwPoint) => $throwPoint->isExplicit()));
Expand Down
5 changes: 4 additions & 1 deletion src/Analyser/ExprHandler/ArrayHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -174,18 +174,21 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
if (
count($expr->items) === 2
&& isset($expr->items[0], $expr->items[1])
&& $type->isCallable()->maybe()
) {
$isCallableCall = new FuncCall(
new FullyQualified('is_callable'),
[new Arg($expr)],
);
// isCallable() is asked last - it reflects the class named by the
// first item, which is expensive and unnecessary for arrays never
// narrowed by is_callable()
if (
$beforeScope->hasExpressionType($isCallableCall)->yes()
// read the narrowed type from expressionTypes directly (the
// synthetic is_callable() call was never processed as a child),
// mirroring ConstFetchHandler's narrowed-constant lookup
&& $beforeScope->expressionTypes[$beforeScope->getNodeKey($isCallableCall)]->getType()->isTrue()->yes()
&& $type->isCallable()->maybe()
) {
$type = TypeCombinator::intersect($type, new CallableType());
}
Expand Down
11 changes: 11 additions & 0 deletions src/Dependency/DependencyResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,17 @@ private function considerArrayForCallableTest(Scope $scope, Array_ $arrayNode):
return false;
}

// a class constant, property default or enum case value is not called where it is
// declared - testing it would reflect whatever class its first item happens to name
if (
$scope->isInClass()
&& $scope->getFunction() === null
&& !$scope->isInAnonymousFunction()
&& $scope->getFunctionCallStack() === []
) {
return false;
}

$itemType = $scope->getType($items[0]->value);
return $itemType->isClassString()->yes();
}
Expand Down
10 changes: 6 additions & 4 deletions src/Reflection/GenericParametersAcceptorResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,6 @@ public static function resolve(array $argTypes, ParametersAcceptor $parametersAc
private static function inferPredicateTemplateTypes(Type $paramType, Type $argType): TemplateTypeMap
{
$typeMap = TemplateTypeMap::createEmpty();
if (!$argType->isCallable()->yes()) {
return $typeMap;
}

foreach ($paramType instanceof UnionType ? $paramType->getTypes() : [$paramType] as $innerType) {
if (!$innerType instanceof CallableParametersAcceptor) {
continue;
Expand All @@ -183,6 +179,12 @@ private static function inferPredicateTemplateTypes(Type $paramType, Type $argTy
continue;
}

// asked only for parameters with asserts - it reflects the class named by
// a callable-like argument
if (!$argType->isCallable()->yes()) {
return $typeMap;
}

foreach ($argType->getCallableParametersAcceptors(new OutOfClassScope()) as $receivedAcceptor) {
$typeMap = $typeMap->union(CallableAssertionsHelper::inferTemplateTypesOnAsserts($innerType, $receivedAcceptor));
}
Expand Down
2 changes: 1 addition & 1 deletion src/Turbo/TurboExtensionEnabler.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
final class TurboExtensionEnabler
{

public const EXPECTED_EXTENSION_VERSION = '341ab11';
public const EXPECTED_EXTENSION_VERSION = '9cebb2a';

private static bool $active = false;

Expand Down
32 changes: 32 additions & 0 deletions tests/PHPStan/Analyser/Bug15292MethodsClassReflectionExtension.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php declare(strict_types = 1);

namespace PHPStan\Analyser;

use PHPStan\Reflection\ClassReflection;
use PHPStan\Reflection\MethodReflection;
use PHPStan\Reflection\MethodsClassReflectionExtension;
use PHPStan\ShouldNotHappenException;

/**
* Records the methods asked about, so a test can assert that a class is not reflected
* for a callable-looking value that is never used as a callable.
*/
final class Bug15292MethodsClassReflectionExtension implements MethodsClassReflectionExtension
{

/** @var list<string> */
public static array $askedMethods = [];

public function hasMethod(ClassReflection $classReflection, string $methodName): bool
{
self::$askedMethods[] = $classReflection->getName() . '::' . $methodName;

return false;
}

public function getMethod(ClassReflection $classReflection, string $methodName): MethodReflection
{
throw new ShouldNotHappenException();
}

}
76 changes: 76 additions & 0 deletions tests/PHPStan/Analyser/Bug15292Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
<?php declare(strict_types = 1);

namespace PHPStan\Analyser;

use PhpParser\Node;
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use PHPUnit\Framework\Attributes\RequiresPhp;
use function array_filter;
use function array_values;
use function str_starts_with;

/**
* @extends RuleTestCase<Rule<Node>>
*/
class Bug15292Test extends RuleTestCase
{

protected function getRule(): Rule
{
return new class implements Rule {

public function getNodeType(): string
{
return Node::class;
}

public function processNode(Node $node, Scope $scope): array
{
return [];
}

};
}

#[RequiresPhp('>= 8.1')]
public function testCallableLikeValuesAreNotReflected(): void
{
Bug15292MethodsClassReflectionExtension::$askedMethods = [];
$this->analyse([__DIR__ . '/data/bug-15292.php'], []);
$this->assertSame([], $this->getAskedMethods('Bug15292\\'));
}

public function testCallableLikeGenericArgumentsAreNotReflected(): void
{
Bug15292MethodsClassReflectionExtension::$askedMethods = [];
$this->analyse([__DIR__ . '/data/bug-15292-generic.php'], []);
$this->assertSame([], $this->getAskedMethods('Bug15292Generic\\'));
}

public function testCallableIsReflected(): void
{
Bug15292MethodsClassReflectionExtension::$askedMethods = [];
$this->analyse([__DIR__ . '/data/bug-15292-callable.php'], []);
$this->assertContains('Bug15292Callable\BigContainer::callable_ident', $this->getAskedMethods('Bug15292Callable\\'));
}

/**
* @return list<string>
*/
private function getAskedMethods(string $prefix): array
{
return array_values(array_filter(
Bug15292MethodsClassReflectionExtension::$askedMethods,
static fn (string $method): bool => str_starts_with($method, $prefix),
));
}

public static function getAdditionalConfigFiles(): array
{
return [
__DIR__ . '/bug-15292.neon',
];
}

}
5 changes: 5 additions & 0 deletions tests/PHPStan/Analyser/bug-15292.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
services:
-
class: PHPStan\Analyser\Bug15292MethodsClassReflectionExtension
tags:
- phpstan.broker.methodsClassReflectionExtension
15 changes: 15 additions & 0 deletions tests/PHPStan/Analyser/data/bug-15292-callable.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

declare(strict_types = 1);

namespace Bug15292Callable;

class BigContainer
{

}

function doFoo(): void
{
usort($list, ['Bug15292Callable\BigContainer', 'callable_ident']);
}
34 changes: 34 additions & 0 deletions tests/PHPStan/Analyser/data/bug-15292-generic.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php declare(strict_types = 1);

namespace Bug15292Generic;

class BigContainer
{

}

/**
* @template T
* @param T $v
* @return T
*/
function identity($v)
{
return $v;
}

final class Foo
{

private const ALLOWED = [
'Bug15292Generic\BigContainer',
'generic_ident',
];

public function doFoo(): void
{
identity(self::ALLOWED);
identity('Bug15292Generic\BigContainer::generic_string_ident');
}

}
46 changes: 46 additions & 0 deletions tests/PHPStan/Analyser/data/bug-15292.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<?php // lint >= 8.1

declare(strict_types = 1);

namespace Bug15292;

class BigContainer
{

}

enum Modality: string
{

case First = 'first';

public const ALLOWED = [
'Bug15292\BigContainer',
'enum_constant_ident',
];

}

final class FooLeaking
{

private const ALLOWED_MODALITIES = [
'Bug15292\BigContainer',
'arbitrary_ident',
];

/** @var list<string> */
private array $allowedProperty = [
'Bug15292\BigContainer',
'property_ident',
];

public function isAllowed(string $item): bool
{
return in_array($item, self::ALLOWED_MODALITIES, true)
|| in_array($item, $this->allowedProperty, true)
|| in_array($item, Modality::ALLOWED, true)
|| in_array('Bug15292\BigContainer::string_ident', [$item], true);
}

}
46 changes: 30 additions & 16 deletions turbo-ext/src/ArgumentsHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2030,10 +2030,8 @@ class ArgumentsHandler

/* the callable-argument bookkeeping of a non-closure argument whose type
* is callable with a single acceptor; false = pending exception */
zend_never_inline bool processCallableArg(Walk &w, zval *value, ArgLocals &a, zval *acceptor) const
zend_never_inline bool processCallableArg(Walk &w, zval *value, zval *acceptor, bool invalidateCallback, bool immediately) const
{
bool invalidateCallback = false;
if (UNEXPECTED(!shouldInvalidateCallbackExpressions(a.parameter, invalidateCallback))) return false;
if (invalidateCallback) {
zv::Val invalidateExpressions = callByName(acceptor, PT_LC("getinvalidateexpressions"), "getInvalidateExpressions", 0, NULL);
if (UNEXPECTED(invalidateExpressions.isUndef())) return false;
Expand All @@ -2042,8 +2040,6 @@ class ArgumentsHandler
w.deferredInvalidateExpressions.push(std::move(invalidateExpressions));
w.deferredUses.push(std::move(usedVariables));
}
bool immediately = false;
if (UNEXPECTED(!callCallbackImmediately(a.parameter, a.parameterType.isUndef() ? NULL : a.parameterType.raw(), w.calleeReflection, immediately))) return false;
if (!immediately) return true;

static pt_method_site throwPointsSite, impurePointsSite;
Expand Down Expand Up @@ -2136,18 +2132,36 @@ class ArgumentsHandler
}
if (!w.hasYield && UNEXPECTED(!pt_expression_result_has_yield(exprResult, w.hasYield))) return false;

if (UNEXPECTED(!exprType.ref().isObject())) {
zend_throw_error(NULL, "Call to a member function isCallable() on %s", zend_zval_value_name(exprType.raw()));
return false;
// only callable objects (closures) carry expressions to invalidate - asking
// isCallable() of other arguments reflects the classes named by callable-like
// strings and arrays, so it is skipped when nothing would come of it
bool invalidateCallback = false;
if (UNEXPECTED(!shouldInvalidateCallbackExpressions(a.parameter, invalidateCallback))) return false;
if (invalidateCallback) {
if (UNEXPECTED(!exprType.ref().isObject())) {
zend_throw_error(NULL, "Call to a member function isObject() on %s", zend_zval_value_name(exprType.raw()));
return false;
}
zend_long isObject = pt_type_call_trinary(Z_OBJ_P(exprType.raw()), PT_LC("isobject"), 0, NULL);
if (UNEXPECTED(isObject < 0)) return false;
invalidateCallback = isObject != PT_TRI_NO;
}
zend_long isCallable = pt_type_op_trinary(Z_OBJ_P(exprType.raw()), PT_OP_IS_CALLABLE, 0, NULL);
if (UNEXPECTED(isCallable < 0)) return false;
if (isCallable == PT_TRI_YES) {
zv::Val acceptors = pt_type_call(Z_OBJ_P(exprType.raw()), PT_LC("getcallableparametersacceptors"), 1, w.scope.raw());
if (UNEXPECTED(acceptors.isUndef() || !requireArray(acceptors.raw(), "count(): Argument #1 ($value)"))) return false;
if (zend_hash_num_elements(Z_ARRVAL_P(acceptors.raw())) == 1) {
zval *acceptor = readIndex(Z_ARRVAL_P(acceptors.raw()), 0);
if (UNEXPECTED(acceptor == NULL || !processCallableArg(w, value, a, acceptor))) return false;
bool immediately = false;
if (UNEXPECTED(!callCallbackImmediately(a.parameter, a.parameterType.isUndef() ? NULL : a.parameterType.raw(), w.calleeReflection, immediately))) return false;
if (invalidateCallback || immediately) {
if (UNEXPECTED(!exprType.ref().isObject())) {
zend_throw_error(NULL, "Call to a member function isCallable() on %s", zend_zval_value_name(exprType.raw()));
return false;
}
zend_long isCallable = pt_type_op_trinary(Z_OBJ_P(exprType.raw()), PT_OP_IS_CALLABLE, 0, NULL);
if (UNEXPECTED(isCallable < 0)) return false;
if (isCallable == PT_TRI_YES) {
zv::Val acceptors = pt_type_call(Z_OBJ_P(exprType.raw()), PT_LC("getcallableparametersacceptors"), 1, w.scope.raw());
if (UNEXPECTED(acceptors.isUndef() || !requireArray(acceptors.raw(), "count(): Argument #1 ($value)"))) return false;
if (zend_hash_num_elements(Z_ARRVAL_P(acceptors.raw())) == 1) {
zval *acceptor = readIndex(Z_ARRVAL_P(acceptors.raw()), 0);
if (UNEXPECTED(acceptor == NULL || !processCallableArg(w, value, acceptor, invalidateCallback, immediately))) return false;
}
}
}

Expand Down
Loading
Loading