Implement Type::isOffsetAccessLegal() to detect offset access that crashes even in isset()

rajyan · web-flow · commit e55a83f81142 · 2024-05-07T14:18:39.000+02:00
diff --git a/src/Rules/Arrays/NonexistentOffsetInArrayDimFetchRule.php b/src/Rules/Arrays/NonexistentOffsetInArrayDimFetchRule.php
@@ -64,7 +64,7 @@ public function processNode(Node $node, Scope $scope): array
 			return [];
 		}
 
-		if ($scope->isUndefinedExpressionAllowed($node) && !$isOffsetAccessible->no()) {
+		if ($scope->isUndefinedExpressionAllowed($node) && $isOffsetAccessibleType->isOffsetAccessLegal()->yes()) {
 			return [];
 		}
 
diff --git a/src/Type/Accessory/AccessoryArrayListType.php b/src/Type/Accessory/AccessoryArrayListType.php
@@ -142,6 +142,11 @@ public function isOffsetAccessible(): TrinaryLogic
 		return TrinaryLogic::createYes();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createYes();
+	}
+
 	public function hasOffsetValueType(Type $offsetType): TrinaryLogic
 	{
 		return $this->getIterableKeyType()->isSuperTypeOf($offsetType)->and(TrinaryLogic::createMaybe());
diff --git a/src/Type/Accessory/AccessoryLiteralStringType.php b/src/Type/Accessory/AccessoryLiteralStringType.php
@@ -132,6 +132,11 @@ public function isOffsetAccessible(): TrinaryLogic
 		return TrinaryLogic::createYes();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createYes();
+	}
+
 	public function hasOffsetValueType(Type $offsetType): TrinaryLogic
 	{
 		return $offsetType->isInteger()->and(TrinaryLogic::createMaybe());
diff --git a/src/Type/Accessory/AccessoryNonEmptyStringType.php b/src/Type/Accessory/AccessoryNonEmptyStringType.php
@@ -134,6 +134,11 @@ public function isOffsetAccessible(): TrinaryLogic
 		return TrinaryLogic::createYes();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createYes();
+	}
+
 	public function hasOffsetValueType(Type $offsetType): TrinaryLogic
 	{
 		return $offsetType->isInteger()->and(TrinaryLogic::createMaybe());
diff --git a/src/Type/Accessory/AccessoryNonFalsyStringType.php b/src/Type/Accessory/AccessoryNonFalsyStringType.php
@@ -134,6 +134,11 @@ public function isOffsetAccessible(): TrinaryLogic
 		return TrinaryLogic::createYes();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createYes();
+	}
+
 	public function hasOffsetValueType(Type $offsetType): TrinaryLogic
 	{
 		return $offsetType->isInteger()->and(TrinaryLogic::createMaybe());
diff --git a/src/Type/Accessory/AccessoryNumericStringType.php b/src/Type/Accessory/AccessoryNumericStringType.php
@@ -137,6 +137,11 @@ public function isOffsetAccessible(): TrinaryLogic
 		return TrinaryLogic::createYes();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createYes();
+	}
+
 	public function hasOffsetValueType(Type $offsetType): TrinaryLogic
 	{
 		return $offsetType->isInteger()->and(TrinaryLogic::createMaybe());
diff --git a/src/Type/Accessory/HasMethodType.php b/src/Type/Accessory/HasMethodType.php
@@ -120,6 +120,11 @@ public function describe(VerbosityLevel $level): string
 		return sprintf('hasMethod(%s)', $this->methodName);
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createMaybe();
+	}
+
 	public function hasMethod(string $methodName): TrinaryLogic
 	{
 		if ($this->getCanonicalMethodName() === strtolower($methodName)) {
diff --git a/src/Type/Accessory/HasOffsetType.php b/src/Type/Accessory/HasOffsetType.php
@@ -138,6 +138,11 @@ public function isOffsetAccessible(): TrinaryLogic
 		return TrinaryLogic::createYes();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createYes();
+	}
+
 	public function hasOffsetValueType(Type $offsetType): TrinaryLogic
 	{
 		if ($offsetType->isConstantScalarValue()->yes() && $offsetType->equals($this->offsetType)) {
diff --git a/src/Type/Accessory/HasOffsetValueType.php b/src/Type/Accessory/HasOffsetValueType.php
@@ -146,6 +146,11 @@ public function isOffsetAccessible(): TrinaryLogic
 		return TrinaryLogic::createYes();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createYes();
+	}
+
 	public function hasOffsetValueType(Type $offsetType): TrinaryLogic
 	{
 		if ($offsetType->isConstantScalarValue()->yes() && $offsetType->equals($this->offsetType)) {
diff --git a/src/Type/Accessory/HasPropertyType.php b/src/Type/Accessory/HasPropertyType.php
@@ -118,6 +118,11 @@ public function describe(VerbosityLevel $level): string
 		return sprintf('hasProperty(%s)', $this->propertyName);
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createMaybe();
+	}
+
 	public function hasProperty(string $propertyName): TrinaryLogic
 	{
 		if ($this->propertyName === $propertyName) {
diff --git a/src/Type/Accessory/NonEmptyArrayType.php b/src/Type/Accessory/NonEmptyArrayType.php
@@ -139,6 +139,11 @@ public function isOffsetAccessible(): TrinaryLogic
 		return TrinaryLogic::createYes();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createYes();
+	}
+
 	public function hasOffsetValueType(Type $offsetType): TrinaryLogic
 	{
 		return TrinaryLogic::createMaybe();
diff --git a/src/Type/Accessory/OversizedArrayType.php b/src/Type/Accessory/OversizedArrayType.php
@@ -135,6 +135,11 @@ public function isOffsetAccessible(): TrinaryLogic
 		return TrinaryLogic::createYes();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createYes();
+	}
+
 	public function hasOffsetValueType(Type $offsetType): TrinaryLogic
 	{
 		return TrinaryLogic::createMaybe();
diff --git a/src/Type/ArrayType.php b/src/Type/ArrayType.php
@@ -393,6 +393,11 @@ public function isOffsetAccessible(): TrinaryLogic
 		return TrinaryLogic::createYes();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createYes();
+	}
+
 	public function hasOffsetValueType(Type $offsetType): TrinaryLogic
 	{
 		$offsetType = $offsetType->toArrayKey();
diff --git a/src/Type/BooleanType.php b/src/Type/BooleanType.php
@@ -96,6 +96,11 @@ public function toArrayKey(): Type
 		return new UnionType([new ConstantIntegerType(0), new ConstantIntegerType(1)]);
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createYes();
+	}
+
 	public function isNull(): TrinaryLogic
 	{
 		return TrinaryLogic::createNo();
diff --git a/src/Type/CallableType.php b/src/Type/CallableType.php
@@ -324,6 +324,11 @@ public function toArrayKey(): Type
 		return new ErrorType();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createMaybe();
+	}
+
 	public function getTemplateTypeMap(): TemplateTypeMap
 	{
 		return $this->templateTypeMap;
diff --git a/src/Type/ClosureType.php b/src/Type/ClosureType.php
@@ -261,6 +261,11 @@ function (): string {
 		);
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createNo();
+	}
+
 	public function isObject(): TrinaryLogic
 	{
 		return $this->objectType->isObject();
diff --git a/src/Type/FloatType.php b/src/Type/FloatType.php
@@ -144,6 +144,11 @@ public function toArrayKey(): Type
 		return new IntegerType();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createYes();
+	}
+
 	public function isNull(): TrinaryLogic
 	{
 		return TrinaryLogic::createNo();
diff --git a/src/Type/IntegerType.php b/src/Type/IntegerType.php
@@ -96,6 +96,11 @@ public function toArrayKey(): Type
 		return $this;
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createYes();
+	}
+
 	public function isNull(): TrinaryLogic
 	{
 		return TrinaryLogic::createNo();
diff --git a/src/Type/IntersectionType.php b/src/Type/IntersectionType.php
@@ -672,6 +672,11 @@ public function isOffsetAccessible(): TrinaryLogic
 		return $this->intersectResults(static fn (Type $type): TrinaryLogic => $type->isOffsetAccessible());
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return $this->intersectResults(static fn (Type $type): TrinaryLogic => $type->isOffsetAccessLegal());
+	}
+
 	public function hasOffsetValueType(Type $offsetType): TrinaryLogic
 	{
 		if ($this->isList()->yes() && $this->isIterableAtLeastOnce()->yes()) {
diff --git a/src/Type/IterableType.php b/src/Type/IterableType.php
@@ -242,6 +242,11 @@ public function toArrayKey(): Type
 		return new ErrorType();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createMaybe();
+	}
+
 	public function isIterable(): TrinaryLogic
 	{
 		return TrinaryLogic::createYes();
diff --git a/src/Type/MixedType.php b/src/Type/MixedType.php
@@ -566,6 +566,16 @@ public function isOffsetAccessible(): TrinaryLogic
 		return TrinaryLogic::createMaybe();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		if ($this->subtractedType !== null) {
+			if ($this->subtractedType->isSuperTypeOf(new ObjectWithoutClassType())->yes()) {
+				return TrinaryLogic::createYes();
+			}
+		}
+		return TrinaryLogic::createMaybe();
+	}
+
 	public function hasOffsetValueType(Type $offsetType): TrinaryLogic
 	{
 		if ($this->isOffsetAccessible()->no()) {
diff --git a/src/Type/NeverType.php b/src/Type/NeverType.php
@@ -257,6 +257,11 @@ public function isOffsetAccessible(): TrinaryLogic
 		return TrinaryLogic::createYes();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createYes();
+	}
+
 	public function hasOffsetValueType(Type $offsetType): TrinaryLogic
 	{
 		return TrinaryLogic::createYes();
diff --git a/src/Type/NonexistentParentClassType.php b/src/Type/NonexistentParentClassType.php
@@ -152,6 +152,11 @@ public function toArrayKey(): Type
 		return new ErrorType();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createNo();
+	}
+
 	public function isScalar(): TrinaryLogic
 	{
 		return TrinaryLogic::createNo();
diff --git a/src/Type/NullType.php b/src/Type/NullType.php
@@ -174,6 +174,11 @@ public function isOffsetAccessible(): TrinaryLogic
 		return TrinaryLogic::createYes();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createYes();
+	}
+
 	public function hasOffsetValueType(Type $offsetType): TrinaryLogic
 	{
 		return TrinaryLogic::createNo();
diff --git a/src/Type/ObjectShapeType.php b/src/Type/ObjectShapeType.php
@@ -428,6 +428,11 @@ public function describe(VerbosityLevel $level): string
 		);
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createMaybe();
+	}
+
 	public function getEnumCases(): array
 	{
 		return [];
diff --git a/src/Type/ObjectType.php b/src/Type/ObjectType.php
@@ -1099,6 +1099,11 @@ public function isOffsetAccessible(): TrinaryLogic
 		);
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return $this->isOffsetAccessible();
+	}
+
 	public function hasOffsetValueType(Type $offsetType): TrinaryLogic
 	{
 		if ($this->isInstanceOf(ArrayAccess::class)->yes()) {
diff --git a/src/Type/ObjectWithoutClassType.php b/src/Type/ObjectWithoutClassType.php
@@ -140,6 +140,11 @@ function () use ($level): string {
 		);
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createMaybe();
+	}
+
 	public function getEnumCases(): array
 	{
 		return [];
diff --git a/src/Type/ResourceType.php b/src/Type/ResourceType.php
@@ -86,6 +86,11 @@ public function toArrayKey(): Type
 		return new ErrorType();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createYes();
+	}
+
 	public function isScalar(): TrinaryLogic
 	{
 		return TrinaryLogic::createNo();
diff --git a/src/Type/StaticType.php b/src/Type/StaticType.php
@@ -375,6 +375,11 @@ public function isOffsetAccessible(): TrinaryLogic
 		return $this->getStaticObjectType()->isOffsetAccessible();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return $this->getStaticObjectType()->isOffsetAccessLegal();
+	}
+
 	public function hasOffsetValueType(Type $offsetType): TrinaryLogic
 	{
 		return $this->getStaticObjectType()->hasOffsetValueType($offsetType);
diff --git a/src/Type/StrictMixedType.php b/src/Type/StrictMixedType.php
@@ -310,6 +310,11 @@ public function isOffsetAccessible(): TrinaryLogic
 		return TrinaryLogic::createNo();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createNo();
+	}
+
 	public function hasOffsetValueType(Type $offsetType): TrinaryLogic
 	{
 		return TrinaryLogic::createNo();
diff --git a/src/Type/StringType.php b/src/Type/StringType.php
@@ -56,6 +56,11 @@ public function isOffsetAccessible(): TrinaryLogic
 		return TrinaryLogic::createYes();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return TrinaryLogic::createYes();
+	}
+
 	public function hasOffsetValueType(Type $offsetType): TrinaryLogic
 	{
 		return $offsetType->isInteger()->and(TrinaryLogic::createMaybe());
diff --git a/src/Type/Traits/LateResolvableTypeTrait.php b/src/Type/Traits/LateResolvableTypeTrait.php
@@ -222,6 +222,11 @@ public function isOffsetAccessible(): TrinaryLogic
 		return $this->resolve()->isOffsetAccessible();
 	}
 
+	public function isOffsetAccessLegal(): TrinaryLogic
+	{
+		return $this->resolve()->isOffsetAccessLegal();
+	}
+
 	public function hasOffsetValueType(Type $offsetType): TrinaryLogic
 	{
 		return $this->resolve()->hasOffsetValueType($offsetType);
diff --git a/src/Type/Type.php b/src/Type/Type.php
@@ -131,6 +131,8 @@ public function isList(): TrinaryLogic;
 
 	public function isOffsetAccessible(): TrinaryLogic;
 
+	public function isOffsetAccessLegal(): TrinaryLogic;
+
 	public function hasOffsetValueType(Type $offsetType): TrinaryLogic;
 
 	public function getOffsetValueType(Type $offsetType): Type;
diff --git a/src/Type/UnionType.php b/src/Type/UnionType.php
diff --git a/src/Type/VoidType.php b/src/Type/VoidType.php
diff --git a/tests/PHPStan/Levels/data/arrayDimFetches-7.json b/tests/PHPStan/Levels/data/arrayDimFetches-7.json
diff --git a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php
diff --git a/tests/PHPStan/Rules/Arrays/data/bug-10926.php b/tests/PHPStan/Rules/Arrays/data/bug-10926.php
diff --git a/tests/PHPStan/Rules/Arrays/data/offset-access-legal-non-existent-parent.php b/tests/PHPStan/Rules/Arrays/data/offset-access-legal-non-existent-parent.php
diff --git a/tests/PHPStan/Rules/Arrays/data/offset-access-legal.php b/tests/PHPStan/Rules/Arrays/data/offset-access-legal.php
diff --git a/tests/PHPStan/Type/MixedTypeTest.php b/tests/PHPStan/Type/MixedTypeTest.php

Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,7 @@ public function processNode(Node $node, Scope $scope): array`
`64`	`64`	`return [];`
`65`	`65`	`}`
`66`	`66`
`67`		`- if ($scope->isUndefinedExpressionAllowed($node) && !$isOffsetAccessible->no()) {`
	`67`	`+ if ($scope->isUndefinedExpressionAllowed($node) && $isOffsetAccessibleType->isOffsetAccessLegal()->yes()) {`
`68`	`68`	`return [];`
`69`	`69`	`}`
`70`	`70`
Original file line number	Diff line number	Diff line change
`@@ -142,6 +142,11 @@ public function isOffsetAccessible(): TrinaryLogic`
`142`	`142`	`return TrinaryLogic::createYes();`
`143`	`143`	`}`
`144`	`144`
	`145`	`+ public function isOffsetAccessLegal(): TrinaryLogic`
	`146`	`+ {`
	`147`	`+ return TrinaryLogic::createYes();`
	`148`	`+ }`
	`149`	`+`
`145`	`150`	`public function hasOffsetValueType(Type $offsetType): TrinaryLogic`
`146`	`151`	`{`
`147`	`152`	`return $this->getIterableKeyType()->isSuperTypeOf($offsetType)->and(TrinaryLogic::createMaybe());`
Original file line number	Diff line number	Diff line change
`@@ -132,6 +132,11 @@ public function isOffsetAccessible(): TrinaryLogic`
`132`	`132`	`return TrinaryLogic::createYes();`
`133`	`133`	`}`
`134`	`134`
	`135`	`+ public function isOffsetAccessLegal(): TrinaryLogic`
	`136`	`+ {`
	`137`	`+ return TrinaryLogic::createYes();`
	`138`	`+ }`
	`139`	`+`
`135`	`140`	`public function hasOffsetValueType(Type $offsetType): TrinaryLogic`
`136`	`141`	`{`
`137`	`142`	`return $offsetType->isInteger()->and(TrinaryLogic::createMaybe());`
Original file line number	Diff line number	Diff line change
`@@ -134,6 +134,11 @@ public function isOffsetAccessible(): TrinaryLogic`
`134`	`134`	`return TrinaryLogic::createYes();`
`135`	`135`	`}`
`136`	`136`
	`137`	`+ public function isOffsetAccessLegal(): TrinaryLogic`
	`138`	`+ {`
	`139`	`+ return TrinaryLogic::createYes();`
	`140`	`+ }`
	`141`	`+`
`137`	`142`	`public function hasOffsetValueType(Type $offsetType): TrinaryLogic`
`138`	`143`	`{`
`139`	`144`	`return $offsetType->isInteger()->and(TrinaryLogic::createMaybe());`
Original file line number	Diff line number	Diff line change
`@@ -137,6 +137,11 @@ public function isOffsetAccessible(): TrinaryLogic`
`137`	`137`	`return TrinaryLogic::createYes();`
`138`	`138`	`}`
`139`	`139`
	`140`	`+ public function isOffsetAccessLegal(): TrinaryLogic`
	`141`	`+ {`
	`142`	`+ return TrinaryLogic::createYes();`
	`143`	`+ }`
	`144`	`+`
`140`	`145`	`public function hasOffsetValueType(Type $offsetType): TrinaryLogic`
`141`	`146`	`{`
`142`	`147`	`return $offsetType->isInteger()->and(TrinaryLogic::createMaybe());`
Original file line number	Diff line number	Diff line change
`@@ -120,6 +120,11 @@ public function describe(VerbosityLevel $level): string`
`120`	`120`	`return sprintf('hasMethod(%s)', $this->methodName);`
`121`	`121`	`}`
`122`	`122`
	`123`	`+ public function isOffsetAccessLegal(): TrinaryLogic`
	`124`	`+ {`
	`125`	`+ return TrinaryLogic::createMaybe();`
	`126`	`+ }`
	`127`	`+`
`123`	`128`	`public function hasMethod(string $methodName): TrinaryLogic`
`124`	`129`	`{`
`125`	`130`	`if ($this->getCanonicalMethodName() === strtolower($methodName)) {`
Original file line number	Diff line number	Diff line change
`@@ -138,6 +138,11 @@ public function isOffsetAccessible(): TrinaryLogic`
`138`	`138`	`return TrinaryLogic::createYes();`
`139`	`139`	`}`
`140`	`140`
	`141`	`+ public function isOffsetAccessLegal(): TrinaryLogic`
	`142`	`+ {`
	`143`	`+ return TrinaryLogic::createYes();`
	`144`	`+ }`
	`145`	`+`
`141`	`146`	`public function hasOffsetValueType(Type $offsetType): TrinaryLogic`
`142`	`147`	`{`
`143`	`148`	`if ($offsetType->isConstantScalarValue()->yes() && $offsetType->equals($this->offsetType)) {`
Original file line number	Diff line number	Diff line change
`@@ -146,6 +146,11 @@ public function isOffsetAccessible(): TrinaryLogic`
`146`	`146`	`return TrinaryLogic::createYes();`
`147`	`147`	`}`
`148`	`148`
	`149`	`+ public function isOffsetAccessLegal(): TrinaryLogic`
	`150`	`+ {`
	`151`	`+ return TrinaryLogic::createYes();`
	`152`	`+ }`
	`153`	`+`
`149`	`154`	`public function hasOffsetValueType(Type $offsetType): TrinaryLogic`
`150`	`155`	`{`
`151`	`156`	`if ($offsetType->isConstantScalarValue()->yes() && $offsetType->equals($this->offsetType)) {`
Original file line number	Diff line number	Diff line change
`@@ -118,6 +118,11 @@ public function describe(VerbosityLevel $level): string`
`118`	`118`	`return sprintf('hasProperty(%s)', $this->propertyName);`
`119`	`119`	`}`
`120`	`120`
	`121`	`+ public function isOffsetAccessLegal(): TrinaryLogic`
	`122`	`+ {`
	`123`	`+ return TrinaryLogic::createMaybe();`
	`124`	`+ }`
	`125`	`+`
`121`	`126`	`public function hasProperty(string $propertyName): TrinaryLogic`
`122`	`127`	`{`
`123`	`128`	`if ($this->propertyName === $propertyName) {`