From 849b39b322f4a1144dc422d2f68a3dc1f1c9b906 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 31 Oct 2024 09:30:15 +0100 Subject: [PATCH 1/5] Report invalid string offsets --- src/Type/StringType.php | 3 +- ...nexistentOffsetInArrayDimFetchRuleTest.php | 26 +++++++++++++++ tests/PHPStan/Rules/Arrays/data/bug-11946.php | 33 +++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Rules/Arrays/data/bug-11946.php diff --git a/src/Type/StringType.php b/src/Type/StringType.php index 4605c92efe..64e5ade4e4 100644 --- a/src/Type/StringType.php +++ b/src/Type/StringType.php @@ -63,7 +63,8 @@ public function isOffsetAccessLegal(): TrinaryLogic public function hasOffsetValueType(Type $offsetType): TrinaryLogic { - return $offsetType->isInteger()->and(TrinaryLogic::createMaybe()); + $zeroOrMore = IntegerRangeType::fromInterval(0, null); + return $zeroOrMore->isSuperTypeOf($offsetType)->result->and(TrinaryLogic::createMaybe()); } public function getOffsetValueType(Type $offsetType): Type diff --git a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php index e212c65218..47d62b214c 100644 --- a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php @@ -780,4 +780,30 @@ public function testInternalClassesWithOverloadedOffsetAccessInvalid84(): void $this->analyse([__DIR__ . '/data/internal-classes-overload-offset-access-invalid-php84.php'], []); } + public function testBug11946(): void + { + $this->analyse([__DIR__ . '/data/bug-11946.php'], [ + [ + 'Offset -1 does not exist on string.', + 21, + ], + [ + 'Offset -1 does not exist on numeric-string.', + 22, + ], + [ + 'Offset -1 does not exist on non-empty-string.', + 23, + ], + [ + 'Offset -1 does not exist on non-falsy-string.', + 24, + ], + [ + 'Offset -1 does not exist on string.', + 25, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Arrays/data/bug-11946.php b/tests/PHPStan/Rules/Arrays/data/bug-11946.php new file mode 100644 index 0000000000..c896b2853c --- /dev/null +++ b/tests/PHPStan/Rules/Arrays/data/bug-11946.php @@ -0,0 +1,33 @@ + Date: Thu, 31 Oct 2024 09:49:15 +0100 Subject: [PATCH 2/5] Report maybe invalid string int-offsets --- .../NonexistentOffsetInArrayDimFetchCheck.php | 10 +++++++ ...nexistentOffsetInArrayDimFetchRuleTest.php | 20 +++++++++++++ tests/PHPStan/Rules/Arrays/data/bug-11946.php | 28 +++++++++++++++---- 3 files changed, 53 insertions(+), 5 deletions(-) diff --git a/src/Rules/Arrays/NonexistentOffsetInArrayDimFetchCheck.php b/src/Rules/Arrays/NonexistentOffsetInArrayDimFetchCheck.php index 5b81f82f1a..bf5faaa3a0 100644 --- a/src/Rules/Arrays/NonexistentOffsetInArrayDimFetchCheck.php +++ b/src/Rules/Arrays/NonexistentOffsetInArrayDimFetchCheck.php @@ -10,6 +10,7 @@ use PHPStan\Rules\RuleLevelHelper; use PHPStan\Type\BenevolentUnionType; use PHPStan\Type\ErrorType; +use PHPStan\Type\IntegerRangeType; use PHPStan\Type\Type; use PHPStan\Type\TypeUtils; use PHPStan\Type\UnionType; @@ -65,6 +66,15 @@ public function check( if ($this->reportMaybes) { $report = false; + $zeroOrMore = IntegerRangeType::fromInterval(0, null); + if ( + $type->isString()->yes() + && $dimType->isInteger()->yes() + && !$zeroOrMore->isSuperTypeOf($dimType)->yes() + ) { + $report = true; + } + if ($type instanceof BenevolentUnionType) { $flattenedTypes = [$type]; } else { diff --git a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php index 47d62b214c..c3d1d446ee 100644 --- a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php @@ -803,6 +803,26 @@ public function testBug11946(): void 'Offset -1 does not exist on string.', 25, ], + [ + 'Offset int<-5, 5> might not exist on string.', + 45 + ], + [ + 'Offset int<-5, 5> might not exist on numeric-string.', + 46 + ], + [ + 'Offset int<-5, 5> might not exist on non-empty-string.', + 47 + ], + [ + 'Offset int<-5, 5> might not exist on non-falsy-string.', + 48 + ], + [ + 'Offset int<-5, 5> might not exist on string.', + 49 + ], ]); } diff --git a/tests/PHPStan/Rules/Arrays/data/bug-11946.php b/tests/PHPStan/Rules/Arrays/data/bug-11946.php index c896b2853c..c7d75e7440 100644 --- a/tests/PHPStan/Rules/Arrays/data/bug-11946.php +++ b/tests/PHPStan/Rules/Arrays/data/bug-11946.php @@ -23,11 +23,29 @@ public function nonExistentStringOffset( echo $nonEmpty[-1]; echo $nonFalsy[-1]; echo $lowerCase[-1]; + } - echo $s[0]; - echo $numericS[0]; - echo $nonEmpty[0]; - echo $nonFalsy[0]; - echo $lowerCase[0]; + /** + * @param numeric-string $numericS + * @param non-empty-string $nonEmpty + * @param non-falsy-string $nonFalsy + * @param lowercase-string $lowerCase + * + * @param int<-5, 5> $maybeWrong + */ + public function maybeNonExistentStringOffset( + string $s, + string $numericS, + string $nonEmpty, + string $nonFalsy, + string $lowerCase, + int $maybeWrong + ) + { + echo $s[$maybeWrong]; + echo $numericS[$maybeWrong]; + echo $nonEmpty[$maybeWrong]; + echo $nonFalsy[$maybeWrong]; + echo $lowerCase[$maybeWrong]; } } From be60a88731ffe97e43fba0481a3c1e8412203a06 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 31 Oct 2024 09:55:03 +0100 Subject: [PATCH 3/5] Add `reportPossiblyNonexistentStringOffset` parameter --- conf/bleedingEdge.neon | 1 + conf/config.neon | 2 ++ conf/parametersSchema.neon | 1 + .../NonexistentOffsetInArrayDimFetchCheck.php | 6 ++++-- .../Rules/Arrays/ArrayDestructuringRuleTest.php | 2 +- .../NonexistentOffsetInArrayDimFetchRuleTest.php | 16 ++++++++++------ tests/PHPStan/Rules/Arrays/data/bug-11946.php | 2 +- 7 files changed, 20 insertions(+), 10 deletions(-) diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 76dd0d8904..9a945a7c63 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -5,3 +5,4 @@ parameters: skipCheckGenericClasses!: [] stricterFunctionMap: true reportPreciseLineForUnusedFunctionParameter: true + reportPossiblyNonexistentStringOffset: true diff --git a/conf/config.neon b/conf/config.neon index d77e889531..e4d65fc858 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -63,6 +63,7 @@ parameters: reportAnyTypeWideningInVarTag: false reportPossiblyNonexistentGeneralArrayOffset: false reportPossiblyNonexistentConstantArrayOffset: false + reportPossiblyNonexistentStringOffset: false checkMissingOverrideMethodAttribute: false mixinExcludeClasses: [] scanFiles: [] @@ -837,6 +838,7 @@ services: reportMaybes: %reportMaybes% reportPossiblyNonexistentGeneralArrayOffset: %reportPossiblyNonexistentGeneralArrayOffset% reportPossiblyNonexistentConstantArrayOffset: %reportPossiblyNonexistentConstantArrayOffset% + reportPossiblyNonexistentStringOffset: %reportPossiblyNonexistentStringOffset% - class: PHPStan\Rules\ClassNameCheck diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index f7328f7863..4834472c6f 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -74,6 +74,7 @@ parametersSchema: reportAnyTypeWideningInVarTag: bool() reportPossiblyNonexistentGeneralArrayOffset: bool() reportPossiblyNonexistentConstantArrayOffset: bool() + reportPossiblyNonexistentStringOffset: bool() checkMissingOverrideMethodAttribute: bool() parallel: structure([ jobSize: int(), diff --git a/src/Rules/Arrays/NonexistentOffsetInArrayDimFetchCheck.php b/src/Rules/Arrays/NonexistentOffsetInArrayDimFetchCheck.php index bf5faaa3a0..7b15273295 100644 --- a/src/Rules/Arrays/NonexistentOffsetInArrayDimFetchCheck.php +++ b/src/Rules/Arrays/NonexistentOffsetInArrayDimFetchCheck.php @@ -26,6 +26,7 @@ public function __construct( private bool $reportMaybes, private bool $reportPossiblyNonexistentGeneralArrayOffset, private bool $reportPossiblyNonexistentConstantArrayOffset, + private bool $reportPossiblyNonexistentStringOffset, ) { } @@ -68,8 +69,9 @@ public function check( $zeroOrMore = IntegerRangeType::fromInterval(0, null); if ( - $type->isString()->yes() - && $dimType->isInteger()->yes() + $this->reportPossiblyNonexistentStringOffset + && $type->isString()->yes() + && $dimType instanceof IntegerRangeType && !$zeroOrMore->isSuperTypeOf($dimType)->yes() ) { $report = true; diff --git a/tests/PHPStan/Rules/Arrays/ArrayDestructuringRuleTest.php b/tests/PHPStan/Rules/Arrays/ArrayDestructuringRuleTest.php index d37f09084b..8e2696da10 100644 --- a/tests/PHPStan/Rules/Arrays/ArrayDestructuringRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/ArrayDestructuringRuleTest.php @@ -19,7 +19,7 @@ protected function getRule(): Rule return new ArrayDestructuringRule( $ruleLevelHelper, - new NonexistentOffsetInArrayDimFetchCheck($ruleLevelHelper, true, false, false), + new NonexistentOffsetInArrayDimFetchCheck($ruleLevelHelper, true, false, false, false), ); } diff --git a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php index c3d1d446ee..b197dbbb40 100644 --- a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php @@ -21,13 +21,15 @@ class NonexistentOffsetInArrayDimFetchRuleTest extends RuleTestCase private bool $reportPossiblyNonexistentConstantArrayOffset = false; + private bool $reportPossiblyNonexistentStringOffset = false; + protected function getRule(): Rule { $ruleLevelHelper = new RuleLevelHelper($this->createReflectionProvider(), true, false, true, $this->checkExplicitMixed, $this->checkImplicitMixed, false); return new NonexistentOffsetInArrayDimFetchRule( $ruleLevelHelper, - new NonexistentOffsetInArrayDimFetchCheck($ruleLevelHelper, true, $this->reportPossiblyNonexistentGeneralArrayOffset, $this->reportPossiblyNonexistentConstantArrayOffset), + new NonexistentOffsetInArrayDimFetchCheck($ruleLevelHelper, true, $this->reportPossiblyNonexistentGeneralArrayOffset, $this->reportPossiblyNonexistentConstantArrayOffset, $this->reportPossiblyNonexistentStringOffset), true, ); } @@ -782,6 +784,8 @@ public function testInternalClassesWithOverloadedOffsetAccessInvalid84(): void public function testBug11946(): void { + $this->reportPossiblyNonexistentStringOffset = true; + $this->analyse([__DIR__ . '/data/bug-11946.php'], [ [ 'Offset -1 does not exist on string.', @@ -805,23 +809,23 @@ public function testBug11946(): void ], [ 'Offset int<-5, 5> might not exist on string.', - 45 + 45, ], [ 'Offset int<-5, 5> might not exist on numeric-string.', - 46 + 46, ], [ 'Offset int<-5, 5> might not exist on non-empty-string.', - 47 + 47, ], [ 'Offset int<-5, 5> might not exist on non-falsy-string.', - 48 + 48, ], [ 'Offset int<-5, 5> might not exist on string.', - 49 + 49, ], ]); } diff --git a/tests/PHPStan/Rules/Arrays/data/bug-11946.php b/tests/PHPStan/Rules/Arrays/data/bug-11946.php index c7d75e7440..2a4ff143af 100644 --- a/tests/PHPStan/Rules/Arrays/data/bug-11946.php +++ b/tests/PHPStan/Rules/Arrays/data/bug-11946.php @@ -15,7 +15,7 @@ public function nonExistentStringOffset( string $numericS, string $nonEmpty, string $nonFalsy, - string $lowerCase, + string $lowerCase ) { echo $s[-1]; From 9963fa9e58fa4a6deb6e19da7445fc5c30b96b00 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 31 Oct 2024 10:23:15 +0100 Subject: [PATCH 4/5] test constant strings --- ...onexistentOffsetInArrayDimFetchRuleTest.php | 18 +++++++++++++----- tests/PHPStan/Rules/Arrays/data/bug-11946.php | 7 +++++++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php index b197dbbb40..a45541ff64 100644 --- a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php @@ -807,25 +807,33 @@ public function testBug11946(): void 'Offset -1 does not exist on string.', 25, ], + [ + "Offset 10 does not exist on 'hi'.", + 29, + ], [ 'Offset int<-5, 5> might not exist on string.', - 45, + 49, ], [ 'Offset int<-5, 5> might not exist on numeric-string.', - 46, + 50, ], [ 'Offset int<-5, 5> might not exist on non-empty-string.', - 47, + 51, ], [ 'Offset int<-5, 5> might not exist on non-falsy-string.', - 48, + 52, ], [ 'Offset int<-5, 5> might not exist on string.', - 49, + 53, + ], + [ + "Offset int<-5, 5> might not exist on 'hi'.", + 56, ], ]); } diff --git a/tests/PHPStan/Rules/Arrays/data/bug-11946.php b/tests/PHPStan/Rules/Arrays/data/bug-11946.php index 2a4ff143af..b890512b1b 100644 --- a/tests/PHPStan/Rules/Arrays/data/bug-11946.php +++ b/tests/PHPStan/Rules/Arrays/data/bug-11946.php @@ -23,6 +23,10 @@ public function nonExistentStringOffset( echo $nonEmpty[-1]; echo $nonFalsy[-1]; echo $lowerCase[-1]; + + $s = 'hi'; + echo $s[1]; + echo $s[10]; } /** @@ -47,5 +51,8 @@ public function maybeNonExistentStringOffset( echo $nonEmpty[$maybeWrong]; echo $nonFalsy[$maybeWrong]; echo $lowerCase[$maybeWrong]; + + $s = 'hi'; + echo $s[$maybeWrong]; } } From 08359cac088d599bfea6ab7ee6045a359ad72cac Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 31 Oct 2024 15:26:44 +0100 Subject: [PATCH 5/5] use featureToggle --- conf/config.neon | 4 ++-- conf/parametersSchema.neon | 2 +- .../Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php | 2 +- tests/PHPStan/Rules/Arrays/data/bug-11946.php | 7 +++++-- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/conf/config.neon b/conf/config.neon index e4d65fc858..54277ed99e 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -26,6 +26,7 @@ parameters: skipCheckGenericClasses: [] stricterFunctionMap: false reportPreciseLineForUnusedFunctionParameter: false + reportPossiblyNonexistentStringOffset: false fileExtensions: - php checkAdvancedIsset: false @@ -63,7 +64,6 @@ parameters: reportAnyTypeWideningInVarTag: false reportPossiblyNonexistentGeneralArrayOffset: false reportPossiblyNonexistentConstantArrayOffset: false - reportPossiblyNonexistentStringOffset: false checkMissingOverrideMethodAttribute: false mixinExcludeClasses: [] scanFiles: [] @@ -838,7 +838,7 @@ services: reportMaybes: %reportMaybes% reportPossiblyNonexistentGeneralArrayOffset: %reportPossiblyNonexistentGeneralArrayOffset% reportPossiblyNonexistentConstantArrayOffset: %reportPossiblyNonexistentConstantArrayOffset% - reportPossiblyNonexistentStringOffset: %reportPossiblyNonexistentStringOffset% + reportPossiblyNonexistentStringOffset: %featureToggles.reportPossiblyNonexistentStringOffset% - class: PHPStan\Rules\ClassNameCheck diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index 4834472c6f..facabc44b9 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -32,6 +32,7 @@ parametersSchema: skipCheckGenericClasses: listOf(string()), stricterFunctionMap: bool() reportPreciseLineForUnusedFunctionParameter: bool() + reportPossiblyNonexistentStringOffset: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() @@ -74,7 +75,6 @@ parametersSchema: reportAnyTypeWideningInVarTag: bool() reportPossiblyNonexistentGeneralArrayOffset: bool() reportPossiblyNonexistentConstantArrayOffset: bool() - reportPossiblyNonexistentStringOffset: bool() checkMissingOverrideMethodAttribute: bool() parallel: structure([ jobSize: int(), diff --git a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php index a45541ff64..a57adc7da4 100644 --- a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php @@ -832,7 +832,7 @@ public function testBug11946(): void 53, ], [ - "Offset int<-5, 5> might not exist on 'hi'.", + "Offset int<-5, 5> might not exist on 'hia'.", 56, ], ]); diff --git a/tests/PHPStan/Rules/Arrays/data/bug-11946.php b/tests/PHPStan/Rules/Arrays/data/bug-11946.php index b890512b1b..e0479377d2 100644 --- a/tests/PHPStan/Rules/Arrays/data/bug-11946.php +++ b/tests/PHPStan/Rules/Arrays/data/bug-11946.php @@ -43,7 +43,7 @@ public function maybeNonExistentStringOffset( string $nonEmpty, string $nonFalsy, string $lowerCase, - int $maybeWrong + int $maybeWrong, int $oneToTwo ) { echo $s[$maybeWrong]; @@ -52,7 +52,10 @@ public function maybeNonExistentStringOffset( echo $nonFalsy[$maybeWrong]; echo $lowerCase[$maybeWrong]; - $s = 'hi'; + $s = 'hia'; echo $s[$maybeWrong]; + if ($maybeWrong >= 1 && $maybeWrong < 3) { + echo $s[$maybeWrong]; + } } }