Skip to content

Fix false positives on non-existing-offset's #3766

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.

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

Merged
merged 4 commits into from
Mar 9, 2025
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
78 changes: 76 additions & 2 deletions src/Analyser/TypeSpecifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -664,11 +664,85 @@ public function specifyTypesInCondition(
if (!$scope instanceof MutatingScope) {
throw new ShouldNotHappenException();
}

if ($context->null()) {
return $this->specifyTypesInCondition($scope->exitFirstLevelStatements(), $expr->expr, $context)->setRootExpr($expr);
$specifiedTypes = $this->specifyTypesInCondition($scope->exitFirstLevelStatements(), $expr->expr, $context)->setRootExpr($expr);

// infer $arr[$key] after $key = array_key_first/last($arr)
if (
$expr->expr instanceof FuncCall
&& $expr->expr->name instanceof Name
&& in_array($expr->expr->name->toLowerString(), ['array_key_first', 'array_key_last'], true)
&& count($expr->expr->getArgs()) >= 1
) {
$arrayArg = $expr->expr->getArgs()[0]->value;
$arrayType = $scope->getType($arrayArg);
if (
$arrayType->isArray()->yes()
&& $arrayType->isIterableAtLeastOnce()->yes()
) {
$dimFetch = new ArrayDimFetch($arrayArg, $expr->var);
$iterableValueType = $expr->expr->name->toLowerString() === 'array_key_first'
? $arrayType->getFirstIterableValueType()
: $arrayType->getLastIterableValueType();

return $specifiedTypes->unionWith(
$this->create($dimFetch, $iterableValueType, TypeSpecifierContext::createTrue(), $scope),
);
}
}

// infer $list[$count] after $count = count($list) - 1
if (
$expr->expr instanceof Expr\BinaryOp\Minus
&& $expr->expr->left instanceof FuncCall
&& $expr->expr->left->name instanceof Name
&& in_array($expr->expr->left->name->toLowerString(), ['count', 'sizeof'], true)
&& count($expr->expr->left->getArgs()) >= 1
&& $expr->expr->right instanceof Node\Scalar\Int_
&& $expr->expr->right->value === 1
) {
$arrayArg = $expr->expr->left->getArgs()[0]->value;
$arrayType = $scope->getType($arrayArg);
if (
$arrayType->isList()->yes()
&& $arrayType->isIterableAtLeastOnce()->yes()
) {
$dimFetch = new ArrayDimFetch($arrayArg, $expr->var);

return $specifiedTypes->unionWith(
$this->create($dimFetch, $arrayType->getLastIterableValueType(), TypeSpecifierContext::createTrue(), $scope),
);
}
}

return $specifiedTypes;
}

return $this->specifyTypesInCondition($scope->exitFirstLevelStatements(), $expr->var, $context)->setRootExpr($expr);
$specifiedTypes = $this->specifyTypesInCondition($scope->exitFirstLevelStatements(), $expr->var, $context)->setRootExpr($expr);

if ($context->true()) {
// infer $arr[$key] after $key = array_search($needle, $arr)
if (
$expr->expr instanceof FuncCall
&& $expr->expr->name instanceof Name
&& $expr->expr->name->toLowerString() === 'array_search'
&& count($expr->expr->getArgs()) >= 2
) {
$arrayArg = $expr->expr->getArgs()[1]->value;
$arrayType = $scope->getType($arrayArg);

if ($arrayType->isArray()->yes()) {
$dimFetch = new ArrayDimFetch($arrayArg, $expr->var);
$iterableValueType = $arrayType->getIterableValueType();

return $specifiedTypes->unionWith(
$this->create($dimFetch, $iterableValueType, TypeSpecifierContext::createTrue(), $scope),
);
}
}
}
return $specifiedTypes;
} elseif (
$expr instanceof Expr\Isset_
&& count($expr->vars) > 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -785,4 +785,48 @@ public function testBug12122(): void
$this->analyse([__DIR__ . '/data/bug-12122.php'], []);
}

public function testArrayDimFetchAfterArrayKeyFirstOrLast(): void
{
$this->reportPossiblyNonexistentGeneralArrayOffset = true;

$this->analyse([__DIR__ . '/data/array-dim-after-array-key-first-or-last.php'], [
[
'Offset null does not exist on array{}.',
19,
],
]);
}

public function testArrayDimFetchAfterCount(): void
{
$this->reportPossiblyNonexistentGeneralArrayOffset = true;

$this->analyse([__DIR__ . '/data/array-dim-after-count.php'], [
[
'Offset int<0, max> might not exist on list<string>.',
26,
],
[
'Offset int<-1, max> might not exist on array<string>.',
35,
],
[
'Offset int<0, max> might not exist on non-empty-array<string>.',
42,
],
]);
}

public function testArrayDimFetchAfterArraySearch(): void
{
$this->reportPossiblyNonexistentGeneralArrayOffset = true;

$this->analyse([__DIR__ . '/data/array-dim-after-array-search.php'], [
[
'Offset int|string might not exist on array.',
20,
],
]);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
<?php // lint >= 8.0

declare(strict_types = 1);

namespace ArrayDimAfterArrayKeyFirstOrLast;

class HelloWorld
{
/**
* @param list<string> $hellos
*/
public function last(array $hellos): string
{
if ($hellos !== []) {
$last = array_key_last($hellos);
return $hellos[$last];
} else {
$last = array_key_last($hellos);
return $hellos[$last];
}
}

/**
* @param array<string> $hellos
*/
public function lastOnArray(array $hellos): string
{
if ($hellos !== []) {
$last = array_key_last($hellos);
return $hellos[$last];
}

return 'nothing';
}

/**
* @param list<string> $hellos
*/
public function first(array $hellos): string
{
if ($hellos !== []) {
$first = array_key_first($hellos);
return $hellos[$first];
}

return 'nothing';
}

/**
* @param array<string> $hellos
*/
public function firstOnArray(array $hellos): string
{
if ($hellos !== []) {
$first = array_key_first($hellos);
return $hellos[$first];
}

return 'nothing';
}

/**
* @param array{first: int, middle: float, last: bool} $hellos
*/
public function shape(array $hellos): int|bool
{
$first = array_key_first($hellos);
$last = array_key_last($hellos);

if (rand(0,1)) {
return $hellos[$first];
}
return $hellos[$last];
}
}
37 changes: 37 additions & 0 deletions tests/PHPStan/Rules/Arrays/data/array-dim-after-array-search.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php // lint >= 8.0

declare(strict_types = 1);

namespace ArrayDimAfterArraySeach;

class HelloWorld
{
public function doFoo(array $arr, string $needle): string
{
if (($key = array_search($needle, $arr, true)) !== false) {
echo $arr[$key];
}
}

public function doBar(array $arr, string $needle): string
{
$key = array_search($needle, $arr, true);
if ($key !== false) {
echo $arr[$key];
}
}

public function doFooBar(array $arr, string $needle): string
{
if (($key = array_search($needle, $arr, false)) !== false) {
echo $arr[$key];
}
}

public function doBaz(array $arr, string $needle): string
{
if (($key = array_search($needle, $arr)) !== false) {
echo $arr[$key];
}
}
}
45 changes: 45 additions & 0 deletions tests/PHPStan/Rules/Arrays/data/array-dim-after-count.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php declare(strict_types = 1);

namespace ArrayDimFetchOnCount;

class HelloWorld
{
/**
* @param list<string> $hellos
*/
public function works(array $hellos): string
{
if ($hellos === []) {
return 'nothing';
}

$count = count($hellos) - 1;
return $hellos[$count];
}

/**
* @param list<string> $hellos
*/
public function offByOne(array $hellos): string
{
$count = count($hellos);
return $hellos[$count];
}

/**
* @param array<string> $hellos
*/
public function maybeInvalid(array $hellos): string
{
$count = count($hellos) - 1;
echo $hellos[$count];

if ($hellos === []) {
return 'nothing';
}

$count = count($hellos) - 1;
return $hellos[$count];
}

}
Loading