From 8b30032ed5c7637880ea4ff7b631b8d4d22dcbab Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 16 Jan 2025 09:33:59 +0100 Subject: [PATCH 1/3] Fix negative offset false positive on constant string --- src/Type/Constant/ConstantStringType.php | 7 ++++--- tests/PHPStan/Analyser/nsrt/string-offsets.php | 18 ++++++++++++++++-- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/Type/Constant/ConstantStringType.php b/src/Type/Constant/ConstantStringType.php index f0e78cf903..c0ad9dee9e 100644 --- a/src/Type/Constant/ConstantStringType.php +++ b/src/Type/Constant/ConstantStringType.php @@ -366,7 +366,7 @@ public function isUppercaseString(): TrinaryLogic public function hasOffsetValueType(Type $offsetType): TrinaryLogic { if ($offsetType->isInteger()->yes()) { - $strLenType = IntegerRangeType::fromInterval(0, strlen($this->value) - 1); + $strLenType = IntegerRangeType::fromInterval(-strlen($this->value), strlen($this->value) - 1); return $strLenType->isSuperTypeOf($offsetType); } @@ -376,15 +376,16 @@ public function hasOffsetValueType(Type $offsetType): TrinaryLogic public function getOffsetValueType(Type $offsetType): Type { if ($offsetType->isInteger()->yes()) { + $strLenType = IntegerRangeType::fromInterval(-strlen($this->value), strlen($this->value) - 1); + if ($offsetType instanceof ConstantIntegerType) { - if ($offsetType->getValue() < strlen($this->value)) { + if ($strLenType->isSuperTypeOf($offsetType)->yes()) { return new self($this->value[$offsetType->getValue()]); } return new ErrorType(); } - $strLenType = IntegerRangeType::fromInterval(0, strlen($this->value) - 1); $intersected = TypeCombinator::intersect($strLenType, $offsetType); if ($intersected instanceof IntegerRangeType) { $finiteTypes = $intersected->getFiniteTypes(); diff --git a/tests/PHPStan/Analyser/nsrt/string-offsets.php b/tests/PHPStan/Analyser/nsrt/string-offsets.php index 6fc8eeabd0..449246f707 100644 --- a/tests/PHPStan/Analyser/nsrt/string-offsets.php +++ b/tests/PHPStan/Analyser/nsrt/string-offsets.php @@ -9,11 +9,12 @@ * @param int<3, 10> $threeToTen * @param int<10, max> $tenOrMore * @param int<-10, -5> $negative + * @param int $smallerMinusSix * @param lowercase-string $lowercase * * @return void */ -function doFoo($oneToThree, $threeToTen, $tenOrMore, $negative, int $i, string $lowercase) { +function doFoo($oneToThree, $threeToTen, $tenOrMore, $negative, int $smallerMinusSix, int $i, string $lowercase) { $s = "world"; if (rand(0, 1)) { $s = "hello"; @@ -26,10 +27,23 @@ function doFoo($oneToThree, $threeToTen, $tenOrMore, $negative, int $i, string $ assertType("'e'|'l'|'o'|'r'", $s[$oneToThree]); assertType('*ERROR*', $s[$tenOrMore]); assertType("''|'d'|'l'|'o'", $s[$threeToTen]); - assertType("*ERROR*", $s[$negative]); + assertType("non-empty-string", $s[$negative]); + assertType("*ERROR*", $s[$smallerMinusSix]); $longString = "myF5HnJv799kWf8VRI7g97vwnABTwN9y2CzAVELCBfRqyqkdTzXg7BkGXcwuIOscAiT6tSuJGzVZOJnYXvkiKQzYBNjjkCPOzSKXR5YHRlVxV1BetqZz4XOmaH9mtacJ9azNYL6bNXezSBjX13BSZy02SK2udzQLbTPNQwlKadKaNkUxjtWegkb8QDFaXbzH1JENVSLVH0FYd6POBU82X1xu7FDDKYLzwsWJHBGVhG8iugjEGwLj22x5ViosUyKR"; assertType("non-empty-string", $longString[$i]); assertType("lowercase-string&non-empty-string", $lowercase[$i]); } + +function bug12122() +{ + // see https://3v4l.org/8EMdX + $foo = 'fo'; + assertType('*ERROR*', $foo[2]); + assertType("'o'", $foo[1]); + assertType("'f'", $foo[0]); + assertType("'o'", $foo[-1]); + assertType("'f'", $foo[-2]); + assertType('*ERROR*', $foo[-3]); +} From 9c8fac6d8392fffcc451447e543ab6304da005a6 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 16 Jan 2025 09:55:49 +0100 Subject: [PATCH 2/3] added rule test --- .../Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php | 5 +++++ tests/PHPStan/Rules/Arrays/data/bug-12122.php | 8 ++++++++ 2 files changed, 13 insertions(+) create mode 100644 tests/PHPStan/Rules/Arrays/data/bug-12122.php diff --git a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php index 6699adb5f7..aa4d8cd83b 100644 --- a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php @@ -924,4 +924,9 @@ public function testInternalClassesWithOverloadedOffsetAccessInvalid84(): void $this->analyse([__DIR__ . '/data/internal-classes-overload-offset-access-invalid-php84.php'], []); } + public function testBug12122(): void + { + $this->analyse([__DIR__ . '/data/bug-12122.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Arrays/data/bug-12122.php b/tests/PHPStan/Rules/Arrays/data/bug-12122.php new file mode 100644 index 0000000000..acd1816675 --- /dev/null +++ b/tests/PHPStan/Rules/Arrays/data/bug-12122.php @@ -0,0 +1,8 @@ + Date: Fri, 24 Jan 2025 16:47:59 +0100 Subject: [PATCH 3/3] Update ConstantStringType.php --- src/Type/Constant/ConstantStringType.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Type/Constant/ConstantStringType.php b/src/Type/Constant/ConstantStringType.php index c0ad9dee9e..7b0f109b7c 100644 --- a/src/Type/Constant/ConstantStringType.php +++ b/src/Type/Constant/ConstantStringType.php @@ -366,7 +366,8 @@ public function isUppercaseString(): TrinaryLogic public function hasOffsetValueType(Type $offsetType): TrinaryLogic { if ($offsetType->isInteger()->yes()) { - $strLenType = IntegerRangeType::fromInterval(-strlen($this->value), strlen($this->value) - 1); + $strlen = strlen($this->value); + $strLenType = IntegerRangeType::fromInterval(-$strlen, $strlen - 1); return $strLenType->isSuperTypeOf($offsetType); } @@ -376,7 +377,8 @@ public function hasOffsetValueType(Type $offsetType): TrinaryLogic public function getOffsetValueType(Type $offsetType): Type { if ($offsetType->isInteger()->yes()) { - $strLenType = IntegerRangeType::fromInterval(-strlen($this->value), strlen($this->value) - 1); + $strlen = strlen($this->value); + $strLenType = IntegerRangeType::fromInterval(-$strlen, $strlen - 1); if ($offsetType instanceof ConstantIntegerType) { if ($strLenType->isSuperTypeOf($offsetType)->yes()) {