From b0aaede9e1599634682b17d49d03a53113866a16 Mon Sep 17 00:00:00 2001 From: Brown Date: Fri, 4 Oct 2019 11:08:08 -0400 Subject: [PATCH] Add support for checking integer array offsets --- config.xsd | 1 + psalm.xml.dist | 1 + src/Psalm/Config.php | 6 ++ .../Expression/Fetch/ArrayFetchAnalyzer.php | 89 +++++++++++++++++-- src/Psalm/Type/Union.php | 8 ++ tests/ArrayAccessTest.php | 24 +++++ 6 files changed, 121 insertions(+), 8 deletions(-) diff --git a/config.xsd b/config.xsd index 8ba86e71a..efdbf7a9f 100644 --- a/config.xsd +++ b/config.xsd @@ -56,6 +56,7 @@ + diff --git a/psalm.xml.dist b/psalm.xml.dist index 7f1f41bb4..8ab60ae84 100644 --- a/psalm.xml.dist +++ b/psalm.xml.dist @@ -11,6 +11,7 @@ throwExceptionOnError="0" findUnusedCode="true" ensureArrayStringOffsetsExist="true" + ensureArrayIntOffsetsExist="false" resolveFromConfigFile="true" xsi:schemaLocation="https://getpsalm.org/schema/config config.xsd" > diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php index 2f3480f88..f80dc16ee 100644 --- a/src/Psalm/Config.php +++ b/src/Psalm/Config.php @@ -326,6 +326,11 @@ class Config */ public $ensure_array_string_offsets_exist = false; + /** + * @var bool + */ + public $ensure_array_int_offsets_exist = false; + /** * @var array */ @@ -697,6 +702,7 @@ class Config 'includePhpVersionsInErrorBaseline' => 'include_php_versions_in_error_baseline', 'loadXdebugStub' => 'load_xdebug_stub', 'ensureArrayStringOffsetsExist' => 'ensure_array_string_offsets_exist', + 'ensureArrayIntOffsetsExist' => 'ensure_array_int_offsets_exist', ]; foreach ($booleanAttributes as $xmlName => $internalName) { diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php index 0f0a8955c..06fbbd27d 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php @@ -537,7 +537,20 @@ class ArrayFetchAnalyzer if ($codebase->config->ensure_array_string_offsets_exist && $offset_type_contained_by_expected ) { - self::checkLiteralArrayOffset( + self::checkLiteralStringArrayOffset( + $offset_type, + $expected_offset_type, + $array_var_id, + $stmt, + $context, + $statements_analyzer + ); + } + + if ($codebase->config->ensure_array_int_offsets_exist + && $offset_type_contained_by_expected + ) { + self::checkLiteralIntArrayOffset( $offset_type, $expected_offset_type, $array_var_id, @@ -662,7 +675,18 @@ class ArrayFetchAnalyzer $has_valid_offset = true; if ($codebase->config->ensure_array_string_offsets_exist) { - self::checkLiteralArrayOffset( + self::checkLiteralStringArrayOffset( + $offset_type, + $type->getGenericKeyType(), + $array_var_id, + $stmt, + $context, + $statements_analyzer + ); + } + + if ($codebase->config->ensure_array_int_offsets_exist) { + self::checkLiteralIntArrayOffset( $offset_type, $type->getGenericKeyType(), $array_var_id, @@ -1122,7 +1146,7 @@ class ArrayFetchAnalyzer return $array_access_type; } - private static function checkLiteralArrayOffset( + private static function checkLiteralIntArrayOffset( Type\Union $offset_type, Type\Union $expected_offset_type, ?string $array_var_id, @@ -1130,11 +1154,60 @@ class ArrayFetchAnalyzer Context $context, StatementsAnalyzer $statements_analyzer ) : void { - if ($offset_type->hasLiteralString() - && !$expected_offset_type->hasLiteralClassString() - && !$context->inside_isset - && !$context->inside_unset - ) { + if ($context->inside_isset || $context->inside_unset) { + return; + } + + if ($offset_type->hasLiteralInt()) { + $found_match = false; + + foreach ($offset_type->getTypes() as $offset_type_part) { + if ($array_var_id + && $offset_type_part instanceof TLiteralInt + && isset( + $context->vars_in_scope[ + $array_var_id . '[' . $offset_type_part->value . ']' + ] + ) + && !$context->vars_in_scope[ + $array_var_id . '[' . $offset_type_part->value . ']' + ]->possibly_undefined + ) { + $found_match = true; + } + } + + if (!$found_match) { + if (IssueBuffer::accepts( + new PossiblyUndefinedArrayOffset( + 'Possibly undefined array offset \'' + . $offset_type->getId() . '\' ' + . 'is risky given expected type \'' + . $expected_offset_type->getId() . '\'.' + . ' Consider using isset beforehand.', + new CodeLocation($statements_analyzer->getSource(), $stmt) + ), + $statements_analyzer->getSuppressedIssues() + )) { + // fall through + } + } + } + } + + private static function checkLiteralStringArrayOffset( + Type\Union $offset_type, + Type\Union $expected_offset_type, + ?string $array_var_id, + PhpParser\Node\Expr\ArrayDimFetch $stmt, + Context $context, + StatementsAnalyzer $statements_analyzer + ) : void { + if ($context->inside_isset || $context->inside_unset) { + return; + } + + if ($offset_type->hasLiteralString() && !$expected_offset_type->hasLiteralClassString()) { $found_match = false; foreach ($offset_type->getTypes() as $offset_type_part) { diff --git a/src/Psalm/Type/Union.php b/src/Psalm/Type/Union.php index c578a71b5..6e83fafa5 100644 --- a/src/Psalm/Type/Union.php +++ b/src/Psalm/Type/Union.php @@ -1726,6 +1726,14 @@ class Union return count($this->literal_string_types) > 0; } + /** + * @return bool + */ + public function hasLiteralInt() + { + return count($this->literal_int_types) > 0; + } + /** * @return bool true if this is a int literal with only one possible value */ diff --git a/tests/ArrayAccessTest.php b/tests/ArrayAccessTest.php index 5f350e3e9..05d8f9424 100644 --- a/tests/ArrayAccessTest.php +++ b/tests/ArrayAccessTest.php @@ -116,6 +116,30 @@ class ArrayAccessTest extends TestCase $this->analyzeFile('somefile.php', new \Psalm\Context()); } + /** + * @return void + */ + public function testEnsureArrayIntOffsetsExist() + { + $this->expectException(\Psalm\Exception\CodeException::class); + $this->expectExceptionMessage('PossiblyUndefinedArrayOffset'); + + \Psalm\Config::getInstance()->ensure_array_int_offsets_exist = true; + + $this->addFile( + 'somefile.php', + ' $arr */ + function takesArrayIteratorOfString(array $arr): void { + echo $arr[4]; + }' + ); + + $this->analyzeFile('somefile.php', new \Psalm\Context()); + } + /** * @return iterable,error_levels?:string[]}> */