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

Allow testing expected CallMap return types #8166

Merged
merged 1 commit into from Jul 6, 2022
+240 −34
Merged

Allow testing expected CallMap return types #8166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions dictionaries/CallMap.php
View file
Open in desktop
Expand Up @@ -11453,7 +11453,9 @@
'ReflectionFunctionAbstract::getShortName' => ['string'],
'ReflectionFunctionAbstract::getStartLine' => ['int|false'],
'ReflectionFunctionAbstract::getStaticVariables' => ['array'],
'ReflectionFunctionAbstract::getTentativeReturnType' => ['?ReflectionType'],
'ReflectionFunctionAbstract::hasReturnType' => ['bool'],
'ReflectionFunctionAbstract::hasTentativeReturnType' => ['bool'],
'ReflectionFunctionAbstract::inNamespace' => ['bool'],
'ReflectionFunctionAbstract::isClosure' => ['bool'],
'ReflectionFunctionAbstract::isDeprecated' => ['bool'],
Expand Down
2 changes: 2 additions & 0 deletions dictionaries/CallMap_81_delta.php
View file
Open in desktop
Expand Up @@ -46,6 +46,8 @@
'ReflectionEnumUnitCase::getEnum' => ['ReflectionEnum'],
'ReflectionEnumUnitCase::getValue' => ['UnitEnum'],
'ReflectionEnumBackedCase::getBackingValue' => ['string|int'],
'ReflectionFunctionAbstract::getTentativeReturnType' => ['?ReflectionType'],
'ReflectionFunctionAbstract::hasTentativeReturnType' => ['bool'],
'ReflectionFunctionAbstract::isStatic' => ['bool'],
'ReflectionObject::isEnum' => ['bool'],
],
Expand Down
270 changes: 236 additions & 34 deletions tests/Internal/Codebase/InternalCallMapHandlerTest.php
View file
Open in desktop
Expand Up @@ -2,7 +2,9 @@

namespace Psalm\Tests\Internal\Codebase;

use Exception;
use InvalidArgumentException;
use PHPUnit\Framework\AssertionFailedError;
use PHPUnit\Framework\ExpectationFailedException;
use PHPUnit\Framework\SkippedTestError;
use Psalm\Codebase;
Expand All @@ -16,6 +18,7 @@
use Psalm\Tests\TestCase;
use Psalm\Tests\TestConfig;
use Psalm\Type;
use ReflectionException;
use ReflectionFunction;
use ReflectionParameter;
use ReflectionType;
Expand Down Expand Up @@ -336,6 +339,167 @@ class InternalCallMapHandlerTest extends TestCase
'zlib_encode',
];

/**
* List of function names to ignore only for return type checks.
*
* @var list<string>
*/
private static $ignoredReturnTypeOnlyFunctions = [
'bcsqrt',
'bzopen',
'cal_from_jd',
'collator_get_strength',
'curl_multi_init',
'date_add',
'date_date_set',
'date_diff',
'date_offset_get',
'date_parse',
'date_sub',
'date_sun_info',
'date_sunrise',
'date_sunset',
'date_time_set',
'date_timestamp_set',
'date_timezone_set',
'datefmt_set_lenient',
'dba_open',
'dba_popen',
'deflate_init',
'enchant_broker_init',
'fgetcsv',
'filter_input_array',
'fopen',
'fpassthru',
'fsockopen',
'ftp_get_option',
'get_declared_traits',
'gmp_export',
'gmp_hamdist',
'gmp_import',
'gzeof',
'gzopen',
'gzpassthru',
'hash',
'hash_hkdf',
'hash_hmac',
'iconv_get_encoding',
'igbinary_serialize',
'imagecolorclosest',
'imagecolorclosestalpha',
'imagecolorclosesthwb',
'imagecolorexact',
'imagecolorexactalpha',
'imagecolorresolve',
'imagecolorresolvealpha',
'imagecolorset',
'imagecolorsforindex',
'imagecolorstotal',
'imagecolortransparent',
'imageloadfont',
'imagesx',
'imagesy',
'imap_mailboxmsginfo',
'imap_msgno',
'imap_num_recent',
'inflate_init',
'intlcal_get',
'intlcal_get_keyword_values_for_locale',
'intlgregcal_set_gregorian_change',
'intltz_get_offset',
'jddayofweek',
'jdtounix',
'ldap_count_entries',
'ldap_exop',
'ldap_get_attributes',
'mb_encoding_aliases',
'metaphone',
'mongodb\\bson\\fromjson',
'mongodb\\bson\\fromphp',
'mongodb\\bson\\tojson',
'msg_get_queue',
'mysqli_stmt_get_warnings',
'mysqli_stmt_insert_id',
'numfmt_create',
'ob_list_handlers',
'odbc_autocommit',
'odbc_columnprivileges',
'odbc_columns',
'odbc_connect',
'odbc_do',
'odbc_exec',
'odbc_fetch_object',
'odbc_foreignkeys',
'odbc_gettypeinfo',
'odbc_pconnect',
'odbc_prepare',
'odbc_primarykeys',
'odbc_specialcolumns',
'odbc_statistics',
'odbc_tableprivileges',
'odbc_tables',
'opendir',
'openssl_random_pseudo_bytes',
'openssl_spki_export',
'openssl_spki_export_challenge',
'pack',
'parse_url',
'passthru',
'pcntl_exec',
'pcntl_signal_get_handler',
'pcntl_strerror',
'pfsockopen',
'pg_port',
'pg_socket',
'popen',
'proc_open',
'pspell_config_create',
'pspell_new',
'pspell_new_config',
'pspell_new_personal',
'register_shutdown_function',
'rewinddir',
'set_error_handler',
'set_exception_handler',
'shm_attach',
'shmop_open',
'simplexml_import_dom',
'sleep',
'snmp_set_oid_numeric_print',
'socket_export_stream',
'socket_import_stream',
'sodium_crypto_aead_chacha20poly1305_encrypt',
'sodium_crypto_aead_chacha20poly1305_ietf_encrypt',
'sodium_crypto_aead_xchacha20poly1305_ietf_encrypt',
'spl_autoload_functions',
'stream_bucket_new',
'stream_context_create',
'stream_context_get_default',
'stream_context_set_default',
'stream_filter_append',
'stream_filter_prepend',
'stream_set_chunk_size',
'stream_socket_accept',
'stream_socket_client',
'stream_socket_server',
'substr',
'substr_compare',
'timezone_abbreviations_list',
'timezone_offset_get',
'tmpfile',
'user_error',
'xml_get_current_byte_index',
'xml_get_current_column_number',
'xml_get_current_line_number',
'xml_get_error_code',
'xml_parser_get_option',
'yaml_parse',
'yaml_parse_file',
'yaml_parse_url',
'zip_open',
'zip_read',
];

/**
*
* @var Codebase
Expand Down Expand Up @@ -391,7 +555,7 @@ public function testGetcallmapReturnsAValidCallmap(): void

/**
*
* @return iterable<string, array{0: callable-string, 1: array<int|string>}>
* @return iterable<string, array{0: callable-string, 1: array<int|string, string>}>
*/
public function callMapEntryProvider(): iterable
{
Expand Down Expand Up @@ -437,45 +601,59 @@ private function isIgnored(string $functionName): bool
return false;
}

/**
*/
private function isReturnTypeOnlyIgnored(string $functionName): bool
{
return in_array($functionName, static::$ignoredReturnTypeOnlyFunctions, true);
}

/**
* @depends testIgnoresAreSortedAndUnique
* @depends testGetcallmapReturnsAValidCallmap
* @dataProvider callMapEntryProvider
* @coversNothing
* @psalm-param callable-string $functionName
* @param array<int|string, string> $callMapEntry
*/
public function testIgnoredFunctionsStillFail(string $functionName, array $callMapEntry): void
{
if (!$this->isIgnored($functionName)) {
$functionIgnored = $this->isIgnored($functionName);
if (!$functionIgnored && !$this->isReturnTypeOnlyIgnored($functionName)) {
// Dummy assertion to mark it as passed
$this->assertTrue(true);
return;
}

$this->expectException(ExpectationFailedException::class);
$function = new ReflectionFunction($functionName);
/** @var string $entryReturnType */
$entryReturnType = array_shift($callMapEntry);

if ($functionIgnored) {
try {
/** @var array<string, string> $callMapEntry */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you document this with an @param maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added one for the CallMap entry. I don't think there's a way to define the shape so it's just array<int|string, string>

I am traveling until Friday so you can make changes or wait until I'm back for further issues.

$this->assertEntryParameters($function, $callMapEntry);
$this->assertEntryReturnType($function, $entryReturnType);
} catch (AssertionFailedError $e) {
$this->assertTrue(true);
return;
} catch (ExpectationFailedException $e) {
$this->assertTrue(true);
return;
}
$this->fail("Remove '{$functionName}' from InternalCallMapHandlerTest::\$ignoredFunctions");
}

try {
unset($callMapEntry[0]);
/** @var array<string, string> $callMapEntry */
$this->assertEntryIsCorrect($callMapEntry, $functionName);
} catch (InvalidArgumentException $t) {
// Silence this one for now.
$this->markTestSkipped('IA');
} catch (SkippedTestError $t) {
die('this should not happen');
$this->assertEntryReturnType($function, $entryReturnType);
} catch (AssertionFailedError $e) {
$this->assertTrue(true);
return;
} catch (ExpectationFailedException $e) {
// This is good!
throw $e;
} catch (InvalidArgumentException $e) {
// This can happen if a class does not exist, we handle the message to check for this case.
if (preg_match('/^Could not get class storage for (.*)$/', $e->getMessage(), $matches)
&& !class_exists($matches[1])
) {
die("Class mentioned in callmap does not exist: " . $matches[1]);
}
$this->assertTrue(true);
return;
}

$this->markTestIncomplete("Remove function '{$functionName}' from your ignores");
$this->fail("Remove '{$functionName}' from InternalCallMapHandlerTest::\$ignoredReturnTypeOnlyFunctions");
}

/**
Expand All @@ -485,35 +663,39 @@ public function testIgnoredFunctionsStillFail(string $functionName, array $callM
* @depends testIgnoresAreSortedAndUnique
* @dataProvider callMapEntryProvider
* @psalm-param callable-string $functionName
* @param array $callMapEntry
* @param array<int|string, string> $callMapEntry
*/
public function testCallMapCompliesWithReflection(string $functionName, array $callMapEntry): void
{
if ($this->isIgnored($functionName)) {
$this->markTestSkipped("Function $functionName is ignored in config");
}

unset($callMapEntry[0]);
$function = new ReflectionFunction($functionName);
/** @var string $entryReturnType */
$entryReturnType = array_shift($callMapEntry);

/** @var array<string, string> $callMapEntry */
$this->assertEntryIsCorrect($callMapEntry, $functionName);
$this->assertEntryParameters($function, $callMapEntry);

if (!$this->isReturnTypeOnlyIgnored($functionName)) {
$this->assertEntryReturnType($function, $entryReturnType);
}
}

/**
*
* @param array<string, string> $callMapEntryWithoutReturn
* @psalm-param callable-string $functionName
* @param array<string, string> $entryParameters
*/
private function assertEntryIsCorrect(array $callMapEntryWithoutReturn, string $functionName): void
private function assertEntryParameters(ReflectionFunction $function, array $entryParameters): void
{
$rF = new ReflectionFunction($functionName);

/**
* Parse the parameter names from the map.
* @var array<string, array{byRef: bool, refMode: 'rw'|'w', variadic: bool, optional: bool, type: string}>
*/
$normalizedEntries = [];

foreach ($callMapEntryWithoutReturn as $key => $entry) {
foreach ($entryParameters as $key => $entry) {
$normalizedKey = $key;
/**
*
Expand Down Expand Up @@ -555,8 +737,8 @@ private function assertEntryIsCorrect(array $callMapEntryWithoutReturn, string $
$normalizedEntry['name'] = $normalizedKey;
$normalizedEntries[$normalizedKey] = $normalizedEntry;
}
foreach ($rF->getParameters() as $parameter) {
$this->assertArrayHasKey($parameter->getName(), $normalizedEntries, "Callmap is missing entry for param {$parameter->getName()} in $functionName: " . print_r($normalizedEntries, true));
foreach ($function->getParameters() as $parameter) {
$this->assertArrayHasKey($parameter->getName(), $normalizedEntries, "Callmap is missing entry for param {$parameter->getName()} in {$function->getName()}: " . print_r($normalizedEntries, true));
$this->assertParameter($normalizedEntries[$parameter->getName()], $parameter);
}
}
Expand All @@ -579,6 +761,26 @@ private function assertParameter(array $normalizedEntry, ReflectionParameter $pa
}
}

/**
*
* @psalm-suppress UndefinedMethod
*/
public function assertEntryReturnType(ReflectionFunction $function, string $entryReturnType): void
{
if (version_compare(PHP_VERSION, '8.1.0', '>=')) {
/** @var \ReflectionType|null $expectedType */
$expectedType = $function->hasTentativeReturnType() ? $function->getTentativeReturnType() : $function->getReturnType();
} else {
$expectedType = $function->getReturnType();
}
if ($expectedType === null) {
$this->assertSame('', $entryReturnType, 'CallMap entry has incorrect return type');
return;
}

$this->assertTypeValidity($expectedType, $entryReturnType, 'CallMap entry has incorrect return type');
}

/**
* Since string equality is too strict, we do some extra checking here
*/
Expand All @@ -594,7 +796,7 @@ private function assertTypeValidity(ReflectionType $reflected, string $specified
if (preg_match('/^Could not get class storage for (.*)$/', $e->getMessage(), $matches)
&& !class_exists($matches[1])
) {
die("Class mentioned in callmap does not exist: " . $matches[1]);
$this->fail("Class used in CallMap does not exist: {$matches[1]}");
}
}
}
Expand Down