Skip to content
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

Improve in_array extension to get rid of related impossible check adaptions #1514

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
63 changes: 0 additions & 63 deletions src/Rules/Comparison/ImpossibleCheckTypeHelper.php
Expand Up @@ -12,14 +12,10 @@
use PHPStan\Analyser\TypeSpecifierContext;
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\TrinaryLogic;
use PHPStan\Type\Constant\ConstantArrayType;
use PHPStan\Type\Constant\ConstantBooleanType;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\MixedType;
use PHPStan\Type\NeverType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeUtils;
use PHPStan\Type\TypeWithClassName;
use PHPStan\Type\VerbosityLevel;
use function array_map;
Expand Down Expand Up @@ -77,65 +73,6 @@ public function findSpecifiedType(
return null;
} elseif ($functionName === 'array_search') {
return null;
} elseif ($functionName === 'in_array' && $argsCount >= 3) {
$haystackType = $scope->getType($node->getArgs()[1]->value);
if ($haystackType instanceof MixedType) {
return null;
}

if (!$haystackType->isArray()->yes()) {
return null;
}

$constantArrays = TypeUtils::getOldConstantArrays($haystackType);
$needleType = $scope->getType($node->getArgs()[0]->value);
$valueType = $haystackType->getIterableValueType();
$constantNeedleTypesCount = count(TypeUtils::getConstantScalars($needleType));
$constantHaystackTypesCount = count(TypeUtils::getConstantScalars($valueType));
$isNeedleSupertype = $needleType->isSuperTypeOf($valueType);
if (count($constantArrays) === 0) {
if ($haystackType->isIterableAtLeastOnce()->yes()) {
if ($constantNeedleTypesCount === 1 && $constantHaystackTypesCount === 1) {
if ($isNeedleSupertype->yes()) {
return true;
}
if ($isNeedleSupertype->no()) {
return false;
}
}
}
return null;
}

if (!$haystackType instanceof ConstantArrayType || count($haystackType->getValueTypes()) > 0) {
$haystackArrayTypes = TypeUtils::getArrays($haystackType);
if (count($haystackArrayTypes) === 1 && $haystackArrayTypes[0]->getIterableValueType() instanceof NeverType) {
return null;
}

if ($isNeedleSupertype->maybe() || $isNeedleSupertype->yes()) {
foreach ($haystackArrayTypes as $haystackArrayType) {
foreach (TypeUtils::getConstantScalars($haystackArrayType->getIterableValueType()) as $constantScalarType) {
if ($constantScalarType->isSuperTypeOf($needleType)->yes()) {
continue 2;
}
}

return null;
}
}

if ($isNeedleSupertype->yes()) {
$hasConstantNeedleTypes = $constantNeedleTypesCount > 0;
$hasConstantHaystackTypes = $constantHaystackTypesCount > 0;
if (
(!$hasConstantNeedleTypes && !$hasConstantHaystackTypes)
|| $hasConstantNeedleTypes !== $hasConstantHaystackTypes
) {
return null;
}
}
}
} elseif ($functionName === 'method_exists' && $argsCount >= 2) {
$objectType = $scope->getType($node->getArgs()[0]->value);
$methodType = $scope->getType($node->getArgs()[1]->value);
Expand Down
10 changes: 7 additions & 3 deletions src/Type/Accessory/HasOffsetType.php
Expand Up @@ -33,9 +33,12 @@ class HasOffsetType implements CompoundType, AccessoryType
use NonRemoveableTypeTrait;
use NonGeneralizableTypeTrait;

private Type $offsetValueType;

/** @api */
public function __construct(private Type $offsetType)
public function __construct(private Type $offsetType, ?Type $offsetValueType = null)
{
$this->offsetValueType = $offsetValueType ?? new MixedType();
}

public function getOffsetType(): Type
Expand Down Expand Up @@ -64,7 +67,8 @@ public function isSuperTypeOf(Type $type): TrinaryLogic
return TrinaryLogic::createYes();
}
return $type->isOffsetAccessible()
->and($type->hasOffsetValueType($this->offsetType));
->and($type->hasOffsetValueType($this->offsetType))
->and($this->offsetValueType->isSuperTypeOf($type->getOffsetValueType($this->offsetType)));
Comment on lines 69 to +71
Copy link
Contributor Author

@herndlm herndlm Jul 13, 2022

Choose a reason for hiding this comment

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

not sure if isSuperTypeOf and friends are really correct, e.g. I did not touch isSubTypeOf or accept at all

}

public function isSubTypeOf(Type $otherType): TrinaryLogic
Expand Down Expand Up @@ -110,7 +114,7 @@ public function hasOffsetValueType(Type $offsetType): TrinaryLogic

public function getOffsetValueType(Type $offsetType): Type
{
return new MixedType();
return $this->offsetValueType;
herndlm marked this conversation as resolved.
Show resolved Hide resolved
}

public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $unionValues = true): Type
Expand Down
18 changes: 18 additions & 0 deletions src/Type/Constant/ConstantArrayType.php
Expand Up @@ -29,6 +29,7 @@
use PHPStan\Type\StringType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use PHPStan\Type\TypeTraverser;
use PHPStan\Type\UnionType;
use PHPStan\Type\VerbosityLevel;
use function array_keys;
Expand Down Expand Up @@ -532,6 +533,23 @@ public function unsetOffset(Type $offsetType): Type
return new ArrayType($this->getKeyType(), $this->getItemType());
}

public function getOffsetType(Type $offsetValueType): Type
herndlm marked this conversation as resolved.
Show resolved Hide resolved
{
return TypeTraverser::map($offsetValueType, function (Type $type, callable $traverse): Type {
if ($type instanceof UnionType || $type instanceof IntersectionType) {
return $traverse($type);
}

foreach ($this->valueTypes as $i => $valueType) {
if ($valueType->equals($type)) {
return $this->keyTypes[$i];
}
}

return new ErrorType(); // undefined offset value
});
}

public function isIterableAtLeastOnce(): TrinaryLogic
{
$keysCount = count($this->keyTypes);
Expand Down
35 changes: 24 additions & 11 deletions src/Type/Php/InArrayFunctionTypeSpecifyingExtension.php
Expand Up @@ -9,11 +9,13 @@
use PHPStan\Analyser\TypeSpecifierAwareExtension;
use PHPStan\Analyser\TypeSpecifierContext;
use PHPStan\Reflection\FunctionReflection;
use PHPStan\Type\Accessory\HasOffsetType;
use PHPStan\Type\Accessory\NonEmptyArrayType;
use PHPStan\Type\ArrayType;
use PHPStan\Type\Constant\ConstantArrayType;
use PHPStan\Type\Constant\ConstantBooleanType;
use PHPStan\Type\ConstantScalarType;
use PHPStan\Type\FunctionTypeSpecifyingExtension;
use PHPStan\Type\MixedType;
use PHPStan\Type\TypeCombinator;
use PHPStan\Type\TypeUtils;
use function count;
Expand Down Expand Up @@ -56,6 +58,7 @@ public function specifyTypes(FunctionReflection $functionReflection, FuncCall $n
|| count(TypeUtils::getConstantScalars($arrayValueType)) > 0
|| count(TypeUtils::getEnumCaseObjects($arrayValueType)) > 0
) {
// Specify needle type
$specifiedTypes = $this->typeSpecifier->create(
$node->getArgs()[0]->value,
$arrayValueType,
Expand All @@ -70,22 +73,32 @@ public function specifyTypes(FunctionReflection $functionReflection, FuncCall $n
|| count(TypeUtils::getConstantScalars($needleType)) > 0
|| count(TypeUtils::getEnumCaseObjects($needleType)) > 0
) {
// Specify haystack type
$arrayKeyType = $arrayType->getIterableKeyType();
if ($context->truthy()) {
$arrayType = TypeCombinator::intersect(
new ArrayType(new MixedType(), TypeCombinator::union($arrayValueType, $needleType)),
$newArrayType = TypeCombinator::intersect(
new ArrayType($arrayKeyType, TypeCombinator::union($arrayValueType, $needleType)),
new NonEmptyArrayType(),
);

if (!$needleType instanceof ConstantScalarType || !$arrayType->isIterableAtLeastOnce()->yes() || !$arrayValueType->equals($needleType)) {
// TODO: hasOffset(int, 'a')::isSuperTypeOf(non-empty-array<int, 'a'>) does not return yes because ArrayType::hasOffsetValueType returns maybe
herndlm marked this conversation as resolved.
Show resolved Hide resolved
$offsetType = $arrayType instanceof ConstantArrayType
? $arrayType->getOffsetType($needleType)
: $arrayKeyType;
$newArrayType = TypeCombinator::intersect($newArrayType, new HasOffsetType($offsetType, $needleType));
}
} else {
$arrayType = new ArrayType(new MixedType(), TypeCombinator::remove($arrayValueType, $needleType));
$newArrayType = new ArrayType($arrayKeyType, TypeCombinator::remove($arrayValueType, $needleType));
}

$specifiedTypes = $specifiedTypes->unionWith($this->typeSpecifier->create(
$node->getArgs()[1]->value,
$arrayType,
TypeSpecifierContext::createTrue(),
false,
$scope,
));
$specifiedTypes = $specifiedTypes->unionWith($this->typeSpecifier->create(
$node->getArgs()[1]->value,
$newArrayType,
TypeSpecifierContext::createTrue(),
false,
$scope,
));
}

return $specifiedTypes;
Expand Down
4 changes: 2 additions & 2 deletions tests/PHPStan/Analyser/data/in-array-non-empty.php
Expand Up @@ -14,15 +14,15 @@ class HelloWorld
public function sayHello(array $array): void
{
if(in_array("thing", $array, true)){
assertType('non-empty-array<int, string>', $array);
assertType('non-empty-array<int, string>&hasOffset(int)', $array);
}
}

/** @param array<int> $haystack */
public function nonConstantNeedle(int $needle, array $haystack): void
{
if (in_array($needle, $haystack, true)) {
assertType('non-empty-array<int>', $haystack);
assertType('non-empty-array<int>&hasOffset((int|string))', $haystack);
}
}
}
6 changes: 4 additions & 2 deletions tests/PHPStan/Analyser/data/in-array.php
Expand Up @@ -10,12 +10,14 @@ class Foo
* @param string $r
* @param $mixed
* @param string[] $strings
* @param string[] $moreStrings
*/
public function doFoo(
string $s,
string $r,
$mixed,
array $strings
array $strings,
array $moreStrings
)
{
if (!in_array($s, ['foo', 'bar'], true)) {
Expand All @@ -26,7 +28,7 @@ public function doFoo(
return;
}

if (in_array($r, $strings, true)) {
if (in_array($r, $moreStrings, true)) {
Copy link
Contributor Author

@herndlm herndlm Jul 13, 2022

Choose a reason for hiding this comment

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

I had to do that because $strings was already type-specified from previous conditionals in the same scope which messed the result up here since types are specified better now.

return;
}

Expand Down
Expand Up @@ -108,6 +108,10 @@ public function testImpossibleCheckTypeFunctionCall(): void
'Call to function in_array() with arguments \'bar\'|\'foo\', array{\'baz\', \'lorem\'} and true will always evaluate to false.',
244,
],
[
'Call to function in_array() with arguments \'bar\'|\'foo\', array{\'foo\', \'bar\'} and true will always evaluate to true.',
248,
],
[
'Call to function in_array() with arguments \'foo\', array{\'foo\'} and true will always evaluate to true.',
252,
Expand Down Expand Up @@ -474,6 +478,7 @@ public function testBugInArrayDateFormat(): void
[
'Call to function in_array() with arguments \'a\', non-empty-array<int, \'a\'> and true will always evaluate to true.',
39,
'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.',
herndlm marked this conversation as resolved.
Show resolved Hide resolved
],
[
'Call to function in_array() with arguments \'b\', non-empty-array<int, \'a\'> and true will always evaluate to false.',
Expand All @@ -483,6 +488,10 @@ public function testBugInArrayDateFormat(): void
'Call to function in_array() with arguments int, array{} and true will always evaluate to false.',
47,
],
[
'Call to function in_array() with arguments int, array<int, string> and true will always evaluate to false.',
61,
],
]);
}

Expand Down
13 changes: 13 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/check-type-function-call.php
Expand Up @@ -860,3 +860,16 @@ public function doFoo(int $i, array $is): void
}

}

class InArray2
{

/** @param non-empty-array<int, int> $haystack */
public function checkWithNonConstants(int $needle, array $haystack): void
{
if (in_array($needle, $haystack, true)) {

}
}

}