Skip to content
Open
Show file tree
Hide file tree
Changes from 13 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
25 changes: 25 additions & 0 deletions src/Analyser/ExprHandler/ArrayHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@
use PHPStan\Analyser\MutatingScope;
use PHPStan\Analyser\NodeScopeResolver;
use PHPStan\DependencyInjection\AutowiredService;
use PHPStan\Node\Expr\IntertwinedVariableByReferenceWithExpr;
use PHPStan\Node\LiteralArrayItem;
use PHPStan\Node\LiteralArrayNode;
use PHPStan\Reflection\InitializerExprTypeResolver;
use PHPStan\Type\Type;
use function array_merge;
use function is_string;

/**
* @implements ExprHandler<Array_>
Expand Down Expand Up @@ -60,12 +62,35 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
$scope = $keyResult->getScope();
}

if ($arrayItem->byRef) {
$scope = $nodeScopeResolver->lookForSetAllowedUndefinedExpressions($scope, $arrayItem->value);
}

$valueResult = $nodeScopeResolver->processExprNode($stmt, $arrayItem->value, $scope, $storage, $nodeCallback, $context->enterDeep());
$hasYield = $hasYield || $valueResult->hasYield();
$throwPoints = array_merge($throwPoints, $valueResult->getThrowPoints());
$impurePoints = array_merge($impurePoints, $valueResult->getImpurePoints());
$isAlwaysTerminating = $isAlwaysTerminating || $valueResult->isAlwaysTerminating();
$scope = $valueResult->getScope();
if (!$arrayItem->byRef) {
continue;
}

$scope = $nodeScopeResolver->lookForUnsetAllowedUndefinedExpressions($scope, $arrayItem->value);

if ($arrayItem->value instanceof Expr\Variable && is_string($arrayItem->value->name)) {
$varName = $arrayItem->value->name;
$type = $scope->getType($arrayItem->value);
$nativeType = $scope->getNativeType($arrayItem->value);
// Ensure the variable is defined (PHP creates it if undefined when used by-ref)
$scope = $scope->assignExpression($arrayItem->value, $type, $nativeType);
// Register intertwined relationship
$scope = $scope->assignExpression(
new IntertwinedVariableByReferenceWithExpr($varName, $expr, new Expr\Variable($varName)),
$type,
$nativeType,
);
}
}
$nodeScopeResolver->callNodeCallback($nodeCallback, new LiteralArrayNode($expr, $itemNodes), $scope, $storage);

Expand Down
32 changes: 32 additions & 0 deletions src/Analyser/ExprHandler/AssignHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,38 @@ static function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $contex
);
}

if (
$expr instanceof Assign
&& $expr->var instanceof Variable
&& is_string($expr->var->name)
&& $expr->expr instanceof Expr\Array_
) {
$targetVarName = $expr->var->name;
foreach ($expr->expr->items as $i => $item) {
if (!$item->byRef) {
continue;
}
if (!($item->value instanceof Variable) || !is_string($item->value->name)) {
continue;
}
$refVarName = $item->value->name;
$key = $item->key ?? new Node\Scalar\Int_($i);
$type = $scope->getType($item->value);
$nativeType = $scope->getNativeType($item->value);

// When $refVarName is assigned, update $targetVar[$key]
$scope = $scope->assignExpression(
new IntertwinedVariableByReferenceWithExpr(
$refVarName,
new ArrayDimFetch(new Variable($targetVarName), $key),
new Variable($refVarName),
),
$type,
$nativeType,
);
}
}

$vars = $nodeScopeResolver->getAssignedVariables($expr->var);
if (count($vars) > 0) {
$varChangedScope = false;
Expand Down
24 changes: 24 additions & 0 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -3532,6 +3532,30 @@
$scope = $scope->invalidateExpression($arg->value, true);
}
}

if (
!$assignByReference
&& $calleeReflection !== null
&& !$calleeReflection->hasSideEffects()->no()

Check warning on line 3539 in src/Analyser/NodeScopeResolver.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ if ( !$assignByReference && $calleeReflection !== null - && !$calleeReflection->hasSideEffects()->no() + && $calleeReflection->hasSideEffects()->yes() && $arg->value instanceof Array_ ) { foreach ($arg->value->items as $item) {

Check warning on line 3539 in src/Analyser/NodeScopeResolver.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ if ( !$assignByReference && $calleeReflection !== null - && !$calleeReflection->hasSideEffects()->no() + && $calleeReflection->hasSideEffects()->yes() && $arg->value instanceof Array_ ) { foreach ($arg->value->items as $item) {
&& $arg->value instanceof Array_
) {
foreach ($arg->value->items as $item) {
if (!$item->byRef) {
continue;
}
if (!($item->value instanceof Variable) || !is_string($item->value->name)) {
continue;
}
$scope = $this->processVirtualAssign(
$scope,
$storage,
$stmt,
$item->value,
new TypeExpr(new MixedType()),
$nodeCallback,
)->getScope();
}
}
}
}

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

namespace Bug6799;

use function PHPStan\Testing\assertType;

class HelloWorld
{
/**
* @param string[] $where
* @param string $sqlTableName
* @param mixed[] $filter
* @param string $value
*/
protected function listingAddWhereFilterAtableDefault(array &$where, string $sqlTableName, array $filter, string $value): void
{
if ($value != "" && !empty($filter) && !empty($filter['sql']) && is_string($filter['sql'])) {
$where[] = "`" . $sqlTableName . "`.`" . (string)$filter['sql'] . "` = '" . $value . "'";
}
}

/**
* @param string[] $filterValues
* @param string[] $where
* @param string[] $tables
* @param mixed[] $filters
*/
protected function listingAddWhereFilterAtable(array $filterValues, array &$where, array &$tables, array $filters): void
{
if (!empty($filterValues) && !empty($filters)) {
$whereFilter = array();
foreach ($filterValues as $type => $value) {
call_user_func_array(array($this, 'listingAddWhereFilterAtableDefault'), array(&$whereFilter, 'xxxx', $filters[$type], $value));
}
assertType('mixed', $whereFilter);
if (count($whereFilter) > 0) {
$where[] = "(" . implode(" AND ", $whereFilter) . ")";
}
}
}
}

/**
* @param mixed $foo
*/
function foo($foo): void {}

function testByRefInArray(): void
{
$a = [];
assertType('array{}', $a);

$b = [&$a];
assertType('array{}', $a);

foo($b);
assertType('array{}', $a);
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be modified since $b was passed to foo and is related to $a

}

function testByRefInArrayWithKey(): void
{
$a = 'hello';
assertType("'hello'", $a);

$b = ['key' => &$a];
assertType("'hello'", $a);

$b['key'] = 42;
assertType("'hello'", $a);
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be 42 since $b['key'] = 42; override $a reference.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Already handled — that was the final phpstan run which passed with no errors. All work is complete and pushed.

}

function testMultipleByRefInArray(): void
{
$a = 1;
$c = 'test';

$b = [&$a, 'normal', &$c];
assertType('1', $a);
assertType("'test'", $c);

$b[0] = 2;
$b[1] = 'foo';
$b[2] = 'bar';

assertType('1', $a);
assertType("'test'", $c);
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be 2 and bar since $b[0] and $b[2] overrides $c reference

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

namespace Bug6799b;

use function PHPStan\Testing\assertType;

class HelloWorld
{


/**
* listingAddWhereFilterAtableRoleCategory
*
* @param string[] $where
* @param string $sqlTableName
* @param mixed[] $filter
* @param string $value
*
* @return void
*/
protected function listingAddWhereFilterAtableDefault(array &$where, string $sqlTableName, array $filter, string $value): void
{
if ($value != "" && !empty($filter) && !empty($filter['sql']) && is_string($filter['sql'])) {
$where[] = "`" . $sqlTableName . "`.`" . (string)$filter['sql'] . "` = '" . $value . "'";
}
}

/**
* listingAddWhereFilterAtableFilter
*
* @param string[] $filterValues
* @param string[] $where
* @param string[] $tables
* @param mixed[] $filters
* @return void
*/
protected function listingAddWhereFilterAtable(array $filterValues, array &$where, array &$tables, array $filters): void
{
if (!empty($filterValues) && !empty($filters)) {
$whereFilter = array();
foreach ($filterValues as $type => $value) {
$this->listingAddWhereFilterAtableDefault($whereFilter, 'xxxxx', $filters[$type], $value);
}
assertType('array<string>', $whereFilter);
}
}
}
17 changes: 17 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-6799c.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php declare(strict_types = 1);

namespace Bug6799C;

use function PHPStan\Testing\assertType;

// https://3v4l.org/g5UjS

$a = [&$x];
assertType('mixed', $x);

function doFoo(array &$arr) {
$arr[0] = 'string';
}

doFoo($a);
assertType('mixed', $x);
Original file line number Diff line number Diff line change
Expand Up @@ -298,4 +298,9 @@ public function testBug12163(): void
]);
}

public function testBug6799(): void
{
$this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-6799.php'], []);
}

}
10 changes: 10 additions & 0 deletions tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1514,4 +1514,14 @@ public function testBug14117(): void
]);
}

public function testBug6799c(): void
{
$this->cliArgumentsVariablesRegistered = true;
$this->polluteScopeWithLoopInitialAssignments = false;
$this->checkMaybeUndefinedVariables = true;
$this->polluteScopeWithAlwaysIterableForeach = true;

$this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-6799c.php'], []);
}

}
Loading