From 82ff58228092cb15f6bd17adfba5aa607179a4a9 Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Tue, 12 Dec 2023 23:29:54 +0100 Subject: [PATCH 1/2] add error for invalid array key type in docblock --- src/Psalm/Internal/Type/TypeParser.php | 58 ++++++++++++++++++++++++++ tests/AnnotationTest.php | 10 ++++- tests/ArrayAssignmentTest.php | 9 ++-- tests/KeyOfArrayTest.php | 18 ++++---- 4 files changed, 81 insertions(+), 14 deletions(-) diff --git a/src/Psalm/Internal/Type/TypeParser.php b/src/Psalm/Internal/Type/TypeParser.php index 770e7efc958..b498c9944e5 100644 --- a/src/Psalm/Internal/Type/TypeParser.php +++ b/src/Psalm/Internal/Type/TypeParser.php @@ -50,11 +50,13 @@ use Psalm\Type\Atomic\TLiteralInt; use Psalm\Type\Atomic\TMixed; use Psalm\Type\Atomic\TNamedObject; +use Psalm\Type\Atomic\TNever; use Psalm\Type\Atomic\TNonEmptyArray; use Psalm\Type\Atomic\TNull; use Psalm\Type\Atomic\TObject; use Psalm\Type\Atomic\TObjectWithProperties; use Psalm\Type\Atomic\TPropertiesOf; +use Psalm\Type\Atomic\TString; use Psalm\Type\Atomic\TTemplateIndexedAccess; use Psalm\Type\Atomic\TTemplateKeyOf; use Psalm\Type\Atomic\TTemplateParam; @@ -643,6 +645,34 @@ private static function getTypeFromGenericTree( throw new TypeParseTreeException('Too many template parameters for array'); } + foreach ($generic_params[0]->getAtomicTypes() as $key => $atomic_type) { + if ($atomic_type instanceof TInt + || $atomic_type instanceof TString + || $atomic_type instanceof TArrayKey + || $atomic_type instanceof TClassConstant // @todo resolve and check types + || $atomic_type instanceof TMixed + || $atomic_type instanceof TNever + || $atomic_type instanceof TTemplateParam + || $atomic_type instanceof TValueOf + ) { + continue; + } + + if ($codebase->register_stub_files || $codebase->register_autoload_files) { + $builder = $generic_params[0]->getBuilder(); + $builder->removeType($key); + + if (count($generic_params[0]->getAtomicTypes()) <= 1) { + $builder = $builder->addType(new TArrayKey($from_docblock)); + } + + $generic_params[0] = $builder->freeze(); + continue; + } + + throw new TypeParseTreeException('Invalid array key type ' . $atomic_type->getKey()); + } + return new TArray($generic_params, $from_docblock); } @@ -671,6 +701,34 @@ private static function getTypeFromGenericTree( throw new TypeParseTreeException('Too many template parameters for non-empty-array'); } + foreach ($generic_params[0]->getAtomicTypes() as $key => $atomic_type) { + if ($atomic_type instanceof TInt + || $atomic_type instanceof TString + || $atomic_type instanceof TArrayKey + || $atomic_type instanceof TClassConstant // @todo resolve and check types + || $atomic_type instanceof TMixed + || $atomic_type instanceof TNever + || $atomic_type instanceof TTemplateParam + || $atomic_type instanceof TValueOf + ) { + continue; + } + + if ($codebase->register_stub_files || $codebase->register_autoload_files) { + $builder = $generic_params[0]->getBuilder(); + $builder->removeType($key); + + if (count($generic_params[0]->getAtomicTypes()) <= 1) { + $builder = $builder->addType(new TArrayKey($from_docblock)); + } + + $generic_params[0] = $builder->freeze(); + continue; + } + + throw new TypeParseTreeException('Invalid array key type ' . $atomic_type->getKey()); + } + return new TNonEmptyArray($generic_params, null, null, 'non-empty-array', $from_docblock); } diff --git a/tests/AnnotationTest.php b/tests/AnnotationTest.php index b06bd217e7f..e0855c3bdc8 100644 --- a/tests/AnnotationTest.php +++ b/tests/AnnotationTest.php @@ -1366,7 +1366,15 @@ public function barBar() { }', 'error_message' => 'MissingDocblockType', ], - + 'invalidArrayKeyType' => [ + 'code' => ' $arg + * @return void + */ + function foo($arg) {}', + 'error_message' => 'InvalidDocblock', + ], 'invalidClassMethodReturnBrackets' => [ 'code' => ' [ 'code' => ' 'InvalidArrayOffset', + 'error_message' => 'MixedArrayAccess', + 'ignored_issues' => ['InvalidDocblock'], ], 'unpackTypedIterableWithStringKeysIntoArray' => [ 'code' => ' [ 'code' => '|array> + * @return key-of|array> */ - function getKey(bool $asFloat) { - if ($asFloat) { - return 42.0; + function getKey(bool $asString) { + if ($asString) { + return "42"; } return 42; } @@ -194,14 +194,14 @@ public function getKey() { ', 'error_message' => 'InvalidReturnStatement', ], - 'noStringAllowedInKeyOfIntFloatArray' => [ + 'noStringAllowedInKeyOfIntFloatStringArray' => [ 'code' => '|array> + * @return key-of|array<"42.0", string>> */ - function getKey(bool $asFloat) { - if ($asFloat) { - return 42.0; + function getKey(bool $asInt) { + if ($asInt) { + return 42; } return "42"; } From 9be7fceb594eb1fdfe9886c39c54db48a815afca Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Wed, 13 Dec 2023 00:17:43 +0100 Subject: [PATCH 2/2] Fix literal string keys int not handled as int as PHP does Fix https://github.com/vimeo/psalm/issues/8680 See also https://github.com/vimeo/psalm/issues/9295 --- src/Psalm/Internal/Type/TypeParser.php | 29 ++++++++++++++ tests/ArrayAssignmentTest.php | 23 +++++++++++ tests/ArrayKeysTest.php | 55 ++++++++++++++++++++++++++ 3 files changed, 107 insertions(+) diff --git a/src/Psalm/Internal/Type/TypeParser.php b/src/Psalm/Internal/Type/TypeParser.php index b498c9944e5..58718eae2dd 100644 --- a/src/Psalm/Internal/Type/TypeParser.php +++ b/src/Psalm/Internal/Type/TypeParser.php @@ -48,6 +48,7 @@ use Psalm\Type\Atomic\TLiteralClassString; use Psalm\Type\Atomic\TLiteralFloat; use Psalm\Type\Atomic\TLiteralInt; +use Psalm\Type\Atomic\TLiteralString; use Psalm\Type\Atomic\TMixed; use Psalm\Type\Atomic\TNamedObject; use Psalm\Type\Atomic\TNever; @@ -85,6 +86,7 @@ use function defined; use function end; use function explode; +use function filter_var; use function get_class; use function in_array; use function is_int; @@ -98,6 +100,9 @@ use function strtolower; use function strtr; use function substr; +use function trim; + +use const FILTER_VALIDATE_INT; /** * @psalm-suppress InaccessibleProperty Allowed during construction @@ -646,6 +651,18 @@ private static function getTypeFromGenericTree( } foreach ($generic_params[0]->getAtomicTypes() as $key => $atomic_type) { + // PHP 8 values with whitespace after number are counted as numeric + // and filter_var treats them as such too + if ($atomic_type instanceof TLiteralString + && trim($atomic_type->value) === $atomic_type->value + && ($string_to_int = filter_var($atomic_type->value, FILTER_VALIDATE_INT)) !== false + ) { + $builder = $generic_params[0]->getBuilder(); + $builder->removeType($key); + $generic_params[0] = $builder->addType(new TLiteralInt($string_to_int, $from_docblock))->freeze(); + continue; + } + if ($atomic_type instanceof TInt || $atomic_type instanceof TString || $atomic_type instanceof TArrayKey @@ -702,6 +719,18 @@ private static function getTypeFromGenericTree( } foreach ($generic_params[0]->getAtomicTypes() as $key => $atomic_type) { + // PHP 8 values with whitespace after number are counted as numeric + // and filter_var treats them as such too + if ($atomic_type instanceof TLiteralString + && trim($atomic_type->value) === $atomic_type->value + && ($string_to_int = filter_var($atomic_type->value, FILTER_VALIDATE_INT)) !== false + ) { + $builder = $generic_params[0]->getBuilder(); + $builder->removeType($key); + $generic_params[0] = $builder->addType(new TLiteralInt($string_to_int, $from_docblock))->freeze(); + continue; + } + if ($atomic_type instanceof TInt || $atomic_type instanceof TString || $atomic_type instanceof TArrayKey diff --git a/tests/ArrayAssignmentTest.php b/tests/ArrayAssignmentTest.php index 769ebadc290..fa65da8223c 100644 --- a/tests/ArrayAssignmentTest.php +++ b/tests/ArrayAssignmentTest.php @@ -2115,6 +2115,29 @@ function getQueryParams(): array return $queryParams; }', ], + 'stringIntKeys' => [ + 'code' => ' $arg + * @return bool + */ + function foo($arg) { + foreach ($arg as $k => $v) { + if ( $k === 15 ) { + return true; + } + + if ( $k === 17 ) { + return false; + } + } + + return true; + } + + $x = ["15" => "a", 17 => "b"]; + foo($x);', + ], ]; } diff --git a/tests/ArrayKeysTest.php b/tests/ArrayKeysTest.php index 070ea52873b..e3f9cc897a4 100644 --- a/tests/ArrayKeysTest.php +++ b/tests/ArrayKeysTest.php @@ -97,6 +97,33 @@ function getKey() { } ', ], + 'literalStringAsIntArrayKey' => [ + 'code' => ' [ + "from" => "79268724911", + "to" => "74950235931", + ], + "b" => [ + "from" => "79313044964", + "to" => "78124169167", + ], + ]; + + private const SIP_FORMAT = "sip:%s@voip.test.com:9090"; + + /** @return array */ + public function test(): array { + $redirects = []; + foreach (self::REDIRECTS as $redirect) { + $redirects[$redirect["from"]] = sprintf(self::SIP_FORMAT, $redirect["to"]); + } + + return $redirects; + } + }', + ], ]; } @@ -126,6 +153,34 @@ function getKeys() { ', 'error_message' => 'InvalidReturnStatement', ], + 'literalStringAsIntArrayKey' => [ + 'code' => ' [ + "from" => "79268724911", + "to" => "74950235931", + ], + "b" => [ + "from" => "79313044964", + "to" => "78124169167", + ], + ]; + + private const SIP_FORMAT = "sip:%s@voip.test.com:9090"; + + /** @return array */ + public function test(): array { + $redirects = []; + foreach (self::REDIRECTS as $redirect) { + $redirects[$redirect["from"]] = sprintf(self::SIP_FORMAT, $redirect["to"]); + } + + return $redirects; + } + }', + 'error_message' => 'InvalidReturnStatement', + ], ]; } }