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
Changes from 4 commits
e994f69
b02bd1b
12c1301
7b35446
7d3a859
46a92e1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,9 +9,12 @@ | |
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; | ||
|
@@ -56,6 +59,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, | ||
|
@@ -65,27 +69,44 @@ public function specifyTypes(FunctionReflection $functionReflection, FuncCall $n | |
); | ||
} | ||
|
||
// "Simple" all constant cases that always evaluate to true still need a nudge (e.g. needle 'foo' in haystack {'foo', 'bar'}) | ||
if ($arrayType instanceof ConstantArrayType && $needleType instanceof ConstantScalarType) { | ||
foreach ($arrayType->getValueTypes() as $i => $valueType) { | ||
if (!$arrayType->isOptionalKey($i) && $valueType->equals($needleType)) { | ||
return $specifiedTypes; | ||
} | ||
} | ||
} | ||
|
||
// If e.g. needle is 'a' and haystack non-empty-array<int, 'a'> we can be sure that this always evaluates to true | ||
// Belows HasOffset::isSuperTypeOf cannot deal with that since it calls ArrayType::hasOffsetValueType and that returns maybe at max | ||
if ($needleType instanceof ConstantScalarType && $arrayType->isIterableAtLeastOnce()->yes() && $arrayValueType->equals($needleType)) { | ||
return $specifiedTypes; | ||
} | ||
|
||
herndlm marked this conversation as resolved.
Show resolved
Hide resolved
Comment on lines
+87
to
+92
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also dislike this edge case, maybe it is possible to adapt |
||
if ( | ||
$context->truthy() | ||
|| count(TypeUtils::getConstantScalars($needleType)) > 0 | ||
|| count(TypeUtils::getEnumCaseObjects($needleType)) > 0 | ||
) { | ||
// Specify haystack type | ||
if ($context->truthy()) { | ||
$arrayType = TypeCombinator::intersect( | ||
$newArrayType = TypeCombinator::intersect( | ||
new ArrayType(new MixedType(), TypeCombinator::union($arrayValueType, $needleType)), | ||
new HasOffsetType(new MixedType(), $needleType), | ||
new NonEmptyArrayType(), | ||
); | ||
} else { | ||
$arrayType = new ArrayType(new MixedType(), TypeCombinator::remove($arrayValueType, $needleType)); | ||
$newArrayType = new ArrayType(new MixedType(), 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; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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(mixed)', $array); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is obviously weird. I'd prefer to rather have an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe print offset and offsetValue type within the bracket, like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that would be easy to do and makes sense, yeah. I did not do it yet because I was scared that it will lead to too many changes in e.g. baselines. but we can't think always about that, right? I'll check it out :) |
||
} | ||
} | ||
|
||
/** @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(mixed)', $haystack); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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)) { | ||
|
@@ -26,7 +28,7 @@ public function doFoo( | |
return; | ||
} | ||
|
||
if (in_array($r, $strings, true)) { | ||
if (in_array($r, $moreStrings, true)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had to do that because |
||
return; | ||
} | ||
|
||
|
There was a problem hiding this comment.
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 touchisSubTypeOf
oraccept
at all