From 04020f904e886d3c11cf4c49fcb3c37a15043d8c Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 11 Dec 2024 15:07:38 +0000 Subject: [PATCH 1/2] Add an ArrayAccessMode enum to better analyse offset accesses --- .../NonexistentOffsetInArrayDimFetchRule.php | 15 +++++++++++++-- src/Type/AccessOffsetMode.php | 15 +++++++++++++++ src/Type/Accessory/AccessoryArrayListType.php | 3 ++- .../Accessory/AccessoryLiteralStringType.php | 8 +++----- .../Accessory/AccessoryLowercaseStringType.php | 8 +++----- .../Accessory/AccessoryNonEmptyStringType.php | 8 +++----- .../Accessory/AccessoryNonFalsyStringType.php | 9 +++------ .../Accessory/AccessoryNumericStringType.php | 8 +++----- .../Accessory/AccessoryUppercaseStringType.php | 8 +++----- src/Type/Accessory/HasMethodType.php | 3 ++- src/Type/Accessory/HasOffsetType.php | 3 ++- src/Type/Accessory/HasOffsetValueType.php | 3 ++- src/Type/Accessory/HasPropertyType.php | 3 ++- src/Type/Accessory/NonEmptyArrayType.php | 3 ++- src/Type/Accessory/OversizedArrayType.php | 3 ++- src/Type/BooleanType.php | 3 ++- src/Type/CallableType.php | 2 +- src/Type/ClosureType.php | 2 +- src/Type/FloatType.php | 2 +- src/Type/IntegerType.php | 2 +- src/Type/IntersectionType.php | 4 ++-- src/Type/IterableType.php | 2 +- src/Type/MixedType.php | 2 +- src/Type/NeverType.php | 2 +- src/Type/NonexistentParentClassType.php | 2 +- src/Type/NullType.php | 2 +- src/Type/ObjectShapeType.php | 2 +- src/Type/ObjectType.php | 3 ++- src/Type/ObjectWithoutClassType.php | 2 +- src/Type/ResourceType.php | 3 ++- src/Type/StaticType.php | 4 ++-- src/Type/StrictMixedType.php | 2 +- src/Type/StringType.php | 7 ++----- src/Type/Traits/ArrayTypeTrait.php | 3 ++- src/Type/Traits/LateResolvableTypeTrait.php | 5 +++-- src/Type/Traits/StringTypeTrait.php | 18 ++++++++++++++++++ src/Type/Type.php | 2 +- src/Type/UnionType.php | 4 ++-- src/Type/VoidType.php | 2 +- tests/PHPStan/Type/MixedTypeTest.php | 3 ++- 40 files changed, 113 insertions(+), 72 deletions(-) create mode 100644 src/Type/AccessOffsetMode.php create mode 100644 src/Type/Traits/StringTypeTrait.php diff --git a/src/Rules/Arrays/NonexistentOffsetInArrayDimFetchRule.php b/src/Rules/Arrays/NonexistentOffsetInArrayDimFetchRule.php index d3ef021189..78b23e9813 100644 --- a/src/Rules/Arrays/NonexistentOffsetInArrayDimFetchRule.php +++ b/src/Rules/Arrays/NonexistentOffsetInArrayDimFetchRule.php @@ -9,6 +9,7 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Rules\RuleLevelHelper; +use PHPStan\Type\AccessOffsetMode; use PHPStan\Type\ErrorType; use PHPStan\Type\Type; use PHPStan\Type\VerbosityLevel; @@ -62,10 +63,20 @@ public function processNode(Node $node, Scope $scope): array $isOffsetAccessible = $isOffsetAccessibleType->isOffsetAccessible(); if ($scope->isInExpressionAssign($node) && $isOffsetAccessible->yes()) { - return []; + if ($isOffsetAccessibleType->isOffsetAccessLegal(AccessOffsetMode::Write)->yes()) { + return []; + } else { + // TODO Improve + return [ + RuleErrorBuilder::message(sprintf( + 'Cannot access an offset on %s.', + $isOffsetAccessibleType->describe(VerbosityLevel::typeOnly()), + ))->identifier('offsetAccess.nonOffsetAccessible')->build(), + ]; + } } - if ($scope->isUndefinedExpressionAllowed($node) && $isOffsetAccessibleType->isOffsetAccessLegal()->yes()) { + if ($scope->isUndefinedExpressionAllowed($node) && $isOffsetAccessibleType->isOffsetAccessLegal(AccessOffsetMode::Read)->yes()) { return []; } diff --git a/src/Type/AccessOffsetMode.php b/src/Type/AccessOffsetMode.php new file mode 100644 index 0000000000..5e1545e57f --- /dev/null +++ b/src/Type/AccessOffsetMode.php @@ -0,0 +1,15 @@ +isInteger()->and(TrinaryLogic::createMaybe()); diff --git a/src/Type/Accessory/AccessoryLowercaseStringType.php b/src/Type/Accessory/AccessoryLowercaseStringType.php index c4b8b0b364..e5ac19d930 100644 --- a/src/Type/Accessory/AccessoryLowercaseStringType.php +++ b/src/Type/Accessory/AccessoryLowercaseStringType.php @@ -7,6 +7,7 @@ use PHPStan\PhpDocParser\Ast\Type\TypeNode; use PHPStan\TrinaryLogic; use PHPStan\Type\AcceptsResult; +use PHPStan\Type\AccessOffsetMode; use PHPStan\Type\BenevolentUnionType; use PHPStan\Type\BooleanType; use PHPStan\Type\CompoundType; @@ -26,6 +27,7 @@ use PHPStan\Type\Traits\NonIterableTypeTrait; use PHPStan\Type\Traits\NonObjectTypeTrait; use PHPStan\Type\Traits\NonRemoveableTypeTrait; +use PHPStan\Type\Traits\StringTypeTrait; use PHPStan\Type\Traits\UndecidedComparisonCompoundTypeTrait; use PHPStan\Type\Type; use PHPStan\Type\UnionType; @@ -41,6 +43,7 @@ class AccessoryLowercaseStringType implements CompoundType, AccessoryType use UndecidedComparisonCompoundTypeTrait; use NonGenericTypeTrait; use NonRemoveableTypeTrait; + use StringTypeTrait; /** @api */ public function __construct() @@ -119,11 +122,6 @@ 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 index 08f4790001..6655641061 100644 --- a/src/Type/Accessory/AccessoryNonEmptyStringType.php +++ b/src/Type/Accessory/AccessoryNonEmptyStringType.php @@ -7,6 +7,7 @@ use PHPStan\PhpDocParser\Ast\Type\TypeNode; use PHPStan\TrinaryLogic; use PHPStan\Type\AcceptsResult; +use PHPStan\Type\AccessOffsetMode; use PHPStan\Type\BenevolentUnionType; use PHPStan\Type\BooleanType; use PHPStan\Type\CompoundType; @@ -26,6 +27,7 @@ use PHPStan\Type\Traits\NonGenericTypeTrait; use PHPStan\Type\Traits\NonIterableTypeTrait; use PHPStan\Type\Traits\NonObjectTypeTrait; +use PHPStan\Type\Traits\StringTypeTrait; use PHPStan\Type\Traits\UndecidedBooleanTypeTrait; use PHPStan\Type\Traits\UndecidedComparisonCompoundTypeTrait; use PHPStan\Type\Type; @@ -43,6 +45,7 @@ class AccessoryNonEmptyStringType implements CompoundType, AccessoryType use UndecidedComparisonCompoundTypeTrait; use NonGenericTypeTrait; use UndecidedBooleanTypeTrait; + use StringTypeTrait; /** @api */ public function __construct() @@ -125,11 +128,6 @@ 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 index 04fd4fb60e..cfe52aeae6 100644 --- a/src/Type/Accessory/AccessoryNonFalsyStringType.php +++ b/src/Type/Accessory/AccessoryNonFalsyStringType.php @@ -7,6 +7,7 @@ use PHPStan\PhpDocParser\Ast\Type\TypeNode; use PHPStan\TrinaryLogic; use PHPStan\Type\AcceptsResult; +use PHPStan\Type\AccessOffsetMode; use PHPStan\Type\BenevolentUnionType; use PHPStan\Type\BooleanType; use PHPStan\Type\CompoundType; @@ -26,6 +27,7 @@ use PHPStan\Type\Traits\NonIterableTypeTrait; use PHPStan\Type\Traits\NonObjectTypeTrait; use PHPStan\Type\Traits\NonRemoveableTypeTrait; +use PHPStan\Type\Traits\StringTypeTrait; use PHPStan\Type\Traits\TruthyBooleanTypeTrait; use PHPStan\Type\Traits\UndecidedComparisonCompoundTypeTrait; use PHPStan\Type\Type; @@ -43,6 +45,7 @@ class AccessoryNonFalsyStringType implements CompoundType, AccessoryType use UndecidedComparisonCompoundTypeTrait; use NonGenericTypeTrait; use NonRemoveableTypeTrait; + use StringTypeTrait; /** @api */ public function __construct() @@ -124,12 +127,6 @@ 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 index e0f4964c93..f9f6bbc22e 100644 --- a/src/Type/Accessory/AccessoryNumericStringType.php +++ b/src/Type/Accessory/AccessoryNumericStringType.php @@ -7,6 +7,7 @@ use PHPStan\PhpDocParser\Ast\Type\TypeNode; use PHPStan\TrinaryLogic; use PHPStan\Type\AcceptsResult; +use PHPStan\Type\AccessOffsetMode; use PHPStan\Type\BenevolentUnionType; use PHPStan\Type\BooleanType; use PHPStan\Type\CompoundType; @@ -25,6 +26,7 @@ use PHPStan\Type\Traits\NonGenericTypeTrait; use PHPStan\Type\Traits\NonIterableTypeTrait; use PHPStan\Type\Traits\NonObjectTypeTrait; +use PHPStan\Type\Traits\StringTypeTrait; use PHPStan\Type\Traits\UndecidedBooleanTypeTrait; use PHPStan\Type\Traits\UndecidedComparisonCompoundTypeTrait; use PHPStan\Type\Type; @@ -42,6 +44,7 @@ class AccessoryNumericStringType implements CompoundType, AccessoryType use UndecidedBooleanTypeTrait; use UndecidedComparisonCompoundTypeTrait; use NonGenericTypeTrait; + use StringTypeTrait; /** @api */ public function __construct() @@ -128,11 +131,6 @@ 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/AccessoryUppercaseStringType.php b/src/Type/Accessory/AccessoryUppercaseStringType.php index f9ae03d0ac..4214c88748 100644 --- a/src/Type/Accessory/AccessoryUppercaseStringType.php +++ b/src/Type/Accessory/AccessoryUppercaseStringType.php @@ -7,6 +7,7 @@ use PHPStan\PhpDocParser\Ast\Type\TypeNode; use PHPStan\TrinaryLogic; use PHPStan\Type\AcceptsResult; +use PHPStan\Type\AccessOffsetMode; use PHPStan\Type\BenevolentUnionType; use PHPStan\Type\BooleanType; use PHPStan\Type\CompoundType; @@ -26,6 +27,7 @@ use PHPStan\Type\Traits\NonIterableTypeTrait; use PHPStan\Type\Traits\NonObjectTypeTrait; use PHPStan\Type\Traits\NonRemoveableTypeTrait; +use PHPStan\Type\Traits\StringTypeTrait; use PHPStan\Type\Traits\UndecidedComparisonCompoundTypeTrait; use PHPStan\Type\Type; use PHPStan\Type\UnionType; @@ -41,6 +43,7 @@ class AccessoryUppercaseStringType implements CompoundType, AccessoryType use UndecidedComparisonCompoundTypeTrait; use NonGenericTypeTrait; use NonRemoveableTypeTrait; + use StringTypeTrait; /** @api */ public function __construct() @@ -119,11 +122,6 @@ 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 index 94364bb21f..cbd5667830 100644 --- a/src/Type/Accessory/HasMethodType.php +++ b/src/Type/Accessory/HasMethodType.php @@ -12,6 +12,7 @@ use PHPStan\Reflection\Type\UnresolvedMethodPrototypeReflection; use PHPStan\TrinaryLogic; use PHPStan\Type\AcceptsResult; +use PHPStan\Type\AccessOffsetMode; use PHPStan\Type\CompoundType; use PHPStan\Type\ErrorType; use PHPStan\Type\IntersectionType; @@ -111,7 +112,7 @@ public function describe(VerbosityLevel $level): string return sprintf('hasMethod(%s)', $this->methodName); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { return TrinaryLogic::createMaybe(); } diff --git a/src/Type/Accessory/HasOffsetType.php b/src/Type/Accessory/HasOffsetType.php index 492bd1ba27..b643cb4d41 100644 --- a/src/Type/Accessory/HasOffsetType.php +++ b/src/Type/Accessory/HasOffsetType.php @@ -7,6 +7,7 @@ use PHPStan\PhpDocParser\Ast\Type\TypeNode; use PHPStan\TrinaryLogic; use PHPStan\Type\AcceptsResult; +use PHPStan\Type\AccessOffsetMode; use PHPStan\Type\BooleanType; use PHPStan\Type\CompoundType; use PHPStan\Type\Constant\ConstantIntegerType; @@ -130,7 +131,7 @@ public function isOffsetAccessible(): TrinaryLogic return TrinaryLogic::createYes(); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { return TrinaryLogic::createYes(); } diff --git a/src/Type/Accessory/HasOffsetValueType.php b/src/Type/Accessory/HasOffsetValueType.php index 296c5b7308..5e871e3dc1 100644 --- a/src/Type/Accessory/HasOffsetValueType.php +++ b/src/Type/Accessory/HasOffsetValueType.php @@ -8,6 +8,7 @@ use PHPStan\ShouldNotHappenException; use PHPStan\TrinaryLogic; use PHPStan\Type\AcceptsResult; +use PHPStan\Type\AccessOffsetMode; use PHPStan\Type\BooleanType; use PHPStan\Type\CompoundType; use PHPStan\Type\Constant\ConstantIntegerType; @@ -142,7 +143,7 @@ public function isOffsetAccessible(): TrinaryLogic return TrinaryLogic::createYes(); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { return TrinaryLogic::createYes(); } diff --git a/src/Type/Accessory/HasPropertyType.php b/src/Type/Accessory/HasPropertyType.php index 71b6b42759..9608b515bc 100644 --- a/src/Type/Accessory/HasPropertyType.php +++ b/src/Type/Accessory/HasPropertyType.php @@ -8,6 +8,7 @@ use PHPStan\Reflection\TrivialParametersAcceptor; use PHPStan\TrinaryLogic; use PHPStan\Type\AcceptsResult; +use PHPStan\Type\AccessOffsetMode; use PHPStan\Type\CompoundType; use PHPStan\Type\ErrorType; use PHPStan\Type\IntersectionType; @@ -106,7 +107,7 @@ public function describe(VerbosityLevel $level): string return sprintf('hasProperty(%s)', $this->propertyName); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { return TrinaryLogic::createMaybe(); } diff --git a/src/Type/Accessory/NonEmptyArrayType.php b/src/Type/Accessory/NonEmptyArrayType.php index e99bb4cbaa..38953effa3 100644 --- a/src/Type/Accessory/NonEmptyArrayType.php +++ b/src/Type/Accessory/NonEmptyArrayType.php @@ -7,6 +7,7 @@ use PHPStan\PhpDocParser\Ast\Type\TypeNode; use PHPStan\TrinaryLogic; use PHPStan\Type\AcceptsResult; +use PHPStan\Type\AccessOffsetMode; use PHPStan\Type\BooleanType; use PHPStan\Type\CompoundType; use PHPStan\Type\Constant\ConstantFloatType; @@ -128,7 +129,7 @@ public function isOffsetAccessible(): TrinaryLogic return TrinaryLogic::createYes(); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { return TrinaryLogic::createYes(); } diff --git a/src/Type/Accessory/OversizedArrayType.php b/src/Type/Accessory/OversizedArrayType.php index 70ba480a7e..a88c8c7722 100644 --- a/src/Type/Accessory/OversizedArrayType.php +++ b/src/Type/Accessory/OversizedArrayType.php @@ -7,6 +7,7 @@ use PHPStan\PhpDocParser\Ast\Type\TypeNode; use PHPStan\TrinaryLogic; use PHPStan\Type\AcceptsResult; +use PHPStan\Type\AccessOffsetMode; use PHPStan\Type\BooleanType; use PHPStan\Type\CompoundType; use PHPStan\Type\Constant\ConstantFloatType; @@ -124,7 +125,7 @@ public function isOffsetAccessible(): TrinaryLogic return TrinaryLogic::createYes(); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { return TrinaryLogic::createYes(); } diff --git a/src/Type/BooleanType.php b/src/Type/BooleanType.php index df059481e6..8ae1a0b6f2 100644 --- a/src/Type/BooleanType.php +++ b/src/Type/BooleanType.php @@ -101,8 +101,9 @@ public function toArrayKey(): Type return new UnionType([new ConstantIntegerType(0), new ConstantIntegerType(1)]); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { + // TODO Should this be NO? return TrinaryLogic::createYes(); } diff --git a/src/Type/CallableType.php b/src/Type/CallableType.php index fe6e412ad2..0617495ea3 100644 --- a/src/Type/CallableType.php +++ b/src/Type/CallableType.php @@ -321,7 +321,7 @@ public function toArrayKey(): Type return new ErrorType(); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { return TrinaryLogic::createMaybe(); } diff --git a/src/Type/ClosureType.php b/src/Type/ClosureType.php index a76aa90596..4ac478c64f 100644 --- a/src/Type/ClosureType.php +++ b/src/Type/ClosureType.php @@ -269,7 +269,7 @@ function (): string { ); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { return TrinaryLogic::createNo(); } diff --git a/src/Type/FloatType.php b/src/Type/FloatType.php index 880006af90..943f30f8a4 100644 --- a/src/Type/FloatType.php +++ b/src/Type/FloatType.php @@ -143,7 +143,7 @@ public function toArrayKey(): Type return new IntegerType(); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { return TrinaryLogic::createYes(); } diff --git a/src/Type/IntegerType.php b/src/Type/IntegerType.php index 31ef751715..274f30ca71 100644 --- a/src/Type/IntegerType.php +++ b/src/Type/IntegerType.php @@ -97,7 +97,7 @@ public function toArrayKey(): Type return $this; } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { return TrinaryLogic::createYes(); } diff --git a/src/Type/IntersectionType.php b/src/Type/IntersectionType.php index 255f56dd7a..f3fe16b3b1 100644 --- a/src/Type/IntersectionType.php +++ b/src/Type/IntersectionType.php @@ -724,9 +724,9 @@ public function isOffsetAccessible(): TrinaryLogic return $this->intersectResults(static fn (Type $type): TrinaryLogic => $type->isOffsetAccessible()); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { - return $this->intersectResults(static fn (Type $type): TrinaryLogic => $type->isOffsetAccessLegal()); + return $this->intersectResults(static fn (Type $type): TrinaryLogic => $type->isOffsetAccessLegal($mode)); } public function hasOffsetValueType(Type $offsetType): TrinaryLogic diff --git a/src/Type/IterableType.php b/src/Type/IterableType.php index 502fb6330a..0aefa0bb1c 100644 --- a/src/Type/IterableType.php +++ b/src/Type/IterableType.php @@ -234,7 +234,7 @@ public function toArrayKey(): Type return new ErrorType(); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { return TrinaryLogic::createMaybe(); } diff --git a/src/Type/MixedType.php b/src/Type/MixedType.php index c63c78f214..2ec3cd2d69 100644 --- a/src/Type/MixedType.php +++ b/src/Type/MixedType.php @@ -651,7 +651,7 @@ public function isOffsetAccessible(): TrinaryLogic return TrinaryLogic::createMaybe(); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { if ($this->subtractedType !== null) { if ($this->subtractedType->isSuperTypeOf(new ObjectWithoutClassType())->yes()) { diff --git a/src/Type/NeverType.php b/src/Type/NeverType.php index eab6009abb..1ba61f7648 100644 --- a/src/Type/NeverType.php +++ b/src/Type/NeverType.php @@ -243,7 +243,7 @@ public function isOffsetAccessible(): TrinaryLogic return TrinaryLogic::createYes(); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { return TrinaryLogic::createYes(); } diff --git a/src/Type/NonexistentParentClassType.php b/src/Type/NonexistentParentClassType.php index f52c5ea8de..22576634c6 100644 --- a/src/Type/NonexistentParentClassType.php +++ b/src/Type/NonexistentParentClassType.php @@ -157,7 +157,7 @@ public function toArrayKey(): Type return new ErrorType(); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { return TrinaryLogic::createNo(); } diff --git a/src/Type/NullType.php b/src/Type/NullType.php index 7d66fa7e79..45e62ad434 100644 --- a/src/Type/NullType.php +++ b/src/Type/NullType.php @@ -171,7 +171,7 @@ public function isOffsetAccessible(): TrinaryLogic return TrinaryLogic::createYes(); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { return TrinaryLogic::createYes(); } diff --git a/src/Type/ObjectShapeType.php b/src/Type/ObjectShapeType.php index 1a1beed6c0..5450912422 100644 --- a/src/Type/ObjectShapeType.php +++ b/src/Type/ObjectShapeType.php @@ -420,7 +420,7 @@ public function describe(VerbosityLevel $level): string ); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { return TrinaryLogic::createMaybe(); } diff --git a/src/Type/ObjectType.php b/src/Type/ObjectType.php index 3290fb97c7..6d15962e94 100644 --- a/src/Type/ObjectType.php +++ b/src/Type/ObjectType.php @@ -1101,8 +1101,9 @@ public function isOffsetAccessible(): TrinaryLogic ); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { + // TODO Narrow down NodeList/Map types return $this->isOffsetAccessible(); } diff --git a/src/Type/ObjectWithoutClassType.php b/src/Type/ObjectWithoutClassType.php index 3abe064e3f..e89eeba4a6 100644 --- a/src/Type/ObjectWithoutClassType.php +++ b/src/Type/ObjectWithoutClassType.php @@ -134,7 +134,7 @@ function () use ($level): string { ); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { return TrinaryLogic::createMaybe(); } diff --git a/src/Type/ResourceType.php b/src/Type/ResourceType.php index f62023f2c6..1bc174b6d7 100644 --- a/src/Type/ResourceType.php +++ b/src/Type/ResourceType.php @@ -91,8 +91,9 @@ public function toArrayKey(): Type return new ErrorType(); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { + // TODO Should be NO? return TrinaryLogic::createYes(); } diff --git a/src/Type/StaticType.php b/src/Type/StaticType.php index 8885972a66..2bb0539997 100644 --- a/src/Type/StaticType.php +++ b/src/Type/StaticType.php @@ -366,9 +366,9 @@ public function isOffsetAccessible(): TrinaryLogic return $this->getStaticObjectType()->isOffsetAccessible(); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { - return $this->getStaticObjectType()->isOffsetAccessLegal(); + return $this->getStaticObjectType()->isOffsetAccessLegal($mode); } public function hasOffsetValueType(Type $offsetType): TrinaryLogic diff --git a/src/Type/StrictMixedType.php b/src/Type/StrictMixedType.php index 355d186986..baed1fc2bf 100644 --- a/src/Type/StrictMixedType.php +++ b/src/Type/StrictMixedType.php @@ -310,7 +310,7 @@ public function isOffsetAccessible(): TrinaryLogic return TrinaryLogic::createNo(); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { return TrinaryLogic::createNo(); } diff --git a/src/Type/StringType.php b/src/Type/StringType.php index 4605c92efe..8c5382642e 100644 --- a/src/Type/StringType.php +++ b/src/Type/StringType.php @@ -18,6 +18,7 @@ use PHPStan\Type\Traits\NonGenericTypeTrait; use PHPStan\Type\Traits\NonIterableTypeTrait; use PHPStan\Type\Traits\NonObjectTypeTrait; +use PHPStan\Type\Traits\StringTypeTrait; use PHPStan\Type\Traits\UndecidedBooleanTypeTrait; use PHPStan\Type\Traits\UndecidedComparisonTypeTrait; use function count; @@ -35,6 +36,7 @@ class StringType implements Type use UndecidedComparisonTypeTrait; use NonGenericTypeTrait; use NonGeneralizableTypeTrait; + use StringTypeTrait; /** @api */ public function __construct() @@ -56,11 +58,6 @@ 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/ArrayTypeTrait.php b/src/Type/Traits/ArrayTypeTrait.php index ddc501d022..6043974ba4 100644 --- a/src/Type/Traits/ArrayTypeTrait.php +++ b/src/Type/Traits/ArrayTypeTrait.php @@ -3,6 +3,7 @@ namespace PHPStan\Type\Traits; use PHPStan\TrinaryLogic; +use PHPStan\Type\AccessOffsetMode; use PHPStan\Type\Accessory\AccessoryArrayListType; use PHPStan\Type\Accessory\NonEmptyArrayType; use PHPStan\Type\ArrayType; @@ -34,7 +35,7 @@ public function isOffsetAccessible(): TrinaryLogic return TrinaryLogic::createYes(); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { return TrinaryLogic::createYes(); } diff --git a/src/Type/Traits/LateResolvableTypeTrait.php b/src/Type/Traits/LateResolvableTypeTrait.php index 3c5a4e5b5f..05cd59603a 100644 --- a/src/Type/Traits/LateResolvableTypeTrait.php +++ b/src/Type/Traits/LateResolvableTypeTrait.php @@ -11,6 +11,7 @@ use PHPStan\Reflection\Type\UnresolvedPropertyPrototypeReflection; use PHPStan\TrinaryLogic; use PHPStan\Type\AcceptsResult; +use PHPStan\Type\AccessOffsetMode; use PHPStan\Type\BooleanType; use PHPStan\Type\CompoundType; use PHPStan\Type\Generic\TemplateTypeMap; @@ -218,9 +219,9 @@ public function isOffsetAccessible(): TrinaryLogic return $this->resolve()->isOffsetAccessible(); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { - return $this->resolve()->isOffsetAccessLegal(); + return $this->resolve()->isOffsetAccessLegal($mode); } public function hasOffsetValueType(Type $offsetType): TrinaryLogic diff --git a/src/Type/Traits/StringTypeTrait.php b/src/Type/Traits/StringTypeTrait.php new file mode 100644 index 0000000000..0299cd715e --- /dev/null +++ b/src/Type/Traits/StringTypeTrait.php @@ -0,0 +1,18 @@ + TrinaryLogic::createYes(), + default => TrinaryLogic::createNo(), + }; + } + +} diff --git a/src/Type/Type.php b/src/Type/Type.php index 8021baae62..aee4c9b344 100644 --- a/src/Type/Type.php +++ b/src/Type/Type.php @@ -129,7 +129,7 @@ public function isList(): TrinaryLogic; public function isOffsetAccessible(): TrinaryLogic; - public function isOffsetAccessLegal(): TrinaryLogic; + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic; public function hasOffsetValueType(Type $offsetType): TrinaryLogic; diff --git a/src/Type/UnionType.php b/src/Type/UnionType.php index c69888cf30..e3ee74e1f0 100644 --- a/src/Type/UnionType.php +++ b/src/Type/UnionType.php @@ -684,9 +684,9 @@ public function isOffsetAccessible(): TrinaryLogic return $this->unionResults(static fn (Type $type): TrinaryLogic => $type->isOffsetAccessible()); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { - return $this->unionResults(static fn (Type $type): TrinaryLogic => $type->isOffsetAccessLegal()); + return $this->unionResults(static fn (Type $type): TrinaryLogic => $type->isOffsetAccessLegal($mode)); } public function hasOffsetValueType(Type $offsetType): TrinaryLogic diff --git a/src/Type/VoidType.php b/src/Type/VoidType.php index b6bac5908c..24ff9137c0 100644 --- a/src/Type/VoidType.php +++ b/src/Type/VoidType.php @@ -119,7 +119,7 @@ public function toArrayKey(): Type return new ErrorType(); } - public function isOffsetAccessLegal(): TrinaryLogic + public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { return TrinaryLogic::createYes(); } diff --git a/tests/PHPStan/Type/MixedTypeTest.php b/tests/PHPStan/Type/MixedTypeTest.php index 3ffc8f9db7..29fb9e0d6d 100644 --- a/tests/PHPStan/Type/MixedTypeTest.php +++ b/tests/PHPStan/Type/MixedTypeTest.php @@ -1097,7 +1097,8 @@ public function dataSubstractedIsOffsetLegal(): array public function testSubstractedIsOffsetLegal(MixedType $mixedType, Type $typeToSubtract, TrinaryLogic $expectedResult): void { $subtracted = $mixedType->subtract($typeToSubtract); - $actualResult = $subtracted->isOffsetAccessLegal(); + // TODO Propagate Access Mode? + $actualResult = $subtracted->isOffsetAccessLegal(AccessOffsetMode::Read); $this->assertSame( $expectedResult->describe(), From 85e39d42b9f9f27f38e36ebd1d0af314f407e02a Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 11 Dec 2024 15:14:34 +0000 Subject: [PATCH 2/2] Restrict access for DOM classes --- src/Type/ObjectType.php | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/src/Type/ObjectType.php b/src/Type/ObjectType.php index 6d15962e94..666059eaa1 100644 --- a/src/Type/ObjectType.php +++ b/src/Type/ObjectType.php @@ -61,7 +61,16 @@ class ObjectType implements TypeWithClassName, SubtractableType use UndecidedComparisonTypeTrait; use NonGeneralizableTypeTrait; - private const EXTRA_OFFSET_CLASSES = ['SimpleXMLElement', 'DOMNodeList', 'Threaded']; + private const EXTRA_OFFSET_CLASSES = [ + 'DOMNamedNodeMap', + 'Dom\NamedNodeMap', + 'DOMNodeList', + 'Dom\NodeList', + 'Dom\HTMLCollection', + 'Dom\DtdNamedNodeMap', + 'SimpleXMLElement', + 'Threaded', + ]; private ?Type $subtractedType; @@ -1067,19 +1076,26 @@ public function looseCompare(Type $type, PhpVersion $phpVersion): BooleanType : new BooleanType(); } - private function isExtraOffsetAccessibleClass(): TrinaryLogic + private function isExtraOffsetAccessibleClass(AccessOffsetMode $mode): TrinaryLogic { $classReflection = $this->getClassReflection(); if ($classReflection === null) { return TrinaryLogic::createMaybe(); } + // TODO Narrow down NodeList/Map types foreach (self::EXTRA_OFFSET_CLASSES as $extraOffsetClass) { - if ($classReflection->getName() === $extraOffsetClass) { - return TrinaryLogic::createYes(); - } - if ($classReflection->isSubclassOf($extraOffsetClass)) { - return TrinaryLogic::createYes(); + if ( + $classReflection->getName() === $extraOffsetClass + || $classReflection->isSubclassOf($extraOffsetClass) + ) { + return match ($extraOffsetClass) { + 'DOMNamedNodeMap', 'Dom\NamedNodeMap', 'DOMNodeList', 'Dom\NodeList', 'Dom\HTMLCollection', 'Dom\DtdNamedNodeMap' => match ($mode) { + AccessOffsetMode::Read, AccessOffsetMode::Exist => TrinaryLogic::createYes(), + default => TrinaryLogic::createNo(), + }, + default => TrinaryLogic::createYes(), + }; } } @@ -1097,14 +1113,16 @@ private function isExtraOffsetAccessibleClass(): TrinaryLogic public function isOffsetAccessible(): TrinaryLogic { return $this->isInstanceOf(ArrayAccess::class)->or( - $this->isExtraOffsetAccessibleClass(), + // TODO Back propagate this? + $this->isExtraOffsetAccessibleClass(AccessOffsetMode::Read), ); } public function isOffsetAccessLegal(AccessOffsetMode $mode): TrinaryLogic { - // TODO Narrow down NodeList/Map types - return $this->isOffsetAccessible(); + return $this->isInstanceOf(ArrayAccess::class)->or( + $this->isExtraOffsetAccessibleClass($mode), + ); } public function hasOffsetValueType(Type $offsetType): TrinaryLogic