Skip to content

Commit

Permalink
Fix various things around isset() and ArrayDimFetch
Browse files Browse the repository at this point in the history
* add regression tests for phpstan/phpstan#7000

* specify expression even it doesn't change types

* skip UnionType offset because it's hard to solve in some cases

* skip specified expressions in NonexistentOffsetInArrayDimFetchRule

* fix tests

* add regression tests

* fix useless call in foreach

* fix issue-7190 too

* Avoided duplicate namespace

Co-authored-by: Ondřej Mirtes <ondrej@mirtes.cz>
  • Loading branch information
rajyan and ondrejmirtes committed May 12, 2022
1 parent dbbced9 commit 21164d6
Show file tree
Hide file tree
Showing 10 changed files with 130 additions and 18 deletions.
27 changes: 15 additions & 12 deletions src/Analyser/MutatingScope.php
Expand Up @@ -4292,14 +4292,19 @@ public function specifyExpressionType(Expr $expr, Type $type, ?Type $nativeType
$constantArrays = TypeUtils::getOldConstantArrays($this->getType($expr->var));
if (count($constantArrays) > 0) {
$setArrays = [];
$dimType = $this->getType($expr->dim);
foreach ($constantArrays as $constantArray) {
$setArrays[] = $constantArray->setOffsetValueType($dimType, $type);
$dimType = ArrayType::castToArrayKeyType($this->getType($expr->dim));
if (!$dimType instanceof UnionType) {
foreach ($constantArrays as $constantArray) {
$setArrays[] = $constantArray->setOffsetValueType(
TypeCombinator::intersect($dimType, $constantArray->getKeyType()),
$type,
);
}
$scope = $this->specifyExpressionType(
$expr->var,
TypeCombinator::union(...$setArrays),
);
}
$scope = $this->specifyExpressionType(
$expr->var,
TypeCombinator::union(...$setArrays),
);
}
}

Expand Down Expand Up @@ -4470,15 +4475,13 @@ public function invalidateMethodsOnExpression(Expr $expressionToInvalidate): sel
public function removeTypeFromExpression(Expr $expr, Type $typeToRemove): self
{
$exprType = $this->getType($expr);
$typeAfterRemove = TypeCombinator::remove($exprType, $typeToRemove);
if (
!$expr instanceof Variable
&& $exprType->equals($typeAfterRemove)
&& !$exprType instanceof ErrorType
&& !$exprType instanceof NeverType
$exprType instanceof NeverType ||
$typeToRemove instanceof NeverType
) {
return $this;
}
$typeAfterRemove = TypeCombinator::remove($exprType, $typeToRemove);
$scope = $this->specifyExpressionType(
$expr,
$typeAfterRemove,
Expand Down
2 changes: 1 addition & 1 deletion src/Rules/Arrays/NonexistentOffsetInArrayDimFetchRule.php
Expand Up @@ -83,7 +83,7 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

if ($dimType === null) {
if ($dimType === null || $scope->isSpecified($node)) {
return [];
}

Expand Down
6 changes: 1 addition & 5 deletions src/Rules/IssetCheck.php
Expand Up @@ -86,13 +86,9 @@ public function check(Expr $expr, Scope $scope, string $operatorDescription, cal
)->build();
}

if ($hasOffsetValue->maybe()) {
return null;
}

// If offset is cannot be null, store this error message and see if one of the earlier offsets is.
// E.g. $array['a']['b']['c'] ?? null; is a valid coalesce if a OR b or C might be null.
if ($hasOffsetValue->yes()) {
if ($hasOffsetValue->yes() || $scope->isSpecified($expr)) {
if ($error !== null) {
return $error;
}
Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
Expand Up @@ -832,6 +832,7 @@ public function dataFileAsserts(): iterable
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-3853.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/conditional-types.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/constant-array-optional-set.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-7000.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6383.php');
yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/Methods/data/bug-3284.php');

Expand Down
20 changes: 20 additions & 0 deletions tests/PHPStan/Analyser/data/bug-7000.php
@@ -0,0 +1,20 @@
<?php

namespace Bug7000Analyser;

use function PHPStan\Testing\assertType;

class Foo
{
public function doBar(): void
{
/** @var array{require?: array<string, string>, require-dev?: array<string, string>} $composer */
$composer = array();
foreach (array('require', 'require-dev') as $linkType) {
if (isset($composer[$linkType])) {
assertType('array{require?: array<string, string>, require-dev?: array<string, string>}', $composer);
foreach ($composer[$linkType] as $x) {}
}
}
}
}
Expand Up @@ -382,4 +382,19 @@ public function testBug4885(): void
$this->analyse([__DIR__ . '/data/bug-4885.php'], []);
}

public function testBug7000(): void
{
$this->analyse([__DIR__ . '/data/bug-7000.php'], [
[
"Offset 'require'|'require-dev' does not exist on array{require?: array<string, string>, require-dev?: array<string, string>}.",
16,
],
]);
}

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

}
18 changes: 18 additions & 0 deletions tests/PHPStan/Rules/Arrays/data/bug-6508.php
@@ -0,0 +1,18 @@
<?php

namespace Bug6508;

class Foo
{
/**
* @param array{
* type1?: array{bool: bool},
* type2?: array{bool: bool}
* } $types
* @param 'type1'|'type2' $type
*/
function test(array $types, string $type): void
{
if (isset($types[$type]) && $types[$type]['bool']) {}
}
}
20 changes: 20 additions & 0 deletions tests/PHPStan/Rules/Arrays/data/bug-7000.php
@@ -0,0 +1,20 @@
<?php declare(strict_types = 1);

namespace Bug7000;

class Foo
{
public function doBar(): void
{
/** @var array{require?: array<string, string>, require-dev?: array<string, string>} $composer */
$composer = array();
/** @var 'require'|'require-dev' $foo */
$foo = '';
foreach (array('require', 'require-dev') as $linkType) {
if (isset($composer[$linkType])) {
foreach ($composer[$linkType] as $x) {} // should not report error
foreach ($composer[$foo] as $x) {} // should report error. It can be $linkType = 'require', $foo = 'require-dev'
}
}
}
}
22 changes: 22 additions & 0 deletions tests/PHPStan/Rules/Properties/data/bug-7190.php
@@ -0,0 +1,22 @@
<?php declare(strict_types = 1); // lint >= 7.4

namespace Bug7190;

interface MyObject {
public function getId(): int;
}

class HelloWorld
{
/**
* @param array<int, int> $array
*/
public function sayHello(array $array, MyObject $object): int
{
if (!isset($array[$object->getId()])) {
return 1;
}

return $array[$object->getId()] ?? 2;
}
}
17 changes: 17 additions & 0 deletions tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php
Expand Up @@ -322,4 +322,21 @@ public function testBug7109Strict(): void
]);
}

public function testBug7190(): void
{
if (PHP_VERSION_ID < 70400) {
$this->markTestSkipped('Test requires PHP 7.4.');
}

$this->treatPhpDocTypesAsCertain = true;
$this->strictUnnecessaryNullsafePropertyFetch = false;

$this->analyse([__DIR__ . '/../Properties/data/bug-7190.php'], [
[
'Offset int on array<int, int> on left side of ?? always exists and is not nullable.',
20,
],
]);
}

}

0 comments on commit 21164d6

Please sign in to comment.