Do not parenthesize a benevolent union whose members describe the same

src/Type/BenevolentUnionType.php

@@ -41,11 +41,6 @@ public function tryRemove(Type $typeToRemove): ?Type
 		return TypeUtils::toBenevolentUnion($result);
 	}
 
-	public function describe(VerbosityLevel $level): string
-	{
-		return '(' . parent::describe($level) . ')';
-	}
-
 	protected function unionTypes(callable $getType): Type
 	{
 		$changed = false;

src/Type/UnionType.php

@@ -508,7 +508,8 @@ public function describe(VerbosityLevel $level): string
 		if (isset($this->cachedDescriptions[$level->getLevelValue()])) {
 			return $this->cachedDescriptions[$level->getLevelValue()];
 		}
-		$joinTypes = static function (array $types) use ($level): string {
+		$benevolent = $this instanceof BenevolentUnionType;
+		$joinTypes = static function (array $types) use ($level, $benevolent): string {
 			$typeNames = [];
 			foreach ($types as $i => $type) {
 				if ($type instanceof ClosureType || $type instanceof CallableType || $type instanceof TemplateUnionType) {
@@ -554,10 +555,17 @@ public function describe(VerbosityLevel $level): string
 			}
 
 			if (count($typeNames) > 1024) {
-				return implode('|', array_slice($typeNames, 0, 1024)) . "|\u{2026}";
+				$description = implode('|', array_slice($typeNames, 0, 1024)) . "|\u{2026}";
+			} else {
+				$description = implode('|', $typeNames);
+			}
+
+			// the members of a benevolent union can describe the same, like decimal-int-string and non-empty-string as string
+			if ($benevolent && count($typeNames) > 1) {
+				return '(' . $description . ')';
 			}
 
-			return implode('|', $typeNames);
+			return $description;
 		};
 
 		return $this->cachedDescriptions[$level->getLevelValue()] = $level->handle(

tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php

@@ -3450,4 +3450,19 @@ public function testFloatRangeArgument(): void
 		]);
 	}
 
+	#[RequiresPhp('>= 8.0.0')]
+	public function testBenevolentUnionDescription(): void
+	{
+		$this->analyse([__DIR__ . '/data/benevolent-union-description.php'], [
+			[
+				'Parameter #1 $num of function BenevolentUnionDescription\\takesNumber expects float|int, string given.',
+				18,
+			],
+			[
+				'Parameter #1 $s of function BenevolentUnionDescription\\takesString expects string, (float|int) given.',
+				19,
+			],
+		]);
+	}
+
 }

tests/PHPStan/Rules/Functions/data/benevolent-union-description.php

@@ -0,0 +1,20 @@
+<?php // lint >= 8.0
+
+declare(strict_types = 1);
+
+namespace BenevolentUnionDescription;
+
+function takesNumber(float|int $num): void
+{
+}
+
+function takesString(string $s): void
+{
+}
+
+function doFoo(int|float $number, int $scale): void
+{
+	$coefficient = 10 ** $scale;
+	takesNumber((string) ($number * $coefficient));
+	takesString($coefficient);
+}

tests/PHPStan/Type/UnionTypeTest.php

@@ -961,6 +961,30 @@ public static function dataDescribe(): array
 				'Foo',
 				'Foo',
 			],
+			[
+				new BenevolentUnionType([new IntegerType(), new StringType()]),
+				'(int|string)',
+				'(int|string)',
+				'(int|string)',
+				'(int|string)',
+			],
+			[
+				new BenevolentUnionType([new ObjectType('Foo'), new ObjectType('Foo')]),
+				'(Foo-PHPStan\Type\ObjectType-#1|Foo-PHPStan\Type\ObjectType-#2)',
+				'(Foo#1|Foo#2)',
+				'Foo',
+				'Foo',
+			],
+			[
+				new BenevolentUnionType([
+					new GenericObjectType('Foo', [new UnionType([new IntegerType(), new StringType()])]),
+					new GenericObjectType('Foo', [new UnionType([new IntegerType(), new StringType()])]),
+				]),
+				'(Foo-PHPStan\Type\Generic\GenericObjectType-<int|string>#1|Foo-PHPStan\Type\Generic\GenericObjectType-<int|string>#2)',
+				'(Foo<int|string>#1|Foo<int|string>#2)',
+				'Foo<int|string>',
+				'Foo<int|string>',
+			],
 		];
 	}
 

turbo-ext/src/BenevolentUnionType.cpp

@@ -8,8 +8,8 @@
  * the constructor forwarding to the parent's, and the overrides that make
  * the union benevolent: unionResults() answering yes as soon as one member
  * does, unionTypes()/getOffsetValueType()/traverse()/traverseSimultaneously()/
- * tryRemove()/filterTypes() keeping the result benevolent, describe()
- * parenthesizing the parent's, pickFromTypes() consulting the criteria,
+ * tryRemove()/filterTypes() keeping the result benevolent,
+ * pickFromTypes() consulting the criteria,
  * isAcceptedBy() or'ing the members, the template maps combined with
  * benevolentUnion(). Everything else is inherited from the parent's
  * class entry, as the PHP class inherits it.
@@ -108,19 +108,6 @@ class BenevolentUnionType
 		return toBenevolent(result.raw());
 	}
 
-	/* '(' . parent::describe($level) . ')' */
-	zv::Val describe(zval *level) const
-	{
-		zv::Val inner = pt_union_type_describe(self, level);
-		if (UNEXPECTED(inner.isUndef())) return zv::Val();
-		smart_str description = {NULL, 0};
-		smart_str_appendc(&description, '(');
-		smart_str_append(&description, zv::Ref(inner.raw()).asString());
-		smart_str_appendc(&description, ')');
-		smart_str_0(&description);
-		return zv::Val::adoptString(description.s);
-	}
-
 	/* the members' results with the ErrorTypes dropped: $this when nothing
 	 * changed, an ErrorType when nothing is left, the benevolent union
 	 * otherwise; UNDEF = pending exception */
@@ -514,9 +501,6 @@ PT_MINIT_REGISTRATION(pt_register_benevolent_union_type)
 
 	cls.method<&BenevolentUnionType::tryRemove, zp::Obj>(sigs::tryRemove);
 
-	cls.method<&BenevolentUnionType::describe, zp::Obj>(sigs::describe);
-	cls.op<PT_OP_DESCRIBE, &BenevolentUnionType::describe>();
-
 	cls.method(sigs::unionTypes, buUnionTypes);
 	cls.method(sigs::pickFromTypes, buPickFromTypes);
 

turbo-ext/src/UnionType.cpp

@@ -966,6 +966,7 @@ class UnionType
 
 		pt_verbosity_case which;
 		if (UNEXPECTED(!pt_type_verbosity_case(level, which))) return zv::Val();
+		bool benevolent = instanceof_function(self->ce, pt_ce_benevolent_union_type);
 		zv::Val description;
 		if (which == PT_VERBOSITY_TYPE_ONLY) {
 			zv::Val sorted = thisGetSortedTypes();
@@ -999,16 +1000,16 @@ class UnionType
 			if (zv::Ref(types.raw()).instanceOf(pt_ce_union_type)) {
 				zv::Val innerSorted = sortedTypesOf(Z_OBJ_P(types.raw()));
 				if (UNEXPECTED(innerSorted.isUndef())) return zv::Val();
-				description = joinTypes(zv::ArrRef(innerSorted.raw()), level, which);
+				description = joinTypes(zv::ArrRef(innerSorted.raw()), level, which, benevolent);
 			} else {
 				zv::Arr single = zv::Arr::create(1);
 				single.push(std::move(types));
-				description = joinTypes(single.arrRef(), level, which);
+				description = joinTypes(single.arrRef(), level, which, benevolent);
 			}
 		} else {
 			zv::Val sorted = thisGetSortedTypes();
 			if (UNEXPECTED(sorted.isUndef())) return zv::Val();
-			description = joinTypes(zv::ArrRef(sorted.raw()), level, which);
+			description = joinTypes(zv::ArrRef(sorted.raw()), level, which, benevolent);
 		}
 		if (UNEXPECTED(description.isUndef())) return zv::Val();
 
@@ -1040,8 +1041,9 @@ class UnionType
 	 * for a closure/callable/template union, for a template with a bound
 	 * that must be shown, and for an intersection containing '&'; the
 	 * duplicates numbered (#n) at the precise and cache levels and dropped
-	 * elsewhere; at most PT_UT_DESCRIBE_TYPES_LIMIT joined with '|' */
-	static zv::Val joinTypes(zv::ArrRef types, zval *level, pt_verbosity_case which)
+	 * elsewhere; at most PT_UT_DESCRIBE_TYPES_LIMIT joined with '|',
+	 * parenthesized for a benevolent union of more than one name */
+	static zv::Val joinTypes(zv::ArrRef types, zval *level, pt_verbosity_case which, bool benevolent)
 	{
 		uint32_t count = types.size();
 		zv::Arr typeNames = zv::Arr::create(count);
@@ -1155,6 +1157,10 @@ class UnionType
 		smart_str result = {NULL, 0};
 		uint32_t n = 0;
 		bool truncated = zend_hash_num_elements(joined.table()) > PT_UT_DESCRIBE_TYPES_LIMIT;
+		bool wrap = benevolent && zend_hash_num_elements(joined.table()) > 1;
+		if (wrap) {
+			smart_str_appendc(&result, '(');
+		}
 		for (zv::ArrayEntry entry : joined.arrRef()) {
 			if (truncated && n == PT_UT_DESCRIBE_TYPES_LIMIT) break;
 			if (n > 0) {
@@ -1166,6 +1172,9 @@ class UnionType
 		if (truncated) {
 			smart_str_appendl(&result, "|\xE2\x80\xA6", 4);
 		}
+		if (wrap) {
+			smart_str_appendc(&result, ')');
+		}
 		smart_str_0(&result);
 		if (result.s == NULL) return zv::Val::string("", 0);
 		return zv::Val::adoptString(result.s);

turbo-ext/src/generated/BenevolentUnionType.h

@@ -32,65 +32,60 @@ inline constexpr char strings[] =
 	"filterTypes\0" /* 63 */
 	"typeToRemove\0" /* 75 */
 	"tryRemove\0" /* 88 */
-	"level\0" /* 98 */
-	"PHPStan\\Type\\VerbosityLevel\0" /* 104 */
-	"describe\0" /* 132 */
-	"getType\0" /* 141 */
-	"unionTypes\0" /* 149 */
-	"getValues\0" /* 160 */
-	"criteria\0" /* 170 */
-	"pickFromTypes\0" /* 179 */
-	"offsetType\0" /* 193 */
-	"getOffsetValueType\0" /* 204 */
-	"getResult\0" /* 223 */
-	"PHPStan\\TrinaryLogic\0" /* 233 */
-	"unionResults\0" /* 254 */
-	"acceptingType\0" /* 267 */
-	"strictTypes\0" /* 281 */
-	"PHPStan\\Type\\AcceptsResult\0" /* 293 */
-	"isAcceptedBy\0" /* 320 */
-	"receivedType\0" /* 333 */
-	"PHPStan\\Type\\Generic\\TemplateTypeMap\0" /* 346 */
-	"inferTemplateTypes\0" /* 383 */
-	"templateType\0" /* 402 */
-	"inferTemplateTypesOn\0" /* 415 */
-	"cb\0" /* 436 */
-	"traverse\0" /* 439 */
-	"right\0" /* 448 */
-	"traverseSimultaneously\0" /* 454 */
-	"PHPStan\\Type\\ValueSet\0" /* 477 */
-	"toValueSet"; /* 499 */
+	"getType\0" /* 98 */
+	"unionTypes\0" /* 106 */
+	"getValues\0" /* 117 */
+	"criteria\0" /* 127 */
+	"pickFromTypes\0" /* 136 */
+	"offsetType\0" /* 150 */
+	"getOffsetValueType\0" /* 161 */
+	"getResult\0" /* 180 */
+	"PHPStan\\TrinaryLogic\0" /* 190 */
+	"unionResults\0" /* 211 */
+	"acceptingType\0" /* 224 */
+	"strictTypes\0" /* 238 */
+	"PHPStan\\Type\\AcceptsResult\0" /* 250 */
+	"isAcceptedBy\0" /* 277 */
+	"receivedType\0" /* 290 */
+	"PHPStan\\Type\\Generic\\TemplateTypeMap\0" /* 303 */
+	"inferTemplateTypes\0" /* 340 */
+	"templateType\0" /* 359 */
+	"inferTemplateTypesOn\0" /* 372 */
+	"cb\0" /* 393 */
+	"traverse\0" /* 396 */
+	"right\0" /* 405 */
+	"traverseSimultaneously\0" /* 411 */
+	"PHPStan\\Type\\ValueSet\0" /* 434 */
+	"toValueSet"; /* 456 */
 inline constexpr reg::PackedArg args[] = {
 	reg::packed(0, MAY_BE_ARRAY), /* __construct $types */
 	reg::packed(6, MAY_BE_BOOL, reg::NoString, false, false, 17), /* __construct $normalized */
 	reg::packed(35, MAY_BE_CALLABLE), /* filterTypes $filterCb */
 	reg::packed(44, 0, 45), /* filterTypes return */
 	reg::packed(75, 0, 45), /* tryRemove $typeToRemove */
 	reg::packed(44, MAY_BE_NULL, 45), /* tryRemove return */
-	reg::packed(98, 0, 104), /* describe $level */
-	reg::packed(44, MAY_BE_STRING), /* describe return */
-	reg::packed(141, MAY_BE_CALLABLE), /* unionTypes $getType */
+	reg::packed(98, MAY_BE_CALLABLE), /* unionTypes $getType */
 	reg::packed(44, 0, 45), /* unionTypes return */
-	reg::packed(160, MAY_BE_CALLABLE), /* pickFromTypes $getValues */
-	reg::packed(170, MAY_BE_CALLABLE), /* pickFromTypes $criteria */
+	reg::packed(117, MAY_BE_CALLABLE), /* pickFromTypes $getValues */
+	reg::packed(127, MAY_BE_CALLABLE), /* pickFromTypes $criteria */
 	reg::packed(44, MAY_BE_ARRAY), /* pickFromTypes return */
-	reg::packed(193, 0, 45), /* getOffsetValueType $offsetType */
+	reg::packed(150, 0, 45), /* getOffsetValueType $offsetType */
 	reg::packed(44, 0, 45), /* getOffsetValueType return */
-	reg::packed(223, MAY_BE_CALLABLE), /* unionResults $getResult */
-	reg::packed(44, 0, 233), /* unionResults return */
-	reg::packed(267, 0, 45), /* isAcceptedBy $acceptingType */
-	reg::packed(281, MAY_BE_BOOL), /* isAcceptedBy $strictTypes */
-	reg::packed(44, 0, 293), /* isAcceptedBy return */
-	reg::packed(333, 0, 45), /* inferTemplateTypes $receivedType */
-	reg::packed(44, 0, 346), /* inferTemplateTypes return */
-	reg::packed(402, 0, 45), /* inferTemplateTypesOn $templateType */
-	reg::packed(44, 0, 346), /* inferTemplateTypesOn return */
-	reg::packed(436, MAY_BE_CALLABLE), /* traverse $cb */
+	reg::packed(180, MAY_BE_CALLABLE), /* unionResults $getResult */
+	reg::packed(44, 0, 190), /* unionResults return */
+	reg::packed(224, 0, 45), /* isAcceptedBy $acceptingType */
+	reg::packed(238, MAY_BE_BOOL), /* isAcceptedBy $strictTypes */
+	reg::packed(44, 0, 250), /* isAcceptedBy return */
+	reg::packed(290, 0, 45), /* inferTemplateTypes $receivedType */
+	reg::packed(44, 0, 303), /* inferTemplateTypes return */
+	reg::packed(359, 0, 45), /* inferTemplateTypesOn $templateType */
+	reg::packed(44, 0, 303), /* inferTemplateTypesOn return */
+	reg::packed(393, MAY_BE_CALLABLE), /* traverse $cb */
 	reg::packed(44, 0, 45), /* traverse return */
-	reg::packed(448, 0, 45), /* traverseSimultaneously $right */
-	reg::packed(436, MAY_BE_CALLABLE), /* traverseSimultaneously $cb */
+	reg::packed(405, 0, 45), /* traverseSimultaneously $right */
+	reg::packed(393, MAY_BE_CALLABLE), /* traverseSimultaneously $cb */
 	reg::packed(44, 0, 45), /* traverseSimultaneously return */
-	reg::packed(44, 0, 477), /* toValueSet return */
+	reg::packed(44, 0, 434), /* toValueSet return */
 };
 using Sig = reg::Sig<strings, args>;
 } // namespace sigtab
@@ -100,17 +95,16 @@ namespace sig {
 inline constexpr sigtab::Sig __construct = { { 23 /* __construct */, 1, 0, 2, reg::NoArg, ZEND_ACC_PUBLIC } };
 inline constexpr sigtab::Sig filterTypes = { { 63 /* filterTypes */, 1, 2, 1, 3, ZEND_ACC_PUBLIC } };
 inline constexpr sigtab::Sig tryRemove = { { 88 /* tryRemove */, 1, 4, 1, 5, ZEND_ACC_PUBLIC } };
-inline constexpr sigtab::Sig describe = { { 132 /* describe */, 1, 6, 1, 7, ZEND_ACC_PUBLIC } };
-inline constexpr sigtab::Sig unionTypes = { { 149 /* unionTypes */, 1, 8, 1, 9, ZEND_ACC_PROTECTED } };
-inline constexpr sigtab::Sig pickFromTypes = { { 179 /* pickFromTypes */, 2, 10, 2, 12, ZEND_ACC_PROTECTED } };
-inline constexpr sigtab::Sig getOffsetValueType = { { 204 /* getOffsetValueType */, 1, 13, 1, 14, ZEND_ACC_PUBLIC } };
-inline constexpr sigtab::Sig unionResults = { { 254 /* unionResults */, 1, 15, 1, 16, ZEND_ACC_PROTECTED } };
-inline constexpr sigtab::Sig isAcceptedBy = { { 320 /* isAcceptedBy */, 2, 17, 2, 19, ZEND_ACC_PUBLIC } };
-inline constexpr sigtab::Sig inferTemplateTypes = { { 383 /* inferTemplateTypes */, 1, 20, 1, 21, ZEND_ACC_PUBLIC } };
-inline constexpr sigtab::Sig inferTemplateTypesOn = { { 415 /* inferTemplateTypesOn */, 1, 22, 1, 23, ZEND_ACC_PUBLIC } };
-inline constexpr sigtab::Sig traverse = { { 439 /* traverse */, 1, 24, 1, 25, ZEND_ACC_PUBLIC } };
-inline constexpr sigtab::Sig traverseSimultaneously = { { 454 /* traverseSimultaneously */, 2, 26, 2, 28, ZEND_ACC_PUBLIC } };
-inline constexpr sigtab::Sig toValueSet = { { 499 /* toValueSet */, 0, 29, 0, 29, ZEND_ACC_PUBLIC } };
+inline constexpr sigtab::Sig unionTypes = { { 106 /* unionTypes */, 1, 6, 1, 7, ZEND_ACC_PROTECTED } };
+inline constexpr sigtab::Sig pickFromTypes = { { 136 /* pickFromTypes */, 2, 8, 2, 10, ZEND_ACC_PROTECTED } };
+inline constexpr sigtab::Sig getOffsetValueType = { { 161 /* getOffsetValueType */, 1, 11, 1, 12, ZEND_ACC_PUBLIC } };
+inline constexpr sigtab::Sig unionResults = { { 211 /* unionResults */, 1, 13, 1, 14, ZEND_ACC_PROTECTED } };
+inline constexpr sigtab::Sig isAcceptedBy = { { 277 /* isAcceptedBy */, 2, 15, 2, 17, ZEND_ACC_PUBLIC } };
+inline constexpr sigtab::Sig inferTemplateTypes = { { 340 /* inferTemplateTypes */, 1, 18, 1, 19, ZEND_ACC_PUBLIC } };
+inline constexpr sigtab::Sig inferTemplateTypesOn = { { 372 /* inferTemplateTypesOn */, 1, 20, 1, 21, ZEND_ACC_PUBLIC } };
+inline constexpr sigtab::Sig traverse = { { 396 /* traverse */, 1, 22, 1, 23, ZEND_ACC_PUBLIC } };
+inline constexpr sigtab::Sig traverseSimultaneously = { { 411 /* traverseSimultaneously */, 2, 24, 2, 26, ZEND_ACC_PUBLIC } };
+inline constexpr sigtab::Sig toValueSet = { { 456 /* toValueSet */, 0, 27, 0, 27, ZEND_ACC_PUBLIC } };
 } // namespace sig
 
 } // namespace ptdecl::BenevolentUnionType

turbo-ext/tests/type-family.php

@@ -3645,6 +3645,10 @@ public function getValuesArray(): \PHPStan\Type\Constant\ConstantArrayType
 		'benevolentArrays' => new $benevolentClass([new \PHPStan\Type\ArrayType(new \PHPStan\Type\IntegerType(), new \PHPStan\Type\StringType()), $compoundConstShape(['a' => new \PHPStan\Type\IntegerType()])]),
 		'benevolentConsts' => new $benevolentClass([new \PHPStan\Type\Constant\ConstantIntegerType(1), new \PHPStan\Type\Constant\ConstantStringType('a')]),
 		'benevolentObjects' => new $benevolentClass([new \PHPStan\Type\ObjectType(\stdClass::class), new \PHPStan\Type\StringType()]),
+		// both members describe as array<int|string> at the value and type-only levels
+		'benevolentSameArrays' => new $benevolentClass([new \PHPStan\Type\ArrayType(new \PHPStan\Type\IntegerType(), new $unionClass([new \PHPStan\Type\IntegerType(), new \PHPStan\Type\StringType()])), new \PHPStan\Type\ArrayType(new \PHPStan\Type\IntegerType(), new $unionClass([new \PHPStan\Type\IntegerType(), new \PHPStan\Type\StringType()]))]),
+		// both members describe as string at the type-only level
+		'benevolentStrings' => new $benevolentClass([new $intersectionClass([new \PHPStan\Type\StringType(), new \PHPStan\Type\Accessory\AccessoryDecimalIntegerStringType()]), new $intersectionClass([new \PHPStan\Type\StringType(), new \PHPStan\Type\Accessory\AccessoryNonEmptyStringType(), new \PHPStan\Type\Accessory\AccessoryUppercaseStringType()])]),
 		'templateUnion' => \PHPStan\Type\Generic\TemplateTypeFactory::create($compoundScope, 'U', new $unionClass([new \PHPStan\Type\IntegerType(), new \PHPStan\Type\StringType()]), \PHPStan\Type\Generic\TemplateTypeVariance::createInvariant()),
 		'templateBenevolentUnion' => \PHPStan\Type\Generic\TemplateTypeFactory::create($compoundScope, 'B', new $benevolentClass([new \PHPStan\Type\IntegerType(), new \PHPStan\Type\StringType()]), \PHPStan\Type\Generic\TemplateTypeVariance::createInvariant()),
 		'nonEmptyString' => new $intersectionClass([new \PHPStan\Type\StringType(), new \PHPStan\Type\Accessory\AccessoryNonEmptyStringType()]),