From 723001d814874d24edf222079f4320e959c41f3f Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sat, 24 Dec 2022 13:00:40 +0100 Subject: [PATCH 1/6] Failing test --- .../RedundantConditionTest.php | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/TypeReconciliation/RedundantConditionTest.php b/tests/TypeReconciliation/RedundantConditionTest.php index 7b105c7ac3a..12424087477 100644 --- a/tests/TypeReconciliation/RedundantConditionTest.php +++ b/tests/TypeReconciliation/RedundantConditionTest.php @@ -1546,6 +1546,27 @@ function f(array $p) : void { }', 'error_message' => 'RedundantCondition', ], + 'https://github.com/vimeo/psalm/issues/8932' => [ + 'code' => ' 'DocblockTypeContradiction', + ], ]; } } From 06010b40ce83bf08749356e5df66e4bc355570f6 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sat, 24 Dec 2022 13:24:28 +0100 Subject: [PATCH 2/6] Fix --- src/Psalm/Type/MutableUnion.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Psalm/Type/MutableUnion.php b/src/Psalm/Type/MutableUnion.php index d22059efcab..ed1eb405bdd 100644 --- a/src/Psalm/Type/MutableUnion.php +++ b/src/Psalm/Type/MutableUnion.php @@ -234,7 +234,7 @@ public function setTypes(array $types): self $this->typed_class_strings = []; $this->checked = false; - $from_docblock = false; + $from_docblock = $this->from_docblock; $keyed_types = []; foreach ($types as $type) { From ebd5727dec257059629b9db1d371b01e680fcd62 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sat, 24 Dec 2022 13:29:41 +0100 Subject: [PATCH 3/6] Update type --- tests/TypeReconciliation/RedundantConditionTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/TypeReconciliation/RedundantConditionTest.php b/tests/TypeReconciliation/RedundantConditionTest.php index 12424087477..9e0411fabd1 100644 --- a/tests/TypeReconciliation/RedundantConditionTest.php +++ b/tests/TypeReconciliation/RedundantConditionTest.php @@ -1085,7 +1085,7 @@ function takesA(A $a): void {} /** @psalm-suppress PossiblyNullArgument */ takesA($a); if ($a instanceof A) {}', - 'error_message' => 'RedundantCondition - src' . DIRECTORY_SEPARATOR . 'somefile.php:15', + 'error_message' => 'RedundantConditionGivenDocblockType - src' . DIRECTORY_SEPARATOR . 'somefile.php:15', ], 'replaceFalseType' => [ 'code' => ' Date: Sun, 25 Dec 2022 17:35:47 +0100 Subject: [PATCH 4/6] Real fix --- src/Psalm/Type/Atomic.php | 2 +- src/Psalm/Type/MutableUnion.php | 2 +- tests/TypeReconciliation/RedundantConditionTest.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Psalm/Type/Atomic.php b/src/Psalm/Type/Atomic.php index f8de4d373df..ac47951ff8b 100644 --- a/src/Psalm/Type/Atomic.php +++ b/src/Psalm/Type/Atomic.php @@ -162,7 +162,7 @@ public static function create( ?string $text = null, bool $from_docblock = false ): Atomic { - $result = self::createInner($value, $analysis_php_version_id, $template_type_map, $type_aliases); + $result = self::createInner($value, $analysis_php_version_id, $template_type_map, $type_aliases, $from_docblock); $result->offset_start = $offset_start; $result->offset_end = $offset_end; $result->text = $text; diff --git a/src/Psalm/Type/MutableUnion.php b/src/Psalm/Type/MutableUnion.php index ed1eb405bdd..d22059efcab 100644 --- a/src/Psalm/Type/MutableUnion.php +++ b/src/Psalm/Type/MutableUnion.php @@ -234,7 +234,7 @@ public function setTypes(array $types): self $this->typed_class_strings = []; $this->checked = false; - $from_docblock = $this->from_docblock; + $from_docblock = false; $keyed_types = []; foreach ($types as $type) { diff --git a/tests/TypeReconciliation/RedundantConditionTest.php b/tests/TypeReconciliation/RedundantConditionTest.php index 9e0411fabd1..12424087477 100644 --- a/tests/TypeReconciliation/RedundantConditionTest.php +++ b/tests/TypeReconciliation/RedundantConditionTest.php @@ -1085,7 +1085,7 @@ function takesA(A $a): void {} /** @psalm-suppress PossiblyNullArgument */ takesA($a); if ($a instanceof A) {}', - 'error_message' => 'RedundantConditionGivenDocblockType - src' . DIRECTORY_SEPARATOR . 'somefile.php:15', + 'error_message' => 'RedundantCondition - src' . DIRECTORY_SEPARATOR . 'somefile.php:15', ], 'replaceFalseType' => [ 'code' => ' Date: Sun, 25 Dec 2022 18:25:40 +0100 Subject: [PATCH 5/6] Try --- src/Psalm/Internal/Type/TypeCombiner.php | 62 ++++++++++++++----- src/Psalm/Type/Atomic.php | 8 ++- .../RedundantConditionTest.php | 8 ++- 3 files changed, 60 insertions(+), 18 deletions(-) diff --git a/src/Psalm/Internal/Type/TypeCombiner.php b/src/Psalm/Internal/Type/TypeCombiner.php index 89abc13b52c..4af0f2b821c 100644 --- a/src/Psalm/Internal/Type/TypeCombiner.php +++ b/src/Psalm/Internal/Type/TypeCombiner.php @@ -210,6 +210,7 @@ public static function combine( $new_types = self::handleKeyedArrayEntries( $combination, $overwrite_empty_array, + $from_docblock, ); } @@ -225,6 +226,7 @@ public static function combine( $allow_mixed_union, $type, $combination->array_type_params, + $from_docblock, ); } @@ -239,7 +241,7 @@ public static function combine( foreach ($combination->builtin_type_params as $generic_type => $generic_type_params) { if ($generic_type === 'iterable') { assert(count($generic_type_params) <= 2); - $new_types[] = new TIterable($generic_type_params); + $new_types[] = new TIterable($generic_type_params, [], $from_docblock); } else { /** @psalm-suppress ArgumentTypeCoercion Caused by the PropertyTypeCoercion above */ $generic_object = new TGenericObject( @@ -248,6 +250,7 @@ public static function combine( false, false, $combination->extra_types, + $from_docblock, ); $new_types[] = $generic_object; @@ -267,6 +270,7 @@ public static function combine( false, $combination->object_static[$generic_type] ?? false, $combination->extra_types, + $from_docblock, ); $new_types[] = $generic_object; @@ -293,9 +297,16 @@ public static function combine( foreach ($object_type->getAtomicTypes() as $object_atomic_type) { if ($object_atomic_type instanceof TNamedObject) { - $new_types[] = new TClassString($object_atomic_type->value, $object_atomic_type); + $new_types[] = new TClassString( + $object_atomic_type->value, + $object_atomic_type, + false, + false, + false, + $from_docblock, + ); } elseif ($object_atomic_type instanceof TObject) { - $new_types[] = new TClassString(); + $new_types[] = new TClassString('object', null, false, false, false, $from_docblock); } } } @@ -351,7 +362,7 @@ public static function combine( continue; } - $new_types[] = $type; + $new_types[] = $type->setFromDocblock($from_docblock); } if (!$new_types && !$has_never) { @@ -1321,7 +1332,8 @@ private static function getClassLikes(Codebase $codebase, string $fq_classlike_n */ private static function handleKeyedArrayEntries( TypeCombination $combination, - bool $overwrite_empty_array + bool $overwrite_empty_array, + bool $from_docblock ): array { $new_types = []; @@ -1397,6 +1409,7 @@ private static function handleKeyedArrayEntries( ? null : [$fallback_key_type, $fallback_value_type], (bool)$combination->all_arrays_lists, + $from_docblock, ); } else { $objectlike = new TKeyedArray( @@ -1406,6 +1419,7 @@ private static function handleKeyedArrayEntries( ? null : [$fallback_key_type, $fallback_value_type], (bool)$combination->all_arrays_lists, + $from_docblock, ); } @@ -1414,9 +1428,15 @@ private static function handleKeyedArrayEntries( $key_type = $combination->objectlike_key_type ?? Type::getArrayKey(); $value_type = $combination->objectlike_value_type ?? Type::getMixed(); if ($combination->array_always_filled) { - $new_types[] = new TNonEmptyArray([$key_type, $value_type]); + $new_types[] = new TNonEmptyArray( + [$key_type, $value_type], + null, + null, + 'non-empty-array', + $from_docblock, + ); } else { - $new_types[] = new TArray([$key_type, $value_type]); + $new_types[] = new TArray([$key_type, $value_type], $from_docblock); } } @@ -1436,7 +1456,8 @@ private static function getArrayTypeFromGenericParams( bool $overwrite_empty_array, bool $allow_mixed_union, Atomic $type, - array $generic_type_params + array $generic_type_params, + bool $from_docblock ): Atomic { if ($combination->objectlike_entries) { $objectlike_generic_type = null; @@ -1455,11 +1476,11 @@ private static function getArrayTypeFromGenericParams( ); if (is_int($property_name)) { - $objectlike_keys[$property_name] = new TLiteralInt($property_name); + $objectlike_keys[$property_name] = new TLiteralInt($property_name, $from_docblock); } elseif ($type instanceof TKeyedArray && isset($type->class_strings[$property_name])) { - $objectlike_keys[$property_name] = new TLiteralClassString($property_name); + $objectlike_keys[$property_name] = new TLiteralClassString($property_name, $from_docblock); } else { - $objectlike_keys[$property_name] = new TLiteralString($property_name); + $objectlike_keys[$property_name] = new TLiteralString($property_name, $from_docblock); } } @@ -1504,7 +1525,13 @@ private static function getArrayTypeFromGenericParams( } if ($combination->all_arrays_callable) { - $array_type = new TCallableArray($generic_type_params); + $array_type = new TCallableArray( + $generic_type_params, + null, + null, + 'callable-array', + $from_docblock, + ); } elseif ($combination->array_always_filled || ($combination->array_sometimes_filled && $overwrite_empty_array) || ($combination->objectlike_entries @@ -1522,6 +1549,7 @@ private static function getArrayTypeFromGenericParams( null, [Type::getInt(), $combination->array_type_params[1]], true, + $from_docblock, ); } elseif ($combination->array_counts && count($combination->array_counts) === 1) { $cnt = array_keys($combination->array_counts)[0]; @@ -1535,6 +1563,7 @@ private static function getArrayTypeFromGenericParams( null, null, true, + $from_docblock, ); } else { $cnt = $combination->array_min_counts @@ -1552,6 +1581,7 @@ private static function getArrayTypeFromGenericParams( null, [Type::getListKey(), $generic_type_params[1]], true, + $from_docblock, ); } } else { @@ -1561,6 +1591,9 @@ private static function getArrayTypeFromGenericParams( $combination->array_min_counts ? min(array_keys($combination->array_min_counts)) : null, + null, + 'non-empty-array', + $from_docblock, ); } } else { @@ -1572,11 +1605,12 @@ private static function getArrayTypeFromGenericParams( array_keys($combination->class_string_map_names)[0], array_values($combination->class_string_map_as_types)[0], $generic_type_params[1], + $from_docblock, ); } elseif ($combination->all_arrays_lists) { - $array_type = Type::getListAtomic($generic_type_params[1]); + $array_type = Type::getListAtomic($generic_type_params[1], $from_docblock); } else { - $array_type = new TArray($generic_type_params); + $array_type = new TArray($generic_type_params, $from_docblock); } } diff --git a/src/Psalm/Type/Atomic.php b/src/Psalm/Type/Atomic.php index ac47951ff8b..520ff00f6c8 100644 --- a/src/Psalm/Type/Atomic.php +++ b/src/Psalm/Type/Atomic.php @@ -162,7 +162,13 @@ public static function create( ?string $text = null, bool $from_docblock = false ): Atomic { - $result = self::createInner($value, $analysis_php_version_id, $template_type_map, $type_aliases, $from_docblock); + $result = self::createInner( + $value, + $analysis_php_version_id, + $template_type_map, + $type_aliases, + $from_docblock, + ); $result->offset_start = $offset_start; $result->offset_end = $offset_end; $result->text = $text; diff --git a/tests/TypeReconciliation/RedundantConditionTest.php b/tests/TypeReconciliation/RedundantConditionTest.php index 12424087477..88142c7187e 100644 --- a/tests/TypeReconciliation/RedundantConditionTest.php +++ b/tests/TypeReconciliation/RedundantConditionTest.php @@ -1546,12 +1546,14 @@ function f(array $p) : void { }', 'error_message' => 'RedundantCondition', ], - 'https://github.com/vimeo/psalm/issues/8932' => [ + 'from_docblock should be kept when removing types' => [ 'code' => ' Date: Sun, 25 Dec 2022 19:21:48 +0100 Subject: [PATCH 6/6] Simplify --- src/Psalm/Internal/Type/TypeCombiner.php | 48 +++++++----------------- 1 file changed, 14 insertions(+), 34 deletions(-) diff --git a/src/Psalm/Internal/Type/TypeCombiner.php b/src/Psalm/Internal/Type/TypeCombiner.php index 4af0f2b821c..1593c13d498 100644 --- a/src/Psalm/Internal/Type/TypeCombiner.php +++ b/src/Psalm/Internal/Type/TypeCombiner.php @@ -297,17 +297,14 @@ public static function combine( foreach ($object_type->getAtomicTypes() as $object_atomic_type) { if ($object_atomic_type instanceof TNamedObject) { - $new_types[] = new TClassString( - $object_atomic_type->value, - $object_atomic_type, - false, - false, - false, - $from_docblock, - ); + $class_type = new TClassString($object_atomic_type->value, $object_atomic_type); } elseif ($object_atomic_type instanceof TObject) { - $new_types[] = new TClassString('object', null, false, false, false, $from_docblock); + $class_type = new TClassString(); + } else { + continue; } + + $new_types[] = $class_type->setFromDocblock($from_docblock); } } } @@ -1428,16 +1425,12 @@ private static function handleKeyedArrayEntries( $key_type = $combination->objectlike_key_type ?? Type::getArrayKey(); $value_type = $combination->objectlike_value_type ?? Type::getMixed(); if ($combination->array_always_filled) { - $new_types[] = new TNonEmptyArray( - [$key_type, $value_type], - null, - null, - 'non-empty-array', - $from_docblock, - ); + $array_type = new TNonEmptyArray([$key_type, $value_type]); } else { - $new_types[] = new TArray([$key_type, $value_type], $from_docblock); + $array_type = new TArray([$key_type, $value_type]); } + + $new_types[] = $array_type->setFromDocblock($from_docblock); } // if we're merging an empty array with an object-like, clobber empty array @@ -1525,13 +1518,7 @@ private static function getArrayTypeFromGenericParams( } if ($combination->all_arrays_callable) { - $array_type = new TCallableArray( - $generic_type_params, - null, - null, - 'callable-array', - $from_docblock, - ); + $array_type = new TCallableArray($generic_type_params); } elseif ($combination->array_always_filled || ($combination->array_sometimes_filled && $overwrite_empty_array) || ($combination->objectlike_entries @@ -1549,7 +1536,6 @@ private static function getArrayTypeFromGenericParams( null, [Type::getInt(), $combination->array_type_params[1]], true, - $from_docblock, ); } elseif ($combination->array_counts && count($combination->array_counts) === 1) { $cnt = array_keys($combination->array_counts)[0]; @@ -1563,7 +1549,6 @@ private static function getArrayTypeFromGenericParams( null, null, true, - $from_docblock, ); } else { $cnt = $combination->array_min_counts @@ -1581,7 +1566,6 @@ private static function getArrayTypeFromGenericParams( null, [Type::getListKey(), $generic_type_params[1]], true, - $from_docblock, ); } } else { @@ -1591,9 +1575,6 @@ private static function getArrayTypeFromGenericParams( $combination->array_min_counts ? min(array_keys($combination->array_min_counts)) : null, - null, - 'non-empty-array', - $from_docblock, ); } } else { @@ -1605,15 +1586,14 @@ private static function getArrayTypeFromGenericParams( array_keys($combination->class_string_map_names)[0], array_values($combination->class_string_map_as_types)[0], $generic_type_params[1], - $from_docblock, ); } elseif ($combination->all_arrays_lists) { - $array_type = Type::getListAtomic($generic_type_params[1], $from_docblock); + $array_type = Type::getListAtomic($generic_type_params[1]); } else { - $array_type = new TArray($generic_type_params, $from_docblock); + $array_type = new TArray($generic_type_params); } } - return $array_type; + return $array_type->setFromDocblock($from_docblock); } }