From 6e361aa9e67fecbbc70e2147e1cf016efcbf00a7 Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Wed, 7 Feb 2024 08:59:49 +0000 Subject: [PATCH 01/59] Failed and regression tests for magic static methods List failed tests: ``` 1) Psalm\Tests\MagicMethodAnnotationTest::testNoSealAllMethodsWithStatic Failed asserting that exception of type "Psalm\Exception\CodeException" is thrown. 2) Psalm\Tests\MagicMethodAnnotationTest::testSealAllMethodsWithoutFooWithStatic Failed asserting that exception of type "Psalm\Exception\CodeException" is thrown. 3) Psalm\Tests\MagicMethodAnnotationTest::testInvalidCode with data set "inheritSealedMethodsWithStatic" Failed asserting that exception of type "Psalm\Exception\CodeException" is thrown. ``` --- tests/MagicMethodAnnotationTest.php | 238 ++++++++++++++++++++++++++++ 1 file changed, 238 insertions(+) diff --git a/tests/MagicMethodAnnotationTest.php b/tests/MagicMethodAnnotationTest.php index 6b7f05e32af..6a92fa4de6d 100644 --- a/tests/MagicMethodAnnotationTest.php +++ b/tests/MagicMethodAnnotationTest.php @@ -46,6 +46,35 @@ class Child {} $this->analyzeFile('somefile.php', new Context()); } + public function testPhpDocMethodWhenUndefinedWithStatic(): void + { + Config::getInstance()->use_phpdoc_method_without_magic_or_parent = true; + + $this->addFile( + 'somefile.php', + 'analyzeFile('somefile.php', new Context()); + } + public function testPhpDocMethodWhenTemplated(): void { Config::getInstance()->use_phpdoc_method_without_magic_or_parent = true; @@ -99,6 +128,28 @@ class Child {} $this->analyzeFile('somefile.php', $context); } + public function testAnnotationWithoutCallConfigWithStatic(): void + { + $this->expectExceptionMessage('UndefinedMethod'); + $this->expectException(CodeException::class); + Config::getInstance()->use_phpdoc_method_without_magic_or_parent = false; + + $this->addFile( + 'somefile.php', + 'analyzeFile('somefile.php', $context); + } + public function testOverrideParentClassRetunType(): void { Config::getInstance()->use_phpdoc_method_without_magic_or_parent = true; @@ -193,6 +244,48 @@ class Child extends ParentClass {} '$e' => 'callable():string', ], ], + 'validSimpleAnnotationsWithStatic' => [ + 'code' => ' [ + '$a' => 'string', + '$b' => 'mixed', + '$c' => 'bool', + '$d' => 'array', + '$e' => 'callable():string', + '$f' => 'Child', + ], + ], 'validAnnotationWithDefault' => [ 'code' => 'foo();', 'error_message' => 'UndefinedMagicMethod', ], + 'inheritSealedMethodsWithStatic' => [ + 'code' => ' 'UndefinedMagicMethod', + ], 'lonelyMethod' => [ 'code' => 'analyzeFile('somefile.php', new Context()); } + public function testSealAllMethodsWithoutFooWithStatic(): void + { + Config::getInstance()->seal_all_methods = true; + + $this->addFile( + 'somefile.php', + 'expectException(CodeException::class); + $this->expectExceptionMessage($error_message); + $this->analyzeFile('somefile.php', new Context()); + } + public function testNoSealAllMethods(): void { Config::getInstance()->seal_all_methods = true; @@ -1199,6 +1329,30 @@ class B extends A {} $this->analyzeFile('somefile.php', new Context()); } + public function testNoSealAllMethodsWithStatic(): void + { + Config::getInstance()->seal_all_methods = true; + + $this->addFile( + 'somefile.php', + 'expectException(CodeException::class); + $this->expectExceptionMessage($error_message); + $this->analyzeFile('somefile.php', new Context()); + } + public function testSealAllMethodsWithFoo(): void { Config::getInstance()->seal_all_methods = true; @@ -1221,6 +1375,27 @@ class B extends A {} $this->analyzeFile('somefile.php', new Context()); } + public function testSealAllMethodsWithFooWithStatic(): void + { + Config::getInstance()->seal_all_methods = true; + + $this->addFile( + 'somefile.php', + 'analyzeFile('somefile.php', new Context()); + } + public function testSealAllMethodsWithFooInSubclass(): void { Config::getInstance()->seal_all_methods = true; @@ -1244,6 +1419,28 @@ public function foo(): void {} $this->analyzeFile('somefile.php', new Context()); } + public function testSealAllMethodsWithFooInSubclassWithStatic(): void + { + Config::getInstance()->seal_all_methods = true; + + $this->addFile( + 'somefile.php', + 'analyzeFile('somefile.php', new Context()); + } + public function testSealAllMethodsWithFooAnnotated(): void { Config::getInstance()->seal_all_methods = true; @@ -1266,6 +1463,27 @@ class B extends A {} $this->analyzeFile('somefile.php', new Context()); } + public function testSealAllMethodsWithFooAnnotatedWithStatic(): void + { + Config::getInstance()->seal_all_methods = true; + + $this->addFile( + 'somefile.php', + 'analyzeFile('somefile.php', new Context()); + } + public function testSealAllMethodsSetToFalse(): void { Config::getInstance()->seal_all_methods = false; @@ -1287,6 +1505,26 @@ class B extends A {} $this->analyzeFile('somefile.php', new Context()); } + public function testSealAllMethodsSetToFalseWithStatic(): void + { + Config::getInstance()->seal_all_methods = false; + + $this->addFile( + 'somefile.php', + 'analyzeFile('somefile.php', new Context()); + } + public function testIntersectionTypeWhenMagicMethodDoesNotExistButIsProvidedBySecondType(): void { $this->addFile( From 29b4c38a944c8c2bc08eb2ab846e66dd73417819 Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Tue, 6 Feb 2024 07:30:33 +0000 Subject: [PATCH 02/59] Add argument with_pseudo for method finding --- src/Psalm/Internal/Codebase/Methods.php | 32 ++++++++++++++++++------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/src/Psalm/Internal/Codebase/Methods.php b/src/Psalm/Internal/Codebase/Methods.php index ad97dfbc65e..bc9abe6572f 100644 --- a/src/Psalm/Internal/Codebase/Methods.php +++ b/src/Psalm/Internal/Codebase/Methods.php @@ -99,7 +99,8 @@ public function methodExists( ?StatementsSource $source = null, ?string $source_file_path = null, bool $use_method_existence_provider = true, - bool $is_used = false + bool $is_used = false, + bool $with_pseudo = false ): bool { $fq_class_name = $method_id->fq_class_name; $method_name = $method_id->method_name; @@ -147,9 +148,11 @@ public function methodExists( $calling_class_name = explode('::', $calling_method_id)[0]; } - if (isset($class_storage->declaring_method_ids[$method_name])) { - $declaring_method_id = $class_storage->declaring_method_ids[$method_name]; - + $declaring_method_id = $class_storage->declaring_method_ids[$method_name] ?? null; + if ($declaring_method_id === null && $with_pseudo) { + $declaring_method_id = $class_storage->declaring_pseudo_method_ids[$method_name] ?? null; + } + if ($declaring_method_id !== null) { if ($calling_method_id === strtolower((string) $declaring_method_id)) { return true; } @@ -1008,7 +1011,8 @@ public function setAppearingMethodId( /** @psalm-mutation-free */ public function getDeclaringMethodId( - MethodIdentifier $method_id + MethodIdentifier $method_id, + bool $with_pseudo = false ): ?MethodIdentifier { $fq_class_name = $this->classlikes->getUnAliasedName($method_id->fq_class_name); @@ -1024,6 +1028,10 @@ public function getDeclaringMethodId( return reset($class_storage->overridden_method_ids[$method_name]); } + if ($with_pseudo && isset($class_storage->declaring_pseudo_method_ids[$method_name])) { + return $class_storage->declaring_pseudo_method_ids[$method_name]; + } + return null; } @@ -1131,7 +1139,7 @@ public function getClassLikeStorageForMethod(MethodIdentifier $method_id): Class } /** @psalm-mutation-free */ - public function getStorage(MethodIdentifier $method_id): MethodStorage + public function getStorage(MethodIdentifier $method_id, bool $with_pseudo = false): MethodStorage { try { $class_storage = $this->classlike_storage_provider->get($method_id->fq_class_name); @@ -1141,13 +1149,21 @@ public function getStorage(MethodIdentifier $method_id): MethodStorage $method_name = $method_id->method_name; - if (!isset($class_storage->methods[$method_name])) { + $storage = $class_storage->methods[$method_name] ?? null; + + if ($storage === null && $with_pseudo) { + $storage = $class_storage->pseudo_methods[$method_name] + ?? $class_storage->pseudo_static_methods[$method_name] + ?? null; + } + + if ($storage === null) { throw new UnexpectedValueException( '$storage should not be null for ' . $method_id, ); } - return $class_storage->methods[$method_name]; + return $storage; } /** @psalm-mutation-free */ From 083b8e26db201d8a527958f7bc667904629a7c4a Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Tue, 6 Feb 2024 07:41:57 +0000 Subject: [PATCH 03/59] Add issue message for magic methods --- .../Internal/Analyzer/MethodAnalyzer.php | 30 +++++++++++++++---- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/MethodAnalyzer.php b/src/Psalm/Internal/Analyzer/MethodAnalyzer.php index bc42192496a..05773590b0f 100644 --- a/src/Psalm/Internal/Analyzer/MethodAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/MethodAnalyzer.php @@ -13,6 +13,7 @@ use Psalm\Issue\InvalidStaticInvocation; use Psalm\Issue\MethodSignatureMustOmitReturnType; use Psalm\Issue\NonStaticSelfCall; +use Psalm\Issue\UndefinedMagicMethod; use Psalm\Issue\UndefinedMethod; use Psalm\IssueBuffer; use Psalm\StatementsSource; @@ -165,7 +166,8 @@ public static function checkMethodExists( MethodIdentifier $method_id, CodeLocation $code_location, array $suppressed_issues, - ?string $calling_method_id = null + ?string $calling_method_id = null, + bool $with_pseudo = false ): ?bool { if ($codebase->methods->methodExists( $method_id, @@ -176,15 +178,31 @@ public static function checkMethodExists( : null, null, $code_location->file_path, + true, + false, + $with_pseudo, )) { return true; } - if (IssueBuffer::accepts( - new UndefinedMethod('Method ' . $method_id . ' does not exist', $code_location, (string) $method_id), - $suppressed_issues, - )) { - return false; + if ($with_pseudo) { + if (IssueBuffer::accepts( + new UndefinedMagicMethod( + 'Magic method ' . $method_id . ' does not exist', + $code_location, + (string) $method_id, + ), + $suppressed_issues, + )) { + return false; + } + } else { + if (IssueBuffer::accepts( + new UndefinedMethod('Method ' . $method_id . ' does not exist', $code_location, (string) $method_id), + $suppressed_issues, + )) { + return false; + } } return null; From 2567a99347dda0e36b530f224bee6eef63079e16 Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Tue, 6 Feb 2024 08:04:05 +0000 Subject: [PATCH 04/59] Extract checking method "__callStatic" from block "if" Appended problem by that commit: ``` 1) FileReferenceTest::testReferencedMethods with data set "getClassReferences" Failed asserting that two arrays are identical. --- Expected +++ Actual @@ @@ 'foo\b::__construct' => true 'foo\c::foo' => true ) - 'foo\c::__construct' => Array &2 ( + 'foo\a::__callstatic' => Array &2 ( + 'foo\b::__construct' => true + ) + 'foo\c::__construct' => Array &3 ( 'foo\b::bar' => true ) ) ``` --- .../StaticMethod/AtomicStaticCallAnalyzer.php | 40 ++++++++++--------- 1 file changed, 21 insertions(+), 19 deletions(-) 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 8e187ab391f..76561582a4b 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php @@ -562,6 +562,26 @@ private static function handleNamedCall( return true; } + $callstatic_id = new MethodIdentifier( + $fq_class_name, + '__callstatic', + ); + + $callstatic_method_exists = $codebase->methods->methodExists( + $callstatic_id, + $context->calling_method_id, + $codebase->collect_locations + ? new CodeLocation($statements_analyzer, $stmt_name) + : null, + !$context->collect_initializations + && !$context->collect_mutations + ? $statements_analyzer + : null, + $statements_analyzer->getFilePath(), + true, + $context->insideUse(), + ); + if (!$naive_method_exists || !MethodAnalyzer::isMethodVisible( $method_id, @@ -572,25 +592,7 @@ private static function handleNamedCall( || ($found_method_and_class_storage && ($config->use_phpdoc_method_without_magic_or_parent || $class_storage->parent_class)) ) { - $callstatic_id = new MethodIdentifier( - $fq_class_name, - '__callstatic', - ); - - if ($codebase->methods->methodExists( - $callstatic_id, - $context->calling_method_id, - $codebase->collect_locations - ? new CodeLocation($statements_analyzer, $stmt_name) - : null, - !$context->collect_initializations - && !$context->collect_mutations - ? $statements_analyzer - : null, - $statements_analyzer->getFilePath(), - true, - $context->insideUse(), - )) { + if ($callstatic_method_exists) { $callstatic_declaring_id = $codebase->methods->getDeclaringMethodId($callstatic_id); assert($callstatic_declaring_id !== null); $callstatic_pure = false; From df2067dd749563d466d06d57beb1624f83c54093 Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Wed, 31 Jan 2024 21:26:00 +0000 Subject: [PATCH 05/59] Resolve testReferencedMethods Resolved problem: ``` FileReferenceTest::testReferencedMethods with data set "getClassReferences" Failed asserting that two arrays are identical. --- Expected +++ Actual @@ @@ 'foo\b::__construct' => true 'foo\c::foo' => true ) - 'foo\c::__construct' => Array &2 ( + 'foo\a::__callstatic' => Array &2 ( + 'foo\b::__construct' => true + ) + 'foo\c::__construct' => Array &3 ( 'foo\b::bar' => true ) ) ``` --- .../StaticMethod/AtomicStaticCallAnalyzer.php | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) 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 76561582a4b..2b76c7d661f 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php @@ -567,20 +567,7 @@ private static function handleNamedCall( '__callstatic', ); - $callstatic_method_exists = $codebase->methods->methodExists( - $callstatic_id, - $context->calling_method_id, - $codebase->collect_locations - ? new CodeLocation($statements_analyzer, $stmt_name) - : null, - !$context->collect_initializations - && !$context->collect_mutations - ? $statements_analyzer - : null, - $statements_analyzer->getFilePath(), - true, - $context->insideUse(), - ); + $callstatic_method_exists = $codebase->methods->methodExists($callstatic_id); if (!$naive_method_exists || !MethodAnalyzer::isMethodVisible( From 0ba346c5a0addf22082c360242c23f5755ad60f4 Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Wed, 31 Jan 2024 16:59:28 +0000 Subject: [PATCH 06/59] Delete code of replacing variable method_id The main task for deleting: ``` $method_id = new MethodIdentifier( $fq_class_name, '__callstatic', ); ``` List of resolved problems: ``` 1) Psalm\Tests\MagicMethodAnnotationTest::testNoSealAllMethodsWithStatic Failed asserting that exception of type "Psalm\Exception\CodeException" is thrown. 2) Psalm\Tests\MagicMethodAnnotationTest::testSealAllMethodsWithoutFooWithStatic Failed asserting that exception of type "Psalm\Exception\CodeException" is thrown. 3) Psalm\Tests\MagicMethodAnnotationTest::testInvalidCode with data set "inheritSealedMethodsWithStatic" Failed asserting that exception of type "Psalm\Exception\CodeException" is thrown. ``` Appended problem by that fix: ``` 1) MagicMethodAnnotationTest::testSealAllMethodsSetToFalseWithStatic Psalm\Exception\CodeException: UndefinedMagicMethod - somefile.php:8:15 - Magic method B::foo does not exist ``` --- .../Internal/Analyzer/MethodAnalyzer.php | 5 ++- .../Call/Method/MethodVisibilityAnalyzer.php | 6 ++- .../StaticMethod/AtomicStaticCallAnalyzer.php | 38 ++----------------- 3 files changed, 10 insertions(+), 39 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/MethodAnalyzer.php b/src/Psalm/Internal/Analyzer/MethodAnalyzer.php index 05773590b0f..94e81f119a5 100644 --- a/src/Psalm/Internal/Analyzer/MethodAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/MethodAnalyzer.php @@ -111,8 +111,9 @@ public static function checkStatic( } $original_method_id = $method_id; + $with_pseudo = true; - $method_id = $codebase_methods->getDeclaringMethodId($method_id); + $method_id = $codebase_methods->getDeclaringMethodId($method_id, $with_pseudo); if (!$method_id) { if (InternalCallMapHandler::inCallMap((string) $original_method_id)) { @@ -122,7 +123,7 @@ public static function checkStatic( throw new LogicException('Declaring method for ' . $original_method_id . ' should not be null'); } - $storage = $codebase_methods->getStorage($method_id); + $storage = $codebase_methods->getStorage($method_id, $with_pseudo); if (!$storage->is_static) { if ($self_call) { diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodVisibilityAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodVisibilityAnalyzer.php index 0a81ee8ecac..e96edfd58cf 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodVisibilityAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodVisibilityAnalyzer.php @@ -40,6 +40,8 @@ public static function analyze( $fq_classlike_name = $method_id->fq_class_name; $method_name = $method_id->method_name; + $with_pseudo = true; + if ($codebase_methods->visibility_provider->has($fq_classlike_name)) { $method_visible = $codebase_methods->visibility_provider->isMethodVisible( $source, @@ -65,7 +67,7 @@ public static function analyze( } } - $declaring_method_id = $codebase_methods->getDeclaringMethodId($method_id); + $declaring_method_id = $codebase_methods->getDeclaringMethodId($method_id, $with_pseudo); if (!$declaring_method_id) { if ($method_name === '__construct' @@ -109,7 +111,7 @@ public static function analyze( return null; } - $storage = $codebase->methods->getStorage($declaring_method_id); + $storage = $codebase->methods->getStorage($declaring_method_id, $with_pseudo); $visibility = $storage->visibility; if ($appearing_method_name 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 2b76c7d661f..c6e36809046 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php @@ -32,12 +32,8 @@ use Psalm\Issue\UndefinedClass; use Psalm\Issue\UndefinedMethod; use Psalm\IssueBuffer; -use Psalm\Node\Expr\VirtualArray; -use Psalm\Node\Expr\VirtualArrayItem; use Psalm\Node\Expr\VirtualMethodCall; use Psalm\Node\Expr\VirtualVariable; -use Psalm\Node\Scalar\VirtualString; -use Psalm\Node\VirtualArg; use Psalm\Storage\ClassLikeStorage; use Psalm\Storage\MethodStorage; use Psalm\Type; @@ -57,7 +53,6 @@ use Psalm\Type\Union; use function array_filter; -use function array_map; use function array_values; use function assert; use function count; @@ -569,6 +564,8 @@ private static function handleNamedCall( $callstatic_method_exists = $codebase->methods->methodExists($callstatic_id); + $with_pseudo = $callstatic_method_exists; + if (!$naive_method_exists || !MethodAnalyzer::isMethodVisible( $method_id, @@ -680,36 +677,6 @@ private static function handleNamedCall( return false; } } - - $array_values = array_map( - static fn(PhpParser\Node\Arg $arg): PhpParser\Node\Expr\ArrayItem => new VirtualArrayItem( - $arg->value, - null, - false, - $arg->getAttributes(), - ), - $args, - ); - - $args = [ - new VirtualArg( - new VirtualString((string) $method_id, $stmt_name->getAttributes()), - false, - false, - $stmt_name->getAttributes(), - ), - new VirtualArg( - new VirtualArray($array_values, $stmt->getAttributes()), - false, - false, - $stmt->getAttributes(), - ), - ]; - - $method_id = new MethodIdentifier( - $fq_class_name, - '__callstatic', - ); } elseif ($found_method_and_class_storage && ($config->use_phpdoc_method_without_magic_or_parent || $class_storage->parent_class) ) { @@ -797,6 +764,7 @@ private static function handleNamedCall( new CodeLocation($statements_analyzer, $stmt), $statements_analyzer->getSuppressedIssues(), $context->calling_method_id, + $with_pseudo, ); if (!$does_method_exist) { From 5f89fa173067529e5c4e4fef2b9668f2dbacbf83 Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Wed, 31 Jan 2024 20:17:21 +0000 Subject: [PATCH 07/59] Resolved all tests Resolved problem: ``` 1) MagicMethodAnnotationTest::testSealAllMethodsSetToFalseWithStatic Psalm\Exception\CodeException: UndefinedMagicMethod - somefile.php:8:15 - Magic method B::foo does not exist ``` --- .../StaticMethod/AtomicStaticCallAnalyzer.php | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) 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 c6e36809046..7d14b2c5eba 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php @@ -758,14 +758,18 @@ private static function handleNamedCall( } } - $does_method_exist = MethodAnalyzer::checkMethodExists( - $codebase, - $method_id, - new CodeLocation($statements_analyzer, $stmt), - $statements_analyzer->getSuppressedIssues(), - $context->calling_method_id, - $with_pseudo, - ); + if (!$callstatic_method_exists || $class_storage->hasSealedMethods($config)) { + $does_method_exist = MethodAnalyzer::checkMethodExists( + $codebase, + $method_id, + new CodeLocation($statements_analyzer, $stmt), + $statements_analyzer->getSuppressedIssues(), + $context->calling_method_id, + $with_pseudo, + ); + } else { + $does_method_exist = null; + } if (!$does_method_exist) { if (ArgumentsAnalyzer::analyze( From 4fce0700bf9981b609424ea8cb1c0e954e3f5b3c Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Wed, 7 Feb 2024 12:37:54 +0000 Subject: [PATCH 08/59] Failed and regression tests with usage config Failed test: ``` 1) MagicMethodAnnotationTest::testAnnotationWithoutCallConfigWithExtendsWithStatic Failed asserting that exception of type "Psalm\Exception\CodeException" is thrown. ``` --- tests/MagicMethodAnnotationTest.php | 48 +++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/MagicMethodAnnotationTest.php b/tests/MagicMethodAnnotationTest.php index 6a92fa4de6d..ca0b28a0bcf 100644 --- a/tests/MagicMethodAnnotationTest.php +++ b/tests/MagicMethodAnnotationTest.php @@ -150,6 +150,54 @@ class Child {} $this->analyzeFile('somefile.php', $context); } + public function testAnnotationWithoutCallConfigWithExtends(): void + { + $this->expectExceptionMessage('UndefinedMethod'); + $this->expectException(CodeException::class); + Config::getInstance()->use_phpdoc_method_without_magic_or_parent = false; + + $this->addFile( + 'somefile.php', + 'getString();', + ); + + $context = new Context(); + + $this->analyzeFile('somefile.php', $context); + } + + public function testAnnotationWithoutCallConfigWithExtendsWithStatic(): void + { + $this->expectExceptionMessage('UndefinedMethod'); + $this->expectException(CodeException::class); + Config::getInstance()->use_phpdoc_method_without_magic_or_parent = false; + + $this->addFile( + 'somefile.php', + 'analyzeFile('somefile.php', $context); + } + public function testOverrideParentClassRetunType(): void { Config::getInstance()->use_phpdoc_method_without_magic_or_parent = true; From 14316f55bacc6642b1161628f87efcc7fa86efcc Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Wed, 31 Jan 2024 19:01:14 +0000 Subject: [PATCH 09/59] Resolve testAnnotationWithoutCallConfigWithExtendsWithStatic Resolved error: ``` MagicMethodAnnotationTest::testAnnotationWithoutCallConfigWithExtendsWithStatic Failed asserting that exception of type "Psalm\Exception\CodeException" is thrown. ``` Appended problem: ``` MagicMethodAnnotationTest::testValidCode with data set "magicStaticMethodInheritanceWithoutCallStatic" Psalm\Exception\CodeException: UndefinedMethod - src/somefile.php:9:32 - Method B::bar does not exist ``` --- .../Call/StaticMethod/AtomicStaticCallAnalyzer.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) 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 7d14b2c5eba..7d2667f93af 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php @@ -564,7 +564,8 @@ private static function handleNamedCall( $callstatic_method_exists = $codebase->methods->methodExists($callstatic_id); - $with_pseudo = $callstatic_method_exists; + $with_pseudo = $callstatic_method_exists + || $codebase->config->use_phpdoc_method_without_magic_or_parent; if (!$naive_method_exists || !MethodAnalyzer::isMethodVisible( @@ -573,8 +574,7 @@ private static function handleNamedCall( $statements_analyzer->getSource(), ) || $fake_method_exists - || ($found_method_and_class_storage - && ($config->use_phpdoc_method_without_magic_or_parent || $class_storage->parent_class)) + || $found_method_and_class_storage ) { if ($callstatic_method_exists) { $callstatic_declaring_id = $codebase->methods->getDeclaringMethodId($callstatic_id); @@ -677,9 +677,7 @@ private static function handleNamedCall( return false; } } - } elseif ($found_method_and_class_storage - && ($config->use_phpdoc_method_without_magic_or_parent || $class_storage->parent_class) - ) { + } elseif ($found_method_and_class_storage) { [$pseudo_method_storage, $defining_class_storage] = $found_method_and_class_storage; if (self::checkPseudoMethod( @@ -696,7 +694,9 @@ private static function handleNamedCall( return false; } - if ($pseudo_method_storage->return_type) { + if ($pseudo_method_storage->return_type + && ($naive_method_exists || $with_pseudo) + ) { return true; } } elseif ($stmt->class instanceof PhpParser\Node\Name && $stmt->class->getFirst() === 'parent' From 60096315671a71da8e995101db19946424dc078a Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Sat, 10 Feb 2024 14:55:57 +0000 Subject: [PATCH 10/59] Fix invalid test This test has been failed with an error "UndefinedMethod". This error is being issued correctly. But so that this error would not interfere with the test, it was suppressed. Fixed problem: ``` MagicMethodAnnotationTest::testValidCode with data set "magicStaticMethodInheritanceWithoutCallStatic" Psalm\Exception\CodeException: UndefinedMethod - src/somefile.php:9:32 - Method B::bar does not exist ``` --- tests/MagicMethodAnnotationTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/MagicMethodAnnotationTest.php b/tests/MagicMethodAnnotationTest.php index ca0b28a0bcf..c5ddffade3d 100644 --- a/tests/MagicMethodAnnotationTest.php +++ b/tests/MagicMethodAnnotationTest.php @@ -958,6 +958,7 @@ class A {} class B extends A {} function consumeInt(int $i): void {} + /** @psalm-suppress UndefinedMethod */ consumeInt(B::bar());', ], 'callUsingParent' => [ From 6f17469b240eefab6b242f2dba2b872b5d2a95f4 Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Sat, 10 Feb 2024 16:03:25 +0000 Subject: [PATCH 11/59] Failed tests for StaticInvocation and NonStaticSelfCall Failed tests: ``` 1) MagicMethodAnnotationTest::testInvalidCode with data set "staticInvocationWithInstanceMethodFoo" Failed asserting that exception of type "Psalm\Exception\CodeException" is thrown. 2) MagicMethodAnnotationTest::testInvalidCode with data set "nonStaticSelfCallWithInstanceMethodFoo" Failed asserting that exception of type "Psalm\Exception\CodeException" is thrown. ``` --- tests/MagicMethodAnnotationTest.php | 62 +++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tests/MagicMethodAnnotationTest.php b/tests/MagicMethodAnnotationTest.php index c5ddffade3d..1ec6b125d78 100644 --- a/tests/MagicMethodAnnotationTest.php +++ b/tests/MagicMethodAnnotationTest.php @@ -1303,6 +1303,68 @@ public function baz(): Foo }', 'error_message' => 'UndefinedVariable', ], + 'staticInvocationWithMagicMethodFoo' => [ + 'code' => ' 'InvalidStaticInvocation', + ], + 'nonStaticSelfCallWithMagicMethodFoo' => [ + 'code' => ' 'NonStaticSelfCall', + ], + 'staticInvocationWithInstanceMethodFoo' => [ + 'code' => ' 'InvalidStaticInvocation', + ], + 'nonStaticSelfCallWithInstanceMethodFoo' => [ + 'code' => ' 'NonStaticSelfCall', + ], ]; } From 4c645e186ccfa760ce18af8661186332d853b3bc Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Wed, 7 Feb 2024 16:04:52 +0000 Subject: [PATCH 12/59] Resolve tests for StaticInvocation and NonStaticSelfCall The code has been moved from down to up. Resolved problems: ``` 1) MagicMethodAnnotationTest::testInvalidCode with data set "staticInvocationWithInstanceMethodFoo" Failed asserting that exception of type "Psalm\Exception\CodeException" is thrown. 2) MagicMethodAnnotationTest::testInvalidCode with data set "nonStaticSelfCallWithInstanceMethodFoo" Failed asserting that exception of type "Psalm\Exception\CodeException" is thrown. ``` --- .../StaticMethod/AtomicStaticCallAnalyzer.php | 64 ++++++++++--------- 1 file changed, 33 insertions(+), 31 deletions(-) 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 7d2667f93af..1b53bc578a8 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php @@ -567,6 +567,39 @@ private static function handleNamedCall( $with_pseudo = $callstatic_method_exists || $codebase->config->use_phpdoc_method_without_magic_or_parent; + if ($codebase->methods->getDeclaringMethodId($method_id, $with_pseudo)) { + if ((!$stmt->class instanceof PhpParser\Node\Name + || $stmt->class->getFirst() !== 'parent' + || $statements_analyzer->isStatic()) + && ( + !$context->self + || $statements_analyzer->isStatic() + || !$codebase->classExtends($context->self, $fq_class_name) + ) + ) { + MethodAnalyzer::checkStatic( + $method_id, + ($stmt->class instanceof PhpParser\Node\Name + && strtolower($stmt->class->getFirst()) === 'self') + || $context->self === $fq_class_name, + !$statements_analyzer->isStatic(), + $codebase, + new CodeLocation($statements_analyzer, $stmt), + $statements_analyzer->getSuppressedIssues(), + $is_dynamic_this_method, + ); + + if ($is_dynamic_this_method) { + return self::forwardCallToInstanceMethod( + $statements_analyzer, + $stmt, + $stmt_name, + $context, + ); + } + } + } + if (!$naive_method_exists || !MethodAnalyzer::isMethodVisible( $method_id, @@ -831,37 +864,6 @@ private static function handleNamedCall( return false; } - if ((!$stmt->class instanceof PhpParser\Node\Name - || $stmt->class->getFirst() !== 'parent' - || $statements_analyzer->isStatic()) - && ( - !$context->self - || $statements_analyzer->isStatic() - || !$codebase->classExtends($context->self, $fq_class_name) - ) - ) { - MethodAnalyzer::checkStatic( - $method_id, - ($stmt->class instanceof PhpParser\Node\Name - && strtolower($stmt->class->getFirst()) === 'self') - || $context->self === $fq_class_name, - !$statements_analyzer->isStatic(), - $codebase, - new CodeLocation($statements_analyzer, $stmt), - $statements_analyzer->getSuppressedIssues(), - $is_dynamic_this_method, - ); - - if ($is_dynamic_this_method) { - return self::forwardCallToInstanceMethod( - $statements_analyzer, - $stmt, - $stmt_name, - $context, - ); - } - } - $has_existing_method = true; ExistingAtomicStaticCallAnalyzer::analyze( From d588b3d251731f10c7140bf0dafd01758f8652da Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Tue, 13 Feb 2024 08:38:13 +0000 Subject: [PATCH 13/59] Support for testing with the creation of a list of issues Description of motivation. We currently have two different behaviors for the code related to "CodeException": ``` $codebase->config->throw_exception = true; // or false ``` If "throw_exception" is set to `true`, code execution stops. If "throw_exception" is set to `false`, the code may continue to execute, and an error may potentially occur. This commit allows testing for the second case, when the value of "throw_exception" will be "false". --- tests/TestCase.php | 28 +++++ ...InvalidCodeAnalysisWithIssuesTestTrait.php | 112 ++++++++++++++++++ 2 files changed, 140 insertions(+) create mode 100644 tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php diff --git a/tests/TestCase.php b/tests/TestCase.php index 39f02d46703..d30afd7d966 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -23,8 +23,11 @@ use function define; use function defined; use function getcwd; +use function implode; use function ini_set; use function is_string; +use function preg_match; +use function preg_quote; use const ARRAY_FILTER_USE_KEY; use const DIRECTORY_SEPARATOR; @@ -155,6 +158,31 @@ protected function getTestName(bool $withDataSet = true): string return $this->getName($withDataSet); } + public static function assertHasIssueType(string $expected, string $message = ''): void + { + $issueMessages = []; + $res = false; + $issues = IssueBuffer::getIssuesData(); + foreach ($issues as $file_issues) { + foreach ($file_issues as $issue) { + $fullIssueMessage = $issue->type . ' - ' . $issue->file_name . ':' . $issue->line_from . ':' . $issue->column_from . ' - ' . $issue->message; + $issueMessages[] = $fullIssueMessage; + if (preg_match('/\b' . preg_quote($expected, '/') . '\b/', $fullIssueMessage)) { + $res = true; + } + } + } + if (!$message) { + $message = "Failed asserting that issue with \"$expected\"."; + if (count($issueMessages)) { + $message .= "\n" . 'Other exists issues:' . "\n - " . implode("\n - ", $issueMessages); + } else { + $message .= ' Issues is not exists.'; + } + } + self::assertTrue($res, $message); + } + public static function assertArrayKeysAreStrings(array $array, string $message = ''): void { $validKeys = array_filter($array, 'is_string', ARRAY_FILTER_USE_KEY); diff --git a/tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php b/tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php new file mode 100644 index 00000000000..ae7c00b624e --- /dev/null +++ b/tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php @@ -0,0 +1,112 @@ +config->throw_exception = true; // or false + * ``` + * + * If "throw_exception" is set to `true`, code execution stops. + * + * If "throw_exception" is set to `false`, the code may continue + * to execute, and an error may potentially occur. + * + * This commit allows testing for the second case, when the value of + * "throw_exception" will be "false". + * + * @psalm-type DeprecatedDataProviderArrayNotation = array{ + * code: string, + * error_message: string, + * ignored_issues?: list, + * php_version?: string + * } + * @psalm-type NamedArgumentsDataProviderArrayNotation = array{ + * code: string, + * error_message: string, + * error_levels?: list, + * php_version?: string + * } + */ +trait InvalidCodeAnalysisWithIssuesTestTrait +{ + /** + * @return iterable< + * string, + * DeprecatedDataProviderArrayNotation|NamedArgumentsDataProviderArrayNotation + * > + */ + abstract public function providerInvalidCodeParse(): iterable; + + /** + * @dataProvider providerInvalidCodeParse + * @small + * @param list $error_levels + */ + public function testInvalidCodeWithIssues( + string $code, + string $error_message, + array $error_levels = [], + string $php_version = '7.4' + ): void { + $test_name = $this->getTestName(); + if (strpos($test_name, 'PHP80-') !== false) { + if (version_compare(PHP_VERSION, '8.0.0', '<')) { + $this->markTestSkipped('Test case requires PHP 8.0.'); + } + } elseif (strpos($test_name, 'SKIPPED-') !== false) { + $this->markTestSkipped('Skipped due to a bug.'); + } + + // sanity check - do we have a PHP tag? + if (strpos($code, 'fail('Test case must have a setCustomErrorLevel($issue_name, $error_level); + } + + $this->project_analyzer->setPhpVersion($php_version, 'tests'); + + $file_path = self::$src_dir_path . 'somefile.php'; + + $codebase = $this->project_analyzer->getCodebase(); + $codebase->enterServerMode(); + $codebase->config->visitPreloadedStubFiles($codebase); + + $codebase->config->throw_exception = false; + + $this->addFile($file_path, $code); + $this->analyzeFile($file_path, new Context()); + + $this->assertHasIssueType($error_message); + } +} From 9b6ef8beef7d166fb2002b223ed0b9658453e3d5 Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Tue, 13 Feb 2024 14:21:57 +0000 Subject: [PATCH 14/59] Failed and regression tests with creation of a list of issues Failed tests: ``` 1) Psalm\Tests\MagicMethodAnnotationTest::testInvalidCodeWithIssues with data set "staticInvocationWithMagicMethodFoo" UnexpectedValueException: Cannot get method params for A::foo 2) Psalm\Tests\MagicMethodAnnotationTest::testInvalidCodeWithIssues with data set "nonStaticSelfCallWithMagicMethodFoo" UnexpectedValueException: Cannot get method params for B::foo ``` --- tests/MagicMethodAnnotationTest.php | 2 ++ tests/MagicPropertyTest.php | 2 ++ tests/MethodCallTest.php | 2 ++ tests/MethodSignatureTest.php | 2 ++ tests/MixinAnnotationTest.php | 2 ++ 5 files changed, 10 insertions(+) diff --git a/tests/MagicMethodAnnotationTest.php b/tests/MagicMethodAnnotationTest.php index 1ec6b125d78..109dc08fd28 100644 --- a/tests/MagicMethodAnnotationTest.php +++ b/tests/MagicMethodAnnotationTest.php @@ -6,12 +6,14 @@ use Psalm\Context; use Psalm\Exception\CodeException; use Psalm\Tests\Traits\InvalidCodeAnalysisTestTrait; +use Psalm\Tests\Traits\InvalidCodeAnalysisWithIssuesTestTrait; use Psalm\Tests\Traits\ValidCodeAnalysisTestTrait; use const DIRECTORY_SEPARATOR; class MagicMethodAnnotationTest extends TestCase { + use InvalidCodeAnalysisWithIssuesTestTrait; use InvalidCodeAnalysisTestTrait; use ValidCodeAnalysisTestTrait; diff --git a/tests/MagicPropertyTest.php b/tests/MagicPropertyTest.php index 27b5b36a5ea..bc1e89faffe 100644 --- a/tests/MagicPropertyTest.php +++ b/tests/MagicPropertyTest.php @@ -6,12 +6,14 @@ use Psalm\Context; use Psalm\Exception\CodeException; use Psalm\Tests\Traits\InvalidCodeAnalysisTestTrait; +use Psalm\Tests\Traits\InvalidCodeAnalysisWithIssuesTestTrait; use Psalm\Tests\Traits\ValidCodeAnalysisTestTrait; use const DIRECTORY_SEPARATOR; class MagicPropertyTest extends TestCase { + use InvalidCodeAnalysisWithIssuesTestTrait; use InvalidCodeAnalysisTestTrait; use ValidCodeAnalysisTestTrait; diff --git a/tests/MethodCallTest.php b/tests/MethodCallTest.php index def613cc537..3426aa38a55 100644 --- a/tests/MethodCallTest.php +++ b/tests/MethodCallTest.php @@ -4,12 +4,14 @@ use Psalm\Context; use Psalm\Tests\Traits\InvalidCodeAnalysisTestTrait; +use Psalm\Tests\Traits\InvalidCodeAnalysisWithIssuesTestTrait; use Psalm\Tests\Traits\ValidCodeAnalysisTestTrait; use const DIRECTORY_SEPARATOR; class MethodCallTest extends TestCase { + use InvalidCodeAnalysisWithIssuesTestTrait; use InvalidCodeAnalysisTestTrait; use ValidCodeAnalysisTestTrait; diff --git a/tests/MethodSignatureTest.php b/tests/MethodSignatureTest.php index c47fd247020..69315f41797 100644 --- a/tests/MethodSignatureTest.php +++ b/tests/MethodSignatureTest.php @@ -5,12 +5,14 @@ use Psalm\Context; use Psalm\Exception\CodeException; use Psalm\Tests\Traits\InvalidCodeAnalysisTestTrait; +use Psalm\Tests\Traits\InvalidCodeAnalysisWithIssuesTestTrait; use Psalm\Tests\Traits\ValidCodeAnalysisTestTrait; use const DIRECTORY_SEPARATOR; class MethodSignatureTest extends TestCase { + use InvalidCodeAnalysisWithIssuesTestTrait; use ValidCodeAnalysisTestTrait; use InvalidCodeAnalysisTestTrait; diff --git a/tests/MixinAnnotationTest.php b/tests/MixinAnnotationTest.php index 1769546aab2..88529223bd8 100644 --- a/tests/MixinAnnotationTest.php +++ b/tests/MixinAnnotationTest.php @@ -3,10 +3,12 @@ namespace Psalm\Tests; use Psalm\Tests\Traits\InvalidCodeAnalysisTestTrait; +use Psalm\Tests\Traits\InvalidCodeAnalysisWithIssuesTestTrait; use Psalm\Tests\Traits\ValidCodeAnalysisTestTrait; class MixinAnnotationTest extends TestCase { + use InvalidCodeAnalysisWithIssuesTestTrait; use ValidCodeAnalysisTestTrait; use InvalidCodeAnalysisTestTrait; From e940de5edd90e318310e08f9d2f93c381148c6ec Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Thu, 1 Feb 2024 12:59:21 +0000 Subject: [PATCH 15/59] Resolve tests with creation of a list of issues Resolved problems: ``` 1) Psalm\Tests\MagicMethodAnnotationTest::testInvalidCodeWithIssues with data set "staticInvocationWithMagicMethodFoo" UnexpectedValueException: Cannot get method params for A::foo 2) Psalm\Tests\MagicMethodAnnotationTest::testInvalidCodeWithIssues with data set "nonStaticSelfCallWithMagicMethodFoo" UnexpectedValueException: Cannot get method params for B::foo ``` --- src/Psalm/Internal/Codebase/Methods.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Psalm/Internal/Codebase/Methods.php b/src/Psalm/Internal/Codebase/Methods.php index bc9abe6572f..3d9f691f4df 100644 --- a/src/Psalm/Internal/Codebase/Methods.php +++ b/src/Psalm/Internal/Codebase/Methods.php @@ -368,7 +368,7 @@ public function getMethodParams( } } - $declaring_method_id = $this->getDeclaringMethodId($method_id); + $declaring_method_id = $this->getDeclaringMethodId($method_id, true); $callmap_id = $declaring_method_id ?? $method_id; @@ -424,7 +424,7 @@ public function getMethodParams( } if ($declaring_method_id) { - $storage = $this->getStorage($declaring_method_id); + $storage = $this->getStorage($declaring_method_id, true); $params = $storage->params; @@ -1088,7 +1088,7 @@ public function getCasedMethodId(MethodIdentifier $original_method_id): string public function getUserMethodStorage(MethodIdentifier $method_id): ?MethodStorage { - $declaring_method_id = $this->getDeclaringMethodId($method_id); + $declaring_method_id = $this->getDeclaringMethodId($method_id, true); if (!$declaring_method_id) { if (InternalCallMapHandler::inCallMap((string) $method_id)) { @@ -1098,7 +1098,7 @@ public function getUserMethodStorage(MethodIdentifier $method_id): ?MethodStorag throw new UnexpectedValueException('$storage should not be null for ' . $method_id); } - $storage = $this->getStorage($declaring_method_id); + $storage = $this->getStorage($declaring_method_id, true); if (!$storage->location) { return null; From db929912feff0fff3749d746f8cb6fba5b65d3f8 Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Wed, 14 Feb 2024 09:57:24 +0300 Subject: [PATCH 16/59] Fix message in TestCase::assertHasIssueType Apply suggestions from code review. Co-authored-by: Bruce Weirdan --- tests/TestCase.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/TestCase.php b/tests/TestCase.php index d30afd7d966..b06e3259d8b 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -173,11 +173,11 @@ public static function assertHasIssueType(string $expected, string $message = '' } } if (!$message) { - $message = "Failed asserting that issue with \"$expected\"."; + $message = "Failed asserting that issue with \"$expected\" was emitted."; if (count($issueMessages)) { - $message .= "\n" . 'Other exists issues:' . "\n - " . implode("\n - ", $issueMessages); + $message .= "\n" . 'Other issues reported:' . "\n - " . implode("\n - ", $issueMessages); } else { - $message .= ' Issues is not exists.'; + $message .= ' No issues reported.'; } } self::assertTrue($res, $message); From cfd0fd155471ef229664a6835f0cb30ea5f06c81 Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Wed, 14 Feb 2024 15:26:42 +0000 Subject: [PATCH 17/59] Use snake_case in TestCase::assertHasIssueType --- tests/TestCase.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/TestCase.php b/tests/TestCase.php index b06e3259d8b..67249585086 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -160,22 +160,22 @@ protected function getTestName(bool $withDataSet = true): string public static function assertHasIssueType(string $expected, string $message = ''): void { - $issueMessages = []; + $issue_messages = []; $res = false; $issues = IssueBuffer::getIssuesData(); foreach ($issues as $file_issues) { foreach ($file_issues as $issue) { - $fullIssueMessage = $issue->type . ' - ' . $issue->file_name . ':' . $issue->line_from . ':' . $issue->column_from . ' - ' . $issue->message; - $issueMessages[] = $fullIssueMessage; - if (preg_match('/\b' . preg_quote($expected, '/') . '\b/', $fullIssueMessage)) { + $full_issue_message = $issue->type . ' - ' . $issue->file_name . ':' . $issue->line_from . ':' . $issue->column_from . ' - ' . $issue->message; + $issue_messages[] = $full_issue_message; + if (preg_match('/\b' . preg_quote($expected, '/') . '\b/', $full_issue_message)) { $res = true; } } } if (!$message) { $message = "Failed asserting that issue with \"$expected\" was emitted."; - if (count($issueMessages)) { - $message .= "\n" . 'Other issues reported:' . "\n - " . implode("\n - ", $issueMessages); + if (count($issue_messages)) { + $message .= "\n" . 'Other issues reported:' . "\n - " . implode("\n - ", $issue_messages); } else { $message .= ' No issues reported.'; } From 08a479aede2d774b2f902bbe6743f307e83f978a Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Wed, 14 Feb 2024 15:42:44 +0000 Subject: [PATCH 18/59] Rename to TestCase::assertHasIssue --- tests/TestCase.php | 2 +- tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/TestCase.php b/tests/TestCase.php index 67249585086..b85a3c5f204 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -158,7 +158,7 @@ protected function getTestName(bool $withDataSet = true): string return $this->getName($withDataSet); } - public static function assertHasIssueType(string $expected, string $message = ''): void + public static function assertHasIssue(string $expected, string $message = ''): void { $issue_messages = []; $res = false; diff --git a/tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php b/tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php index ae7c00b624e..94eab5e616b 100644 --- a/tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php +++ b/tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php @@ -107,6 +107,6 @@ public function testInvalidCodeWithIssues( $this->addFile($file_path, $code); $this->analyzeFile($file_path, new Context()); - $this->assertHasIssueType($error_message); + $this->assertHasIssue($error_message); } } From b7a20802daba7105aa2d881f8aedefde4b8df8dd Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Wed, 14 Feb 2024 15:45:04 +0000 Subject: [PATCH 19/59] Fix description in InvalidCodeAnalysisWithIssuesTestTrait --- tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php b/tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php index 94eab5e616b..f77f79a67a0 100644 --- a/tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php +++ b/tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php @@ -32,7 +32,7 @@ * If "throw_exception" is set to `false`, the code may continue * to execute, and an error may potentially occur. * - * This commit allows testing for the second case, when the value of + * This is trait allows testing for the second case, when the value of * "throw_exception" will be "false". * * @psalm-type DeprecatedDataProviderArrayNotation = array{ From 37240624f6b8771c8b0e4f5733486ff0f72680be Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Thu, 15 Feb 2024 00:10:54 +0300 Subject: [PATCH 20/59] Fix description InvalidCodeAnalysisWithIssuesTestTrait Co-authored-by: Bruce Weirdan --- tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php b/tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php index f77f79a67a0..cfe4dba87b0 100644 --- a/tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php +++ b/tests/Traits/InvalidCodeAnalysisWithIssuesTestTrait.php @@ -27,10 +27,12 @@ * $codebase->config->throw_exception = true; // or false * ``` * - * If "throw_exception" is set to `true`, code execution stops. + * When `throw_exception` is set to `true`, code execution stops once + * the first issue is emitted, thus it may mask any problems after + * that point. * - * If "throw_exception" is set to `false`, the code may continue - * to execute, and an error may potentially occur. + * When `throw_exception` is set to `false`, the code will continue + * to be executed and we can uncover some additional bugs. * * This is trait allows testing for the second case, when the value of * "throw_exception" will be "false". From 5a66742e707fa220aea8dfeae7670d226c57879f Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Wed, 14 Feb 2024 16:51:31 +0000 Subject: [PATCH 21/59] Failed and regression tests with suppression "UndefinedMethod" Apply suggestions from code review. Failed tests: ``` 1) MagicMethodAnnotationTest::testValidCode with data set "magicStaticMethodInheritanceWithoutCallStatic" Psalm\Exception\CodeException: UnusedPsalmSuppress - src/somefile.php:9:58 - This suppression is never used 2) MagicMethodAnnotationTest::testValidCode with data set "magicStaticMethodInheritanceWithoutCallStatic_WithReturnAndManyArgs" Psalm\Exception\CodeException: TooManyArguments - src/somefile.php:9:6 - Too many arguments for B::bar - expecting 0 but saw 2 ``` Co-authored-by: Bruce Weirdan --- tests/MagicMethodAnnotationTest.php | 63 ++++++++++++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/tests/MagicMethodAnnotationTest.php b/tests/MagicMethodAnnotationTest.php index 109dc08fd28..cd091a26336 100644 --- a/tests/MagicMethodAnnotationTest.php +++ b/tests/MagicMethodAnnotationTest.php @@ -960,9 +960,44 @@ class A {} class B extends A {} function consumeInt(int $i): void {} - /** @psalm-suppress UndefinedMethod */ + /** @psalm-suppress UndefinedMethod, MixedArgument */ consumeInt(B::bar());', ], + 'magicStaticMethodInheritanceWithoutCallStatic_WithReturnAndManyArgs' => [ + // This is compatible with "magicMethodInheritanceWithoutCall_WithReturnAndManyArgs" + 'code' => <<<'PHP' + [ + '$a===' => 'mixed', + ], + ], + 'magicMethodInheritanceWithoutCall_WithReturnAndManyArgs' => [ + 'code' => <<<'PHP' + bar(123, "whatever"); + PHP, + 'assertions' => [ + '$a===' => 'mixed', + ], + ], 'callUsingParent' => [ 'code' => ' 'NonStaticSelfCall', ], + 'suppressUndefinedMethodWithObjectCall_WithNotExistsFunc' => [ + 'code' => <<<'PHP' + bar(function_does_not_exist(123)); + PHP, + 'error_message' => 'UndefinedFunction', + ], + 'suppressUndefinedMethodWithStaticCall_WithNotExistsFunc' => [ + 'code' => <<<'PHP' + 'UndefinedFunction', + ], ]; } From 8a70bc250685b747e8b2a15ae2206e8221a68bee Mon Sep 17 00:00:00 2001 From: Ivan Sidorov Date: Wed, 14 Feb 2024 20:40:35 +0000 Subject: [PATCH 22/59] Resolve tests with suppression "UndefinedMethod" Apply suggestions from code review. Resolved problems: ``` 1) MagicMethodAnnotationTest::testValidCode with data set "magicStaticMethodInheritanceWithoutCallStatic" Psalm\Exception\CodeException: UnusedPsalmSuppress - src/somefile.php:9:58 - This suppression is never used 2) MagicMethodAnnotationTest::testValidCode with data set "magicStaticMethodInheritanceWithoutCallStatic_WithReturnAndManyArgs" Psalm\Exception\CodeException: TooManyArguments - src/somefile.php:9:6 - Too many arguments for B::bar - expecting 0 but saw 2 ``` --- .../Call/StaticMethod/AtomicStaticCallAnalyzer.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) 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 1b53bc578a8..5056d25b1f9 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php @@ -710,7 +710,7 @@ private static function handleNamedCall( return false; } } - } elseif ($found_method_and_class_storage) { + } elseif ($found_method_and_class_storage && ($naive_method_exists || $with_pseudo)) { [$pseudo_method_storage, $defining_class_storage] = $found_method_and_class_storage; if (self::checkPseudoMethod( @@ -727,9 +727,7 @@ private static function handleNamedCall( return false; } - if ($pseudo_method_storage->return_type - && ($naive_method_exists || $with_pseudo) - ) { + if ($pseudo_method_storage->return_type) { return true; } } elseif ($stmt->class instanceof PhpParser\Node\Name && $stmt->class->getFirst() === 'parent' From b238bc7dc9ebe52c7d1f2a21cbeeaaf7abe1728e Mon Sep 17 00:00:00 2001 From: Daniil Gentili Date: Thu, 15 Feb 2024 11:51:41 +0100 Subject: [PATCH 23/59] Improve randomizer stubs --- stubs/extensions/random.phpstub | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/stubs/extensions/random.phpstub b/stubs/extensions/random.phpstub index 01cc8fb8028..27135afc376 100644 --- a/stubs/extensions/random.phpstub +++ b/stubs/extensions/random.phpstub @@ -87,10 +87,20 @@ namespace Random */ public function getBytes(int $length): string {} + /** + * @template TValue + * @param array $array + * @return list + */ public function shuffleArray(array $array): array {} public function shuffleBytes(string $bytes): string {} + /** + * @template TKey as array-key + * @param array $array + * @return list + */ public function pickArrayKeys(array $array, int $num): array {} public function __serialize(): array {} From f035c00a21c6dfb3d6887b56d1da524c655dd19d Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Mon, 19 Feb 2024 11:32:52 +0100 Subject: [PATCH 24/59] Fix non-empty-lowercase-string handling with literal non-lowercase strings * Fix https://github.com/vimeo/psalm/issues/9782 and related issues * add explicit handling for non-falsy-string to not fallback non-falsy-string and 0 to string --- src/Psalm/Internal/Type/TypeCombiner.php | 37 +++++++++++++++++++++++- tests/TypeCombinationTest.php | 34 ++++++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/src/Psalm/Internal/Type/TypeCombiner.php b/src/Psalm/Internal/Type/TypeCombiner.php index e21d41b0559..0a084e8a62f 100644 --- a/src/Psalm/Internal/Type/TypeCombiner.php +++ b/src/Psalm/Internal/Type/TypeCombiner.php @@ -1103,18 +1103,53 @@ private static function scrapeStringProperties( } else { $combination->value_types['string'] = $type; } + } elseif ($type instanceof TNonFalsyString) { + $has_empty_string = false; + $has_falsy_string = false; + + foreach ($combination->strings as $string_type) { + if ($string_type->value === '') { + $has_empty_string = true; + $has_falsy_string = true; + break; + } + + if ($string_type->value === '0') { + $has_falsy_string = true; + } + } + + if ($has_empty_string) { + $combination->value_types['string'] = new TString(); + } elseif ($has_falsy_string) { + $combination->value_types['string'] = new TNonEmptyString(); + } else { + $combination->value_types['string'] = $type; + } } elseif ($type instanceof TNonEmptyString) { $has_empty_string = false; foreach ($combination->strings as $string_type) { - if (!$string_type->value) { + if ($string_type->value === '') { $has_empty_string = true; break; } } + $has_non_lowercase_string = false; + if ($type instanceof TNonEmptyLowercaseString) { + foreach ($combination->strings as $string_type) { + if (strtolower($string_type->value) !== $string_type->value) { + $has_non_lowercase_string = true; + break; + } + } + } + if ($has_empty_string) { $combination->value_types['string'] = new TString(); + } elseif ($has_non_lowercase_string && get_class($type) !== TNonEmptyString::class) { + $combination->value_types['string'] = new TNonEmptyString(); } else { $combination->value_types['string'] = $type; } diff --git a/tests/TypeCombinationTest.php b/tests/TypeCombinationTest.php index 56af82a69ed..ba3ded5e83d 100644 --- a/tests/TypeCombinationTest.php +++ b/tests/TypeCombinationTest.php @@ -127,6 +127,40 @@ function takesLiteralString($arg) {} '$x===' => 'non-falsy-string', ], ], + 'loopNonFalsyWithZeroShouldBeNonEmpty' => [ + 'code' => ' [ + '$x===' => 'list', + ], + ], + 'loopNonLowercaseLiteralWithNonEmptyLowercaseShouldBeNonEmptyAndNotLowercase' => [ + 'code' => ' [ + '$x===' => 'list', + ], + ], ]; } From 4ac18720aab9dda3c3fc3d5a2d55d8ce2fdf5150 Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Mon, 19 Feb 2024 11:41:29 +0100 Subject: [PATCH 25/59] Revert https://github.com/vimeo/psalm/pull/10039 and fix type and test --- src/Psalm/Internal/Type/TypeCombiner.php | 15 +++++++++++---- tests/TypeCombinationTest.php | 2 +- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/Psalm/Internal/Type/TypeCombiner.php b/src/Psalm/Internal/Type/TypeCombiner.php index 0a084e8a62f..099ae82c7fc 100644 --- a/src/Psalm/Internal/Type/TypeCombiner.php +++ b/src/Psalm/Internal/Type/TypeCombiner.php @@ -1047,12 +1047,19 @@ private static function scrapeStringProperties( && strtolower($type->value) === $type->value ) { // do nothing + } elseif (isset($combination->value_types['string']) + && $combination->value_types['string'] instanceof TNonFalsyString + && $type->value + ) { + // do nothing + } elseif (isset($combination->value_types['string']) + && $combination->value_types['string'] instanceof TNonFalsyString + && $type->value === '0' + ) { + $combination->value_types['string'] = new TNonEmptyString(); } elseif (isset($combination->value_types['string']) && $combination->value_types['string'] instanceof TNonEmptyString - && ($combination->value_types['string'] instanceof TNonFalsyString - ? $type->value - : $type->value !== '' - ) + && $type->value !== '' ) { // do nothing } else { diff --git a/tests/TypeCombinationTest.php b/tests/TypeCombinationTest.php index ba3ded5e83d..841fbd67e36 100644 --- a/tests/TypeCombinationTest.php +++ b/tests/TypeCombinationTest.php @@ -934,7 +934,7 @@ public function providerTestValidTypeCombination(): array ], ], 'nonFalsyStringAndFalsyLiteral' => [ - 'string', + 'non-empty-string', [ 'non-falsy-string', '"0"', From 407c8b3084825c13ed8373616ce7af73541dedf7 Mon Sep 17 00:00:00 2001 From: George Steel Date: Mon, 19 Feb 2024 17:59:38 +0000 Subject: [PATCH 26/59] Update PHP 8.2 stubs to include `SensitiveParameterValue` Signed-off-by: George Steel --- stubs/Php82.phpstub | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/stubs/Php82.phpstub b/stubs/Php82.phpstub index 81b99f91d1b..a2b9ee46150 100644 --- a/stubs/Php82.phpstub +++ b/stubs/Php82.phpstub @@ -42,4 +42,22 @@ namespace { * @psalm-flow ($string) -> return */ function str_split(string $string, int $length = 1) {} + + /** + * @psalm-immutable + * @template TValue + * + * @since 8.2.0 + */ + final class SensitiveParameterValue + { + /** @param TValue $value */ + public function __construct(private readonly mixed $value) {} + + /** @return array */ + public function __debugInfo(): array {} + + /** @return TValue */ + public function getValue(): mixed {} + } } From 2943cfe11dbcb5a59939f05b01bfee528e0e5abb Mon Sep 17 00:00:00 2001 From: Oliver Hader Date: Mon, 19 Feb 2024 23:55:01 +0100 Subject: [PATCH 27/59] Add list of statements to BeforeFileAnalysisEvent Fixes: #10725 --- src/Psalm/Internal/Analyzer/FileAnalyzer.php | 4 ++-- .../Event/BeforeFileAnalysisEvent.php | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/FileAnalyzer.php b/src/Psalm/Internal/Analyzer/FileAnalyzer.php index 80db22ed9d1..b69f01481d2 100644 --- a/src/Psalm/Internal/Analyzer/FileAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/FileAnalyzer.php @@ -150,9 +150,9 @@ public function analyze( return; } - $event = new BeforeFileAnalysisEvent($this, $this->context, $file_storage, $codebase); - + $event = new BeforeFileAnalysisEvent($this, $this->context, $file_storage, $codebase, $stmts); $codebase->config->eventDispatcher->dispatchBeforeFileAnalysis($event); + $stmts = $event->getStmts(); if ($codebase->alter_code) { foreach ($stmts as $stmt) { diff --git a/src/Psalm/Plugin/EventHandler/Event/BeforeFileAnalysisEvent.php b/src/Psalm/Plugin/EventHandler/Event/BeforeFileAnalysisEvent.php index 249ab125b38..e04305afea3 100644 --- a/src/Psalm/Plugin/EventHandler/Event/BeforeFileAnalysisEvent.php +++ b/src/Psalm/Plugin/EventHandler/Event/BeforeFileAnalysisEvent.php @@ -2,6 +2,7 @@ namespace Psalm\Plugin\EventHandler\Event; +use PhpParser\Node\Stmt; use Psalm\Codebase; use Psalm\Context; use Psalm\StatementsSource; @@ -13,22 +14,29 @@ final class BeforeFileAnalysisEvent private Context $file_context; private FileStorage $file_storage; private Codebase $codebase; + /** + * @var list + */ + private array $stmts; /** * Called before a file has been checked * + * @param list $stmts * @internal */ public function __construct( StatementsSource $statements_source, Context $file_context, FileStorage $file_storage, - Codebase $codebase + Codebase $codebase, + array $stmts ) { $this->statements_source = $statements_source; $this->file_context = $file_context; $this->file_storage = $file_storage; $this->codebase = $codebase; + $this->stmts = $stmts; } public function getStatementsSource(): StatementsSource @@ -50,4 +58,12 @@ public function getCodebase(): Codebase { return $this->codebase; } + + /** + * @return list + */ + public function getStmts(): array + { + return $this->stmts; + } } From 1ff41e68a0a859d27c9bcae46f611f766216290b Mon Sep 17 00:00:00 2001 From: Oliver Hader Date: Tue, 20 Feb 2024 00:22:14 +0100 Subject: [PATCH 28/59] Drop superfluous fetching of statements from event --- src/Psalm/Internal/Analyzer/FileAnalyzer.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Psalm/Internal/Analyzer/FileAnalyzer.php b/src/Psalm/Internal/Analyzer/FileAnalyzer.php index b69f01481d2..a6dc95618b8 100644 --- a/src/Psalm/Internal/Analyzer/FileAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/FileAnalyzer.php @@ -152,7 +152,6 @@ public function analyze( $event = new BeforeFileAnalysisEvent($this, $this->context, $file_storage, $codebase, $stmts); $codebase->config->eventDispatcher->dispatchBeforeFileAnalysis($event); - $stmts = $event->getStmts(); if ($codebase->alter_code) { foreach ($stmts as $stmt) { From 679aaf09395212dd6ec18d24d810c506d4f0dc2a Mon Sep 17 00:00:00 2001 From: Oliver Hader Date: Tue, 20 Feb 2024 13:55:31 +0100 Subject: [PATCH 29/59] Revert "Allow tainted numerics except for 'html' and 'has_quotes'" --- .../Expression/BinaryOpAnalyzer.php | 17 ----- .../Expression/Call/ArgumentAnalyzer.php | 15 ++--- .../Statements/Expression/CastAnalyzer.php | 23 +++++-- tests/TaintTest.php | 66 +++++++++---------- 4 files changed, 57 insertions(+), 64 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php index 309d1b8d537..2b7b8b607ae 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php @@ -32,7 +32,6 @@ use Psalm\Type\Union; use UnexpectedValueException; -use function array_merge; use function in_array; use function strlen; @@ -171,14 +170,6 @@ public static function analyze( $removed_taints = $codebase->config->eventDispatcher->dispatchRemoveTaints($event); if ($stmt_left_type && $stmt_left_type->parent_nodes) { - // numeric types can't be tainted html or has_quotes, neither can bool - if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph - && $stmt_left_type->isSingle() - && ($stmt_left_type->isInt() || $stmt_left_type->isFloat() || $stmt_left_type->isBool()) - ) { - $removed_taints = array_merge($removed_taints, array('html', 'has_quotes')); - } - foreach ($stmt_left_type->parent_nodes as $parent_node) { $statements_analyzer->data_flow_graph->addPath( $parent_node, @@ -191,14 +182,6 @@ public static function analyze( } if ($stmt_right_type && $stmt_right_type->parent_nodes) { - // numeric types can't be tainted html or has_quotes, neither can bool - if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph - && $stmt_right_type->isSingle() - && ($stmt_right_type->isInt() || $stmt_right_type->isFloat() || $stmt_right_type->isBool()) - ) { - $removed_taints = array_merge($removed_taints, array('html', 'has_quotes')); - } - foreach ($stmt_right_type->parent_nodes as $parent_node) { $statements_analyzer->data_flow_graph->addPath( $parent_node, diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php index ec72396f28d..262935153d6 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php @@ -60,7 +60,6 @@ use Psalm\Type\Atomic\TNamedObject; use Psalm\Type\Union; -use function array_merge; use function count; use function explode; use function implode; @@ -1529,19 +1528,19 @@ private static function processTaintedness( return; } - $event = new AddRemoveTaintsEvent($expr, $context, $statements_analyzer, $codebase); - - $added_taints = $codebase->config->eventDispatcher->dispatchAddTaints($event); - $removed_taints = $codebase->config->eventDispatcher->dispatchRemoveTaints($event); - - // numeric types can't be tainted html or has_quotes, neither can bool + // numeric types can't be tainted, neither can bool if ($statements_analyzer->data_flow_graph instanceof TaintFlowGraph && $input_type->isSingle() && ($input_type->isInt() || $input_type->isFloat() || $input_type->isBool()) ) { - $removed_taints = array_merge($removed_taints, array('html', 'has_quotes')); + return; } + $event = new AddRemoveTaintsEvent($expr, $context, $statements_analyzer, $codebase); + + $added_taints = $codebase->config->eventDispatcher->dispatchAddTaints($event); + $removed_taints = $codebase->config->eventDispatcher->dispatchRemoveTaints($event); + if ($function_param->type && $function_param->type->isString() && !$input_type->isString()) { $input_type = CastAnalyzer::castStringAttempt( $statements_analyzer, diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php index bbcb0e3105a..5832bb159fe 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php @@ -142,9 +142,14 @@ public static function analyze( } } - $type = new Union([new TBool()], [ - 'parent_nodes' => $maybe_type->parent_nodes ?? [], - ]); + if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph + ) { + $type = new Union([new TBool()], [ + 'parent_nodes' => $maybe_type->parent_nodes ?? [], + ]); + } else { + $type = Type::getBool(); + } $statements_analyzer->node_data->setType($stmt, $type); @@ -323,7 +328,11 @@ public static function castIntAttempt( $atomic_types = $stmt_type->getAtomicTypes(); - $parent_nodes = $stmt_type->parent_nodes; + $parent_nodes = []; + + if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph) { + $parent_nodes = $stmt_type->parent_nodes; + } while ($atomic_types) { $atomic_type = array_pop($atomic_types); @@ -509,7 +518,11 @@ public static function castFloatAttempt( $atomic_types = $stmt_type->getAtomicTypes(); - $parent_nodes = $stmt_type->parent_nodes; + $parent_nodes = []; + + if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph) { + $parent_nodes = $stmt_type->parent_nodes; + } while ($atomic_types) { $atomic_type = array_pop($atomic_types); diff --git a/tests/TaintTest.php b/tests/TaintTest.php index 7a947ca8127..6439b366c1d 100644 --- a/tests/TaintTest.php +++ b/tests/TaintTest.php @@ -177,6 +177,23 @@ public function deleteUser(PDO $pdo, string $userId) : void { } }', ], + 'untaintedInputAfterIntCast' => [ + 'code' => 'getUserId(); + } + + public function deleteUser(PDO $pdo) : void { + $userId = $this->getAppendedUserId(); + $pdo->exec("delete from users where user_id = " . $userId); + } + }', + ], 'specializedCoreFunctionCall' => [ 'code' => ' [ + 'code' => ' [ 'code' => ' 'TaintedSql', ], - 'taintedInputAfterIntCast' => [ - 'code' => 'getUserId(); - } - - public function deleteUser(PDO $pdo) : void { - $userId = $this->getAppendedUserId(); - $pdo->exec("delete from users where user_id = " . $userId); - } - }', - 'error_message' => 'TaintedSql', - ], - 'TaintForIntTypeCastUsingAnnotatedSink' => [ - 'code' => ' 'TaintedSql', - ], 'taintedInputFromReturnTypeWithBranch' => [ 'code' => ' Date: Tue, 20 Feb 2024 22:15:11 +0100 Subject: [PATCH 30/59] Fix RiskyTruthyFalsyComparison reporting irrelevant errors when there is no explicit truthy/falsy type --- .../issues/RiskyTruthyFalsyComparison.md | 4 ++-- .../Block/IfConditionalAnalyzer.php | 4 +++- .../Expression/BooleanNotAnalyzer.php | 4 +++- .../Statements/Expression/EmptyAnalyzer.php | 4 +++- tests/TypeReconciliation/ConditionalTest.php | 19 +++++++++++++++++++ 5 files changed, 30 insertions(+), 5 deletions(-) diff --git a/docs/running_psalm/issues/RiskyTruthyFalsyComparison.md b/docs/running_psalm/issues/RiskyTruthyFalsyComparison.md index 8d60969633e..601b870c038 100644 --- a/docs/running_psalm/issues/RiskyTruthyFalsyComparison.md +++ b/docs/running_psalm/issues/RiskyTruthyFalsyComparison.md @@ -1,6 +1,6 @@ # RiskyTruthyFalsyComparison -Emitted when comparing a value with multiple types that can both contain truthy and falsy values. +Emitted when comparing a value with multiple types, where at least one type can be only truthy or falsy and other types can contain both truthy and falsy values. ```php getAtomicTypes()) > 1) { + $has_truthy_or_falsy_exclusive_type = false; $both_types = $type->getBuilder(); foreach ($both_types->getAtomicTypes() as $key => $atomic_type) { if ($atomic_type->isTruthy() || $atomic_type->isFalsy() || $atomic_type instanceof TBool) { $both_types->removeType($key); + $has_truthy_or_falsy_exclusive_type = true; } } - if (count($both_types->getAtomicTypes()) > 0) { + if (count($both_types->getAtomicTypes()) > 0 && $has_truthy_or_falsy_exclusive_type) { $both_types = $both_types->freeze(); IssueBuffer::maybeAdd( new RiskyTruthyFalsyComparison( diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BooleanNotAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BooleanNotAnalyzer.php index 93d17c3f7f5..9a2f36602c8 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BooleanNotAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BooleanNotAnalyzer.php @@ -46,16 +46,18 @@ public static function analyze( $stmt_type = new TTrue($expr_type->from_docblock); } else { if (count($expr_type->getAtomicTypes()) > 1) { + $has_truthy_or_falsy_exclusive_type = false; $both_types = $expr_type->getBuilder(); foreach ($both_types->getAtomicTypes() as $key => $atomic_type) { if ($atomic_type->isTruthy() || $atomic_type->isFalsy() || $atomic_type instanceof TBool) { $both_types->removeType($key); + $has_truthy_or_falsy_exclusive_type = true; } } - if (count($both_types->getAtomicTypes()) > 0) { + if (count($both_types->getAtomicTypes()) > 0 && $has_truthy_or_falsy_exclusive_type) { $both_types = $both_types->freeze(); IssueBuffer::maybeAdd( new RiskyTruthyFalsyComparison( diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/EmptyAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/EmptyAnalyzer.php index 8dbcca2f9bb..d96d5d9267a 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/EmptyAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/EmptyAnalyzer.php @@ -65,16 +65,18 @@ public static function analyze( $stmt_type = new TTrue($expr_type->from_docblock); } else { if (count($expr_type->getAtomicTypes()) > 1) { + $has_truthy_or_falsy_exclusive_type = false; $both_types = $expr_type->getBuilder(); foreach ($both_types->getAtomicTypes() as $key => $atomic_type) { if ($atomic_type->isTruthy() || $atomic_type->isFalsy() || $atomic_type instanceof TBool) { $both_types->removeType($key); + $has_truthy_or_falsy_exclusive_type = true; } } - if (count($both_types->getAtomicTypes()) > 0) { + if (count($both_types->getAtomicTypes()) > 0 && $has_truthy_or_falsy_exclusive_type) { $both_types = $both_types->freeze(); IssueBuffer::maybeAdd( new RiskyTruthyFalsyComparison( diff --git a/tests/TypeReconciliation/ConditionalTest.php b/tests/TypeReconciliation/ConditionalTest.php index 4d48add4f1d..66cdeb669b1 100644 --- a/tests/TypeReconciliation/ConditionalTest.php +++ b/tests/TypeReconciliation/ConditionalTest.php @@ -3153,6 +3153,25 @@ function foo( $a, $b ) { '$existing' => 'null|stdClass', ], ], + 'nonStrictConditionWithoutExclusiveTruthyFalsyFuncCallNegated' => [ + 'code' => ' [], + 'ignored_issues' => ['InvalidReturnType'], + ], ]; } From e482c078a7a9d490808d179fad72332e98585c0a Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Tue, 20 Feb 2024 22:16:27 +0100 Subject: [PATCH 31/59] code style whitespace fixes including for unrelated tests --- tests/TypeReconciliation/ConditionalTest.php | 112 +++++++++---------- 1 file changed, 56 insertions(+), 56 deletions(-) diff --git a/tests/TypeReconciliation/ConditionalTest.php b/tests/TypeReconciliation/ConditionalTest.php index 66cdeb669b1..321832df7c5 100644 --- a/tests/TypeReconciliation/ConditionalTest.php +++ b/tests/TypeReconciliation/ConditionalTest.php @@ -41,28 +41,28 @@ function foo($a): void { 'nonStrictConditionTruthyFalsyNoOverlap' => [ 'code' => ' [ 'code' => ' [ 'code' => ' 'RiskyTruthyFalsyComparison', ], 'nonStrictConditionTruthyFalsyNegated' => [ 'code' => ' 'RiskyTruthyFalsyComparison', ], 'nonStrictConditionTruthyFalsyFuncCall' => [ 'code' => ' 'RiskyTruthyFalsyComparison', ], 'nonStrictConditionTruthyFalsyFuncCallNegated' => [ 'code' => ' 'RiskyTruthyFalsyComparison', ], 'redundantConditionForNonEmptyString' => [ From 3cd9658d298bbefa8222dfe41440aa7dffbe7d7f Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Wed, 21 Feb 2024 12:25:30 +0100 Subject: [PATCH 32/59] Allow `Override` attribute to be used in pure contexts Fixes vimeo/psalm#10731 --- stubs/CoreGenericAttributes.phpstub | 3 +++ tests/OverrideTest.php | 17 +++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/stubs/CoreGenericAttributes.phpstub b/stubs/CoreGenericAttributes.phpstub index 92abe9542f8..0871e05448c 100644 --- a/stubs/CoreGenericAttributes.phpstub +++ b/stubs/CoreGenericAttributes.phpstub @@ -1,17 +1,20 @@ [], 'php_version' => '8.3', ], + 'canBeUsedOnPureMethods' => [ + 'code' => <<<'PHP' + [], + 'ignored_issues' => [], + 'php_version' => '8.3', + ], ]; } From f5533924540980a818ec029b15639ab9c39cac2d Mon Sep 17 00:00:00 2001 From: Melech Mizrachi Date: Fri, 23 Feb 2024 11:25:34 -0700 Subject: [PATCH 33/59] Fix a false flag issue with InvalidConstantAssignmentValue being thrown for constants over a certain length. Usually happens with arrays or lists over 100+ entries in length. Check if this type was defined via a dockblock or type hint otherwise the inferred type should always match the assigned type, and we don't even need to do additional checks There is an issue with constants over a certain length where additional values are added to fallback_params in the assigned_type but not in const_storage_type which causes a false flag for this error to appear. Usually happens with arrays/lists. Added two separate tests to cover both lists, and arrays to ensure this issue is fixed. --- .../Expression/ClassConstAnalyzer.php | 16 +- tests/ConstantTest.php | 236 ++++++++++++++++++ 2 files changed, 248 insertions(+), 4 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/ClassConstAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/ClassConstAnalyzer.php index 92025d0bb2f..41c04ed79c0 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/ClassConstAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/ClassConstAnalyzer.php @@ -721,19 +721,27 @@ public static function analyzeAssignment( // Check assigned type matches docblock type if ($assigned_type = $statements_analyzer->node_data->getType($const->value)) { - if ($const_storage->type !== null + $const_storage_type = $const_storage->type; + + if ($const_storage_type !== null && $const_storage->stmt_location !== null - && $assigned_type !== $const_storage->type + && $assigned_type !== $const_storage_type + // Check if this type was defined via a dockblock or type hint otherwise the inferred type + // should always match the assigned type and we don't even need to do additional checks + // There is an issue with constants over a certain length where additional values + // are added to fallback_params in the assigned_type but not in const_storage_type + // which causes a false flag for this error to appear. Usually happens with arrays + && ($const_storage_type->from_docblock || $const_storage_type->from_property) && !UnionTypeComparator::isContainedBy( $statements_analyzer->getCodebase(), $assigned_type, - $const_storage->type, + $const_storage_type, ) ) { IssueBuffer::maybeAdd( new InvalidConstantAssignmentValue( "{$class_storage->name}::{$const->name->name} with declared type " - . "{$const_storage->type->getId()} cannot be assigned type {$assigned_type->getId()}", + . "{$const_storage_type->getId()} cannot be assigned type {$assigned_type->getId()}", $const_storage->stmt_location, "{$class_storage->name}::{$const->name->name}", ), diff --git a/tests/ConstantTest.php b/tests/ConstantTest.php index 828f2ea654e..68b10f2b641 100644 --- a/tests/ConstantTest.php +++ b/tests/ConstantTest.php @@ -824,6 +824,242 @@ public static function foo(int $i) : void {} A::foo(2); A::foo(3);', ], + 'tooLongArrayInvalidConstantAssignmentValueFalsePositiveWithArray' => [ + 'code' => ' null, + "01" => null, + "02" => null, + "03" => null, + "04" => null, + "05" => null, + "06" => null, + "07" => null, + "08" => null, + "09" => null, + "10" => null, + "11" => null, + "12" => null, + "13" => null, + "14" => null, + "15" => null, + "16" => null, + "17" => null, + "18" => null, + "19" => null, + "20" => null, + "21" => null, + "22" => null, + "23" => null, + "24" => null, + "25" => null, + "26" => null, + "27" => null, + "28" => null, + "29" => null, + "30" => null, + "31" => null, + "32" => null, + "33" => null, + "34" => null, + "35" => null, + "36" => null, + "37" => null, + "38" => null, + "39" => null, + "40" => null, + "41" => null, + "42" => null, + "43" => null, + "44" => null, + "45" => null, + "46" => null, + "47" => null, + "48" => null, + "49" => null, + "50" => null, + "51" => null, + "52" => null, + "53" => null, + "54" => null, + "55" => null, + "56" => null, + "57" => null, + "58" => null, + "59" => null, + "60" => null, + "61" => null, + "62" => null, + "63" => null, + "64" => null, + "65" => null, + "66" => null, + "67" => null, + "68" => null, + "69" => null, + "70" => self::SUCCEED, + "71" => self::FAIL, + "72" => null, + "73" => null, + "74" => null, + "75" => null, + "76" => null, + "77" => null, + "78" => null, + "79" => null, + "80" => null, + "81" => null, + "82" => null, + "83" => null, + "84" => null, + "85" => null, + "86" => null, + "87" => null, + "88" => null, + "89" => null, + "90" => null, + "91" => null, + "92" => null, + "93" => null, + "94" => null, + "95" => null, + "96" => null, + "97" => null, + "98" => null, + "99" => null, + "100" => null, + "101" => null, + ]; + + const SUCCEED = "SUCCEED"; + const FAIL = "FAIL"; + + public static function will_succeed(string $code) : bool { + // Seems to fail when the array has over 100+ entries, and at least one value references + // another constant from the same class (even nested) + return (self::LOOKUP[$code] ?? null) === self::SUCCEED; + } + }', + ], + 'tooLongArrayInvalidConstantAssignmentValueFalsePositiveWithList' => [ + 'code' => ' [ 'code' => ' Date: Sat, 24 Feb 2024 14:44:16 +0100 Subject: [PATCH 34/59] Use TaintKind/TaintKindGroup constants instead of string values --- .../Call/FunctionCallReturnTypeFetcher.php | 7 +++--- .../Reflector/FunctionLikeDocblockParser.php | 7 ++++-- .../Reflector/FunctionLikeDocblockScanner.php | 25 ++++++++++++------- .../AddRemoveTaints/HtmlFunctionTainter.php | 21 ++++++++-------- src/Psalm/Type/TaintKindGroup.php | 2 ++ 5 files changed, 38 insertions(+), 24 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallReturnTypeFetcher.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallReturnTypeFetcher.php index 55557576e6b..74400b70a2b 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallReturnTypeFetcher.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallReturnTypeFetcher.php @@ -39,6 +39,7 @@ use Psalm\Type\Atomic\TNonEmptyArray; use Psalm\Type\Atomic\TNull; use Psalm\Type\Atomic\TString; +use Psalm\Type\TaintKind; use Psalm\Type\Union; use UnexpectedValueException; @@ -646,9 +647,9 @@ private static function taintReturnType( $pattern = substr($pattern, 2, -1); if (self::simpleExclusion($pattern, $first_arg_value[0])) { - $removed_taints[] = 'html'; - $removed_taints[] = 'has_quotes'; - $removed_taints[] = 'sql'; + $removed_taints[] = TaintKind::INPUT_HTML; + $removed_taints[] = TaintKind::INPUT_HAS_QUOTES; + $removed_taints[] = TaintKind::INPUT_SQL; } } } diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockParser.php b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockParser.php index 7bedbf9e27f..66fc0fafa94 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockParser.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockParser.php @@ -14,6 +14,7 @@ use Psalm\Internal\Scanner\ParsedDocblock; use Psalm\Issue\InvalidDocblock; use Psalm\IssueBuffer; +use Psalm\Type\TaintKindGroup; use function array_keys; use function array_shift; @@ -264,10 +265,11 @@ public static function parse( $taint_type = substr($taint_type, 5); if ($taint_type === 'tainted') { - $taint_type = 'input'; + $taint_type = TaintKindGroup::GROUP_INPUT; } if ($taint_type === 'misc') { + // @todo `text` is semantically not defined in `TaintKind`, maybe drop it $taint_type = 'text'; } @@ -305,10 +307,11 @@ public static function parse( if ($param_parts[0]) { if ($param_parts[0] === 'tainted') { - $param_parts[0] = 'input'; + $param_parts[0] = TaintKindGroup::GROUP_INPUT; } if ($param_parts[0] === 'misc') { + // @todo `text` is semantically not defined in `TaintKind`, maybe drop it $param_parts[0] = 'text'; } diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php index 270529ee306..cfeb0b15f30 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php @@ -50,6 +50,9 @@ use function array_filter; use function array_merge; +use function array_search; +use function array_splice; +use function array_unique; use function array_values; use function count; use function explode; @@ -352,16 +355,20 @@ public static function addDocblockInfo( } } - foreach ($docblock_info->taint_source_types as $taint_source_type) { - if ($taint_source_type === 'input') { - $storage->taint_source_types = array_merge( - $storage->taint_source_types, - TaintKindGroup::ALL_INPUT, - ); - } else { - $storage->taint_source_types[] = $taint_source_type; - } + $docblock_info->taint_source_types = array_values(array_unique($docblock_info->taint_source_types)); + // expand 'input' group to all items, e.g. `['other', 'input']` -> `['other', 'html', 'sql', 'shell', ...]` + $inputIndex = array_search(TaintKindGroup::GROUP_INPUT, $docblock_info->taint_source_types, true); + if ($inputIndex !== false) { + array_splice( + $docblock_info->taint_source_types, + $inputIndex, + 1, + TaintKindGroup::ALL_INPUT, + ); } + // merge taints from doc block to storage, enforce uniqueness and having consecutive index keys + $storage->taint_source_types = array_merge($storage->taint_source_types, $docblock_info->taint_source_types); + $storage->taint_source_types = array_values(array_unique($storage->taint_source_types)); $storage->added_taints = $docblock_info->added_taints; diff --git a/src/Psalm/Internal/Provider/AddRemoveTaints/HtmlFunctionTainter.php b/src/Psalm/Internal/Provider/AddRemoveTaints/HtmlFunctionTainter.php index d49ebcee48b..bfb973faba0 100644 --- a/src/Psalm/Internal/Provider/AddRemoveTaints/HtmlFunctionTainter.php +++ b/src/Psalm/Internal/Provider/AddRemoveTaints/HtmlFunctionTainter.php @@ -7,6 +7,7 @@ use Psalm\Plugin\EventHandler\AddTaintsInterface; use Psalm\Plugin\EventHandler\Event\AddRemoveTaintsEvent; use Psalm\Plugin\EventHandler\RemoveTaintsInterface; +use Psalm\Type\TaintKind; use function count; use function strtolower; @@ -47,24 +48,24 @@ public static function addTaints(AddRemoveTaintsEvent $event): array if ($second_arg === null) { if ($statements_analyzer->getCodebase()->analysis_php_version_id >= 8_01_00) { - return ['html', 'has_quotes']; + return [TaintKind::INPUT_HTML, TaintKind::INPUT_HAS_QUOTES]; } - return ['html']; + return [TaintKind::INPUT_HTML]; } $second_arg_value = $statements_analyzer->node_data->getType($second_arg); if (!$second_arg_value || !$second_arg_value->isSingleIntLiteral()) { - return ['html']; + return [TaintKind::INPUT_HTML]; } $second_arg_value = $second_arg_value->getSingleIntLiteral()->value; if (($second_arg_value & ENT_QUOTES) === ENT_QUOTES) { - return ['html', 'has_quotes']; + return [TaintKind::INPUT_HTML, TaintKind::INPUT_HAS_QUOTES]; } - return ['html']; + return [TaintKind::INPUT_HTML]; } return []; @@ -99,24 +100,24 @@ public static function removeTaints(AddRemoveTaintsEvent $event): array if ($second_arg === null) { if ($statements_analyzer->getCodebase()->analysis_php_version_id >= 8_01_00) { - return ['html', 'has_quotes']; + return [TaintKind::INPUT_HTML, TaintKind::INPUT_HAS_QUOTES]; } - return ['html']; + return [TaintKind::INPUT_HTML]; } $second_arg_value = $statements_analyzer->node_data->getType($second_arg); if (!$second_arg_value || !$second_arg_value->isSingleIntLiteral()) { - return ['html']; + return [TaintKind::INPUT_HTML]; } $second_arg_value = $second_arg_value->getSingleIntLiteral()->value; if (($second_arg_value & ENT_QUOTES) === ENT_QUOTES) { - return ['html', 'has_quotes']; + return [TaintKind::INPUT_HTML, TaintKind::INPUT_HAS_QUOTES]; } - return ['html']; + return [TaintKind::INPUT_HTML]; } return []; diff --git a/src/Psalm/Type/TaintKindGroup.php b/src/Psalm/Type/TaintKindGroup.php index eb74a2916a6..8f144569a5e 100644 --- a/src/Psalm/Type/TaintKindGroup.php +++ b/src/Psalm/Type/TaintKindGroup.php @@ -7,6 +7,8 @@ */ final class TaintKindGroup { + public const GROUP_INPUT = 'input'; + public const ALL_INPUT = [ TaintKind::INPUT_HTML, TaintKind::INPUT_HAS_QUOTES, From 01dc2fef84f07aef95a7bd5db84904f662d731d1 Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Sun, 25 Feb 2024 12:53:13 +0100 Subject: [PATCH 35/59] Fix loading stubs from phar file on Windows Fixes vimeo/psalm#10747 --- src/Psalm/Config.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php index a4d4fe35e24..a8a08366e43 100644 --- a/src/Psalm/Config.php +++ b/src/Psalm/Config.php @@ -2326,6 +2326,10 @@ public function visitPreloadedStubFiles(Codebase $codebase, ?Progress $progress foreach ($stub_files as $file_path) { $file_path = str_replace(['/', '\\'], DIRECTORY_SEPARATOR, $file_path); + // fix mangled phar paths on Windows + if (strpos($file_path, 'phar:\\\\') === 0) { + $file_path = 'phar://'. substr($file_path, 7); + } $codebase->scanner->addFileToDeepScan($file_path); } @@ -2423,6 +2427,10 @@ public function visitStubFiles(Codebase $codebase, ?Progress $progress = null): foreach ($stub_files as $file_path) { $file_path = str_replace(['/', '\\'], DIRECTORY_SEPARATOR, $file_path); + // fix mangled phar paths on Windows + if (strpos($file_path, 'phar:\\\\') === 0) { + $file_path = 'phar://' . substr($file_path, 7); + } $codebase->scanner->addFileToDeepScan($file_path); } From 57caade0d13ac4b19556e19412aaaffdf69cf1ec Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Sun, 25 Feb 2024 14:25:25 +0100 Subject: [PATCH 36/59] Skip symlink test on Windows Fixes vimeo/psalm#6449 Symlinks on Windows are rare (and quite unreliable) --- tests/Config/ConfigTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/Config/ConfigTest.php b/tests/Config/ConfigTest.php index e8fe9a94c40..d0aa56b1cc0 100644 --- a/tests/Config/ConfigTest.php +++ b/tests/Config/ConfigTest.php @@ -158,6 +158,9 @@ public function testIgnoreMissingProjectDirectory(): void $this->assertFalse($config->isInProjectDirs(realpath('examples/TemplateScanner.php'))); } + /** + * @requires OS ^(?!WIN) + */ public function testIgnoreSymlinkedProjectDirectory(): void { @unlink(dirname(__DIR__, 1) . '/fixtures/symlinktest/ignored/b'); From 10ed0f3cd61b980983b97723dbf7323865cc9cec Mon Sep 17 00:00:00 2001 From: Evan Shaw Date: Mon, 26 Feb 2024 21:19:29 +1300 Subject: [PATCH 37/59] Set inside_isset false when analyzing ArrayDimFetch dim --- .../Statements/Expression/Fetch/ArrayFetchAnalyzer.php | 6 ++++++ tests/ArrayAccessTest.php | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php index 753e2891920..d71499606c6 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php @@ -120,13 +120,19 @@ public static function analyze( $was_inside_unset = $context->inside_unset; $context->inside_unset = false; + $was_inside_isset = $context->inside_isset; + $context->inside_isset = false; + if (ExpressionAnalyzer::analyze($statements_analyzer, $stmt->dim, $context) === false) { + $context->inside_isset = $was_inside_isset; $context->inside_unset = $was_inside_unset; $context->inside_general_use = $was_inside_general_use; return false; } + $context->inside_isset = $was_inside_isset; + $context->inside_unset = $was_inside_unset; $context->inside_general_use = $was_inside_general_use; diff --git a/tests/ArrayAccessTest.php b/tests/ArrayAccessTest.php index 88eb7d15e52..8481ac569e4 100644 --- a/tests/ArrayAccessTest.php +++ b/tests/ArrayAccessTest.php @@ -1281,6 +1281,11 @@ function return_array() { echo $a[0];', 'error_message' => 'PossiblyInvalidArrayAccess', ], + 'insideIssetDisabledForDim' => [ + 'code' => ' 'UndefinedGlobalVariable', + ], 'mixedArrayAccess' => [ 'code' => ' Date: Mon, 26 Feb 2024 21:28:59 +1300 Subject: [PATCH 38/59] Fix new errors --- .../Analyzer/Statements/Block/IfConditionalAnalyzer.php | 4 ++-- src/Psalm/Internal/Type/SimpleAssertionReconciler.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php index 349eaf5ff0a..c00803dd1da 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php @@ -23,7 +23,7 @@ use function array_diff_key; use function array_filter; -use function array_keys; +use function array_key_first; use function array_merge; use function array_values; use function count; @@ -80,7 +80,7 @@ public static function analyze( $entry_clauses, static fn(Clause $c): bool => count($c->possibilities) > 1 || $c->wedge - || !isset($changed_var_ids[array_keys($c->possibilities)[0]]) + || !isset($changed_var_ids[array_key_first($c->possibilities)]) ), ); } diff --git a/src/Psalm/Internal/Type/SimpleAssertionReconciler.php b/src/Psalm/Internal/Type/SimpleAssertionReconciler.php index c8bd3c1b21d..ed40b070143 100644 --- a/src/Psalm/Internal/Type/SimpleAssertionReconciler.php +++ b/src/Psalm/Internal/Type/SimpleAssertionReconciler.php @@ -2982,7 +2982,7 @@ private static function reconcileValueOf( continue; } - $enum_case = $class_storage->enum_cases[$atomic_type->const_name] ?? null; + $enum_case = $class_storage->enum_cases[$enum_case_to_assert] ?? null; if ($enum_case === null) { return null; } From 8a0bc19fa6544e96b4281dd1ed1d16c7f4a44ef2 Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Mon, 12 Feb 2024 05:14:17 +0100 Subject: [PATCH 39/59] Forbid iterating over generators with non-nullable `send()` Fixes vimeo/psalm#6985 --- .../Statements/Block/ForeachAnalyzer.php | 41 ++++++++++++++++++- tests/Loop/ForeachTest.php | 34 +++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php index 8f0ec8a713f..8c16dbaf25f 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php @@ -657,6 +657,7 @@ public static function checkIteratorType( $key_type, $value_type, $has_valid_iterator, + $invalid_iterator_types, ); } else { $raw_object_types[] = $iterator_atomic_type->value; @@ -725,6 +726,7 @@ public static function checkIteratorType( return null; } + /** @param list $invalid_iterator_types */ public static function handleIterable( StatementsAnalyzer $statements_analyzer, TNamedObject $iterator_atomic_type, @@ -733,7 +735,8 @@ public static function handleIterable( Context $context, ?Union &$key_type, ?Union &$value_type, - bool &$has_valid_iterator + bool &$has_valid_iterator, + array &$invalid_iterator_types = [] ): void { if ($iterator_atomic_type->extra_types) { $iterator_atomic_types = array_merge( @@ -753,7 +756,6 @@ public static function handleIterable( } - $has_valid_iterator = true; if ($iterator_atomic_type instanceof TIterable || (strtolower($iterator_atomic_type->value) === 'traversable' @@ -781,6 +783,8 @@ public static function handleIterable( ) ) ) { + $has_valid_iterator = true; + $old_data_provider = $statements_analyzer->node_data; $statements_analyzer->node_data = clone $statements_analyzer->node_data; @@ -867,6 +871,7 @@ public static function handleIterable( $key_type, $value_type, $has_valid_iterator, + $invalid_iterator_types, ); continue; @@ -899,6 +904,37 @@ public static function handleIterable( $value_type = Type::combineUnionTypes($value_type, $value_type_part); } } + } elseif ($iterator_atomic_type instanceof TGenericObject + && strtolower($iterator_atomic_type->value) === 'generator' + ) { + $type_params = $iterator_atomic_type->type_params; + if (isset($type_params[2]) && !$type_params[2]->isNullable() && !$type_params[2]->isMixed()) { + $invalid_iterator_types[] = $iterator_atomic_type->getKey(); + } else { + $has_valid_iterator = true; + } + + $iterator_value_type = self::getFakeMethodCallType( + $statements_analyzer, + $foreach_expr, + $context, + 'current', + ); + + $iterator_key_type = self::getFakeMethodCallType( + $statements_analyzer, + $foreach_expr, + $context, + 'key', + ); + + if ($iterator_value_type && !$iterator_value_type->isMixed()) { + $value_type = Type::combineUnionTypes($value_type, $iterator_value_type); + } + + if ($iterator_key_type && !$iterator_key_type->isMixed()) { + $key_type = Type::combineUnionTypes($key_type, $iterator_key_type); + } } elseif ($codebase->classImplements( $iterator_atomic_type->value, 'Iterator', @@ -911,6 +947,7 @@ public static function handleIterable( ) ) ) { + $has_valid_iterator = true; $iterator_value_type = self::getFakeMethodCallType( $statements_analyzer, $foreach_expr, diff --git a/tests/Loop/ForeachTest.php b/tests/Loop/ForeachTest.php index d7b56e5fa48..ba1ca170b88 100644 --- a/tests/Loop/ForeachTest.php +++ b/tests/Loop/ForeachTest.php @@ -1169,6 +1169,28 @@ function f(array $a): array { } PHP, ], + 'generatorWithUnspecifiedSend' => [ + 'code' => <<<'PHP' + */ + function gen() : Generator { + return yield 1; + } + $gen = gen(); + foreach ($gen as $i) {} + PHP, + ], + 'generatorWithMixedSend' => [ + 'code' => <<<'PHP' + */ + function gen() : Generator { + return yield 1; + } + $gen = gen(); + foreach ($gen as $i) {} + PHP, + ], ]; } @@ -1395,6 +1417,18 @@ function f(array $a): array { PHP, 'error_message' => 'LessSpecificReturnStatement', ], + 'generatorWithNonNullableSend' => [ + 'code' => <<<'PHP' + */ + function gen() : Generator { + return yield 1; + } + $gen = gen(); + foreach ($gen as $i) {} + PHP, + 'error_message' => 'InvalidIterator', + ], ]; } } From 27461c98f6f9def9d1a6a4e4643c6066e65a5f9d Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Mon, 12 Feb 2024 07:47:58 +0100 Subject: [PATCH 40/59] Strip null used to signify completed iterations in foreach context Even though `Generator::current()` can return `null` once generator is exhausted, `foreach()` never iterates after iterator ends, so we can safely remove `null` (unless, of course, generator can yield `null`). --- .../Statements/Block/ForeachAnalyzer.php | 14 ++++++++ tests/Loop/ForeachTest.php | 35 +++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php index 8c16dbaf25f..19099c9c247 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php @@ -929,10 +929,24 @@ public static function handleIterable( ); if ($iterator_value_type && !$iterator_value_type->isMixed()) { + // remove null coming from current() to signify invalid iterations + // we're in a foreach context, so we know we're not going iterate past the end + if (isset($type_params[1]) && !$type_params[1]->isNullable()) { + $iterator_value_type = $iterator_value_type->getBuilder(); + $iterator_value_type->removeType('null'); + $iterator_value_type = $iterator_value_type->freeze(); + } $value_type = Type::combineUnionTypes($value_type, $iterator_value_type); } if ($iterator_key_type && !$iterator_key_type->isMixed()) { + // remove null coming from key() to signify invalid iterations + // we're in a foreach context, so we know we're not going iterate past the end + if (isset($type_params[0]) && !$type_params[0]->isNullable()) { + $iterator_key_type = $iterator_key_type->getBuilder(); + $iterator_key_type->removeType('null'); + $iterator_key_type = $iterator_key_type->freeze(); + } $key_type = Type::combineUnionTypes($key_type, $iterator_key_type); } } elseif ($codebase->classImplements( diff --git a/tests/Loop/ForeachTest.php b/tests/Loop/ForeachTest.php index ba1ca170b88..0777815cb5e 100644 --- a/tests/Loop/ForeachTest.php +++ b/tests/Loop/ForeachTest.php @@ -1191,6 +1191,41 @@ function gen() : Generator { foreach ($gen as $i) {} PHP, ], + 'nullableGenerator' => [ + 'code' => <<<'PHP' + */ + function gen() : Generator { + yield null; + yield 1; + } + $gen = gen(); + $a = ""; + foreach ($gen as $i) { + $a = $i; + } + PHP, + 'assertions' => [ + '$a===' => "''|int|null", + ], + ], + 'nonNullableGenerator' => [ + 'code' => <<<'PHP' + */ + function gen() : Generator { + yield 1; + } + $gen = gen(); + $a = ""; + foreach ($gen as $i) { + $a = $i; + } + PHP, + 'assertions' => [ + '$a===' => "''|int", + ], + ], ]; } From 4eec8bb5e8aedfcd992576addfd154867c50ddae Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Mon, 26 Feb 2024 01:39:38 +0100 Subject: [PATCH 41/59] Disabled wrong test --- tests/GeneratorTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/GeneratorTest.php b/tests/GeneratorTest.php index d744e271aa5..fc12a351dfb 100644 --- a/tests/GeneratorTest.php +++ b/tests/GeneratorTest.php @@ -217,8 +217,9 @@ function gen(): Generator { echo yield; }', ], - 'yieldFromTwiceWithVoidSend' => [ + 'SKIPPED-yieldFromTwiceWithVoidSend' => [ 'code' => ' */ From 0ce62a51ec4e4620d410f5049fca9650a9bef3b9 Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Mon, 26 Feb 2024 01:50:12 +0100 Subject: [PATCH 42/59] Let's see what breaks without this hack --- .../Analyzer/FunctionLike/ReturnTypeAnalyzer.php | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php index e13308e5c0f..b752d9176ff 100644 --- a/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php @@ -307,15 +307,6 @@ public static function verifyReturnType( $source->getParentFQCLN(), ); - // hack until we have proper yield type collection - if ($function_like_storage - && $function_like_storage->has_yield - && !$inferred_yield_type - && !$inferred_return_type->isVoid() - ) { - $inferred_return_type = new Union([new TNamedObject('Generator')]); - } - if ($is_to_string) { $union_comparison_results = new TypeComparisonResult(); if (!$inferred_return_type->hasMixed() && From 22f32c1392f1e8922a8294d1fc23e5a28a197888 Mon Sep 17 00:00:00 2001 From: Evan Shaw Date: Tue, 27 Feb 2024 22:09:03 +1300 Subject: [PATCH 43/59] Set inside_isset = false when analyzing arguments --- .../Statements/Expression/Call/ArgumentsAnalyzer.php | 5 +++++ tests/FunctionCallTest.php | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php index 81843d56a15..3d825668dc9 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php @@ -231,6 +231,9 @@ public static function analyze( $was_inside_call = $context->inside_call; $context->inside_call = true; + $was_inside_isset = $context->inside_isset; + $context->inside_isset = false; + if (ExpressionAnalyzer::analyze( $statements_analyzer, $arg->value, @@ -240,11 +243,13 @@ public static function analyze( false, $high_order_template_result, ) === false) { + $context->inside_isset = $was_inside_isset; $context->inside_call = $was_inside_call; return false; } + $context->inside_isset = $was_inside_isset; $context->inside_call = $was_inside_call; if ($high_order_callable_info && $high_order_template_result) { diff --git a/tests/FunctionCallTest.php b/tests/FunctionCallTest.php index c2ca6040367..d667552666a 100644 --- a/tests/FunctionCallTest.php +++ b/tests/FunctionCallTest.php @@ -2506,6 +2506,16 @@ public function foo() : void { }', 'error_message' => 'InvalidArgument', ], + 'clearIssetContext' => [ + 'code' => ' 'UndefinedGlobalVariable', + ], 'mixedArgument' => [ 'code' => ' Date: Thu, 29 Feb 2024 17:54:51 +0100 Subject: [PATCH 44/59] Fix PHP notice - crash on invalid taint-escape --- .../Reflector/FunctionLikeDocblockParser.php | 20 +++++++++++++++++-- tests/AnnotationTest.php | 10 ++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockParser.php b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockParser.php index 66fc0fafa94..52106d3895e 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockParser.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockParser.php @@ -325,14 +325,30 @@ public static function parse( if (isset($parsed_docblock->tags['psalm-taint-unescape'])) { foreach ($parsed_docblock->tags['psalm-taint-unescape'] as $param) { $param = trim($param); - $info->added_taints[] = $param; + if ($param === '') { + IssueBuffer::maybeAdd( + new InvalidDocblock( + '@psalm-taint-unescape expects 1 argument', + $code_location, + ), + ); + } else { + $info->added_taints[] = $param; + } } } if (isset($parsed_docblock->tags['psalm-taint-escape'])) { foreach ($parsed_docblock->tags['psalm-taint-escape'] as $param) { $param = trim($param); - if ($param[0] === '(') { + if ($param === '') { + IssueBuffer::maybeAdd( + new InvalidDocblock( + '@psalm-taint-escape expects 1 argument', + $code_location, + ), + ); + } elseif ($param[0] === '(') { $line_parts = CommentAnalyzer::splitDocLine($param); $info->removed_taints[] = CommentAnalyzer::sanitizeDocblockType($line_parts[0]); diff --git a/tests/AnnotationTest.php b/tests/AnnotationTest.php index 4252e60b84e..1626c0bcc1a 100644 --- a/tests/AnnotationTest.php +++ b/tests/AnnotationTest.php @@ -1632,6 +1632,16 @@ public function bar() { ', 'error_message' => 'UndefinedDocblockClass', ], + 'invalidTaintEscapeAnnotation' => [ + 'code' => ' 'InvalidDocblock', + ], 'noPhpStormAnnotationsThankYou' => [ 'code' => ' Date: Fri, 1 Mar 2024 01:36:53 +0100 Subject: [PATCH 45/59] Fix version comparison for `@since` Refs vimeo/psalm#10761 --- .../Internal/PhpVisitor/Reflector/FunctionLikeNodeScanner.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeNodeScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeNodeScanner.php index f0a8f34cc63..fa2d09f2a98 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeNodeScanner.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeNodeScanner.php @@ -463,7 +463,7 @@ public function start(PhpParser\Node\FunctionLike $stmt, bool $fake_method = fal if ($docblock_info) { if ($docblock_info->since_php_major_version && !$this->aliases->namespace) { $analysis_major_php_version = $this->codebase->getMajorAnalysisPhpVersion(); - $analysis_minor_php_version = $this->codebase->getMajorAnalysisPhpVersion(); + $analysis_minor_php_version = $this->codebase->getMinorAnalysisPhpVersion(); if ($docblock_info->since_php_major_version > $analysis_major_php_version) { return false; } @@ -1047,7 +1047,7 @@ private function createStorageForFunctionLike( if ($docblock_info) { if ($docblock_info->since_php_major_version && !$this->aliases->namespace) { $analysis_major_php_version = $this->codebase->getMajorAnalysisPhpVersion(); - $analysis_minor_php_version = $this->codebase->getMajorAnalysisPhpVersion(); + $analysis_minor_php_version = $this->codebase->getMinorAnalysisPhpVersion(); if ($docblock_info->since_php_major_version > $analysis_major_php_version) { return false; } From e357b972f6a0765e7a43c6d021b2a99cbc48ec9c Mon Sep 17 00:00:00 2001 From: Theodore Brown Date: Thu, 29 Feb 2024 19:51:19 -0600 Subject: [PATCH 46/59] Avoid duplicating code for RiskyTruthyFalsyComparison --- .../Block/IfConditionalAnalyzer.php | 30 +-------------- .../Expression/BooleanNotAnalyzer.php | 34 +---------------- .../Statements/Expression/EmptyAnalyzer.php | 33 +---------------- .../Statements/ExpressionAnalyzer.php | 37 +++++++++++++++++++ 4 files changed, 41 insertions(+), 93 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php index c00803dd1da..fa1970e3ae6 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php @@ -15,10 +15,8 @@ use Psalm\Issue\DocblockTypeContradiction; use Psalm\Issue\RedundantCondition; use Psalm\Issue\RedundantConditionGivenDocblockType; -use Psalm\Issue\RiskyTruthyFalsyComparison; use Psalm\Issue\TypeDoesNotContainType; use Psalm\IssueBuffer; -use Psalm\Type\Atomic\TBool; use Psalm\Type\Reconciler; use function array_diff_key; @@ -371,33 +369,7 @@ public static function handleParadoxicalCondition( } elseif (!($stmt instanceof PhpParser\Node\Expr\BinaryOp\NotIdentical) && !($stmt instanceof PhpParser\Node\Expr\BinaryOp\Identical) && !($stmt instanceof PhpParser\Node\Expr\BooleanNot)) { - if (count($type->getAtomicTypes()) > 1) { - $has_truthy_or_falsy_exclusive_type = false; - $both_types = $type->getBuilder(); - foreach ($both_types->getAtomicTypes() as $key => $atomic_type) { - if ($atomic_type->isTruthy() - || $atomic_type->isFalsy() - || $atomic_type instanceof TBool) { - $both_types->removeType($key); - $has_truthy_or_falsy_exclusive_type = true; - } - } - - if (count($both_types->getAtomicTypes()) > 0 && $has_truthy_or_falsy_exclusive_type) { - $both_types = $both_types->freeze(); - IssueBuffer::maybeAdd( - new RiskyTruthyFalsyComparison( - 'Operand of type ' . $type->getId() . ' contains ' . - 'type' . (count($both_types->getAtomicTypes()) > 1 ? 's' : '') . ' ' . - $both_types->getId() . ', which can be falsy and truthy. ' . - 'This can cause possibly unexpected behavior. Use strict comparison instead.', - new CodeLocation($statements_analyzer, $stmt), - $type->getId(), - ), - $statements_analyzer->getSuppressedIssues(), - ); - } - } + ExpressionAnalyzer::checkRiskyTruthyFalsyComparison($type, $statements_analyzer, $stmt); } } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BooleanNotAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BooleanNotAnalyzer.php index 9a2f36602c8..e6b29ff736c 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BooleanNotAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BooleanNotAnalyzer.php @@ -3,20 +3,15 @@ namespace Psalm\Internal\Analyzer\Statements\Expression; use PhpParser; -use Psalm\CodeLocation; use Psalm\Context; use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer; use Psalm\Internal\Analyzer\StatementsAnalyzer; -use Psalm\Issue\RiskyTruthyFalsyComparison; -use Psalm\IssueBuffer; use Psalm\Type; use Psalm\Type\Atomic\TBool; use Psalm\Type\Atomic\TFalse; use Psalm\Type\Atomic\TTrue; use Psalm\Type\Union; -use function count; - /** * @internal */ @@ -45,34 +40,7 @@ public static function analyze( } elseif ($expr_type->isAlwaysFalsy()) { $stmt_type = new TTrue($expr_type->from_docblock); } else { - if (count($expr_type->getAtomicTypes()) > 1) { - $has_truthy_or_falsy_exclusive_type = false; - $both_types = $expr_type->getBuilder(); - foreach ($both_types->getAtomicTypes() as $key => $atomic_type) { - if ($atomic_type->isTruthy() - || $atomic_type->isFalsy() - || $atomic_type instanceof TBool) { - $both_types->removeType($key); - $has_truthy_or_falsy_exclusive_type = true; - } - } - - if (count($both_types->getAtomicTypes()) > 0 && $has_truthy_or_falsy_exclusive_type) { - $both_types = $both_types->freeze(); - IssueBuffer::maybeAdd( - new RiskyTruthyFalsyComparison( - 'Operand of type ' . $expr_type->getId() . ' contains ' . - 'type' . (count($both_types->getAtomicTypes()) > 1 ? 's' : '') . ' ' . - $both_types->getId() . ', which can be falsy and truthy. ' . - 'This can cause possibly unexpected behavior. Use strict comparison instead.', - new CodeLocation($statements_analyzer, $stmt), - $expr_type->getId(), - ), - $statements_analyzer->getSuppressedIssues(), - ); - } - } - + ExpressionAnalyzer::checkRiskyTruthyFalsyComparison($expr_type, $statements_analyzer, $stmt); $stmt_type = new TBool(); } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/EmptyAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/EmptyAnalyzer.php index d96d5d9267a..1d0eafe18fd 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/EmptyAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/EmptyAnalyzer.php @@ -5,10 +5,10 @@ use PhpParser; use Psalm\CodeLocation; use Psalm\Context; +use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer; use Psalm\Internal\Analyzer\StatementsAnalyzer; use Psalm\Issue\ForbiddenCode; use Psalm\Issue\InvalidArgument; -use Psalm\Issue\RiskyTruthyFalsyComparison; use Psalm\IssueBuffer; use Psalm\Type; use Psalm\Type\Atomic\TBool; @@ -16,8 +16,6 @@ use Psalm\Type\Atomic\TTrue; use Psalm\Type\Union; -use function count; - /** * @internal */ @@ -64,34 +62,7 @@ public static function analyze( } elseif ($expr_type->isAlwaysFalsy()) { $stmt_type = new TTrue($expr_type->from_docblock); } else { - if (count($expr_type->getAtomicTypes()) > 1) { - $has_truthy_or_falsy_exclusive_type = false; - $both_types = $expr_type->getBuilder(); - foreach ($both_types->getAtomicTypes() as $key => $atomic_type) { - if ($atomic_type->isTruthy() - || $atomic_type->isFalsy() - || $atomic_type instanceof TBool) { - $both_types->removeType($key); - $has_truthy_or_falsy_exclusive_type = true; - } - } - - if (count($both_types->getAtomicTypes()) > 0 && $has_truthy_or_falsy_exclusive_type) { - $both_types = $both_types->freeze(); - IssueBuffer::maybeAdd( - new RiskyTruthyFalsyComparison( - 'Operand of type ' . $expr_type->getId() . ' contains ' . - 'type' . (count($both_types->getAtomicTypes()) > 1 ? 's' : '') . ' ' . - $both_types->getId() . ', which can be falsy and truthy. ' . - 'This can cause possibly unexpected behavior. Use strict comparison instead.', - new CodeLocation($statements_analyzer, $stmt), - $expr_type->getId(), - ), - $statements_analyzer->getSuppressedIssues(), - ); - } - } - + ExpressionAnalyzer::checkRiskyTruthyFalsyComparison($expr_type, $statements_analyzer, $stmt); $stmt_type = new TBool(); } diff --git a/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php index 063277aaff6..c19c8df51a6 100644 --- a/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php @@ -44,6 +44,7 @@ use Psalm\Internal\Analyzer\StatementsAnalyzer; use Psalm\Internal\FileManipulation\FileManipulationBuffer; use Psalm\Internal\Type\TemplateResult; +use Psalm\Issue\RiskyTruthyFalsyComparison; use Psalm\Issue\UnrecognizedExpression; use Psalm\Issue\UnsupportedReferenceUsage; use Psalm\IssueBuffer; @@ -54,7 +55,9 @@ use Psalm\Plugin\EventHandler\Event\AfterExpressionAnalysisEvent; use Psalm\Plugin\EventHandler\Event\BeforeExpressionAnalysisEvent; use Psalm\Type; +use Psalm\Type\Atomic\TBool; +use function count; use function get_class; use function in_array; use function strtolower; @@ -135,6 +138,40 @@ public static function analyze( return true; } + public static function checkRiskyTruthyFalsyComparison( + Type\Union $type, + StatementsAnalyzer $statements_analyzer, + PhpParser\Node\Expr $stmt + ): void { + if (count($type->getAtomicTypes()) > 1) { + $has_truthy_or_falsy_exclusive_type = false; + $both_types = $type->getBuilder(); + foreach ($both_types->getAtomicTypes() as $key => $atomic_type) { + if ($atomic_type->isTruthy() + || $atomic_type->isFalsy() + || $atomic_type instanceof TBool) { + $both_types->removeType($key); + $has_truthy_or_falsy_exclusive_type = true; + } + } + + if (count($both_types->getAtomicTypes()) > 0 && $has_truthy_or_falsy_exclusive_type) { + $both_types = $both_types->freeze(); + IssueBuffer::maybeAdd( + new RiskyTruthyFalsyComparison( + 'Operand of type ' . $type->getId() . ' contains ' . + 'type' . (count($both_types->getAtomicTypes()) > 1 ? 's' : '') . ' ' . + $both_types->getId() . ', which can be falsy and truthy. ' . + 'This can cause possibly unexpected behavior. Use strict comparison instead.', + new CodeLocation($statements_analyzer, $stmt), + $type->getId(), + ), + $statements_analyzer->getSuppressedIssues(), + ); + } + } + } + /** * @param bool $assigned_to_reference This is set to true when the expression being analyzed * here is being assigned to another variable by reference. From d751c202d8534436ab0452fe632e17fb1de6907d Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Sat, 2 Mar 2024 11:30:28 +0100 Subject: [PATCH 47/59] fix unrelated Config bug not honoring PHP composer versions 8.2 + 8.3 --- src/Psalm/Config.php | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php index a8a08366e43..069fd2c7a1f 100644 --- a/src/Psalm/Config.php +++ b/src/Psalm/Config.php @@ -2781,8 +2781,22 @@ public function getPHPVersionFromComposerJson(): ?string $version_parser = new VersionParser(); $constraint = $version_parser->parseConstraints($php_version); - - foreach (['5.4', '5.5', '5.6', '7.0', '7.1', '7.2', '7.3', '7.4', '8.0', '8.1'] as $candidate) { + $php_versions = [ + '5.4', + '5.5', + '5.6', + '7.0', + '7.1', + '7.2', + '7.3', + '7.4', + '8.0', + '8.1', + '8.2', + '8.3', + ]; + + foreach ($php_versions as $candidate) { if ($constraint->matches(new Constraint('<=', "$candidate.0.0-dev")) || $constraint->matches(new Constraint('<=', "$candidate.999")) ) { From a1848a1a1419b602c0266d64513827b5f124842f Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Sat, 2 Mar 2024 11:31:43 +0100 Subject: [PATCH 48/59] @since annotations should only infer PHP version in .phpstub files or for @since 8.0.0 PHP Fix https://github.com/vimeo/psalm/issues/10761 --- .../Reflector/FunctionLikeDocblockParser.php | 17 ++++++++++++----- tests/AnnotationTest.php | 10 ++++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockParser.php b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockParser.php index 52106d3895e..25fcb44bf21 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockParser.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockParser.php @@ -23,6 +23,7 @@ use function explode; use function implode; use function in_array; +use function pathinfo; use function preg_last_error_msg; use function preg_match; use function preg_replace; @@ -37,6 +38,8 @@ use function substr_count; use function trim; +use const PATHINFO_EXTENSION; + /** * @internal */ @@ -417,11 +420,15 @@ public static function parse( if (isset($parsed_docblock->tags['since'])) { $since = trim(reset($parsed_docblock->tags['since'])); - if (preg_match('/^[4578]\.\d(\.\d+)?$/', $since)) { - $since_parts = explode('.', $since); - - $info->since_php_major_version = (int)$since_parts[0]; - $info->since_php_minor_version = (int)$since_parts[1]; + // only for phpstub files or @since 8.0.0 PHP + // since @since is commonly used with the project version, not the PHP version + // https://docs.phpdoc.org/3.0/guide/references/phpdoc/tags/since.html + // https://github.com/vimeo/psalm/issues/10761 + if (preg_match('/^([4578])\.(\d)(\.\d+)?(\s+PHP)?$/i', $since, $since_match) + && isset($since_match[1])&& isset($since_match[2]) + && (!empty($since_match[4]) || pathinfo($code_location->file_name, PATHINFO_EXTENSION) === 'phpstub')) { + $info->since_php_major_version = (int)$since_match[1]; + $info->since_php_minor_version = (int)$since_match[2]; } } diff --git a/tests/AnnotationTest.php b/tests/AnnotationTest.php index 1626c0bcc1a..7a7345bd538 100644 --- a/tests/AnnotationTest.php +++ b/tests/AnnotationTest.php @@ -1384,6 +1384,16 @@ class Bar {} class Foo {}', 'assertions' => [], ], + 'sinceTagNonPhpVersion' => [ + 'code' => ' Date: Sat, 2 Mar 2024 17:49:01 +0100 Subject: [PATCH 49/59] Initial support for named parameters for callables Fixes vimeo/psalm#10766 --- .../Type/ParseTree/CallableParamTree.php | 7 ++++++ src/Psalm/Internal/Type/ParseTreeCreator.php | 7 +++++- src/Psalm/Internal/Type/TypeParser.php | 4 +++- src/Psalm/Storage/FunctionLikeParameter.php | 2 ++ tests/CallableTest.php | 24 +++++++++++++++++++ 5 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/Psalm/Internal/Type/ParseTree/CallableParamTree.php b/src/Psalm/Internal/Type/ParseTree/CallableParamTree.php index 53bc98bbbc8..8cd9cf7354b 100644 --- a/src/Psalm/Internal/Type/ParseTree/CallableParamTree.php +++ b/src/Psalm/Internal/Type/ParseTree/CallableParamTree.php @@ -12,4 +12,11 @@ final class CallableParamTree extends ParseTree public bool $variadic = false; public bool $has_default = false; + + /** + * Param name, without the $ prefix + * + * @var null|non-empty-string + */ + public ?string $name = null; } diff --git a/src/Psalm/Internal/Type/ParseTreeCreator.php b/src/Psalm/Internal/Type/ParseTreeCreator.php index 1b652f09758..29e27602817 100644 --- a/src/Psalm/Internal/Type/ParseTreeCreator.php +++ b/src/Psalm/Internal/Type/ParseTreeCreator.php @@ -30,6 +30,7 @@ use function preg_match; use function strlen; use function strtolower; +use function substr; /** * @internal @@ -258,13 +259,17 @@ private function parseCallableParam(array $current_token, ParseTree $current_par $current_token = $this->t < $this->type_token_count ? $this->type_tokens[$this->t] : null; } - if (!$current_token || $current_token[0][0] !== '$') { + if (!$current_token || $current_token[0][0] !== '$' || strlen($current_token[0]) < 2) { throw new TypeParseTreeException('Unexpected token after space'); } $new_leaf = new CallableParamTree($current_parent); $new_leaf->has_default = $has_default; $new_leaf->variadic = $variadic; + $potential_name = substr($current_token[0], 1); + if ($potential_name !== false && $potential_name !== '') { + $new_leaf->name = $potential_name; + } if ($current_parent !== $this->current_leaf) { $new_leaf->children = [$this->current_leaf]; diff --git a/src/Psalm/Internal/Type/TypeParser.php b/src/Psalm/Internal/Type/TypeParser.php index fd807f584b1..f79cc3cfdc8 100644 --- a/src/Psalm/Internal/Type/TypeParser.php +++ b/src/Psalm/Internal/Type/TypeParser.php @@ -1277,6 +1277,7 @@ private static function getTypeFromCallableTree( foreach ($parse_tree->children as $child_tree) { $is_variadic = false; $is_optional = false; + $param_name = ''; if ($child_tree instanceof CallableParamTree) { if (isset($child_tree->children[0])) { @@ -1294,6 +1295,7 @@ private static function getTypeFromCallableTree( $is_variadic = $child_tree->variadic; $is_optional = $child_tree->has_default; + $param_name = $child_tree->name ?? ''; } else { if ($child_tree instanceof Value && strpos($child_tree->value, '$') > 0) { $child_tree->value = preg_replace('/(.+)\$.*/', '$1', $child_tree->value); @@ -1310,7 +1312,7 @@ private static function getTypeFromCallableTree( } $param = new FunctionLikeParameter( - '', + $param_name, false, $tree_type instanceof Union ? $tree_type : new Union([$tree_type]), null, diff --git a/src/Psalm/Storage/FunctionLikeParameter.php b/src/Psalm/Storage/FunctionLikeParameter.php index 39125c366bd..5078b15bbe8 100644 --- a/src/Psalm/Storage/FunctionLikeParameter.php +++ b/src/Psalm/Storage/FunctionLikeParameter.php @@ -15,6 +15,8 @@ final class FunctionLikeParameter implements HasAttributesInterface, TypeNode use UnserializeMemoryUsageSuppressionTrait; /** + * Parameter name, without `$` + * * @var string */ public $name; diff --git a/tests/CallableTest.php b/tests/CallableTest.php index fc8c36d212f..e10784b0b3f 100644 --- a/tests/CallableTest.php +++ b/tests/CallableTest.php @@ -1908,6 +1908,18 @@ function bar($cb_arg) {} foo("bar");', ], + 'callableWithNamedArguments' => [ + 'code' => <<<'PHP' + [], + 'ignored_issues' => [], + 'php_version' => '8.0', + ], ]; } @@ -2494,6 +2506,18 @@ function int_int_int_int_string(Closure $f): void {} 'ignored_issues' => [], 'php_version' => '8.0', ], + 'callableWithInvalidNamedArguments' => [ + 'code' => <<<'PHP' + 'InvalidNamedArgument', + 'ignored_issues' => [], + 'php_version' => '8.0', + ], ]; } } From edc123b219027e9f051c48c682da79a5104613b3 Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Sun, 3 Mar 2024 13:45:25 +0100 Subject: [PATCH 50/59] Backport `WeakMap` iterator fix from `master` Refs vimeo/psalm#10773 --- stubs/CoreGenericClasses.phpstub | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/stubs/CoreGenericClasses.phpstub b/stubs/CoreGenericClasses.phpstub index 62e0ff33750..542fec35875 100644 --- a/stubs/CoreGenericClasses.phpstub +++ b/stubs/CoreGenericClasses.phpstub @@ -492,6 +492,11 @@ final class WeakMap implements ArrayAccess, Countable, IteratorAggregate, Traver * @return void */ public function offsetUnset($offset) {} + + /** + * @return Traversable + */ + public function getIterator() { } } class mysqli From 963feb91a7ea70aed2220fa840d19a9d5c4794b9 Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Sat, 2 Mar 2024 20:53:40 +0100 Subject: [PATCH 51/59] fix PHP 8 tests running with wrong --php-version=/phpVersion= if not explicitly specified --- tests/CoreStubsTest.php | 4 ++-- tests/Traits/InvalidCodeAnalysisTestTrait.php | 10 +++++++++- tests/Traits/ValidCodeAnalysisTestTrait.php | 14 +++++++++++++- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/tests/CoreStubsTest.php b/tests/CoreStubsTest.php index 142be4a3e8a..c388fc12913 100644 --- a/tests/CoreStubsTest.php +++ b/tests/CoreStubsTest.php @@ -192,7 +192,7 @@ function foo(string $foo): string '$c3===' => 'bool', ], ]; - yield 'PHP8 str_* function assert non-empty-string' => [ + yield 'PHP80-str_* function assert non-empty-string' => [ 'code' => ' 'non-empty-string', ], ]; - yield "PHP8 str_* function doesn't subtract string after assertion" => [ + yield "PHP80-str_* function doesn't subtract string after assertion" => [ 'code' => 'getTestName(); if (strpos($test_name, 'PHP80-') !== false) { if (version_compare(PHP_VERSION, '8.0.0', '<')) { $this->markTestSkipped('Test case requires PHP 8.0.'); } + + if ($php_version === null) { + $php_version = '8.0'; + } } elseif (strpos($test_name, 'SKIPPED-') !== false) { $this->markTestSkipped('Skipped due to a bug.'); } + if ($php_version === null) { + $php_version = '7.4'; + } + // sanity check - do we have a PHP tag? if (strpos($code, 'fail('Test case must have a getTestName(); if (strpos($test_name, 'PHP80-') !== false) { if (version_compare(PHP_VERSION, '8.0.0', '<')) { $this->markTestSkipped('Test case requires PHP 8.0.'); } + + if ($php_version === null) { + $php_version = '8.0'; + } } elseif (strpos($test_name, 'PHP81-') !== false) { if (version_compare(PHP_VERSION, '8.1.0', '<')) { $this->markTestSkipped('Test case requires PHP 8.1.'); } + + if ($php_version === null) { + $php_version = '8.1'; + } } elseif (strpos($test_name, 'SKIPPED-') !== false) { $this->markTestSkipped('Skipped due to a bug.'); } + if ($php_version === null) { + $php_version = '7.4'; + } + // sanity check - do we have a PHP tag? if (strpos($code, 'fail('Test case must have a Date: Sat, 2 Mar 2024 21:00:11 +0100 Subject: [PATCH 52/59] set previous default setting for failing test temporarily Refs https://github.com/vimeo/psalm/issues/10773 --- tests/Template/ClassTemplateTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/Template/ClassTemplateTest.php b/tests/Template/ClassTemplateTest.php index 12c0025461a..9c492f5b69b 100644 --- a/tests/Template/ClassTemplateTest.php +++ b/tests/Template/ClassTemplateTest.php @@ -3498,6 +3498,10 @@ function writesLikeAnArray(WeakMap $wm): void { $wm[$ex] = 42; } ', + 'assertions' => [], + 'ignored_issues' => [], + // temporarily keep the old value until https://github.com/vimeo/psalm/issues/10773 is fixed by @weirdan + 'php_version' => '7.4', ], 'combineTwoTemplatedArrays' => [ 'code' => ' Date: Sun, 3 Mar 2024 15:16:39 +0100 Subject: [PATCH 53/59] Allow the test fixed in vimeo/psalm#10778 --- tests/Template/ClassTemplateTest.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/Template/ClassTemplateTest.php b/tests/Template/ClassTemplateTest.php index 9c492f5b69b..12c0025461a 100644 --- a/tests/Template/ClassTemplateTest.php +++ b/tests/Template/ClassTemplateTest.php @@ -3498,10 +3498,6 @@ function writesLikeAnArray(WeakMap $wm): void { $wm[$ex] = 42; } ', - 'assertions' => [], - 'ignored_issues' => [], - // temporarily keep the old value until https://github.com/vimeo/psalm/issues/10773 is fixed by @weirdan - 'php_version' => '7.4', ], 'combineTwoTemplatedArrays' => [ 'code' => ' Date: Sun, 3 Mar 2024 15:17:31 +0100 Subject: [PATCH 54/59] Use `PHP_VERSION_ID` instead of `version_compare()` As we do elsewhere --- tests/Traits/InvalidCodeAnalysisTestTrait.php | 5 ++--- tests/Traits/ValidCodeAnalysisTestTrait.php | 7 +++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/Traits/InvalidCodeAnalysisTestTrait.php b/tests/Traits/InvalidCodeAnalysisTestTrait.php index b837c40a0e2..bcc7f7be641 100644 --- a/tests/Traits/InvalidCodeAnalysisTestTrait.php +++ b/tests/Traits/InvalidCodeAnalysisTestTrait.php @@ -11,10 +11,9 @@ use function strpos; use function strtoupper; use function substr; -use function version_compare; use const PHP_OS; -use const PHP_VERSION; +use const PHP_VERSION_ID; /** * @psalm-type DeprecatedDataProviderArrayNotation = array{ @@ -53,7 +52,7 @@ public function testInvalidCode( ): void { $test_name = $this->getTestName(); if (strpos($test_name, 'PHP80-') !== false) { - if (version_compare(PHP_VERSION, '8.0.0', '<')) { + if (PHP_VERSION_ID < 8_00_00) { $this->markTestSkipped('Test case requires PHP 8.0.'); } diff --git a/tests/Traits/ValidCodeAnalysisTestTrait.php b/tests/Traits/ValidCodeAnalysisTestTrait.php index 370206423a9..5be98d38931 100644 --- a/tests/Traits/ValidCodeAnalysisTestTrait.php +++ b/tests/Traits/ValidCodeAnalysisTestTrait.php @@ -10,10 +10,9 @@ use function strpos; use function strtoupper; use function substr; -use function version_compare; use const PHP_OS; -use const PHP_VERSION; +use const PHP_VERSION_ID; trait ValidCodeAnalysisTestTrait { @@ -44,7 +43,7 @@ public function testValidCode( ): void { $test_name = $this->getTestName(); if (strpos($test_name, 'PHP80-') !== false) { - if (version_compare(PHP_VERSION, '8.0.0', '<')) { + if (PHP_VERSION_ID < 8_00_00) { $this->markTestSkipped('Test case requires PHP 8.0.'); } @@ -52,7 +51,7 @@ public function testValidCode( $php_version = '8.0'; } } elseif (strpos($test_name, 'PHP81-') !== false) { - if (version_compare(PHP_VERSION, '8.1.0', '<')) { + if (PHP_VERSION_ID < 8_01_00) { $this->markTestSkipped('Test case requires PHP 8.1.'); } From 6b9b4b523dcb7c42db5ae400a4ed4a3ef958225a Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Sun, 3 Mar 2024 16:18:48 +0100 Subject: [PATCH 55/59] Namespace anonymous classes Fixes vimeo/psalm#10755 --- src/Psalm/Internal/Analyzer/ClassAnalyzer.php | 25 +++++++++++++++---- .../Expression/Call/NewAnalyzer.php | 6 ++++- .../Reflector/ClassLikeNodeScanner.php | 2 +- tests/InternalAnnotationTest.php | 24 ++++++++++++++++++ 4 files changed, 50 insertions(+), 7 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/ClassAnalyzer.php b/src/Psalm/Internal/Analyzer/ClassAnalyzer.php index 0db77ba83e2..8bd9884bd2a 100644 --- a/src/Psalm/Internal/Analyzer/ClassAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/ClassAnalyzer.php @@ -123,7 +123,7 @@ public function __construct(PhpParser\Node\Stmt $class, SourceAnalyzer $source, throw new UnexpectedValueException('Anonymous enums are not allowed'); } - $fq_class_name = self::getAnonymousClassName($class, $source->getFilePath()); + $fq_class_name = self::getAnonymousClassName($class, $source->getAliases(), $source->getFilePath()); } parent::__construct($class, $source, $fq_class_name); @@ -137,10 +137,25 @@ public function __construct(PhpParser\Node\Stmt $class, SourceAnalyzer $source, } /** @return non-empty-string */ - public static function getAnonymousClassName(PhpParser\Node\Stmt\Class_ $class, string $file_path): string - { - return preg_replace('/[^A-Za-z0-9]/', '_', $file_path) - . '_' . $class->getLine() . '_' . (int)$class->getAttribute('startFilePos'); + public static function getAnonymousClassName( + PhpParser\Node\Stmt\Class_ $class, + Aliases $aliases, + string $file_path + ): string { + $class_name = preg_replace('/[^A-Za-z0-9]/', '_', $file_path) + . '_' . $class->getLine() + . '_' . (int)$class->getAttribute('startFilePos'); + + $fq_class_name = Type::getFQCLNFromString( + $class_name, + $aliases, + ); + + if ($fq_class_name === '') { + throw new LogicException('Invalid class name, should never happen'); + } + + return $fq_class_name; } public function analyze( diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php index db767a30339..019f159d403 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php @@ -159,7 +159,11 @@ public static function analyze( } } elseif ($stmt->class instanceof PhpParser\Node\Stmt\Class_) { $statements_analyzer->analyze([$stmt->class], $context); - $fq_class_name = ClassAnalyzer::getAnonymousClassName($stmt->class, $statements_analyzer->getFilePath()); + $fq_class_name = ClassAnalyzer::getAnonymousClassName( + $stmt->class, + $statements_analyzer->getAliases(), + $statements_analyzer->getFilePath(), + ); } else { self::analyzeConstructorExpression( $statements_analyzer, diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php index 828012fa790..14309b4349a 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php @@ -161,7 +161,7 @@ public function start(PhpParser\Node\Stmt\ClassLike $node): ?bool throw new LogicException('Anonymous classes are always classes'); } - $fq_classlike_name = ClassAnalyzer::getAnonymousClassName($node, $this->file_path); + $fq_classlike_name = ClassAnalyzer::getAnonymousClassName($node, $this->aliases, $this->file_path); } else { $name_location = new CodeLocation($this->file_scanner, $node->name); diff --git a/tests/InternalAnnotationTest.php b/tests/InternalAnnotationTest.php index 7d0341e8d0c..dc42641d6d5 100644 --- a/tests/InternalAnnotationTest.php +++ b/tests/InternalAnnotationTest.php @@ -606,6 +606,30 @@ public function baz(): void } ', ], + 'callToInternalMethodFromAnonymousClass' => [ + 'code' => <<<'PHP' + a(); + } + }; + PHP, + ], ]; } From 33071242aa27acb91132d505f936e57b99adec69 Mon Sep 17 00:00:00 2001 From: Theodore Brown Date: Thu, 18 Jan 2024 18:07:31 -0600 Subject: [PATCH 56/59] Update CallMap for sqlsrv_connect and sqlsrv_errors to match reflection --- dictionaries/CallMap.php | 4 ++-- dictionaries/CallMap_historical.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dictionaries/CallMap.php b/dictionaries/CallMap.php index 87a8e310855..0c19300cead 100644 --- a/dictionaries/CallMap.php +++ b/dictionaries/CallMap.php @@ -12640,8 +12640,8 @@ 'sqlsrv_close' => ['bool', 'conn'=>'?resource'], 'sqlsrv_commit' => ['bool', 'conn'=>'resource'], 'sqlsrv_configure' => ['bool', 'setting'=>'string', 'value'=>'mixed'], -'sqlsrv_connect' => ['resource|false', 'serverName'=>'string', 'connectionInfo='=>'array'], -'sqlsrv_errors' => ['?array', 'errorsAndOrWarnings='=>'int'], +'sqlsrv_connect' => ['resource|false', 'server_name'=>'string', 'connection_info='=>'array'], +'sqlsrv_errors' => ['?array', 'errors_and_or_warnings='=>'int'], 'sqlsrv_execute' => ['bool', 'stmt'=>'resource'], 'sqlsrv_fetch' => ['?bool', 'stmt'=>'resource', 'row='=>'int', 'offset='=>'int'], 'sqlsrv_fetch_array' => ['array|null|false', 'stmt'=>'resource', 'fetchType='=>'int', 'row='=>'int', 'offset='=>'int'], diff --git a/dictionaries/CallMap_historical.php b/dictionaries/CallMap_historical.php index d8669ba257e..92746fd697b 100644 --- a/dictionaries/CallMap_historical.php +++ b/dictionaries/CallMap_historical.php @@ -14070,8 +14070,8 @@ 'sqlsrv_close' => ['bool', 'conn'=>'?resource'], 'sqlsrv_commit' => ['bool', 'conn'=>'resource'], 'sqlsrv_configure' => ['bool', 'setting'=>'string', 'value'=>'mixed'], - 'sqlsrv_connect' => ['resource|false', 'serverName'=>'string', 'connectionInfo='=>'array'], - 'sqlsrv_errors' => ['?array', 'errorsAndOrWarnings='=>'int'], + 'sqlsrv_connect' => ['resource|false', 'server_name'=>'string', 'connection_info='=>'array'], + 'sqlsrv_errors' => ['?array', 'errors_and_or_warnings='=>'int'], 'sqlsrv_execute' => ['bool', 'stmt'=>'resource'], 'sqlsrv_fetch' => ['?bool', 'stmt'=>'resource', 'row='=>'int', 'offset='=>'int'], 'sqlsrv_fetch_array' => ['array|null|false', 'stmt'=>'resource', 'fetchType='=>'int', 'row='=>'int', 'offset='=>'int'], From 8e1aa46195fe67288c9f29579174fc2338733af1 Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Sat, 9 Mar 2024 18:41:51 +0100 Subject: [PATCH 57/59] CS fix - trailing commas in calls with anon functions - useless array docblock Refs https://github.com/slevomat/coding-standard/commit/8d0f603befc9dbc50735dc5cb4ef62015a6d915b Refs https://github.com/slevomat/coding-standard/commit/6ea0278a67dbcdf605a48dc28cf5b9c6baa91a2c --- src/Psalm/Internal/Analyzer/ClassAnalyzer.php | 2 +- .../Analyzer/FunctionLike/ReturnTypeAnalyzer.php | 2 +- .../Statements/Block/IfConditionalAnalyzer.php | 2 +- .../Statements/Block/IfElse/ElseIfAnalyzer.php | 2 +- .../Analyzer/Statements/Block/IfElseAnalyzer.php | 2 +- .../Statements/Expression/BinaryOp/AndAnalyzer.php | 2 +- .../Statements/Expression/BinaryOp/OrAnalyzer.php | 2 +- .../Method/ExistingAtomicMethodCallAnalyzer.php | 4 ++-- .../Expression/Call/MethodCallAnalyzer.php | 2 +- .../Call/StaticMethod/AtomicStaticCallAnalyzer.php | 2 +- .../Statements/Expression/TernaryAnalyzer.php | 2 +- src/Psalm/Internal/Cli/LanguageServer.php | 2 +- src/Psalm/Internal/Cli/Psalm.php | 4 ++-- src/Psalm/Internal/Cli/Psalter.php | 2 +- src/Psalm/Internal/Cli/Refactor.php | 2 +- src/Psalm/Internal/CliUtils.php | 2 +- src/Psalm/Internal/Codebase/ClassLikes.php | 4 ++-- src/Psalm/Internal/Fork/PsalmRestarter.php | 2 +- .../Internal/LanguageServer/LanguageClient.php | 2 -- .../PhpVisitor/Reflector/ClassLikeNodeScanner.php | 2 +- .../Reflector/FunctionLikeDocblockScanner.php | 2 +- .../Internal/Provider/FileReferenceProvider.php | 2 +- .../ArrayFilterReturnTypeProvider.php | 2 +- src/Psalm/Internal/Type/TypeExpander.php | 2 +- src/Psalm/IssueBuffer.php | 2 +- src/Psalm/Type/Atomic.php | 8 ++++---- src/Psalm/Type/UnionTrait.php | 14 +++++++------- tests/DocumentationTest.php | 2 +- tests/LanguageServer/Message.php | 2 -- 29 files changed, 39 insertions(+), 43 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/ClassAnalyzer.php b/src/Psalm/Internal/Analyzer/ClassAnalyzer.php index 8bd9884bd2a..fead38b71d0 100644 --- a/src/Psalm/Internal/Analyzer/ClassAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/ClassAnalyzer.php @@ -946,7 +946,7 @@ public static function addContextProperties( $stmts, static fn($stmt): bool => $stmt instanceof PhpParser\Node\Stmt\Property && isset($stmt->props[0]->name->name) - && $stmt->props[0]->name->name === $property_name + && $stmt->props[0]->name->name === $property_name, ); $suppressed = []; diff --git a/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php index b752d9176ff..763bd3e2f01 100644 --- a/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php @@ -245,7 +245,7 @@ public static function verifyReturnType( if (count($inferred_return_type_parts) > 1) { $inferred_return_type_parts = array_filter( $inferred_return_type_parts, - static fn(Union $union_type): bool => !$union_type->isNever() + static fn(Union $union_type): bool => !$union_type->isNever(), ); } $inferred_return_type_parts = array_values($inferred_return_type_parts); diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php index fa1970e3ae6..c98700f071a 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php @@ -78,7 +78,7 @@ public static function analyze( $entry_clauses, static fn(Clause $c): bool => count($c->possibilities) > 1 || $c->wedge - || !isset($changed_var_ids[array_key_first($c->possibilities)]) + || !isset($changed_var_ids[array_key_first($c->possibilities)]), ), ); } diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php index 00c7895c2be..e1378100049 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php @@ -144,7 +144,7 @@ public static function analyze( $elseif_context_clauses = array_values( array_filter( $elseif_context_clauses, - static fn(Clause $c): bool => !in_array($c->hash, $reconciled_expression_clauses, true) + static fn(Clause $c): bool => !in_array($c->hash, $reconciled_expression_clauses, true), ), ); } diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php index b4648cd59f2..99ef6c45291 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php @@ -182,7 +182,7 @@ public static function analyze( $if_context->clauses = array_values( array_filter( $if_context->clauses, - static fn(Clause $c): bool => !in_array($c->hash, $reconciled_expression_clauses) + static fn(Clause $c): bool => !in_array($c->hash, $reconciled_expression_clauses), ), ); diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/AndAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/AndAnalyzer.php index d721f481587..982cb4d78c7 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/AndAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/AndAnalyzer.php @@ -105,7 +105,7 @@ public static function analyze( $context_clauses = array_values( array_filter( $context_clauses, - static fn(Clause $c): bool => !in_array($c->hash, $reconciled_expression_clauses, true) + static fn(Clause $c): bool => !in_array($c->hash, $reconciled_expression_clauses, true), ), ); diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php index 2a2e0cf8217..ed716f66e91 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php @@ -173,7 +173,7 @@ public static function analyze( $negated_left_clauses = array_values( array_filter( $negated_left_clauses, - static fn(Clause $c): bool => !in_array($c->hash, $reconciled_expression_clauses) + static fn(Clause $c): bool => !in_array($c->hash, $reconciled_expression_clauses), ), ); diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/ExistingAtomicMethodCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/ExistingAtomicMethodCallAnalyzer.php index bf8d1af4239..6fe0bd3cf80 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/ExistingAtomicMethodCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/ExistingAtomicMethodCallAnalyzer.php @@ -446,7 +446,7 @@ public static function analyze( $possibilities, static fn(Possibilities $assertion): bool => !(is_string($assertion->var_id) && strpos($assertion->var_id, '$this->') === 0 - ) + ), ); } $statements_analyzer->node_data->setIfTrueAssertions( @@ -469,7 +469,7 @@ public static function analyze( $possibilities, static fn(Possibilities $assertion): bool => !(is_string($assertion->var_id) && strpos($assertion->var_id, '$this->') === 0 - ) + ), ); } $statements_analyzer->node_data->setIfFalseAssertions( diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php index ceff9788724..53bbb8097bf 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php @@ -238,7 +238,7 @@ public static function analyze( if (count($possible_new_class_types) > 0) { $class_type = array_reduce( $possible_new_class_types, - static fn(?Union $type_1, Union $type_2): Union => Type::combineUnionTypes($type_1, $type_2, $codebase) + static fn(?Union $type_1, Union $type_2): Union => Type::combineUnionTypes($type_1, $type_2, $codebase), ); } 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 5056d25b1f9..48c50ed0c24 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php @@ -419,7 +419,7 @@ private static function handleNamedCall( $mixin_candidates_no_generic = array_filter( $mixin_candidates, - static fn(Atomic $check): bool => !($check instanceof TGenericObject) + static fn(Atomic $check): bool => !($check instanceof TGenericObject), ); // $mixin_candidates_no_generic will only be empty when there are TGenericObject entries. diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php index 7524c523baa..24133ac4d86 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php @@ -139,7 +139,7 @@ static function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause { $ternary_context_clauses = array_values( array_filter( $ternary_context_clauses, - static fn(Clause $c): bool => !in_array($c->hash, $reconciled_expression_clauses) + static fn(Clause $c): bool => !in_array($c->hash, $reconciled_expression_clauses), ), ); diff --git a/src/Psalm/Internal/Cli/LanguageServer.php b/src/Psalm/Internal/Cli/LanguageServer.php index 0fa174eff1f..99a8521f004 100644 --- a/src/Psalm/Internal/Cli/LanguageServer.php +++ b/src/Psalm/Internal/Cli/LanguageServer.php @@ -282,7 +282,7 @@ static function (string $arg) use ($valid_long_options): void { // we ignore the FQN because of a hack in scoper.inc that needs full path // phpcs:ignore SlevomatCodingStandard.Namespaces.ReferenceUsedNamesOnly.ReferenceViaFullyQualifiedName static fn(): ?\Composer\Autoload\ClassLoader => - CliUtils::requireAutoloaders($current_dir, isset($options['r']), $vendor_dir) + CliUtils::requireAutoloaders($current_dir, isset($options['r']), $vendor_dir), ); if (array_key_exists('v', $options)) { diff --git a/src/Psalm/Internal/Cli/Psalm.php b/src/Psalm/Internal/Cli/Psalm.php index 5aacced0f61..30b6679ecd4 100644 --- a/src/Psalm/Internal/Cli/Psalm.php +++ b/src/Psalm/Internal/Cli/Psalm.php @@ -226,7 +226,7 @@ public static function run(array $argv): void // we ignore the FQN because of a hack in scoper.inc that needs full path // phpcs:ignore SlevomatCodingStandard.Namespaces.ReferenceUsedNamesOnly.ReferenceViaFullyQualifiedName static fn(): ?\Composer\Autoload\ClassLoader => - CliUtils::requireAutoloaders($current_dir, isset($options['r']), $vendor_dir) + CliUtils::requireAutoloaders($current_dir, isset($options['r']), $vendor_dir), ); $run_taint_analysis = self::shouldRunTaintAnalysis($options); @@ -502,7 +502,7 @@ private static function generateConfig(string $current_dir, array &$args): void && $arg !== '--debug-emitted-issues' && strpos($arg, '--disable-extension=') !== 0 && strpos($arg, '--root=') !== 0 - && strpos($arg, '--r=') !== 0 + && strpos($arg, '--r=') !== 0, )); $init_level = null; diff --git a/src/Psalm/Internal/Cli/Psalter.php b/src/Psalm/Internal/Cli/Psalter.php index d42a1f10843..db9810ea12c 100644 --- a/src/Psalm/Internal/Cli/Psalter.php +++ b/src/Psalm/Internal/Cli/Psalter.php @@ -221,7 +221,7 @@ public static function run(array $argv): void // we ignore the FQN because of a hack in scoper.inc that needs full path // phpcs:ignore SlevomatCodingStandard.Namespaces.ReferenceUsedNamesOnly.ReferenceViaFullyQualifiedName static fn(): ?\Composer\Autoload\ClassLoader => - CliUtils::requireAutoloaders($current_dir, isset($options['r']), $vendor_dir) + CliUtils::requireAutoloaders($current_dir, isset($options['r']), $vendor_dir), ); $ini_handler = new PsalmRestarter('PSALTER'); $ini_handler->disableExtensions([ diff --git a/src/Psalm/Internal/Cli/Refactor.php b/src/Psalm/Internal/Cli/Refactor.php index b23bcb863c2..9ffc04d943a 100644 --- a/src/Psalm/Internal/Cli/Refactor.php +++ b/src/Psalm/Internal/Cli/Refactor.php @@ -192,7 +192,7 @@ static function (string $arg) use ($valid_long_options): void { // we ignore the FQN because of a hack in scoper.inc that needs full path // phpcs:ignore SlevomatCodingStandard.Namespaces.ReferenceUsedNamesOnly.ReferenceViaFullyQualifiedName static fn(): ?\Composer\Autoload\ClassLoader => - CliUtils::requireAutoloaders($current_dir, isset($options['r']), $vendor_dir) + CliUtils::requireAutoloaders($current_dir, isset($options['r']), $vendor_dir), ); // If Xdebug is enabled, restart without it diff --git a/src/Psalm/Internal/CliUtils.php b/src/Psalm/Internal/CliUtils.php index 8f0f1fbf9cb..92e7286e342 100644 --- a/src/Psalm/Internal/CliUtils.php +++ b/src/Psalm/Internal/CliUtils.php @@ -549,7 +549,7 @@ public static function checkRuntimeRequirements(): void $missing_extensions = array_filter( $required_extensions, - static fn(string $ext) => !extension_loaded($ext) + static fn(string $ext) => !extension_loaded($ext), ); if ($missing_extensions) { diff --git a/src/Psalm/Internal/Codebase/ClassLikes.php b/src/Psalm/Internal/Codebase/ClassLikes.php index 552fab265d5..d65853cfc3b 100644 --- a/src/Psalm/Internal/Codebase/ClassLikes.php +++ b/src/Psalm/Internal/Codebase/ClassLikes.php @@ -1599,7 +1599,7 @@ public function getConstantsForClass(string $class_name, int $visibility): array $storage->constants, static fn(ClassConstantStorage $constant): bool => $constant->type && ($constant->visibility === ClassLikeAnalyzer::VISIBILITY_PUBLIC - || $constant->visibility === ClassLikeAnalyzer::VISIBILITY_PROTECTED) + || $constant->visibility === ClassLikeAnalyzer::VISIBILITY_PROTECTED), ); } @@ -2410,7 +2410,7 @@ private function getConstantType( fn(ClassConstantStorage $resolved_constant) => $this->filterConstantNameByVisibility( $resolved_constant, $visibility, - ) + ), ); if ($filtered_constants_by_visibility === []) { diff --git a/src/Psalm/Internal/Fork/PsalmRestarter.php b/src/Psalm/Internal/Fork/PsalmRestarter.php index af4d83776b8..6d1dada4c3a 100644 --- a/src/Psalm/Internal/Fork/PsalmRestarter.php +++ b/src/Psalm/Internal/Fork/PsalmRestarter.php @@ -61,7 +61,7 @@ protected function requiresRestart($default): bool { $this->required = (bool) array_filter( $this->disabled_extensions, - static fn(string $extension): bool => extension_loaded($extension) + static fn(string $extension): bool => extension_loaded($extension), ); $opcache_loaded = extension_loaded('opcache') || extension_loaded('Zend OPcache'); diff --git a/src/Psalm/Internal/LanguageServer/LanguageClient.php b/src/Psalm/Internal/LanguageServer/LanguageClient.php index 4575aa3d575..d20b17ec94e 100644 --- a/src/Psalm/Internal/LanguageServer/LanguageClient.php +++ b/src/Psalm/Internal/LanguageServer/LanguageClient.php @@ -145,8 +145,6 @@ public function makeProgress(string $token): ProgressInterface /** * Configuration Refreshed from Client - * - * @param array $config */ private function configurationRefreshed(array $config): void { diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php index 14309b4349a..b0f4003c49a 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php @@ -422,7 +422,7 @@ public function start(PhpParser\Node\Stmt\ClassLike $node): ?bool usort( $docblock_info->templates, - static fn(array $l, array $r): int => $l[4] > $r[4] ? 1 : -1 + static fn(array $l, array $r): int => $l[4] > $r[4] ? 1 : -1, ); foreach ($docblock_info->templates as $i => $template_map) { diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php index cfeb0b15f30..313a3713af4 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php @@ -917,7 +917,7 @@ private static function improveParamsFromDocblock( $params_without_docblock_type = array_filter( $storage->params, - static fn(FunctionLikeParameter $p): bool => !$p->has_docblock_type && (!$p->type || $p->type->hasArray()) + static fn(FunctionLikeParameter $p): bool => !$p->has_docblock_type && (!$p->type || $p->type->hasArray()), ); if ($params_without_docblock_type) { diff --git a/src/Psalm/Internal/Provider/FileReferenceProvider.php b/src/Psalm/Internal/Provider/FileReferenceProvider.php index b89a2604128..87a77167184 100644 --- a/src/Psalm/Internal/Provider/FileReferenceProvider.php +++ b/src/Psalm/Internal/Provider/FileReferenceProvider.php @@ -180,7 +180,7 @@ public function getDeletedReferencedFiles(): array if (self::$deleted_files === null) { self::$deleted_files = array_filter( array_keys(self::$file_references), - fn(string $file_name): bool => !$this->file_provider->fileExists($file_name) + fn(string $file_name): bool => !$this->file_provider->fileExists($file_name), ); } diff --git a/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php b/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php index ada2e2825c6..d5d32ac58c8 100644 --- a/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php +++ b/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php @@ -106,7 +106,7 @@ static function ($keyed_type) use ($statements_source, $context) { }, $first_arg_array->properties, ), - static fn($keyed_type) => !$keyed_type->isNever() + static fn($keyed_type) => !$keyed_type->isNever(), ); if (!$new_properties) { diff --git a/src/Psalm/Internal/Type/TypeExpander.php b/src/Psalm/Internal/Type/TypeExpander.php index 0855a1ab732..8f7225ef37a 100644 --- a/src/Psalm/Internal/Type/TypeExpander.php +++ b/src/Psalm/Internal/Type/TypeExpander.php @@ -634,7 +634,7 @@ private static function expandNamedObject( if ($container_class_storage->template_types && array_filter( $container_class_storage->template_types, - static fn($type_map): bool => !reset($type_map)->hasMixed() + static fn($type_map): bool => !reset($type_map)->hasMixed(), ) ) { $return_type = new TGenericObject( diff --git a/src/Psalm/IssueBuffer.php b/src/Psalm/IssueBuffer.php index 33fc16bc800..45f8d53e9cf 100644 --- a/src/Psalm/IssueBuffer.php +++ b/src/Psalm/IssueBuffer.php @@ -575,7 +575,7 @@ public static function finish( $file_issues, static fn(IssueData $d1, IssueData $d2): int => [$d1->file_path, $d1->line_from, $d1->column_from] <=> - [$d2->file_path, $d2->line_from, $d2->column_from] + [$d2->file_path, $d2->line_from, $d2->column_from], ); self::$issues_data[$file_path] = $file_issues; } diff --git a/src/Psalm/Type/Atomic.php b/src/Psalm/Type/Atomic.php index 46092ec8162..ee0b4d7ca57 100644 --- a/src/Psalm/Type/Atomic.php +++ b/src/Psalm/Type/Atomic.php @@ -453,7 +453,7 @@ public function isNamedObjectType(): bool && ($this->as->hasNamedObjectType() || array_filter( $this->extra_types, - static fn($extra_type): bool => $extra_type->isNamedObjectType() + static fn($extra_type): bool => $extra_type->isNamedObjectType(), ) ) ); @@ -545,7 +545,7 @@ public function hasTraversableInterface(Codebase $codebase): bool $this->extra_types && array_filter( $this->extra_types, - static fn(Atomic $a): bool => $a->hasTraversableInterface($codebase) + static fn(Atomic $a): bool => $a->hasTraversableInterface($codebase), ) ) ); @@ -568,7 +568,7 @@ public function hasCountableInterface(Codebase $codebase): bool $this->extra_types && array_filter( $this->extra_types, - static fn(Atomic $a): bool => $a->hasCountableInterface($codebase) + static fn(Atomic $a): bool => $a->hasCountableInterface($codebase), ) ) ); @@ -606,7 +606,7 @@ public function hasArrayAccessInterface(Codebase $codebase): bool $this->extra_types && array_filter( $this->extra_types, - static fn(Atomic $a): bool => $a->hasArrayAccessInterface($codebase) + static fn(Atomic $a): bool => $a->hasArrayAccessInterface($codebase), ) ) ); diff --git a/src/Psalm/Type/UnionTrait.php b/src/Psalm/Type/UnionTrait.php index a500789cf41..b471b795df3 100644 --- a/src/Psalm/Type/UnionTrait.php +++ b/src/Psalm/Type/UnionTrait.php @@ -381,7 +381,7 @@ public function canBeFullyExpressedInPhp(int $analysis_php_version_id): bool return !array_filter( $types, - static fn($atomic_type): bool => !$atomic_type->canBeFullyExpressedInPhp($analysis_php_version_id) + static fn($atomic_type): bool => !$atomic_type->canBeFullyExpressedInPhp($analysis_php_version_id), ); } @@ -459,7 +459,7 @@ public function hasArrayAccessInterface(Codebase $codebase): bool { return (bool)array_filter( $this->types, - static fn($type): bool => $type->hasArrayAccessInterface($codebase) + static fn($type): bool => $type->hasArrayAccessInterface($codebase), ); } @@ -750,7 +750,7 @@ public function hasTemplate(): bool $type->extra_types, static fn($t): bool => $t instanceof TTemplateParam, ) - ) + ), ); } @@ -782,7 +782,7 @@ public function hasTemplateOrStatic(): bool ) ) ) - ) + ), ); } @@ -993,7 +993,7 @@ public function isInt(bool $check_templates = false): bool || ($check_templates && $type instanceof TTemplateParam && $type->as->isInt() - ) + ), ), ) === count($this->types); } @@ -1024,7 +1024,7 @@ public function isString(bool $check_templates = false): bool || ($check_templates && $type instanceof TTemplateParam && $type->as->isString() - ) + ), ), ) === count($this->types); } @@ -1043,7 +1043,7 @@ public function isNonEmptyString(bool $check_templates = false): bool || ($check_templates && $type instanceof TTemplateParam && $type->as->isNonEmptyString() - ) + ), ), ) === count($this->types); } diff --git a/tests/DocumentationTest.php b/tests/DocumentationTest.php index 74c0cb6eae6..aff06fde78b 100644 --- a/tests/DocumentationTest.php +++ b/tests/DocumentationTest.php @@ -349,7 +349,7 @@ public function testShortcodesAreUnique(): void $duplicate_shortcodes = array_filter( $all_shortcodes, - static fn($issues): bool => count($issues) > 1 + static fn($issues): bool => count($issues) > 1, ); $this->assertEquals( diff --git a/tests/LanguageServer/Message.php b/tests/LanguageServer/Message.php index 075a50a8134..2067c901d29 100644 --- a/tests/LanguageServer/Message.php +++ b/tests/LanguageServer/Message.php @@ -17,8 +17,6 @@ abstract class Message extends AdvancedJsonRpcMessage { /** * Returns the appropriate Message subclass - * - * @param array $msg */ public static function parseArray(array $msg): AdvancedJsonRpcMessage { From 5c780c7b055a50a59c91815e9940bda915773560 Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Sat, 9 Mar 2024 20:06:47 +0100 Subject: [PATCH 58/59] `$resource` parameter of `mkdir()` is nullable since PHP 7.3 Fixes vimeo/psalm#10796 --- dictionaries/CallMap.php | 2 +- dictionaries/CallMap_73_delta.php | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/dictionaries/CallMap.php b/dictionaries/CallMap.php index 0c19300cead..80de3809a85 100644 --- a/dictionaries/CallMap.php +++ b/dictionaries/CallMap.php @@ -6857,7 +6857,7 @@ 'ming_setswfcompression' => ['void', 'level'=>'int'], 'ming_useconstants' => ['void', 'use'=>'int'], 'ming_useswfversion' => ['void', 'version'=>'int'], -'mkdir' => ['bool', 'directory'=>'string', 'permissions='=>'int', 'recursive='=>'bool', 'context='=>'resource'], +'mkdir' => ['bool', 'directory'=>'string', 'permissions='=>'int', 'recursive='=>'bool', 'context='=>'null|resource'], 'mktime' => ['int|false', 'hour'=>'int', 'minute='=>'int|null', 'second='=>'int|null', 'month='=>'int|null', 'day='=>'int|null', 'year='=>'int|null'], 'money_format' => ['string', 'format'=>'string', 'value'=>'float'], 'Mongo::__construct' => ['void', 'server='=>'string', 'options='=>'array', 'driver_options='=>'array'], diff --git a/dictionaries/CallMap_73_delta.php b/dictionaries/CallMap_73_delta.php index 6352fa06292..523bd28444c 100644 --- a/dictionaries/CallMap_73_delta.php +++ b/dictionaries/CallMap_73_delta.php @@ -116,6 +116,10 @@ 'old' => ['resource[]|resource|false', 'ldap'=>'resource|resource[]', 'base'=>'array|string', 'filter'=>'array|string', 'attributes='=>'array', 'attributes_only='=>'int', 'sizelimit='=>'int', 'timelimit='=>'int', 'deref='=>'int'], 'new' => ['resource[]|resource|false', 'ldap'=>'resource|resource[]', 'base'=>'array|string', 'filter'=>'array|string', 'attributes='=>'array', 'attributes_only='=>'int', 'sizelimit='=>'int', 'timelimit='=>'int', 'deref='=>'int', 'controls='=>'array'], ], + 'mkdir' => [ + 'old' => ['bool', 'directory'=>'string', 'permissions='=>'int', 'recursive='=>'bool', 'context='=>'resource'], + 'new' => ['bool', 'directory'=>'string', 'permissions='=>'int', 'recursive='=>'bool', 'context='=>'null|resource'], + ], ], 'removed' => [ ], From 739d87dba7de72e290fe3aeb6e8e9fd3a8cc163f Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Sat, 9 Mar 2024 18:25:46 +0100 Subject: [PATCH 59/59] 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')) {