Skip to content

Commit

Permalink
Allow testing expected CallMap return types and ignore functions that…
Browse files Browse the repository at this point in the history 
… currently fail
  • Loading branch information
@othercorey
othercorey committed Jun 26, 2022
1 parent d626d24 commit 84dae7f
Show file tree
Hide file tree
Showing 3 changed files with 230 additions and 32 deletions.
2 changes: 2 additions & 0 deletions dictionaries/CallMap.php
View file
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
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
258 changes: 226 additions & 32 deletions tests/Internal/Codebase/InternalCallMapHandlerTest.php
View file
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 @@ -437,6 +601,13 @@ 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
Expand All @@ -446,36 +617,42 @@ private function isIgnored(string $functionName): bool
*/
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 */
$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 @@ -493,27 +670,31 @@ public function testCallMapCompliesWithReflection(string $functionName, array $c
$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 +736,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 +760,19 @@ private function assertParameter(array $normalizedEntry, ReflectionParameter $pa
}
}

/**
*/
public function assertEntryReturnType(ReflectionFunction $function, string $entryReturnType): void
{
$expectedType = $function->hasTentativeReturnType() ? $function->getTentativeReturnType() : $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 +788,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

0 comments on commit 84dae7f

Please sign in to comment.