Skip to content

Re-process a stored scope-changing expression at a foreign ask position instead of re-anchoring it #6832

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

Open
phpstan-bot wants to merge 1 commit into
base: 2.3.x
Choose a base branch
from
Open

Re-process a stored scope-changing expression at a foreign ask position instead of re-anchoring it #6832

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 12 additions & 0 deletions src/Analyser/NodeScopeResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,18 @@ public function processExprNode(
return $storedResult;
}

// re-anchoring drops what the node did to the scope (an assignment
// inside a closure body an extension prices on demand) - such a
// node is re-processed on the asking scope instead
if (
!$consume
&& $storedResult->getScope() !== $storedResult->getBeforeScope()
&& !$expr instanceof Expr\Closure
&& !$expr instanceof Expr\ArrowFunction
) {
return $this->processExprNodeInternal($stmt, $expr, $scope, $storage, $nodeCallback, $context);
}

$reanchored = $storedResult->atAskPosition($scope);
if ($consume) {
// the re-anchored view IS this walk's result for the node
Expand Down
162 changes: 162 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-15491.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
<?php declare(strict_types = 1);

namespace Bug15491;

use function PHPStan\Testing\assertType;

function idFromAlias(string $alias): ?string
{
return strpos($alias, '.') !== false ? md5($alias) : null;
}

/**
* @param array<string, mixed> $data
* @return array<string, mixed>
*/
function withCoalesce(array $data): array
{
$data['ids'] = array_filter(
array_map(
function (string $alias): ?string {
$id = idFromAlias($alias);
if ($id === null) {
echo 'invalid';
}
return $id;
},
explode('|', (string) ($data['options'] ?? ''))
)
);
assertType('array<int<0, max>, non-falsy-string>', $data['ids']);

return $data;
}

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

/**
* @param mixed $x
* @return list<string>
*/
function strings($x): array
{
return [];
}

/** @param array<string, mixed> $data */
function assignOp(array $data): void
{
$x = identity(array_map(
function (string $alias): int {
$i = 0;
$i += strlen($alias);
if ($i === 0) {
echo 'empty';
}
return $i;
},
strings($data['options'] ?? '')
));
assertType('list<int<0, max>>', $x);
}

/** @param array<string, mixed> $data */
function coalesceAssign(array $data): void
{
$x = identity(array_map(
function (string $alias): ?string {
$id = null;
$id ??= idFromAlias($alias);
if ($id === null) {
echo 'invalid';
}
return $id;
},
strings($data['options'] ?? '')
));
assertType('list<string|null>', $x);
}

/** @param array<string, mixed> $data */
function increment(array $data): void
{
$x = identity(array_map(
function (string $alias): int {
$i = 0;
$i++;
if ($i === 0) {
echo 'never';
}
return $i;
},
strings($data['options'] ?? '')
));
assertType('list<1>', $x);
}

/** @param array<string, mixed> $data */
function byRefOutParameter(array $data): void
{
$x = identity(array_map(
function (string $alias): ?string {
$m = null;
preg_match('/a/', $alias, $m);
if ($m === null) {
echo 'never';
}
return $m[0] ?? null;
},
strings($data['options'] ?? '')
));
assertType('list<non-empty-string|null>', $x);
}

/** @param array<string, mixed> $data */
function listAssign(array $data): void
{
$x = identity(array_map(
function (string $alias): ?string {
[$id] = [idFromAlias($alias)];
if ($id === null) {
echo 'invalid';
}
return $id;
},
strings($data['options'] ?? '')
));
assertType('list<string|null>', $x);
}

/** @param array<string, mixed> $data */
function arrowFunction(array $data): void
{
$x = identity(array_map(
fn (string $alias): ?string => ($id = idFromAlias($alias)) === null ? $id : $id,
strings($data['options'] ?? '')
));
assertType('list<string|null>', $x);
}

/** @param array<string, mixed> $data */
function arrayFilterCallback(array $data): void
{
$x = identity(array_filter(
strings($data['options'] ?? ''),
function (string $alias): bool {
$id = idFromAlias($alias);
if ($id === null) {
echo 'invalid';
}
return $id !== null;
}
));
assertType('array<int<0, max>, string>', $x);
}
5 changes: 5 additions & 0 deletions tests/PHPStan/Rules/Functions/ArrayFilterRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,9 @@ public function testBug2065WithoutPhpDocTypesAsCertain(): void
$this->analyse([__DIR__ . '/data/bug-array-filter.php'], []);
}

public function testBug15491(): void
{
$this->analyse([__DIR__ . '/data/bug-15491.php'], []);
}

}
71 changes: 71 additions & 0 deletions tests/PHPStan/Rules/Functions/data/bug-15491.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
<?php declare(strict_types = 1);

namespace Bug15491Rule;

function idFromAlias(string $alias): ?string
{
return strpos($alias, '.') !== false ? md5($alias) : null;
}

/**
* @param array<string, mixed> $data
* @return array<string, mixed>
*/
function withCoalesce(array $data): array
{
$data['ids'] = array_filter(
array_map(
function (string $alias): ?string {
$id = idFromAlias($alias);
if ($id === null) {
echo 'invalid';
}
return $id;
},
explode('|', (string) ($data['options'] ?? ''))
)
);

return $data;
}

/**
* @param array<string, mixed> $data
* @return array<string, mixed>
*/
function withoutCoalesce(array $data): array
{
$data['ids'] = array_filter(
array_map(
function (string $alias): ?string {
$id = idFromAlias($alias);
if ($id === null) {
echo 'invalid';
}
return $id;
},
explode('|', (string) $data['options'])
)
);

return $data;
}

/**
* @param array<string, mixed> $data
* @return array<string, mixed>
*/
function withoutIf(array $data): array
{
$data['ids'] = array_filter(
array_map(
function (string $alias): ?string {
$id = idFromAlias($alias);
return $id;
},
explode('|', (string) ($data['options'] ?? ''))
)
);

return $data;
}
14 changes: 14 additions & 0 deletions turbo-ext/src/NodeScopeResolver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1963,6 +1963,20 @@ class NodeScopeResolver
if (UNEXPECTED(beforeScope.isUndef())) return zv::Val();
if (beforeScope.ref().isObject() && Z_OBJ_P(beforeScope.raw()) == Z_OBJ_P(scope)) return storedResult;

// re-anchoring drops what the node did to the scope - such a
// node is re-processed on the asking scope instead
if (!consume) {
zv::Val afterScope = expressionResultScope(storedResult.raw());
if (UNEXPECTED(afterScope.isUndef())) return zv::Val();
bool sameScope = afterScope.ref().isObject() && beforeScope.ref().isObject() && Z_OBJ_P(afterScope.raw()) == Z_OBJ_P(beforeScope.raw());
if (!sameScope) {
bool isClosure;
if (UNEXPECTED(!isInstance(Z_OBJ_P(expr), PT_CLASS_CLOSURE_EXPR, isClosure))) return zv::Val();
if (!isClosure && UNEXPECTED(!isInstance(Z_OBJ_P(expr), PT_CLASS_ARROW_FUNCTION, isClosure))) return zv::Val();
if (!isClosure) return processExprNodeInternal(stmt, expr, scope, storage, nodeCallback, context);
}
}

zv::Val reanchored = pt_expression_result_at_ask_position(storedResult.raw(), scope);
if (UNEXPECTED(reanchored.isUndef())) return zv::Val();
if (consume && UNEXPECTED(!thisStoreExpressionResult(storage, expr, reanchored.raw()))) return zv::Val();
Expand Down
Loading