From 739d87dba7de72e290fe3aeb6e8e9fd3a8cc163f Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Sat, 9 Mar 2024 18:25:46 +0100 Subject: [PATCH 01/28] Use wider class-string when combining class strings with intersections Fixes vimeo/psalm#10799 --- src/Psalm/Internal/Type/TypeCombiner.php | 15 ++++++++++++++- tests/TypeCombinationTest.php | 7 +++++++ tests/TypeParseTest.php | 8 ++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/Psalm/Internal/Type/TypeCombiner.php b/src/Psalm/Internal/Type/TypeCombiner.php index 099ae82c7fc..5e05fea1cff 100644 --- a/src/Psalm/Internal/Type/TypeCombiner.php +++ b/src/Psalm/Internal/Type/TypeCombiner.php @@ -1005,7 +1005,20 @@ private static function scrapeStringProperties( if (!$type->as_type) { $combination->class_string_types['object'] = new TObject(); } else { - $combination->class_string_types[$type->as] = $type->as_type; + if (isset($combination->class_string_types[$type->as]) + && $combination->class_string_types[$type->as] instanceof TNamedObject + ) { + if ($combination->class_string_types[$type->as]->extra_types === []) { + // do nothing, existing type is wider or the same + } elseif ($type->as_type->extra_types === []) { + $combination->class_string_types[$type->as] = $type->as_type; + } else { + // todo: figure out what to do with class-string|class-string + $combination->class_string_types[$type->as] = $type->as_type; + } + } else { + $combination->class_string_types[$type->as] = $type->as_type; + } } } elseif ($type instanceof TLiteralString) { if ($combination->strings !== null && count($combination->strings) < $literal_limit) { diff --git a/tests/TypeCombinationTest.php b/tests/TypeCombinationTest.php index 841fbd67e36..8cab04fcacd 100644 --- a/tests/TypeCombinationTest.php +++ b/tests/TypeCombinationTest.php @@ -940,6 +940,13 @@ public function providerTestValidTypeCombination(): array '"0"', ], ], + 'unionOfClassStringAndClassStringWithIntersection' => [ + 'class-string', + [ + 'class-string', + 'class-string', + ], + ], ]; } diff --git a/tests/TypeParseTest.php b/tests/TypeParseTest.php index 2156257e2e5..e3ddeec2ad2 100644 --- a/tests/TypeParseTest.php +++ b/tests/TypeParseTest.php @@ -1148,6 +1148,14 @@ public function testIntMaskOfWithValidValueOf(): void $this->assertSame('int-mask-of>', $docblock_type->getId()); } + public function testUnionOfClassStringAndClassStringWithIntersection(): void + { + $this->assertSame( + 'class-string', + (string) Type::parseString('class-string|class-string'), + ); + } + public function testReflectionTypeParse(): void { if (!function_exists('Psalm\Tests\someFunction')) { From 00c2fef25d8c038d6faa0c6b5df027d0dd6355b2 Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Sat, 9 Mar 2024 23:10:08 +0100 Subject: [PATCH 02/28] Forbid named arguments for ArrayAcccess methods Fixes vimeo/psalm#10533 --- stubs/CoreGenericClasses.phpstub | 8 ++++++++ tests/ArrayAccessTest.php | 29 +++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/stubs/CoreGenericClasses.phpstub b/stubs/CoreGenericClasses.phpstub index 542fec35875..93cb8cb42f0 100644 --- a/stubs/CoreGenericClasses.phpstub +++ b/stubs/CoreGenericClasses.phpstub @@ -82,6 +82,7 @@ interface ArrayAccess { * The return value will be casted to boolean if non-boolean was returned. * * @since 5.0.0 + * @no-named-arguments because of conflict with ArrayObject */ public function offsetExists($offset); @@ -94,6 +95,7 @@ interface ArrayAccess { * @psalm-ignore-nullable-return * * @since 5.0.0 + * @no-named-arguments because of conflict with ArrayObject */ public function offsetGet($offset); @@ -106,6 +108,7 @@ interface ArrayAccess { * @return void * * @since 5.0.0 + * @no-named-arguments because of conflict with ArrayObject */ public function offsetSet($offset, $value); @@ -117,6 +120,7 @@ interface ArrayAccess { * @return void * * @since 5.0.0 + * @no-named-arguments because of conflict with ArrayObject */ public function offsetUnset($offset); } @@ -162,6 +166,7 @@ class ArrayObject implements IteratorAggregate, ArrayAccess, Serializable, Count * @return bool true if the requested index exists, otherwise false * * @since 5.0.0 + * @no-named-arguments because of conflict with ArrayAccess */ public function offsetExists($offset) { } @@ -173,6 +178,7 @@ class ArrayObject implements IteratorAggregate, ArrayAccess, Serializable, Count * @return TValue The value at the specified index or false. * * @since 5.0.0 + * @no-named-arguments because of conflict with ArrayAccess */ public function offsetGet($offset) { } @@ -185,6 +191,7 @@ class ArrayObject implements IteratorAggregate, ArrayAccess, Serializable, Count * @return void * * @since 5.0.0 + * @no-named-arguments because of conflict with ArrayAccess */ public function offsetSet($offset, $value) { } @@ -196,6 +203,7 @@ class ArrayObject implements IteratorAggregate, ArrayAccess, Serializable, Count * @return void * * @since 5.0.0 + * @no-named-arguments because of conflict with ArrayAccess */ public function offsetUnset($offset) { } diff --git a/tests/ArrayAccessTest.php b/tests/ArrayAccessTest.php index 8481ac569e4..ba4b6ec7988 100644 --- a/tests/ArrayAccessTest.php +++ b/tests/ArrayAccessTest.php @@ -1233,6 +1233,25 @@ public function f(): void { 'assertions' => [], 'ignored_issues' => ['UndefinedDocblockClass'], ], + 'canExtendArrayObjectOffsetSet' => [ + 'code' => <<<'PHP' + */ + class C extends ArrayObject { + public function offsetSet(mixed $key, mixed $value): void { + parent::offsetSet($key, $value); + } + } + PHP, + 'assertions' => [], + 'ignored_issues' => [], + 'php_version' => '8.0', + ], ]; } @@ -1560,6 +1579,16 @@ function takesArrayOfFloats(array $arr): void { if ($x === null) {}', 'error_message' => 'PossiblyUndefinedArrayOffset', ], + 'cannotUseNamedArgumentsForArrayAccess' => [ + 'code' => <<<'PHP' + $a */ + function f(ArrayAccess $a): void { + echo $a->offsetGet(offset: 0); + } + PHP, + 'error_message' => 'NamedArgumentNotAllowed', + ], ]; } } From 20c788911f7e43cf18225c44392c34ebc2f02756 Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Sun, 10 Mar 2024 22:17:38 +0100 Subject: [PATCH 03/28] Allow more callable types as subtypes of `callable` Fixes vimeo/psalm#10461 --- .../Type/Comparator/AtomicTypeComparator.php | 8 +++++- .../Comparator/CallableTypeComparator.php | 25 ++++++++++++++++++- tests/CallableTest.php | 15 +++++++++++ tests/TypeComparatorTest.php | 16 ++++++++++++ 4 files changed, 62 insertions(+), 2 deletions(-) diff --git a/src/Psalm/Internal/Type/Comparator/AtomicTypeComparator.php b/src/Psalm/Internal/Type/Comparator/AtomicTypeComparator.php index 0b3478ea978..85000bec7c7 100644 --- a/src/Psalm/Internal/Type/Comparator/AtomicTypeComparator.php +++ b/src/Psalm/Internal/Type/Comparator/AtomicTypeComparator.php @@ -8,6 +8,7 @@ use Psalm\Type\Atomic\Scalar; use Psalm\Type\Atomic\TArray; use Psalm\Type\Atomic\TCallable; +use Psalm\Type\Atomic\TCallableArray; use Psalm\Type\Atomic\TCallableKeyedArray; use Psalm\Type\Atomic\TCallableObject; use Psalm\Type\Atomic\TCallableString; @@ -191,7 +192,12 @@ public static function isContainedBy( } if (($container_type_part instanceof TCallable - && $input_type_part instanceof TCallable) + && ($input_type_part instanceof TCallable + || $input_type_part instanceof TCallableArray + || $input_type_part instanceof TCallableObject + || $input_type_part instanceof TCallableString + || $input_type_part instanceof TCallableKeyedArray + )) || ($container_type_part instanceof TClosure && $input_type_part instanceof TClosure) ) { diff --git a/src/Psalm/Internal/Type/Comparator/CallableTypeComparator.php b/src/Psalm/Internal/Type/Comparator/CallableTypeComparator.php index 4c7c3b8c7df..393c8369167 100644 --- a/src/Psalm/Internal/Type/Comparator/CallableTypeComparator.php +++ b/src/Psalm/Internal/Type/Comparator/CallableTypeComparator.php @@ -19,6 +19,9 @@ use Psalm\Type\Atomic\TArray; use Psalm\Type\Atomic\TCallable; use Psalm\Type\Atomic\TCallableArray; +use Psalm\Type\Atomic\TCallableKeyedArray; +use Psalm\Type\Atomic\TCallableObject; +use Psalm\Type\Atomic\TCallableString; use Psalm\Type\Atomic\TClassString; use Psalm\Type\Atomic\TClosure; use Psalm\Type\Atomic\TKeyedArray; @@ -41,7 +44,7 @@ final class CallableTypeComparator { /** - * @param TCallable|TClosure $input_type_part + * @param TCallable|TClosure|TCallableArray|TCallableString|TCallableKeyedArray|TCallableObject $input_type_part * @param TCallable|TClosure $container_type_part */ public static function isContainedBy( @@ -50,6 +53,26 @@ public static function isContainedBy( Atomic $container_type_part, ?TypeComparisonResult $atomic_comparison_result ): bool { + if ($container_type_part instanceof TClosure) { + if ($input_type_part instanceof TCallableArray + || $input_type_part instanceof TCallableString + || $input_type_part instanceof TCallableKeyedArray + || $input_type_part instanceof TCallableObject + ) { + if ($atomic_comparison_result) { + $atomic_comparison_result->type_coerced = true; + } + return false; + } + } + if ($input_type_part instanceof TCallableArray + || $input_type_part instanceof TCallableString + || $input_type_part instanceof TCallableKeyedArray + || $input_type_part instanceof TCallableObject + ) { + return true; + } + if ($container_type_part->is_pure && !$input_type_part->is_pure) { if ($atomic_comparison_result) { $atomic_comparison_result->type_coerced = $input_type_part->is_pure === null; diff --git a/tests/CallableTest.php b/tests/CallableTest.php index e10784b0b3f..eff0e838a97 100644 --- a/tests/CallableTest.php +++ b/tests/CallableTest.php @@ -1920,6 +1920,21 @@ function f(callable $c): void { 'ignored_issues' => [], 'php_version' => '8.0', ], + 'callableArrayPassedAsCallable' => [ + 'code' => <<<'PHP' + [ + 'callable', + "callable-array{0: class-string, 1: 'from'}", + ], + 'callableAcceptsCallableObject' => [ + 'callable', + "callable-object", + ], + 'callableAcceptsCallableString' => [ + 'callable', + 'callable-string', + ], + 'callableAcceptsCallableKeyedList' => [ + 'callable', + "callable-list{class-string, 'from'}", + ], ]; } From 4a2cf93d7a2b3733d467bdc267dba55785f0af90 Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Mon, 11 Mar 2024 01:41:52 +0100 Subject: [PATCH 04/28] Don't crash on invalid templates Fixes vimeo/psalm#9596 --- .../PhpVisitor/Reflector/ClassLikeNodeScanner.php | 6 ++++-- tests/Template/ClassTemplateTest.php | 10 ++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php index b0f4003c49a..325fd4e77a5 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php @@ -434,9 +434,11 @@ public function start(PhpParser\Node\Stmt\ClassLike $node): ?bool try { $type_string = CommentAnalyzer::splitDocLine($type_string)[0]; } catch (DocblockParseException $e) { - throw new DocblockParseException( - $type_string . ' is not a valid type: ' . $e->getMessage(), + $storage->docblock_issues[] = new InvalidDocblock( + $e->getMessage() . ' in docblock for ' . $fq_classlike_name, + $name_location ?? $class_location, ); + continue; } $type_string = CommentAnalyzer::sanitizeDocblockType($type_string); try { diff --git a/tests/Template/ClassTemplateTest.php b/tests/Template/ClassTemplateTest.php index 12c0025461a..b7dcf2c582f 100644 --- a/tests/Template/ClassTemplateTest.php +++ b/tests/Template/ClassTemplateTest.php @@ -5025,6 +5025,16 @@ function getMixedCollection(MyCollection $c): MyCollection { }', 'error_message' => 'InvalidReturnStatement', ], + 'noCrashOnBrokenTemplate' => [ + 'code' => <<<'PHP' + |string + */ + class C {} + PHP, + 'error_message' => 'InvalidDocblock', + ], ]; } } From 2a91bd66162df453e63cbcc687320f6c94131619 Mon Sep 17 00:00:00 2001 From: Aleksandr Zhuravlev Date: Mon, 11 Mar 2024 20:09:05 +1000 Subject: [PATCH 05/28] Added test for #10807 --- tests/AssertAnnotationTest.php | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/AssertAnnotationTest.php b/tests/AssertAnnotationTest.php index 972e2e3d72f..5263a5267a0 100644 --- a/tests/AssertAnnotationTest.php +++ b/tests/AssertAnnotationTest.php @@ -90,6 +90,36 @@ function requiresString(string $_str): void {} $this->analyzeFile('somefile.php', new Context()); } + public function testAssertsAllongCallStaticMethodWork(): void + { + $this->addFile( + 'somefile.php', + 'analyzeFile('somefile.php', new Context()); + } + public function testAssertInvalidDocblockMessageDoesNotIncludeTrace(): void { $this->expectException(CodeException::class); From fe42e8821d6f9ae8e75be7f3e5d2189faccafc51 Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Mon, 11 Mar 2024 18:48:38 +0000 Subject: [PATCH 06/28] Resolve fail test #10807 Resolved problem: ``` Psalm\Tests\AssertAnnotationTest::testAssertsAllongCallStaticMethodWork Psalm\Exception\CodeException: LessSpecificReturnStatement The type 'string' is more general than the declared return type 'non-empty-string' for returnNonEmpty ``` --- .../Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php index 48c50ed0c24..c9dce4d1460 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php @@ -789,7 +789,7 @@ private static function handleNamedCall( } } - if (!$callstatic_method_exists || $class_storage->hasSealedMethods($config)) { + if ($naive_method_exists || !$callstatic_method_exists || $class_storage->hasSealedMethods($config)) { $does_method_exist = MethodAnalyzer::checkMethodExists( $codebase, $method_id, From c9468b63e523dcdb67fd94f9bad877639460fa56 Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Tue, 12 Mar 2024 12:51:49 +0100 Subject: [PATCH 07/28] Introduce `TCallableInterface` `TClosure` does not implement it at this point. It's intentional, to maintain the separation between callables and closures that our code relies on. --- .../Type/Comparator/AtomicTypeComparator.php | 10 +++------- .../Comparator/CallableTypeComparator.php | 20 +++++++------------ src/Psalm/Type/Atomic/TCallable.php | 2 +- src/Psalm/Type/Atomic/TCallableArray.php | 2 +- src/Psalm/Type/Atomic/TCallableInterface.php | 7 +++++++ src/Psalm/Type/Atomic/TCallableKeyedArray.php | 2 +- src/Psalm/Type/Atomic/TCallableList.php | 2 +- src/Psalm/Type/Atomic/TCallableObject.php | 2 +- src/Psalm/Type/Atomic/TCallableString.php | 2 +- 9 files changed, 23 insertions(+), 26 deletions(-) create mode 100644 src/Psalm/Type/Atomic/TCallableInterface.php diff --git a/src/Psalm/Internal/Type/Comparator/AtomicTypeComparator.php b/src/Psalm/Internal/Type/Comparator/AtomicTypeComparator.php index 85000bec7c7..15c4512429f 100644 --- a/src/Psalm/Internal/Type/Comparator/AtomicTypeComparator.php +++ b/src/Psalm/Internal/Type/Comparator/AtomicTypeComparator.php @@ -8,7 +8,7 @@ use Psalm\Type\Atomic\Scalar; use Psalm\Type\Atomic\TArray; use Psalm\Type\Atomic\TCallable; -use Psalm\Type\Atomic\TCallableArray; +use Psalm\Type\Atomic\TCallableInterface; use Psalm\Type\Atomic\TCallableKeyedArray; use Psalm\Type\Atomic\TCallableObject; use Psalm\Type\Atomic\TCallableString; @@ -192,12 +192,8 @@ public static function isContainedBy( } if (($container_type_part instanceof TCallable - && ($input_type_part instanceof TCallable - || $input_type_part instanceof TCallableArray - || $input_type_part instanceof TCallableObject - || $input_type_part instanceof TCallableString - || $input_type_part instanceof TCallableKeyedArray - )) + && $input_type_part instanceof TCallableInterface + ) || ($container_type_part instanceof TClosure && $input_type_part instanceof TClosure) ) { diff --git a/src/Psalm/Internal/Type/Comparator/CallableTypeComparator.php b/src/Psalm/Internal/Type/Comparator/CallableTypeComparator.php index 393c8369167..7573743acc8 100644 --- a/src/Psalm/Internal/Type/Comparator/CallableTypeComparator.php +++ b/src/Psalm/Internal/Type/Comparator/CallableTypeComparator.php @@ -19,9 +19,7 @@ use Psalm\Type\Atomic\TArray; use Psalm\Type\Atomic\TCallable; use Psalm\Type\Atomic\TCallableArray; -use Psalm\Type\Atomic\TCallableKeyedArray; -use Psalm\Type\Atomic\TCallableObject; -use Psalm\Type\Atomic\TCallableString; +use Psalm\Type\Atomic\TCallableInterface; use Psalm\Type\Atomic\TClassString; use Psalm\Type\Atomic\TClosure; use Psalm\Type\Atomic\TKeyedArray; @@ -44,20 +42,18 @@ final class CallableTypeComparator { /** - * @param TCallable|TClosure|TCallableArray|TCallableString|TCallableKeyedArray|TCallableObject $input_type_part + * @param TClosure|TCallableInterface $input_type_part * @param TCallable|TClosure $container_type_part */ public static function isContainedBy( Codebase $codebase, - Atomic $input_type_part, + $input_type_part, Atomic $container_type_part, ?TypeComparisonResult $atomic_comparison_result ): bool { if ($container_type_part instanceof TClosure) { - if ($input_type_part instanceof TCallableArray - || $input_type_part instanceof TCallableString - || $input_type_part instanceof TCallableKeyedArray - || $input_type_part instanceof TCallableObject + if ($input_type_part instanceof TCallableInterface + && !$input_type_part instanceof TCallable // it has stricter checks below ) { if ($atomic_comparison_result) { $atomic_comparison_result->type_coerced = true; @@ -65,10 +61,8 @@ public static function isContainedBy( return false; } } - if ($input_type_part instanceof TCallableArray - || $input_type_part instanceof TCallableString - || $input_type_part instanceof TCallableKeyedArray - || $input_type_part instanceof TCallableObject + if ($input_type_part instanceof TCallableInterface + && !$input_type_part instanceof TCallable // it has stricter checks below ) { return true; } diff --git a/src/Psalm/Type/Atomic/TCallable.php b/src/Psalm/Type/Atomic/TCallable.php index 7ef847d4ab1..bcd925b2643 100644 --- a/src/Psalm/Type/Atomic/TCallable.php +++ b/src/Psalm/Type/Atomic/TCallable.php @@ -14,7 +14,7 @@ * * @psalm-immutable */ -final class TCallable extends Atomic +final class TCallable extends Atomic implements TCallableInterface { use CallableTrait; diff --git a/src/Psalm/Type/Atomic/TCallableArray.php b/src/Psalm/Type/Atomic/TCallableArray.php index d4992523b06..695a9d0e8b9 100644 --- a/src/Psalm/Type/Atomic/TCallableArray.php +++ b/src/Psalm/Type/Atomic/TCallableArray.php @@ -7,7 +7,7 @@ * * @psalm-immutable */ -final class TCallableArray extends TNonEmptyArray +final class TCallableArray extends TNonEmptyArray implements TCallableInterface { /** * @var string diff --git a/src/Psalm/Type/Atomic/TCallableInterface.php b/src/Psalm/Type/Atomic/TCallableInterface.php new file mode 100644 index 00000000000..e25f076d1a7 --- /dev/null +++ b/src/Psalm/Type/Atomic/TCallableInterface.php @@ -0,0 +1,7 @@ + Date: Wed, 13 Mar 2024 09:11:17 +0100 Subject: [PATCH 08/28] add support for named arguments for filter_var and filter_input this fixes #10809 --- .../FilterInputReturnTypeProvider.php | 12 +++++++++++- .../FilterVarReturnTypeProvider.php | 12 +++++++++++- tests/FunctionCallTest.php | 8 ++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/Psalm/Internal/Provider/ReturnTypeProvider/FilterInputReturnTypeProvider.php b/src/Psalm/Internal/Provider/ReturnTypeProvider/FilterInputReturnTypeProvider.php index 09b32b2aa65..6b410c306aa 100644 --- a/src/Psalm/Internal/Provider/ReturnTypeProvider/FilterInputReturnTypeProvider.php +++ b/src/Psalm/Internal/Provider/ReturnTypeProvider/FilterInputReturnTypeProvider.php @@ -12,6 +12,7 @@ use Psalm\Type\Union; use UnexpectedValueException; +use function array_flip; use function array_search; use function in_array; use function is_array; @@ -48,7 +49,16 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev throw new UnexpectedValueException('Expected StatementsAnalyzer not StatementsSource'); } - $call_args = $event->getCallArgs(); + $arg_names = array_flip(['type', 'var_name', 'filter', 'options']); + $call_args = []; + foreach ($event->getCallArgs() as $idx => $arg) { + if (isset($arg->name)) { + $call_args[$arg_names[$arg->name->name]] = $arg; + } else { + $call_args[$idx] = $arg; + } + } + $function_id = $event->getFunctionId(); $code_location = $event->getCodeLocation(); $codebase = $statements_analyzer->getCodebase(); diff --git a/src/Psalm/Internal/Provider/ReturnTypeProvider/FilterVarReturnTypeProvider.php b/src/Psalm/Internal/Provider/ReturnTypeProvider/FilterVarReturnTypeProvider.php index c6ea6a15fdf..76f6caa6280 100644 --- a/src/Psalm/Internal/Provider/ReturnTypeProvider/FilterVarReturnTypeProvider.php +++ b/src/Psalm/Internal/Provider/ReturnTypeProvider/FilterVarReturnTypeProvider.php @@ -9,6 +9,7 @@ use Psalm\Type\Union; use UnexpectedValueException; +use function array_flip; use function is_array; use function is_int; @@ -37,7 +38,16 @@ public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $ev throw new UnexpectedValueException(); } - $call_args = $event->getCallArgs(); + $arg_names = array_flip(['value', 'filter', 'options']); + $call_args = []; + foreach ($event->getCallArgs() as $idx => $arg) { + if (isset($arg->name)) { + $call_args[$arg_names[$arg->name->name]] = $arg; + } else { + $call_args[$idx] = $arg; + } + } + $function_id = $event->getFunctionId(); $code_location = $event->getCodeLocation(); $codebase = $statements_analyzer->getCodebase(); diff --git a/tests/FunctionCallTest.php b/tests/FunctionCallTest.php index d667552666a..833b38af431 100644 --- a/tests/FunctionCallTest.php +++ b/tests/FunctionCallTest.php @@ -1075,6 +1075,9 @@ function foo(string $s) : void { ], 'filterInput' => [ 'code' => ' [ 'code' => ' Date: Wed, 13 Mar 2024 11:45:54 +0100 Subject: [PATCH 09/28] Promoted properties missing in extended __construct should report PropertyNotSetInConstructor Fix #10786 --- src/Psalm/Internal/Analyzer/ClassAnalyzer.php | 22 +++++++++++-- .../Expression/Call/StaticCallAnalyzer.php | 22 +++++++++++++ tests/PropertyTypeTest.php | 33 +++++++++++++++++++ 3 files changed, 75 insertions(+), 2 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/ClassAnalyzer.php b/src/Psalm/Internal/Analyzer/ClassAnalyzer.php index fead38b71d0..8e850570012 100644 --- a/src/Psalm/Internal/Analyzer/ClassAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/ClassAnalyzer.php @@ -870,7 +870,12 @@ public static function addContextProperties( $property_type = $property_storage->type; if (!$property_type->isMixed() - && !$property_storage->is_promoted + && (!$property_storage->is_promoted + || (strtolower($fq_class_name) !== strtolower($property_class_name) + && isset($storage->declaring_method_ids['__construct']) + && strtolower( + $storage->declaring_method_ids['__construct']->fq_class_name, + ) === strtolower($fq_class_name))) && !$property_storage->has_default && !($property_type->isNullable() && $property_type->from_docblock) ) { @@ -881,7 +886,13 @@ public static function addContextProperties( ]); } } else { - if (!$property_storage->has_default && !$property_storage->is_promoted) { + if (!$property_storage->has_default + && (!$property_storage->is_promoted + || (strtolower($fq_class_name) !== strtolower($property_class_name) + && isset($storage->declaring_method_ids['__construct']) + && strtolower( + $storage->declaring_method_ids['__construct']->fq_class_name, + ) === strtolower($fq_class_name)))) { $property_type = new Union([new TMixed()], [ 'initialized' => false, 'from_property' => true, @@ -1097,6 +1108,13 @@ private function checkPropertyInitialization( continue; } + if ($property->is_promoted + && strtolower($property_class_name) !== $fq_class_name_lc + && isset($storage->declaring_method_ids['__construct']) + && strtolower($storage->declaring_method_ids['__construct']->fq_class_name) === $fq_class_name_lc) { + $property_is_initialized = false; + } + if ($property->has_default || $property_is_initialized) { continue; } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php index a357afac605..e78cfd8d230 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php @@ -81,6 +81,28 @@ public static function analyze( $class_storage = $codebase->classlike_storage_provider->get($fq_class_name); $fq_class_name = $class_storage->name; + + if ($context->collect_initializations + && isset($stmt->name->name) + && $stmt->name->name === '__construct' + && isset($class_storage->declaring_method_ids['__construct'])) { + $construct_fq_class_name = $class_storage->declaring_method_ids['__construct']->fq_class_name; + $construct_class_storage = $codebase->classlike_storage_provider->get($construct_fq_class_name); + $construct_fq_class_name = $construct_class_storage->name; + + foreach ($construct_class_storage->properties as $property_name => $property_storage) { + if ($property_storage->is_promoted + && isset($context->vars_in_scope['$this->' . $property_name])) { + $context_type = $context->vars_in_scope['$this->' . $property_name]; + $context->vars_in_scope['$this->' . $property_name] = $context_type->setProperties( + [ + 'initialized_class' => $construct_fq_class_name, + 'initialized' => true, + ], + ); + } + } + } } elseif ($context->self) { if ($stmt->class->getFirst() === 'static' && isset($context->vars_in_scope['$this'])) { $fq_class_name = (string) $context->vars_in_scope['$this']; diff --git a/tests/PropertyTypeTest.php b/tests/PropertyTypeTest.php index 3d066210444..a5fd313413d 100644 --- a/tests/PropertyTypeTest.php +++ b/tests/PropertyTypeTest.php @@ -587,6 +587,22 @@ class A { 'MixedAssignment', ], ], + 'promotedPropertyNoExtendedConstructor' => [ + 'code' => ' [], + 'ignored_issues' => [], + 'php_version' => '8.0', + ], 'propertyWithoutTypeSuppressingIssueAndAssertingNull' => [ 'code' => ' 'PropertyNotSetInConstructor', ], + 'promotedPropertyNotSetInExtendedConstructor' => [ + 'code' => ' 'PropertyNotSetInConstructor', + 'ignored_issues' => [], + 'php_version' => '8.0', + ], 'nullableTypedPropertyNoConstructor' => [ 'code' => ' Date: Wed, 13 Mar 2024 19:19:31 +0100 Subject: [PATCH 10/28] report error for single param too since named args can even be used then --- .../Internal/Analyzer/MethodComparator.php | 7 ++---- tests/MethodCallTest.php | 4 +-- tests/MethodSignatureTest.php | 25 +++++++++++++++---- tests/UnusedCodeTest.php | 2 +- 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/MethodComparator.php b/src/Psalm/Internal/Analyzer/MethodComparator.php index 61726c5adbe..1c0c05df9c7 100644 --- a/src/Psalm/Internal/Analyzer/MethodComparator.php +++ b/src/Psalm/Internal/Analyzer/MethodComparator.php @@ -38,7 +38,6 @@ use Psalm\Type\Union; use function array_filter; -use function count; use function in_array; use function strpos; use function strtolower; @@ -457,10 +456,8 @@ private static function compareMethodParams( $implementer_param->location->file_path, ) ) { - if (!$guide_classlike_storage->user_defined && $i === 0 && count($guide_method_storage->params) < 2) { - // if it's third party defined and a single arg, renaming is unnecessary - // if we still want to psalter it, move this if and change the else below to elseif - } elseif ($config->allow_named_arg_calls + // even if it's just a single arg, it needs to be renamed in case it's called with a single named arg + if ($config->allow_named_arg_calls || ($guide_classlike_storage->location && !$config->isInProjectDirs($guide_classlike_storage->location->file_path) ) diff --git a/tests/MethodCallTest.php b/tests/MethodCallTest.php index 3426aa38a55..7ecca414d69 100644 --- a/tests/MethodCallTest.php +++ b/tests/MethodCallTest.php @@ -1144,9 +1144,9 @@ public static function new() : self { class Datetime extends \DateTime { - public static function createFromInterface(\DateTimeInterface $datetime): static + public static function createFromInterface(\DateTimeInterface $object): static { - return parent::createFromInterface($datetime); + return parent::createFromInterface($object); } }', 'assertions' => [], diff --git a/tests/MethodSignatureTest.php b/tests/MethodSignatureTest.php index 69315f41797..c4bbce5882a 100644 --- a/tests/MethodSignatureTest.php +++ b/tests/MethodSignatureTest.php @@ -452,13 +452,13 @@ class A implements Serializable { private $id = 1; /** - * @param string $serialized + * @param string $data */ - public function unserialize($serialized) : void + public function unserialize($data) : void { [ $this->id, - ] = (array) \unserialize($serialized); + ] = (array) \unserialize($data); } public function serialize() : string @@ -1060,6 +1060,21 @@ public function fooFoo(int $a, bool $c): void { }', 'error_message' => 'ParamNameMismatch', ], + 'differentArgumentName' => [ + 'code' => ' 'ParamNameMismatch', + ], 'nonNullableSubclassParam' => [ 'code' => ' [ 'code' => ' 'ImplementedParamTypeMismatch', diff --git a/tests/UnusedCodeTest.php b/tests/UnusedCodeTest.php index 32b85090d1b..32e2ea103c0 100644 --- a/tests/UnusedCodeTest.php +++ b/tests/UnusedCodeTest.php @@ -764,7 +764,7 @@ public function serialize() : string { return ""; } - public function unserialize($_serialized) : void {} + public function unserialize($data) : void {} } new Foo();', From c174e35f158e6e39b134fe9c01d14b65f2db702b Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Thu, 14 Mar 2024 00:35:05 +0100 Subject: [PATCH 11/28] Fix https://github.com/vimeo/psalm/issues/7550 --- dictionaries/CallMap.php | 38 +++++------ dictionaries/CallMap_81_delta.php | 68 +++++++++++++++++++ dictionaries/CallMap_83_delta.php | 8 +++ .../Codebase/InternalCallMapHandlerTest.php | 3 + 4 files changed, 98 insertions(+), 19 deletions(-) diff --git a/dictionaries/CallMap.php b/dictionaries/CallMap.php index 80de3809a85..e4442fcf28f 100644 --- a/dictionaries/CallMap.php +++ b/dictionaries/CallMap.php @@ -1142,17 +1142,17 @@ 'crash' => [''], 'crc32' => ['int', 'string'=>'string'], 'crypt' => ['string', 'string'=>'string', 'salt'=>'string'], -'ctype_alnum' => ['bool', 'text'=>'string|int'], -'ctype_alpha' => ['bool', 'text'=>'string|int'], -'ctype_cntrl' => ['bool', 'text'=>'string|int'], -'ctype_digit' => ['bool', 'text'=>'string|int'], -'ctype_graph' => ['bool', 'text'=>'string|int'], -'ctype_lower' => ['bool', 'text'=>'string|int'], -'ctype_print' => ['bool', 'text'=>'string|int'], -'ctype_punct' => ['bool', 'text'=>'string|int'], -'ctype_space' => ['bool', 'text'=>'string|int'], -'ctype_upper' => ['bool', 'text'=>'string|int'], -'ctype_xdigit' => ['bool', 'text'=>'string|int'], +'ctype_alnum' => ['bool', 'text'=>'string'], +'ctype_alpha' => ['bool', 'text'=>'string'], +'ctype_cntrl' => ['bool', 'text'=>'string'], +'ctype_digit' => ['bool', 'text'=>'string'], +'ctype_graph' => ['bool', 'text'=>'string'], +'ctype_lower' => ['bool', 'text'=>'string'], +'ctype_print' => ['bool', 'text'=>'string'], +'ctype_punct' => ['bool', 'text'=>'string'], +'ctype_space' => ['bool', 'text'=>'string'], +'ctype_upper' => ['bool', 'text'=>'string'], +'ctype_xdigit' => ['bool', 'text'=>'string'], 'cubrid_affected_rows' => ['int', 'req_identifier='=>''], 'cubrid_bind' => ['bool', 'req_identifier'=>'resource', 'bind_param'=>'int', 'bind_value'=>'mixed', 'bind_value_type='=>'string'], 'cubrid_client_encoding' => ['string', 'conn_identifier='=>''], @@ -1293,7 +1293,7 @@ 'CURLFile::setMimeType' => ['void', 'mime_type'=>'string'], 'CURLFile::setPostFilename' => ['void', 'posted_filename'=>'string'], 'CURLStringFile::__construct' => ['void', 'data'=>'string', 'postname'=>'string', 'mime='=>'string'], -'current' => ['mixed|false', 'array'=>'array|object'], +'current' => ['mixed|false', 'array'=>'array'], 'cyrus_authenticate' => ['void', 'connection'=>'resource', 'mechlist='=>'string', 'service='=>'string', 'user='=>'string', 'minssf='=>'int', 'maxssf='=>'int', 'authname='=>'string', 'password='=>'string'], 'cyrus_bind' => ['bool', 'connection'=>'resource', 'callbacks'=>'array'], 'cyrus_close' => ['bool', 'connection'=>'resource'], @@ -3269,7 +3269,7 @@ 'get_call_stack' => [''], 'get_called_class' => ['class-string'], 'get_cfg_var' => ['string|false', 'option'=>'string'], -'get_class' => ['class-string', 'object='=>'object'], +'get_class' => ['class-string', 'object'=>'object'], 'get_class_methods' => ['list', 'object_or_class'=>'object|class-string'], 'get_class_vars' => ['array', 'class'=>'string'], 'get_current_user' => ['string'], @@ -3290,7 +3290,7 @@ 'get_magic_quotes_runtime' => ['int|false'], 'get_meta_tags' => ['array', 'filename'=>'string', 'use_include_path='=>'bool'], 'get_object_vars' => ['array', 'object'=>'object'], -'get_parent_class' => ['class-string|false', 'object_or_class='=>'object|class-string'], +'get_parent_class' => ['class-string|false', 'object_or_class'=>'object|class-string'], 'get_required_files' => ['list'], 'get_resource_id' => ['int', 'resource'=>'resource'], 'get_resource_type' => ['string', 'resource'=>'resource'], @@ -6198,7 +6198,7 @@ 'kadm5_get_principals' => ['array', 'handle'=>'resource'], 'kadm5_init_with_password' => ['resource', 'admin_server'=>'string', 'realm'=>'string', 'principal'=>'string', 'password'=>'string'], 'kadm5_modify_principal' => ['bool', 'handle'=>'resource', 'principal'=>'string', 'options'=>'array'], -'key' => ['int|string|null', 'array'=>'array|object'], +'key' => ['int|string|null', 'array'=>'array'], 'key_exists' => ['bool', 'key'=>'string|int', 'array'=>'array'], 'krsort' => ['true', '&rw_array'=>'array', 'flags='=>'int'], 'ksort' => ['true', '&rw_array'=>'array', 'flags='=>'int'], @@ -6598,7 +6598,7 @@ 'mapObj::zoomScale' => ['int', 'nScaleDenom'=>'float', 'oPixelPos'=>'pointObj', 'nImageWidth'=>'int', 'nImageHeight'=>'int', 'oGeorefExt'=>'rectObj', 'oMaxGeorefExt'=>'rectObj'], 'max' => ['mixed', 'value'=>'non-empty-array'], 'max\'1' => ['mixed', 'value'=>'', 'values'=>'', '...args='=>''], -'mb_check_encoding' => ['bool', 'value='=>'array|string|null', 'encoding='=>'string|null'], +'mb_check_encoding' => ['bool', 'value'=>'array|string', 'encoding='=>'string|null'], 'mb_chr' => ['non-empty-string|false', 'codepoint'=>'int', 'encoding='=>'string|null'], 'mb_convert_case' => ['string', 'string'=>'string', 'mode'=>'int', 'encoding='=>'string|null'], 'mb_convert_encoding' => ['string|false', 'string'=>'string', 'to_encoding'=>'string', 'from_encoding='=>'array|string|null'], @@ -8149,7 +8149,7 @@ 'newrelic_set_appname' => ['bool', 'name'=>'string', 'license='=>'string', 'xmit='=>'bool'], 'newrelic_set_user_attributes' => ['bool', 'user'=>'string', 'account'=>'string', 'product'=>'string'], 'newrelic_start_transaction' => ['bool', 'appname'=>'string', 'license='=>'string'], -'next' => ['mixed', '&r_array'=>'array|object'], +'next' => ['mixed', '&r_array'=>'array'], 'ngettext' => ['string', 'singular'=>'string', 'plural'=>'string', 'count'=>'int'], 'nl2br' => ['string', 'string'=>'string', 'use_xhtml='=>'bool'], 'nl_langinfo' => ['string|false', 'item'=>'int'], @@ -9429,7 +9429,7 @@ 'preg_replace_callback_array\'1' => ['string[]|null', 'pattern'=>'array', 'subject'=>'string[]', 'limit='=>'int', '&w_count='=>'int', 'flags='=>'int'], 'preg_split' => ['list|false', 'pattern'=>'string', 'subject'=>'string', 'limit'=>'int', 'flags='=>'null'], 'preg_split\'1' => ['list|list>|false', 'pattern'=>'string', 'subject'=>'string', 'limit='=>'int', 'flags='=>'int'], -'prev' => ['mixed', '&r_array'=>'array|object'], +'prev' => ['mixed', '&r_array'=>'array'], 'print' => ['int', 'arg'=>'string'], 'print_r' => ['string', 'value'=>'mixed'], 'print_r\'1' => ['true', 'value'=>'mixed', 'return='=>'bool'], @@ -10699,7 +10699,7 @@ 'register_tick_function' => ['bool', 'callback'=>'callable():void', '...args='=>'mixed'], 'rename' => ['bool', 'from'=>'string', 'to'=>'string', 'context='=>'resource'], 'rename_function' => ['bool', 'original_name'=>'string', 'new_name'=>'string'], -'reset' => ['mixed|false', '&r_array'=>'array|object'], +'reset' => ['mixed|false', '&r_array'=>'array'], 'ResourceBundle::__construct' => ['void', 'locale'=>'?string', 'bundle'=>'?string', 'fallback='=>'bool'], 'ResourceBundle::count' => ['int'], 'ResourceBundle::create' => ['?ResourceBundle', 'locale'=>'?string', 'bundle'=>'?string', 'fallback='=>'bool'], diff --git a/dictionaries/CallMap_81_delta.php b/dictionaries/CallMap_81_delta.php index bfb2da40bb6..eaa855863e6 100644 --- a/dictionaries/CallMap_81_delta.php +++ b/dictionaries/CallMap_81_delta.php @@ -1211,6 +1211,74 @@ 'old' => ['int|false', '&rw_read'=>'?resource[]', '&rw_write'=>'?resource[]', '&rw_except'=>'?resource[]', 'seconds'=>'?int', 'microseconds='=>'int'], 'new' => ['int|false', '&rw_read'=>'?resource[]', '&rw_write'=>'?resource[]', '&rw_except'=>'?resource[]', 'seconds'=>'?int', 'microseconds='=>'?int'], ], + 'mb_check_encoding' => [ + 'old' => ['bool', 'value='=>'array|string|null', 'encoding='=>'string|null'], + 'new' => ['bool', 'value'=>'array|string', 'encoding='=>'string|null'], + ], + 'ctype_alnum' => [ + 'old' => ['bool', 'text'=>'string|int'], + 'new' => ['bool', 'text'=>'string'], + ], + 'ctype_alpha' => [ + 'old' => ['bool', 'text'=>'string|int'], + 'new' => ['bool', 'text'=>'string'], + ], + 'ctype_cntrl' => [ + 'old' => ['bool', 'text'=>'string|int'], + 'new' => ['bool', 'text'=>'string'], + ], + 'ctype_digit' => [ + 'old' => ['bool', 'text'=>'string|int'], + 'new' => ['bool', 'text'=>'string'], + ], + 'ctype_graph' => [ + 'old' => ['bool', 'text'=>'string|int'], + 'new' => ['bool', 'text'=>'string'], + ], + 'ctype_lower' => [ + 'old' => ['bool', 'text'=>'string|int'], + 'new' => ['bool', 'text'=>'string'], + ], + 'ctype_print' => [ + 'old' => ['bool', 'text'=>'string|int'], + 'new' => ['bool', 'text'=>'string'], + ], + 'ctype_punct' => [ + 'old' => ['bool', 'text'=>'string|int'], + 'new' => ['bool', 'text'=>'string'], + ], + 'ctype_space' => [ + 'old' => ['bool', 'text'=>'string|int'], + 'new' => ['bool', 'text'=>'string'], + ], + 'ctype_upper' => [ + 'old' => ['bool', 'text'=>'string|int'], + 'new' => ['bool', 'text'=>'string'], + ], + 'ctype_xdigit' => [ + 'old' => ['bool', 'text'=>'string|int'], + 'new' => ['bool', 'text'=>'string'], + ], + 'key' => [ + 'old' => ['int|string|null', 'array'=>'array|object'], + 'new' => ['int|string|null', 'array'=>'array'], + ], + 'current' => [ + 'old' => ['mixed|false', 'array'=>'array|object'], + 'new' => ['mixed|false', 'array'=>'array'], + ], + 'next' => [ + 'old' => ['mixed', '&r_array'=>'array|object'], + 'new' => ['mixed', '&r_array'=>'array'], + ], + 'prev' => [ + 'old' => ['mixed', '&r_array'=>'array|object'], + 'new' => ['mixed', '&r_array'=>'array'], + ], + 'reset' => [ + 'old' => ['mixed|false', '&r_array'=>'array|object'], + 'new' => ['mixed|false', '&r_array'=>'array'], + ], ], 'removed' => [ diff --git a/dictionaries/CallMap_83_delta.php b/dictionaries/CallMap_83_delta.php index 8a4a76077b8..005017e4dfe 100644 --- a/dictionaries/CallMap_83_delta.php +++ b/dictionaries/CallMap_83_delta.php @@ -117,6 +117,14 @@ 'old' => ['string|false', 'haystack'=>'string', 'needle'=>'string'], 'new' => ['string|false', 'haystack'=>'string', 'needle'=>'string', 'before_needle='=>'bool'], ], + 'get_class' => [ + 'old' => ['class-string', 'object='=>'object'], + 'new' => ['class-string', 'object'=>'object'], + ], + 'get_parent_class' => [ + 'old' => ['class-string|false', 'object_or_class='=>'object|class-string'], + 'new' => ['class-string|false', 'object_or_class'=>'object|class-string'], + ], ], 'removed' => [ diff --git a/tests/Internal/Codebase/InternalCallMapHandlerTest.php b/tests/Internal/Codebase/InternalCallMapHandlerTest.php index e4dba7e3081..82909f86027 100644 --- a/tests/Internal/Codebase/InternalCallMapHandlerTest.php +++ b/tests/Internal/Codebase/InternalCallMapHandlerTest.php @@ -78,6 +78,8 @@ class InternalCallMapHandlerTest extends TestCase 'array_multisort', 'datefmt_create' => ['8.0'], 'fiber::start', + 'get_class' => ['8.3'], + 'get_parent_class' => ['8.3'], 'imagefilledpolygon', 'imagegd', 'imagegd2', @@ -95,6 +97,7 @@ class InternalCallMapHandlerTest extends TestCase 'mailparse_msg_get_structure', 'mailparse_msg_parse', 'mailparse_stream_encode', + 'mb_check_encoding' => ['8.1', '8.2', '8.3'], 'memcached::cas', // memcached 3.2.0 has incorrect reflection 'memcached::casbykey', // memcached 3.2.0 has incorrect reflection 'oauth::fetch', From c7fc76ec5747d9fb56265c5a8fad62d7ba4ab9a9 Mon Sep 17 00:00:00 2001 From: Jack Worman Date: Thu, 14 Mar 2024 13:12:01 -0400 Subject: [PATCH 12/28] MissingClassConstType --- config.xsd | 1 + docs/running_psalm/error_levels.md | 1 + docs/running_psalm/issues.md | 1 + .../issues/MissingClassConstType.md | 21 +++++++ .../Reflector/ClassLikeNodeScanner.php | 19 ++++++ src/Psalm/Issue/MissingClassConstType.php | 11 ++++ tests/Config/PluginTest.php | 38 +++++++----- tests/DocumentationTest.php | 1 + tests/MissingClassConstTypeTest.php | 60 +++++++++++++++++++ 9 files changed, 139 insertions(+), 14 deletions(-) create mode 100644 docs/running_psalm/issues/MissingClassConstType.md create mode 100644 src/Psalm/Issue/MissingClassConstType.php create mode 100644 tests/MissingClassConstTypeTest.php diff --git a/config.xsd b/config.xsd index c15a74e6080..9d785d39b04 100644 --- a/config.xsd +++ b/config.xsd @@ -313,6 +313,7 @@ + diff --git a/docs/running_psalm/error_levels.md b/docs/running_psalm/error_levels.md index d0edf8c0236..155e42671e2 100644 --- a/docs/running_psalm/error_levels.md +++ b/docs/running_psalm/error_levels.md @@ -232,6 +232,7 @@ Level 5 and above allows a more non-verifiable code, and higher levels are even - [InvalidDocblockParamName](issues/InvalidDocblockParamName.md) - [InvalidFalsableReturnType](issues/InvalidFalsableReturnType.md) - [InvalidStringClass](issues/InvalidStringClass.md) +- [MissingClassConstType](issues/MissingClassConstType.md) - [MissingClosureParamType](issues/MissingClosureParamType.md) - [MissingClosureReturnType](issues/MissingClosureReturnType.md) - [MissingConstructor](issues/MissingConstructor.md) diff --git a/docs/running_psalm/issues.md b/docs/running_psalm/issues.md index ae683435186..384e3703b3a 100644 --- a/docs/running_psalm/issues.md +++ b/docs/running_psalm/issues.md @@ -113,6 +113,7 @@ - [MismatchingDocblockParamType](issues/MismatchingDocblockParamType.md) - [MismatchingDocblockPropertyType](issues/MismatchingDocblockPropertyType.md) - [MismatchingDocblockReturnType](issues/MismatchingDocblockReturnType.md) + - [MissingClassConstType](issues/MissingClassConstType.md) - [MissingClosureParamType](issues/MissingClosureParamType.md) - [MissingClosureReturnType](issues/MissingClosureReturnType.md) - [MissingConstructor](issues/MissingConstructor.md) diff --git a/docs/running_psalm/issues/MissingClassConstType.md b/docs/running_psalm/issues/MissingClassConstType.md new file mode 100644 index 00000000000..c4fa9049fc6 --- /dev/null +++ b/docs/running_psalm/issues/MissingClassConstType.md @@ -0,0 +1,21 @@ +# MissingClassConstType + +Emitted when a class constant doesn't have a declared type. + +```php +codebase->analysis_php_version_id >= 8_03_00 + && $stmt->type === null + ) { + IssueBuffer::maybeAdd( + new MissingClassConstType( + sprintf( + 'Class constant "%s::%s" should have a declared type.', + $storage->name, + $const->name->name, + ), + new CodeLocation($this->file_scanner, $const), + ), + $suppressed_issues, + ); + } + if ($exists) { $existing_constants[$const->name->name] = $constant_storage; } diff --git a/src/Psalm/Issue/MissingClassConstType.php b/src/Psalm/Issue/MissingClassConstType.php new file mode 100644 index 00000000000..38f29dbcec5 --- /dev/null +++ b/src/Psalm/Issue/MissingClassConstType.php @@ -0,0 +1,11 @@ +addFile( $file_path, - ' "Psalm\Internal\Analyzer\ProjectAnalyzer", - ]; - }', + sprintf( + <<<'PHP' + "Psalm\Internal\Analyzer\ProjectAnalyzer", + ]; + } + PHP, + $this->project_analyzer->getCodebase()->analysis_php_version_id >= 8_03_00 ? 'array' : '', + ), ); $this->analyzeFile($file_path, new Context()); @@ -173,14 +179,18 @@ public function testStringAnalyzerPluginWithClassConstantConcat(): void $this->addFile( $file_path, - ' \Psalm\Internal\Analyzer\ProjectAnalyzer::class . "::foo", - ]; - }', + sprintf( + <<<'PHP' + \Psalm\Internal\Analyzer\ProjectAnalyzer::class . "::foo", + ]; + } + PHP, + $this->project_analyzer->getCodebase()->analysis_php_version_id >= 8_03_00 ? 'array' : '', + ), ); $this->analyzeFile($file_path, new Context()); diff --git a/tests/DocumentationTest.php b/tests/DocumentationTest.php index aff06fde78b..95b2ad3c1b8 100644 --- a/tests/DocumentationTest.php +++ b/tests/DocumentationTest.php @@ -316,6 +316,7 @@ public function providerInvalidCodeParse(): array case 'InvalidOverride': case 'MissingOverrideAttribute': + case 'MissingClassConstType': $php_version = '8.3'; break; } diff --git a/tests/MissingClassConstTypeTest.php b/tests/MissingClassConstTypeTest.php new file mode 100644 index 00000000000..de413050fdb --- /dev/null +++ b/tests/MissingClassConstTypeTest.php @@ -0,0 +1,60 @@ += PHP 8.3' => [ + 'code' => <<<'PHP' + [], + 'ignored_issues' => [], + 'php_version' => '8.3', + ], + 'no type; < PHP 8.3' => [ + 'code' => <<<'PHP' + [], + 'ignored_issues' => [], + 'php_version' => '8.2', + ], + ]; + } + + public function providerInvalidCodeParse(): iterable + { + return [ + 'no type; >= PHP 8.3' => [ + 'code' => <<<'PHP' + MissingClassConstType::getIssueType(), + 'error_levels' => [], + 'php_version' => '8.3', + ], + ]; + } +} From ad87c2e90553f57bc5391ebf78af1635d6d4e99c Mon Sep 17 00:00:00 2001 From: Daniil Gentili Date: Fri, 15 Mar 2024 12:15:05 +0100 Subject: [PATCH 13/28] Allow specifying flags to Codebase::isTypeContainedByType --- src/Psalm/Codebase.php | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/Psalm/Codebase.php b/src/Psalm/Codebase.php index a02f61403f6..f812c63c163 100644 --- a/src/Psalm/Codebase.php +++ b/src/Psalm/Codebase.php @@ -47,6 +47,7 @@ use Psalm\Internal\Provider\FileStorageProvider; use Psalm\Internal\Provider\Providers; use Psalm\Internal\Provider\StatementsProvider; +use Psalm\Internal\Type\Comparator\TypeComparisonResult; use Psalm\Internal\Type\Comparator\UnionTypeComparator; use Psalm\Progress\Progress; use Psalm\Progress\VoidProgress; @@ -2352,9 +2353,22 @@ public function removeTemporaryFileChanges(string $file_path): void */ public function isTypeContainedByType( Union $input_type, - Union $container_type + Union $container_type, + bool $ignore_null = false, + bool $ignore_false = false, + bool $allow_interface_equality = false, + bool $allow_float_int_equality = true ): bool { - return UnionTypeComparator::isContainedBy($this, $input_type, $container_type); + return UnionTypeComparator::isContainedBy( + $this, + $input_type, + $container_type, + $ignore_null, + $ignore_false, + null, + $allow_interface_equality, + $allow_float_int_equality + ); } /** From b431e5fc730d61b8c84e81ebd38fa9acf01c2a02 Mon Sep 17 00:00:00 2001 From: Daniil Gentili Date: Fri, 15 Mar 2024 12:52:36 +0100 Subject: [PATCH 14/28] cs-fix --- src/Psalm/Codebase.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Psalm/Codebase.php b/src/Psalm/Codebase.php index f812c63c163..cdcda459884 100644 --- a/src/Psalm/Codebase.php +++ b/src/Psalm/Codebase.php @@ -47,7 +47,6 @@ use Psalm\Internal\Provider\FileStorageProvider; use Psalm\Internal\Provider\Providers; use Psalm\Internal\Provider\StatementsProvider; -use Psalm\Internal\Type\Comparator\TypeComparisonResult; use Psalm\Internal\Type\Comparator\UnionTypeComparator; use Psalm\Progress\Progress; use Psalm\Progress\VoidProgress; @@ -2367,7 +2366,7 @@ public function isTypeContainedByType( $ignore_false, null, $allow_interface_equality, - $allow_float_int_equality + $allow_float_int_equality, ); } From 431977cb1d3f1562cd259c85e576bbd72c47f42e Mon Sep 17 00:00:00 2001 From: Ayesh Karunaratne Date: Sat, 16 Mar 2024 02:19:34 +0700 Subject: [PATCH 15/28] [PHP 8.4] Fixes for implicit nullability deprecation Fixes all issues that emits a deprecation notice on PHP 8.4. See: - [RFC](https://wiki.php.net/rfc/deprecate-implicitly-nullable-types) - [PHP 8.4: Implicitly nullable parameter declarations deprecated](https://php.watch/versions/8.4/implicitly-marking-parameter-type-nullable-deprecated) --- src/Psalm/Codebase.php | 6 +++--- src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php | 2 +- src/Psalm/Internal/Analyzer/IssueData.php | 2 +- .../Internal/Analyzer/Statements/Block/LoopAnalyzer.php | 2 +- .../Statements/Expression/BinaryOp/ArithmeticOpAnalyzer.php | 2 +- .../Statements/Expression/BinaryOp/ConcatAnalyzer.php | 2 +- .../Statements/Expression/Call/HighOrderFunctionArgInfo.php | 2 +- .../Analyzer/Statements/Expression/Call/NewAnalyzer.php | 4 ++-- .../Statements/Expression/Fetch/ArrayFetchAnalyzer.php | 4 ++-- .../Analyzer/Statements/Expression/SimpleTypeInferer.php | 6 +++--- src/Psalm/Internal/Codebase/Analyzer.php | 2 +- src/Psalm/Internal/Codebase/ConstantTypeResolver.php | 2 +- src/Psalm/Internal/Codebase/Methods.php | 2 +- src/Psalm/Internal/Provider/FakeFileProvider.php | 2 +- src/Psalm/Internal/Provider/FileProvider.php | 2 +- src/Psalm/Internal/Type/ParseTree/Value.php | 2 +- src/Psalm/Plugin/DynamicTemplateProvider.php | 2 +- .../EventHandler/Event/AfterMethodCallAnalysisEvent.php | 2 +- src/Psalm/Storage/FunctionLikeStorage.php | 2 +- src/Psalm/Type/Atomic.php | 2 +- 20 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/Psalm/Codebase.php b/src/Psalm/Codebase.php index a02f61403f6..c73b4f210d1 100644 --- a/src/Psalm/Codebase.php +++ b/src/Psalm/Codebase.php @@ -889,7 +889,7 @@ public function getMethodReturnsByRef($method_id): bool */ public function getMethodReturnTypeLocation( $method_id, - CodeLocation &$defined_location = null + ?CodeLocation &$defined_location = null ): ?CodeLocation { return $this->methods->getMethodReturnTypeLocation( MethodIdentifier::wrap($method_id), @@ -1651,7 +1651,7 @@ public function getFunctionArgumentAtPosition(string $file_path, Position $posit */ public function getSignatureInformation( string $function_symbol, - string $file_path = null + ?string $file_path = null ): ?SignatureInformation { $signature_label = ''; $signature_documentation = null; @@ -1876,7 +1876,7 @@ public function getCompletionItemsForClassishThing( string $type_string, string $gap, bool $snippets_supported = false, - array $allow_visibilities = null, + ?array $allow_visibilities = null, array $ignore_fq_class_names = [] ): array { if ($allow_visibilities === null) { diff --git a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php index 70786016bf7..c38849ff6f2 100644 --- a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php @@ -1596,7 +1596,7 @@ public function examineParamTypes( StatementsAnalyzer $statements_analyzer, Context $context, Codebase $codebase, - PhpParser\Node $stmt = null + ?PhpParser\Node $stmt = null ): void { $storage = $this->getFunctionLikeStorage($statements_analyzer); diff --git a/src/Psalm/Internal/Analyzer/IssueData.php b/src/Psalm/Internal/Analyzer/IssueData.php index 3bfb9a414cc..67c845a2134 100644 --- a/src/Psalm/Internal/Analyzer/IssueData.php +++ b/src/Psalm/Internal/Analyzer/IssueData.php @@ -122,7 +122,7 @@ public function __construct( int $shortcode = 0, int $error_level = -1, ?array $taint_trace = null, - array $other_references = null, + ?array $other_references = null, ?string $dupe_key = null ) { $this->severity = $severity; diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php index f925f961336..7d36fd3399b 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php @@ -44,7 +44,7 @@ public static function analyze( array $pre_conditions, array $post_expressions, LoopScope $loop_scope, - Context &$continue_context = null, + ?Context &$continue_context = null, bool $is_do = false, bool $always_enters_loop = false ): ?bool { diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ArithmeticOpAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ArithmeticOpAnalyzer.php index f4ec365b722..7fc47efd872 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ArithmeticOpAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ArithmeticOpAnalyzer.php @@ -309,7 +309,7 @@ private static function analyzeOperands( bool &$has_valid_left_operand, bool &$has_valid_right_operand, bool &$has_string_increment, - Union &$result_type = null + ?Union &$result_type = null ): ?Union { if (($left_type_part instanceof TLiteralInt || $left_type_part instanceof TLiteralFloat) && ($right_type_part instanceof TLiteralInt || $right_type_part instanceof TLiteralFloat) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ConcatAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ConcatAnalyzer.php index 6a59185b55f..a870acd6a70 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ConcatAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/ConcatAnalyzer.php @@ -59,7 +59,7 @@ public static function analyze( PhpParser\Node\Expr $left, PhpParser\Node\Expr $right, Context $context, - Union &$result_type = null + ?Union &$result_type = null ): void { $codebase = $statements_analyzer->getCodebase(); diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/HighOrderFunctionArgInfo.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/HighOrderFunctionArgInfo.php index 526e6ee1141..87019029000 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/HighOrderFunctionArgInfo.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/HighOrderFunctionArgInfo.php @@ -35,7 +35,7 @@ final class HighOrderFunctionArgInfo public function __construct( string $type, FunctionLikeStorage $function_storage, - ClassLikeStorage $class_storage = null + ?ClassLikeStorage $class_storage = null ) { $this->type = $type; $this->function_storage = $function_storage; diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php index 019f159d403..8f35fee56b3 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php @@ -76,7 +76,7 @@ public static function analyze( StatementsAnalyzer $statements_analyzer, PhpParser\Node\Expr\New_ $stmt, Context $context, - TemplateResult $template_result = null + ?TemplateResult $template_result = null ): bool { $fq_class_name = null; @@ -310,7 +310,7 @@ private static function analyzeNamedConstructor( string $fq_class_name, bool $from_static, bool $can_extend, - TemplateResult $template_result = null + ?TemplateResult $template_result = null ): void { $storage = $codebase->classlike_storage_provider->get($fq_class_name); diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php index d71499606c6..7f81211330a 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php @@ -475,8 +475,8 @@ public static function getArrayAccessTypeGivenOffset( bool $in_assignment, ?string $extended_var_id, Context $context, - PhpParser\Node\Expr $assign_value = null, - Union $replacement_type = null + ?PhpParser\Node\Expr $assign_value = null, + ?Union $replacement_type = null ): Union { $offset_type = $offset_type_original->getBuilder(); diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/SimpleTypeInferer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/SimpleTypeInferer.php index 3d60782d1b9..8b39f79871c 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/SimpleTypeInferer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/SimpleTypeInferer.php @@ -56,7 +56,7 @@ public static function infer( NodeDataProvider $nodes, PhpParser\Node\Expr $stmt, Aliases $aliases, - FileSource $file_source = null, + ?FileSource $file_source = null, ?array $existing_class_constants = null, ?string $fq_classlike_name = null ): ?Union { @@ -544,7 +544,7 @@ private static function inferArrayType( NodeDataProvider $nodes, PhpParser\Node\Expr\Array_ $stmt, Aliases $aliases, - FileSource $file_source = null, + ?FileSource $file_source = null, ?array $existing_class_constants = null, ?string $fq_classlike_name = null ): ?Union { @@ -628,7 +628,7 @@ private static function handleArrayItem( ArrayCreationInfo $array_creation_info, PhpParser\Node\Expr\ArrayItem $item, Aliases $aliases, - FileSource $file_source = null, + ?FileSource $file_source = null, ?array $existing_class_constants = null, ?string $fq_classlike_name = null ): bool { diff --git a/src/Psalm/Internal/Codebase/Analyzer.php b/src/Psalm/Internal/Codebase/Analyzer.php index 72825b05baf..afd8b6b272e 100644 --- a/src/Psalm/Internal/Codebase/Analyzer.php +++ b/src/Psalm/Internal/Codebase/Analyzer.php @@ -1188,7 +1188,7 @@ public function addNodeType( string $file_path, PhpParser\Node $node, string $node_type, - PhpParser\Node $parent_node = null + ?PhpParser\Node $parent_node = null ): void { if ($node_type === '') { throw new UnexpectedValueException('non-empty node_type expected'); diff --git a/src/Psalm/Internal/Codebase/ConstantTypeResolver.php b/src/Psalm/Internal/Codebase/ConstantTypeResolver.php index fc6940b1cfb..6506bed61f5 100644 --- a/src/Psalm/Internal/Codebase/ConstantTypeResolver.php +++ b/src/Psalm/Internal/Codebase/ConstantTypeResolver.php @@ -58,7 +58,7 @@ final class ConstantTypeResolver public static function resolve( ClassLikes $classlikes, UnresolvedConstantComponent $c, - StatementsAnalyzer $statements_analyzer = null, + ?StatementsAnalyzer $statements_analyzer = null, array $visited_constant_ids = [] ): Atomic { $c_id = spl_object_id($c); diff --git a/src/Psalm/Internal/Codebase/Methods.php b/src/Psalm/Internal/Codebase/Methods.php index 3d9f691f4df..cdb067a43bd 100644 --- a/src/Psalm/Internal/Codebase/Methods.php +++ b/src/Psalm/Internal/Codebase/Methods.php @@ -941,7 +941,7 @@ public function getMethodReturnsByRef(MethodIdentifier $method_id): bool public function getMethodReturnTypeLocation( MethodIdentifier $method_id, - CodeLocation &$defined_location = null + ?CodeLocation &$defined_location = null ): ?CodeLocation { $method_id = $this->getDeclaringMethodId($method_id); diff --git a/src/Psalm/Internal/Provider/FakeFileProvider.php b/src/Psalm/Internal/Provider/FakeFileProvider.php index 734cd64dab8..8611bac1c3c 100644 --- a/src/Psalm/Internal/Provider/FakeFileProvider.php +++ b/src/Psalm/Internal/Provider/FakeFileProvider.php @@ -79,7 +79,7 @@ public function deleteFile(string $file_path): void * @param null|callable(string):bool $filter * @return list */ - public function getFilesInDir(string $dir_path, array $file_extensions, callable $filter = null): array + public function getFilesInDir(string $dir_path, array $file_extensions, ?callable $filter = null): array { $file_paths = parent::getFilesInDir($dir_path, $file_extensions, $filter); diff --git a/src/Psalm/Internal/Provider/FileProvider.php b/src/Psalm/Internal/Provider/FileProvider.php index aa4202d032c..948dec5d33b 100644 --- a/src/Psalm/Internal/Provider/FileProvider.php +++ b/src/Psalm/Internal/Provider/FileProvider.php @@ -155,7 +155,7 @@ public function isDirectory(string $file_path): bool * @param null|callable(string):bool $filter * @return list */ - public function getFilesInDir(string $dir_path, array $file_extensions, callable $filter = null): array + public function getFilesInDir(string $dir_path, array $file_extensions, ?callable $filter = null): array { $file_paths = []; diff --git a/src/Psalm/Internal/Type/ParseTree/Value.php b/src/Psalm/Internal/Type/ParseTree/Value.php index 1f9a506f42f..3a6d252f887 100644 --- a/src/Psalm/Internal/Type/ParseTree/Value.php +++ b/src/Psalm/Internal/Type/ParseTree/Value.php @@ -22,7 +22,7 @@ public function __construct( int $offset_start, int $offset_end, ?string $text, - ParseTree $parent = null + ?ParseTree $parent = null ) { $this->offset_start = $offset_start; $this->offset_end = $offset_end; diff --git a/src/Psalm/Plugin/DynamicTemplateProvider.php b/src/Psalm/Plugin/DynamicTemplateProvider.php index fbbcb3a36fd..c3778d47c11 100644 --- a/src/Psalm/Plugin/DynamicTemplateProvider.php +++ b/src/Psalm/Plugin/DynamicTemplateProvider.php @@ -23,7 +23,7 @@ public function __construct(string $defining_class) /** * If {@see DynamicFunctionStorage} requires template params this method can create it. */ - public function createTemplate(string $param_name, Union $as = null): TTemplateParam + public function createTemplate(string $param_name, ?Union $as = null): TTemplateParam { return new TTemplateParam($param_name, $as ?? Type::getMixed(), $this->defining_class); } diff --git a/src/Psalm/Plugin/EventHandler/Event/AfterMethodCallAnalysisEvent.php b/src/Psalm/Plugin/EventHandler/Event/AfterMethodCallAnalysisEvent.php index f699a6d4e03..7a071d6d94a 100644 --- a/src/Psalm/Plugin/EventHandler/Event/AfterMethodCallAnalysisEvent.php +++ b/src/Psalm/Plugin/EventHandler/Event/AfterMethodCallAnalysisEvent.php @@ -43,7 +43,7 @@ public function __construct( StatementsSource $statements_source, Codebase $codebase, array $file_replacements = [], - Union $return_type_candidate = null + ?Union $return_type_candidate = null ) { $this->expr = $expr; $this->method_id = $method_id; diff --git a/src/Psalm/Storage/FunctionLikeStorage.php b/src/Psalm/Storage/FunctionLikeStorage.php index ee6128e787f..01ab5d13242 100644 --- a/src/Psalm/Storage/FunctionLikeStorage.php +++ b/src/Psalm/Storage/FunctionLikeStorage.php @@ -329,7 +329,7 @@ public function setParams(array $params): void /** * @internal */ - public function addParam(FunctionLikeParameter $param, bool $lookup_value = null): void + public function addParam(FunctionLikeParameter $param, ?bool $lookup_value = null): void { $this->params[] = $param; $this->param_lookup[$param->name] = $lookup_value ?? true; diff --git a/src/Psalm/Type/Atomic.php b/src/Psalm/Type/Atomic.php index ee0b4d7ca57..c6a247d769d 100644 --- a/src/Psalm/Type/Atomic.php +++ b/src/Psalm/Type/Atomic.php @@ -753,7 +753,7 @@ public function replaceTemplateTypesWithStandins( TemplateResult $template_result, Codebase $codebase, ?StatementsAnalyzer $statements_analyzer = null, - Atomic $input_type = null, + ?Atomic $input_type = null, ?int $input_arg_offset = null, ?string $calling_class = null, ?string $calling_function = null, From ff168a9c7ac155346e70cfee0aafe50cf014dda9 Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Sat, 16 Mar 2024 21:43:36 +0100 Subject: [PATCH 16/28] Fix undefined parent not reported in callable Fix https://github.com/vimeo/psalm/issues/10836 --- .../Expression/Call/ArgumentAnalyzer.php | 11 ++++++ tests/CallableTest.php | 34 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php index 262935153d6..e89cd6faffc 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php @@ -38,6 +38,7 @@ use Psalm\Issue\NamedArgumentNotAllowed; use Psalm\Issue\NoValue; use Psalm\Issue\NullArgument; +use Psalm\Issue\ParentNotFound; use Psalm\Issue\PossiblyFalseArgument; use Psalm\Issue\PossiblyInvalidArgument; use Psalm\Issue\PossiblyNullArgument; @@ -1297,6 +1298,16 @@ private static function verifyExplicitParam( if ($callable_fq_class_name === 'parent') { $container_class = $statements_analyzer->getParentFQCLN(); + if ($container_class === null) { + IssueBuffer::accepts( + new ParentNotFound( + 'Cannot call method on parent' + . ' as this class does not extend another', + $arg_location, + ), + $statements_analyzer->getSuppressedIssues(), + ); + } } if (!$container_class) { diff --git a/tests/CallableTest.php b/tests/CallableTest.php index e10784b0b3f..c70020a80b6 100644 --- a/tests/CallableTest.php +++ b/tests/CallableTest.php @@ -2518,6 +2518,40 @@ function f(callable $c): void { 'ignored_issues' => [], 'php_version' => '8.0', ], + 'parentCallableArrayWithoutParent' => [ + 'code' => 'run(["parent", "hello"]); + } + + /** + * @param callable $callable + * @return void + */ + public function run($callable) { + call_user_func($callable); + } + }', + 'error_message' => 'ParentNotFound', + ], + 'parentCallableWithoutParent' => [ + 'code' => 'run("parent::hello"); + } + + /** + * @param callable $callable + * @return void + */ + public function run($callable) { + call_user_func($callable); + } + }', + 'error_message' => 'ParentNotFound', + ], ]; } } From 881546340c97039f6bc25208f8226d7957db156c Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Tue, 19 Mar 2024 13:26:25 +0100 Subject: [PATCH 17/28] fix tests running with other than called PHP binary if called with a non-default PHP binary e.g. you run the tests with a php83 executable but your default php is PHP 7.4, it will suddenly change while running the tests leading to false positive errors --- tests/EndToEnd/PsalmEndToEndTest.php | 5 +++-- tests/EndToEnd/PsalmRunnerTrait.php | 6 ++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/EndToEnd/PsalmEndToEndTest.php b/tests/EndToEnd/PsalmEndToEndTest.php index bdbb2cb1a0e..bd5930ee023 100644 --- a/tests/EndToEnd/PsalmEndToEndTest.php +++ b/tests/EndToEnd/PsalmEndToEndTest.php @@ -25,6 +25,7 @@ use function unlink; use const DIRECTORY_SEPARATOR; +use const PHP_BINARY; /** * Tests some of the most important use cases of the psalm and psalter commands, by launching a new @@ -116,7 +117,7 @@ public function testAlter(): void public function testPsalter(): void { $this->runPsalmInit(); - (new Process(['php', $this->psalter, '--alter', '--issues=InvalidReturnType'], self::$tmpDir))->mustRun(); + (new Process([PHP_BINARY, $this->psalter, '--alter', '--issues=InvalidReturnType'], self::$tmpDir))->mustRun(); $this->assertSame(0, $this->runPsalm([], self::$tmpDir)['CODE']); } @@ -229,7 +230,7 @@ public function testLegacyConfigWithoutresolveFromConfigFile(): void file_put_contents(self::$tmpDir . '/src/psalm.xml', $psalmXmlContent); - $process = new Process(['php', $this->psalm, '--config=src/psalm.xml'], self::$tmpDir); + $process = new Process([PHP_BINARY, $this->psalm, '--config=src/psalm.xml'], self::$tmpDir); $process->run(); $this->assertSame(2, $process->getExitCode()); $this->assertStringContainsString('InvalidReturnType', $process->getOutput()); diff --git a/tests/EndToEnd/PsalmRunnerTrait.php b/tests/EndToEnd/PsalmRunnerTrait.php index 03e7f714ec2..92900ec9b39 100644 --- a/tests/EndToEnd/PsalmRunnerTrait.php +++ b/tests/EndToEnd/PsalmRunnerTrait.php @@ -8,6 +8,8 @@ use function array_unshift; use function in_array; +use const PHP_BINARY; + trait PsalmRunnerTrait { private string $psalm = __DIR__ . '/../../psalm'; @@ -37,9 +39,9 @@ private function runPsalm( // we run `php psalm` rather than just `psalm`. if ($relyOnConfigDir) { - $process = new Process(array_merge(['php', $this->psalm, '-c=' . $workingDir . '/psalm.xml'], $args), null); + $process = new Process(array_merge([PHP_BINARY, $this->psalm, '-c=' . $workingDir . '/psalm.xml'], $args), null); } else { - $process = new Process(array_merge(['php', $this->psalm], $args), $workingDir); + $process = new Process(array_merge([PHP_BINARY, $this->psalm], $args), $workingDir); } if (!$shouldFail) { From ea825c626d99638362c62cba750d679c1520980e Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Tue, 19 Mar 2024 21:06:20 +0100 Subject: [PATCH 18/28] Explicitly set value in config to fix warning in tests --- tests/fixtures/DestructiveAutoloader/psalm.xml | 3 ++- tests/fixtures/DummyProject/psalm.xml | 3 ++- tests/fixtures/ModularConfig/psalm.xml | 3 ++- tests/fixtures/SuicidalAutoloader/psalm.xml | 3 ++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/fixtures/DestructiveAutoloader/psalm.xml b/tests/fixtures/DestructiveAutoloader/psalm.xml index 9df9c0cdd13..3fe8955268c 100644 --- a/tests/fixtures/DestructiveAutoloader/psalm.xml +++ b/tests/fixtures/DestructiveAutoloader/psalm.xml @@ -2,7 +2,8 @@ Date: Tue, 19 Mar 2024 21:17:26 +0100 Subject: [PATCH 19/28] fix bad class --- tests/fixtures/SuicidalAutoloader/autoloader.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/fixtures/SuicidalAutoloader/autoloader.php b/tests/fixtures/SuicidalAutoloader/autoloader.php index d506219c141..ac6e8d54a47 100644 --- a/tests/fixtures/SuicidalAutoloader/autoloader.php +++ b/tests/fixtures/SuicidalAutoloader/autoloader.php @@ -10,6 +10,7 @@ Transliterator::class, // symfony/string InstalledVersions::class, // composer v2 'Mockery\Closure', // Mockery/mockery 1.6.1 + 'Mockery\Matcher\TExpected', // Mockery/mockery 1.6.10, possibly before 'parent', // it's unclear why Psalm tries to autoload parent 'PHPUnit\Framework\ArrayAccess', 'PHPUnit\Framework\Countable', From a59248f5a1225e56f646c0931cc69fa12c633e60 Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Sat, 16 Mar 2024 23:13:00 +0100 Subject: [PATCH 20/28] self, parent and static in callable are deprecated since PHP 8.2 Fix https://github.com/vimeo/psalm/issues/10837 --- .../Expression/Call/ArgumentAnalyzer.php | 38 +++++- tests/CallableTest.php | 119 ++++++++++++++++++ 2 files changed, 156 insertions(+), 1 deletion(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php index 262935153d6..0109b3365bc 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php @@ -29,6 +29,7 @@ use Psalm\Internal\Type\TemplateStandinTypeReplacer; use Psalm\Internal\Type\TypeExpander; use Psalm\Issue\ArgumentTypeCoercion; +use Psalm\Issue\DeprecatedConstant; use Psalm\Issue\ImplicitToStringCast; use Psalm\Issue\InvalidArgument; use Psalm\Issue\InvalidLiteralArgument; @@ -951,6 +952,24 @@ public static function verifyType( $statements_analyzer->getFilePath(), ); + if ($potential_method_id === null && $codebase->analysis_php_version_id >= 8_02_00) { + [$lhs,] = $input_type_part->properties; + if ($lhs->isSingleStringLiteral() + && in_array( + strtolower($lhs->getSingleStringLiteral()->value), + ['self', 'parent', 'static'], + true, + )) { + IssueBuffer::maybeAdd( + new DeprecatedConstant( + 'Use of "' . $lhs->getSingleStringLiteral()->value . '" in callables is deprecated', + $arg_location, + ), + $statements_analyzer->getSuppressedIssues(), + ); + } + } + if ($potential_method_id && $potential_method_id !== 'not-callable') { $potential_method_ids[] = $potential_method_id; } @@ -959,10 +978,27 @@ public static function verifyType( ) { $parts = explode('::', $input_type_part->value); /** @psalm-suppress PossiblyUndefinedIntArrayOffset */ - $potential_method_ids[] = new MethodIdentifier( + $potential_method_id = new MethodIdentifier( $parts[0], strtolower($parts[1]), ); + + if ($codebase->analysis_php_version_id >= 8_02_00 + && in_array( + strtolower($potential_method_id->fq_class_name), + ['self', 'parent', 'static'], + true, + )) { + IssueBuffer::maybeAdd( + new DeprecatedConstant( + 'Use of "' . $potential_method_id->fq_class_name . '" in callables is deprecated', + $arg_location, + ), + $statements_analyzer->getSuppressedIssues(), + ); + } + + $potential_method_ids[] = $potential_method_id; } } diff --git a/tests/CallableTest.php b/tests/CallableTest.php index eff0e838a97..4b2c100b350 100644 --- a/tests/CallableTest.php +++ b/tests/CallableTest.php @@ -2457,6 +2457,125 @@ function ($_a) { }', 'error_message' => 'InvalidArgument', ], + 'callableArrayParentConstantDeprecated' => [ + 'code' => 'run(["parent", "hello"]); + } + + /** + * @param callable $callable + * @return void + */ + public function run($callable) { + call_user_func($callable); + } + }', + 'error_message' => 'DeprecatedConstant', + 'ignored_issues' => [], + 'php_version' => '8.2', + ], + 'callableParentConstantDeprecated' => [ + 'code' => 'run("parent::hello"); + } + + /** + * @param callable $callable + * @return void + */ + public function run($callable) { + call_user_func($callable); + } + }', + 'error_message' => 'DeprecatedConstant', + 'ignored_issues' => [], + 'php_version' => '8.2', + ], + 'callableSelfConstantDeprecated' => [ + 'code' => 'run("self::hello"); + } + + public static function hello(): void { + echo "hello"; + } + + /** + * @param callable $callable + * @return void + */ + public function run($callable) { + call_user_func($callable); + } + }', + 'error_message' => 'DeprecatedConstant', + 'ignored_issues' => [], + 'php_version' => '8.2', + ], + 'callableStaticConstantDeprecated' => [ + 'code' => 'run("static::hello"); + } + + public static function hello(): void { + echo "hello"; + } + + /** + * @param callable $callable + * @return void + */ + public function run($callable) { + call_user_func($callable); + } + }', + 'error_message' => 'DeprecatedConstant', + 'ignored_issues' => [], + 'php_version' => '8.2', + ], + 'callableArrayStaticConstantDeprecated' => [ + 'code' => 'run(["static", "hello"]); + } + + public static function hello(): void { + echo "hello"; + } + + /** + * @param callable $callable + * @return void + */ + public function run($callable) { + call_user_func($callable); + } + }', + 'error_message' => 'DeprecatedConstant', + 'ignored_issues' => [], + 'php_version' => '8.2', + ], 'invalidFirstClassCallableCannotBeInferred' => [ 'code' => ' Date: Tue, 19 Mar 2024 13:21:31 +0100 Subject: [PATCH 21/28] report invalid callable if callable cannot be called like this from current context Fix https://github.com/vimeo/psalm/issues/10823 Fix https://github.com/vimeo/psalm/issues/8509 --- .../Expression/Call/ArgumentAnalyzer.php | 168 ++++ tests/CallableTest.php | 951 +++++++++++++++++- 2 files changed, 1118 insertions(+), 1 deletion(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php index 0109b3365bc..0516f87febb 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php @@ -60,6 +60,7 @@ use Psalm\Type\Atomic\TMixed; use Psalm\Type\Atomic\TNamedObject; use Psalm\Type\Union; +use UnexpectedValueException; use function count; use function explode; @@ -886,6 +887,20 @@ public static function verifyType( true, $context->insideUse(), ); + + if (self::verifyCallableInContext( + $potential_method_id, + $cased_method_id, + $method_id, + $atomic_type, + $argument_offset, + $arg_location, + $context, + $codebase, + $statements_analyzer, + ) === false) { + continue; + } } $input_type->removeType($key); @@ -971,6 +986,20 @@ public static function verifyType( } if ($potential_method_id && $potential_method_id !== 'not-callable') { + if (self::verifyCallableInContext( + $potential_method_id, + $cased_method_id, + $method_id, + $input_type_part, + $argument_offset, + $arg_location, + $context, + $codebase, + $statements_analyzer, + ) === false) { + continue; + } + $potential_method_ids[] = $potential_method_id; } } elseif ($input_type_part instanceof TLiteralString @@ -998,6 +1027,20 @@ public static function verifyType( ); } + if (self::verifyCallableInContext( + $potential_method_id, + $cased_method_id, + $method_id, + $input_type_part, + $argument_offset, + $arg_location, + $context, + $codebase, + $statements_analyzer, + ) === false) { + continue; + } + $potential_method_ids[] = $potential_method_id; } } @@ -1235,6 +1278,131 @@ public static function verifyType( return null; } + private static function verifyCallableInContext( + MethodIdentifier $potential_method_id, + ?string $cased_method_id, + ?MethodIdentifier $method_id, + Atomic $input_type_part, + int $argument_offset, + CodeLocation $arg_location, + Context $context, + Codebase $codebase, + StatementsAnalyzer $statements_analyzer + ): ?bool { + $method_identifier = $cased_method_id !== null ? ' of ' . $cased_method_id : ''; + + if (!$method_id + || $potential_method_id->fq_class_name !== $context->self + || $method_id->fq_class_name !== $context->self) { + if ($input_type_part instanceof TKeyedArray) { + [$lhs,] = $input_type_part->properties; + } else { + $lhs = Type::getString($potential_method_id->fq_class_name); + } + + try { + $method_storage = $codebase->methods->getStorage($potential_method_id); + + $lhs_atomic = $lhs->getSingleAtomic(); + if ($lhs->isSingle() + && $lhs->hasNamedObjectType() + && ($lhs->isStaticObject() + || ($lhs_atomic instanceof TNamedObject + && !$lhs_atomic->definite_class + && $lhs_atomic->value === $context->self))) { + // callable $this + // some PHP-internal functions (e.g. array_filter) will call the callback within the current context + // unlike user-defined functions which call the callback in their context + // however this doesn't apply to all + // e.g. header_register_callback will not throw an error immediately like user-land functions + // however error log "Could not call the sapi_header_callback" if it's not public + // this is NOT a complete list, but just what was easily available and to be extended + $php_native_non_public_cb = [ + 'array_diff_uassoc', + 'array_diff_ukey', + 'array_filter', + 'array_intersect_uassoc', + 'array_intersect_ukey', + 'array_map', + 'array_reduce', + 'array_udiff', + 'array_udiff_assoc', + 'array_udiff_uassoc', + 'array_uintersect', + 'array_uintersect_assoc', + 'array_uintersect_uassoc', + 'array_walk', + 'array_walk_recursive', + 'preg_replace_callback', + 'preg_replace_callback_array', + 'call_user_func', + 'call_user_func_array', + 'forward_static_call', + 'forward_static_call_array', + 'is_callable', + 'ob_start', + 'register_shutdown_function', + 'register_tick_function', + 'session_set_save_handler', + 'set_error_handler', + 'set_exception_handler', + 'spl_autoload_register', + 'spl_autoload_unregister', + 'uasort', + 'uksort', + 'usort', + ]; + + if ($potential_method_id->fq_class_name !== $context->self + || ($cased_method_id !== null + && !$method_id + && !in_array($cased_method_id, $php_native_non_public_cb, true)) + || ($method_id + && $method_id->fq_class_name !== $context->self + && $method_id->fq_class_name !== 'Closure') + ) { + if ($method_storage->visibility !== ClassLikeAnalyzer::VISIBILITY_PUBLIC) { + IssueBuffer::maybeAdd( + new InvalidArgument( + 'Argument ' . ($argument_offset + 1) . $method_identifier + . ' expects a public callable, but a non-public callable provided', + $arg_location, + $cased_method_id, + ), + $statements_analyzer->getSuppressedIssues(), + ); + return false; + } + } + } elseif ($lhs->isSingle()) { + // instance from e.g. new Foo() or static string like Foo::bar + if ((!$method_storage->is_static && !$lhs->hasNamedObjectType()) + || $method_storage->visibility !== ClassLikeAnalyzer::VISIBILITY_PUBLIC) { + IssueBuffer::maybeAdd( + new InvalidArgument( + 'Argument ' . ($argument_offset + 1) . $method_identifier + . ' expects a public static callable, but a ' + . ($method_storage->visibility !== ClassLikeAnalyzer::VISIBILITY_PUBLIC ? + 'non-public ' : '') + . (!$method_storage->is_static ? 'non-static ' : '') + . 'callable provided', + $arg_location, + $cased_method_id, + ), + $statements_analyzer->getSuppressedIssues(), + ); + + return false; + } + } + } catch (UnexpectedValueException $e) { + // do nothing + } + } + + return null; + } + /** * @param PhpParser\Node\Scalar\String_|PhpParser\Node\Expr\Array_|PhpParser\Node\Expr\BinaryOp\Concat $input_expr */ diff --git a/tests/CallableTest.php b/tests/CallableTest.php index 4b2c100b350..ba9e5467da4 100644 --- a/tests/CallableTest.php +++ b/tests/CallableTest.php @@ -1786,6 +1786,421 @@ function takesCallable(callable $c) : void {} takesCallable(function() { return; });', ], + 'callableMethodOutOfClassContextStaticPublic' => [ + 'code' => ' [ + 'code' => ' [ + 'code' => ' [ + 'code' => ' [ + 'code' => ' [ + 'code' => ' [ + 'code' => ' [ + 'code' => ' [ + 'code' => ' [ + 'code' => ' [ + 'code' => ' [ + 'code' => ' [ + 'code' => ' [ + 'code' => ' [ + 'code' => ' [ + 'code' => 'run_in_c(array($this, "hello")); + } + + public function hello(): void { + echo "hello"; + } + + /** + * @param callable $callable + * @return void + */ + public function run_in_c($callable) { + call_user_func($callable); + } + }', + ], + 'callableInstanceArrayMethodClassContextNonStaticNonPublic' => [ + 'code' => 'run_in_c(array($this, "hello")); + } + + protected function hello(): void { + echo "hello"; + } + + /** + * @param callable $callable + * @return void + */ + private function run_in_c($callable) { + call_user_func($callable); + } + }', + ], + 'callableClassConstantArrayMethodClassContextStaticNonPublic' => [ + 'code' => 'run_in_c(array(Foo::class, "hello")); + } + + protected static function hello(): void { + echo "hello"; + } + + /** + * @param callable $callable + * @return void + */ + private function run_in_c($callable) { + call_user_func($callable); + } + }', + ], + 'callableClassConstantArrayMethodClassContextNonStaticNonPublic' => [ + 'code' => 'run_in_c(array(Foo::class, "hello")); + } + + protected function hello(): void { + echo "hello"; + } + + /** + * @param callable $callable + * @return void + */ + private function run_in_c($callable) { + call_user_func($callable); + } + }', + ], + 'callableClassStringArrayMethodOtherClassContextStaticPublic' => [ + 'code' => 'run_in_c(array(Foo::class, "hello")); + } + + public static function hello(): void { + echo "hello"; + } + } + + class Bar { + /** + * @param callable $callable + * @return void + */ + public function run_in_c($callable) { + call_user_func($callable); + } + } + ', + ], + 'callableInstanceArrayMethodOtherClassContextNonStaticPublic' => [ + 'code' => 'run_in_c(array($this, "hello")); + } + + public function hello(): void { + echo "hello"; + } + } + + class Bar { + /** + * @param callable $callable + * @return void + */ + public function run_in_c($callable) { + call_user_func($callable); + } + } + ', + ], + 'callableClassLiteralStringMethodOtherClassContextStaticPublic' => [ + 'code' => 'run_in_c("Foo::hello"); + } + + public static function hello(): void { + echo "hello"; + } + } + + class Bar { + /** + * @param callable $callable + * @return void + */ + public function run_in_c($callable) { + call_user_func($callable); + } + } + ', + ], + # @todo valid 'notCallableListNoUndefinedClass' => [ 'code' => ' 'InvalidFunctionCall', ], + 'callableMethodOutOfClassContextNonPublic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableMethodOutOfClassContextNonStaticNonPublic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableClassStringArrayMethodOutOfClassContextNonStatic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableClassStringArrayMethodOutOfClassContextNonPublic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableClassStringArrayMethodOutOfClassContextNonStaticNonPublic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableClassStringMethodOutOfClassContextNonStatic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableClassStringMethodOutOfClassContextNonPublic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableClassStringMethodOutOfClassContextNonStaticNonPublic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableInClassStringArrayMethodOutOfClassContextNonStatic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableInClassStringArrayMethodOutOfClassContextNonPublic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableInClassStringArrayMethodOutOfClassContextNonStaticNonPublic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableInClassLiteralStringArrayMethodOutOfClassContextNonStatic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableInClassLiteralStringArrayMethodOutOfClassContextNonPublic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableInClassLiteralStringArrayMethodOutOfClassContextNonStaticNonPublic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableInClassConstantArrayMethodOutOfClassContextNonStatic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableInClassConstantArrayMethodOutOfClassContextNonPublic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableInClassConstantArrayMethodOutOfClassContextNonStaticNonPublic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableInClassStringMethodOutOfClassContextNonStatic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableInClassStringMethodOutOfClassContextNonPublic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableInClassStringMethodOutOfClassContextNonStaticNonPublic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableInstanceArrayMethodClassContextPhpNativeUnsupportedNonStaticNonPublic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableInstanceArrayMethodOutOfClassContextNonStaticNonPublic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableInstanceArrayMethodOutOfClassContextStaticNonPublic' => [ + 'code' => ' 'InvalidArgument', + ], + 'callableClassStringArrayMethodOtherClassContextNonStaticPublic' => [ + 'code' => 'run_in_c(array(Foo::class, "hello")); + } + + public function hello(): void { + echo "hello"; + } + } + + class Bar { + /** + * @param callable $callable + * @return void + */ + public function run_in_c($callable) { + call_user_func($callable); + } + }', + 'error_message' => 'InvalidArgument', + ], + 'callableInstanceArrayMethodOtherClassContextNonStaticNonPublic' => [ + 'code' => 'run_in_c(array($this, "hello")); + } + + protected function hello(): void { + echo "hello"; + } + } + + class Bar { + /** + * @param callable $callable + * @return void + */ + public function run_in_c($callable) { + call_user_func($callable); + } + }', + 'error_message' => 'InvalidArgument', + ], + 'callableClassLiteralStringMethodOtherClassContextStaticNonPublic' => [ + 'code' => 'run_in_c("Foo::hello"); + } + + protected static function hello(): void { + echo "hello"; + } + } + + class Bar { + /** + * @param callable $callable + * @return void + */ + public function run_in_c($callable) { + call_user_func($callable); + } + }', + 'error_message' => 'InvalidArgument', + ], + # @todo invalid 'ImpureFunctionCall' => [ 'code' => ' Date: Wed, 13 Mar 2024 19:05:13 +0100 Subject: [PATCH 22/28] ParamNameMismatch not reported for __construct Fix https://github.com/vimeo/psalm/issues/10784 --- .../issues/ConstructorSignatureMismatch.md | 2 +- .../Internal/Analyzer/MethodComparator.php | 4 +++- tests/ClassTest.php | 23 +++++++++++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/docs/running_psalm/issues/ConstructorSignatureMismatch.md b/docs/running_psalm/issues/ConstructorSignatureMismatch.md index 767375e14fa..710e313ea01 100644 --- a/docs/running_psalm/issues/ConstructorSignatureMismatch.md +++ b/docs/running_psalm/issues/ConstructorSignatureMismatch.md @@ -9,7 +9,7 @@ Emitted when a constructor parameter differs from a parent constructor parameter * @psalm-consistent-constructor */ class A { - public function __construct(int $i) {} + public function __construct(int $s) {} } class B extends A { public function __construct(string $s) {} diff --git a/src/Psalm/Internal/Analyzer/MethodComparator.php b/src/Psalm/Internal/Analyzer/MethodComparator.php index 1c0c05df9c7..2fc7a24ca20 100644 --- a/src/Psalm/Internal/Analyzer/MethodComparator.php +++ b/src/Psalm/Internal/Analyzer/MethodComparator.php @@ -451,7 +451,9 @@ private static function compareMethodParams( && $implementer_classlike_storage->user_defined && $implementer_param->location && $guide_method_storage->cased_name - && strpos($guide_method_storage->cased_name, '__') !== 0 + && (strpos($guide_method_storage->cased_name, '__') !== 0 + || ($guide_classlike_storage->preserve_constructor_signature + && $guide_method_storage->cased_name === '__construct')) && $config->isInProjectDirs( $implementer_param->location->file_path, ) diff --git a/tests/ClassTest.php b/tests/ClassTest.php index d245fb44fc9..1d1984d4b12 100644 --- a/tests/ClassTest.php +++ b/tests/ClassTest.php @@ -985,6 +985,29 @@ class A {} echo A::HELLO;', 'error_message' => 'UndefinedConstant', ], + 'consistentNamesConstructor' => [ + 'code' => ' 'ParamNameMismatch', + ], 'overridePublicAccessLevelToPrivate' => [ 'code' => ' Date: Mon, 26 Feb 2024 20:48:42 +1300 Subject: [PATCH 23/28] When inside isset, array fetch can return null This prevents false positive for various types of issues inside empty, such as RedundantConditionGivenDocblockType and TypeDoesNotContainType. --- .../Expression/Fetch/ArrayFetchAnalyzer.php | 8 ++++++-- tests/ArrayAssignmentTest.php | 2 ++ tests/ArrayFunctionCallTest.php | 1 + tests/TypeReconciliation/EmptyTest.php | 13 +++++++++++++ tests/TypeReconciliation/RedundantConditionTest.php | 2 ++ 5 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php index 7f81211330a..34ed8f51076 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php @@ -171,7 +171,7 @@ public static function analyze( $codebase = $statements_analyzer->getCodebase(); - if ($keyed_array_var_id + if ($keyed_array_var_id !== null && $context->hasVariable($keyed_array_var_id) && !$context->vars_in_scope[$keyed_array_var_id]->possibly_undefined && $stmt_var_type @@ -250,6 +250,10 @@ public static function analyze( } } + if ($context->inside_isset && !$stmt_type->hasMixed()) { + $stmt_type = Type::combineUnionTypes($stmt_type, Type::getNull()); + } + $statements_analyzer->node_data->setType($stmt, $stmt_type); if ($context->inside_isset @@ -304,7 +308,7 @@ public static function analyze( } } - if ($keyed_array_var_id + if ($keyed_array_var_id !== null && $context->hasVariable($keyed_array_var_id) && (!($stmt_type = $statements_analyzer->node_data->getType($stmt)) || $stmt_type->isVanillaMixed()) ) { diff --git a/tests/ArrayAssignmentTest.php b/tests/ArrayAssignmentTest.php index fa65da8223c..488550453b3 100644 --- a/tests/ArrayAssignmentTest.php +++ b/tests/ArrayAssignmentTest.php @@ -1234,6 +1234,8 @@ function takesList(array $arr) : void { foreach ($arr[0] as $k => $v) {} } }', + 'assertions' => [], + 'ignored_issues' => ['RiskyTruthyFalsyComparison'], ], 'nonEmptyAssignmentToListElement' => [ 'code' => ' [ '$line===' => 'array{0: int, ...}', ], + 'ignored_issues' => ['RiskyTruthyFalsyComparison'], ], 'arrayUnshiftOnEmptyArrayMeansNonEmptyList' => [ 'code' => ' [], + 'ignored_issues' => ['RiskyTruthyFalsyComparison'], ], 'multipleEmptiesInConditionWithMixedOffset' => [ 'code' => ' [], + 'ignored_issues' => ['RiskyTruthyFalsyComparison'], ], 'doubleEmptyCheckTwoArrays' => [ 'code' => ' [], + 'ignored_issues' => ['RiskyTruthyFalsyComparison'], ], 'doubleEmptyCheckOnTKeyedArrayVariableOffsets' => [ 'code' => ' [], + 'ignored_issues' => ['RiskyTruthyFalsyComparison'], ], 'checkArrayEmptyUnknownRoot' => [ 'code' => ' 'true', ], ], + 'emptyArrayFetch' => [ + 'code' => ' $a */ + if (empty($a["a"])) {}', + ], ]; } diff --git a/tests/TypeReconciliation/RedundantConditionTest.php b/tests/TypeReconciliation/RedundantConditionTest.php index 74f06e44ea1..e54aa2bd369 100644 --- a/tests/TypeReconciliation/RedundantConditionTest.php +++ b/tests/TypeReconciliation/RedundantConditionTest.php @@ -575,6 +575,8 @@ function foo(array $a) : void { if (empty($a["foo"])) {} } }', + 'assertions' => [], + 'ignored_issues' => ['RiskyTruthyFalsyComparison'], ], 'suppressRedundantConditionAfterAssertNonEmpty' => [ 'code' => ' Date: Wed, 28 Feb 2024 21:46:58 +1300 Subject: [PATCH 24/28] Update baseline --- psalm-baseline.xml | 59 +++++++++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 8d205b058f7..987aab124ad 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1,10 +1,5 @@ - - - - - - + tags['variablesfrom'][0]]]> @@ -75,7 +70,6 @@ - function_id]]> @@ -716,6 +710,7 @@ template_extended_params]]> template_types]]> + overridden_method_ids[$method_name])]]> @@ -865,8 +860,6 @@ self]]> mixin_declaring_fqcln]]> - parent_class]]> - parent_class]]> calling_method_id]]> calling_method_id]]> self]]> @@ -945,8 +938,6 @@ - - @@ -1104,7 +1095,6 @@ - @@ -1115,9 +1105,6 @@ error_baseline]]> - - - threads]]> @@ -1128,7 +1115,6 @@ - @@ -1140,7 +1126,6 @@ - @@ -1201,11 +1186,7 @@ - - - - - value]]> + overridden_method_ids[$method_name])]]> @@ -1441,7 +1422,6 @@ - children[0]]]> children[1]]]> @@ -1475,6 +1455,9 @@ line_number]]> type_end]]> type_start]]> + + + @@ -1498,12 +1481,12 @@ - - - 0]]> + + + @@ -1636,6 +1619,13 @@ cache->getFileMapCache()]]> + + + + + + + @@ -1730,6 +1720,7 @@ + @@ -1884,6 +1875,7 @@ template_extended_params]]> + offset_param_name])]]> @@ -1897,6 +1889,7 @@ template_extended_params[$container_class])]]> template_extended_params[$base_type->as_type->value])]]> template_extended_params[$base_type->value])]]> + lower_bounds[$atomic_type->offset_param_name])]]> @@ -1929,9 +1922,6 @@ strings]]> strings]]> strings]]> - value_types['string'] instanceof TNonFalsyString - ? $type->value - : $type->value !== '']]> @@ -2278,7 +2268,6 @@ - @@ -2348,6 +2337,11 @@ + + + + + @@ -2357,4 +2351,9 @@ + + + + + From 4b707d1233610f5b81a87cef575c3ce8d4e80aac Mon Sep 17 00:00:00 2001 From: Evan Shaw Date: Mon, 4 Mar 2024 21:39:18 +1300 Subject: [PATCH 25/28] Additional array fetch test case --- tests/TypeReconciliation/EmptyTest.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/TypeReconciliation/EmptyTest.php b/tests/TypeReconciliation/EmptyTest.php index d144faa39e9..bce863e0d34 100644 --- a/tests/TypeReconciliation/EmptyTest.php +++ b/tests/TypeReconciliation/EmptyTest.php @@ -771,6 +771,13 @@ function bar() { }', 'error_message' => 'RedundantConditionGivenDocblockType', ], + 'redundantEmptyArrayFetch' => [ + 'code' => ' $a */; + assert(isset($a["a"])); + if (empty($a["a"])) {}', + 'error_message' => 'DocblockTypeContradiction', + ], ]; } } From 375fe32992f63634380d255797aa39287c28b3c2 Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Wed, 20 Mar 2024 05:32:13 +0100 Subject: [PATCH 26/28] Update tests/CallableTest.php --- tests/CallableTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/CallableTest.php b/tests/CallableTest.php index ba9e5467da4..250afae7261 100644 --- a/tests/CallableTest.php +++ b/tests/CallableTest.php @@ -2637,7 +2637,7 @@ public function bar(): bool { function foo($arg) {} class A { - public static function bar(): bool { + public function bar(): bool { return true; } } From 2f908817d4bbe4d15b77685fbca082ee6ec40ae8 Mon Sep 17 00:00:00 2001 From: Thomas Landauer Date: Thu, 21 Mar 2024 18:34:34 +0100 Subject: [PATCH 27/28] Update CallMap.php --- dictionaries/CallMap.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dictionaries/CallMap.php b/dictionaries/CallMap.php index e4442fcf28f..ad879c10104 100644 --- a/dictionaries/CallMap.php +++ b/dictionaries/CallMap.php @@ -3307,7 +3307,7 @@ 'getimagesize' => ['array{0:int, 1: int, 2: int, 3: string, mime: string, channels?: 3|4, bits?: int}|false', 'filename'=>'string', '&w_image_info='=>'array'], 'getimagesizefromstring' => ['array{0:int, 1: int, 2: int, 3: string, mime: string, channels?: 3|4, bits?: int}|false', 'string'=>'string', '&w_image_info='=>'array'], 'getlastmod' => ['int|false'], -'getmxrr' => ['bool', 'hostname'=>'string', '&w_hosts'=>'array', '&w_weights='=>'array'], +'getmxrr' => ['bool', 'hostname'=>'string', '&w_hosts'=>'array', '&w_weights='=>'array'], 'getmygid' => ['int|false'], 'getmyinode' => ['int|false'], 'getmypid' => ['int|false'], From 2ae91aa95c6072d95eba870b4ed653ebc2a10d80 Mon Sep 17 00:00:00 2001 From: Thomas Landauer Date: Thu, 21 Mar 2024 20:33:41 +0100 Subject: [PATCH 28/28] Update CallMap_historical.php --- dictionaries/CallMap_historical.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dictionaries/CallMap_historical.php b/dictionaries/CallMap_historical.php index 92746fd697b..dd45575237d 100644 --- a/dictionaries/CallMap_historical.php +++ b/dictionaries/CallMap_historical.php @@ -10671,7 +10671,7 @@ 'getimagesize' => ['array{0:int, 1: int, 2: int, 3: string, mime: string, channels?: 3|4, bits?: int}|false', 'filename'=>'string', '&w_image_info='=>'array'], 'getimagesizefromstring' => ['array{0:int, 1: int, 2: int, 3: string, mime: string, channels?: 3|4, bits?: int}|false', 'string'=>'string', '&w_image_info='=>'array'], 'getlastmod' => ['int|false'], - 'getmxrr' => ['bool', 'hostname'=>'string', '&w_hosts'=>'array', '&w_weights='=>'array'], + 'getmxrr' => ['bool', 'hostname'=>'string', '&w_hosts'=>'array', '&w_weights='=>'array'], 'getmygid' => ['int|false'], 'getmyinode' => ['int|false'], 'getmypid' => ['int|false'],