Skip to content

Commit

Permalink
Merge pull request #10481 from kkmuffme/invalid-array-key-types-and-l…
Browse files Browse the repository at this point in the history 
…iteral-string-as-int-key

report error for invalid array key type
  • Loading branch information
@orklah
orklah committed Dec 13, 2023
2 parents 1df5b35 + 9be7fce commit 955e7fe
Show file tree
Hide file tree
Showing 5 changed files with 188 additions and 14 deletions.
87 changes: 87 additions & 0 deletions src/Psalm/Internal/Type/TypeParser.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,16 @@
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;
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;
Expand Down Expand Up @@ -83,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;
Expand All @@ -96,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
Expand Down Expand Up @@ -643,6 +650,46 @@ private static function getTypeFromGenericTree(
throw new TypeParseTreeException('Too many template parameters for array');
}

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
|| $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);
}

Expand Down Expand Up @@ -671,6 +718,46 @@ private static function getTypeFromGenericTree(
throw new TypeParseTreeException('Too many template parameters for non-empty-array');
}

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
|| $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);
}

Expand Down
10 changes: 9 additions & 1 deletion tests/AnnotationTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1366,7 +1366,15 @@ public function barBar() {
}',
'error_message' => 'MissingDocblockType',
],

'invalidArrayKeyType' => [
'code' => '<?php
/**
* @param array<float, string> $arg
* @return void
*/
function foo($arg) {}',
'error_message' => 'InvalidDocblock',
],
'invalidClassMethodReturnBrackets' => [
'code' => '<?php
class C {
Expand Down
32 changes: 28 additions & 4 deletions tests/ArrayAssignmentTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ public function providerValidCodeParse(): iterable
'assignUnionOfLiterals' => [
'code' => '<?php
$result = [];
foreach (["a", "b"] as $k) {
$result[$k] = true;
}
$resultOpt = [];
foreach (["a", "b"] as $k) {
if (random_int(0, 1)) {
continue;
Expand Down Expand Up @@ -2115,6 +2115,29 @@ function getQueryParams(): array
return $queryParams;
}',
],
'stringIntKeys' => [
'code' => '<?php
/**
* @param array<15|"17"|"hello", string> $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);',
],
];
}

Expand Down Expand Up @@ -2492,7 +2515,8 @@ public function getThisName($offset, $weird_array): string {
return $weird_array[$offset];
}
}',
'error_message' => 'InvalidArrayOffset',
'error_message' => 'MixedArrayAccess',
'ignored_issues' => ['InvalidDocblock'],
],
'unpackTypedIterableWithStringKeysIntoArray' => [
'code' => '<?php
Expand Down
55 changes: 55 additions & 0 deletions tests/ArrayKeysTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,33 @@ function getKey() {
}
',
],
'literalStringAsIntArrayKey' => [
'code' => '<?php
class a {
private const REDIRECTS = [
"a" => [
"from" => "79268724911",
"to" => "74950235931",
],
"b" => [
"from" => "79313044964",
"to" => "78124169167",
],
];
private const SIP_FORMAT = "sip:%s@voip.test.com:9090";
/** @return array<int, string> */
public function test(): array {
$redirects = [];
foreach (self::REDIRECTS as $redirect) {
$redirects[$redirect["from"]] = sprintf(self::SIP_FORMAT, $redirect["to"]);
}
return $redirects;
}
}',
],
];
}

Expand Down Expand Up @@ -126,6 +153,34 @@ function getKeys() {
',
'error_message' => 'InvalidReturnStatement',
],
'literalStringAsIntArrayKey' => [
'code' => '<?php
class a {
private const REDIRECTS = [
"a" => [
"from" => "79268724911",
"to" => "74950235931",
],
"b" => [
"from" => "79313044964",
"to" => "78124169167",
],
];
private const SIP_FORMAT = "sip:%s@voip.test.com:9090";
/** @return array<string, string> */
public function test(): array {
$redirects = [];
foreach (self::REDIRECTS as $redirect) {
$redirects[$redirect["from"]] = sprintf(self::SIP_FORMAT, $redirect["to"]);
}
return $redirects;
}
}',
'error_message' => 'InvalidReturnStatement',
],
];
}
}
18 changes: 9 additions & 9 deletions tests/KeyOfArrayTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,11 @@ function getKey() {
'keyOfUnionArrayLiteral' => [
'code' => '<?php
/**
* @return key-of<array<int, string>|array<float, string>>
* @return key-of<array<int, string>|array<string, string>>
*/
function getKey(bool $asFloat) {
if ($asFloat) {
return 42.0;
function getKey(bool $asString) {
if ($asString) {
return "42";
}
return 42;
}
Expand Down Expand Up @@ -194,14 +194,14 @@ public function getKey() {
',
'error_message' => 'InvalidReturnStatement',
],
'noStringAllowedInKeyOfIntFloatArray' => [
'noStringAllowedInKeyOfIntFloatStringArray' => [
'code' => '<?php
/**
* @return key-of<array<int, string>|array<float, string>>
* @return key-of<array<int, string>|array<"42.0", string>>
*/
function getKey(bool $asFloat) {
if ($asFloat) {
return 42.0;
function getKey(bool $asInt) {
if ($asInt) {
return 42;
}
return "42";
}
Expand Down

0 comments on commit 955e7fe

Please sign in to comment.