Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

report error for invalid array key type #10481

Merged
merged 2 commits into from
Dec 13, 2023
Merged

report error for invalid array key type #10481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
82ff582
add error for invalid array key type in docblock
kkmuffme Dec 12, 2023
9be7fce
Fix literal string keys int not handled as int as PHP does
kkmuffme Dec 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
87 changes: 87 additions & 0 deletions src/Psalm/Internal/Type/TypeParser.php
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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