mirror of
https://github.com/danog/psalm.git
synced 2025-01-21 21:31:13 +01:00
Add stricter checks after first isset
This commit is contained in:
parent
60214ab0c7
commit
d85fbaec09
@ -536,44 +536,15 @@ class ArrayFetchAnalyzer
|
||||
|
||||
if ($codebase->config->ensure_array_string_offsets_exist
|
||||
&& $offset_type_contained_by_expected
|
||||
&& $offset_type->hasLiteralString()
|
||||
&& !$expected_offset_type->hasLiteralClassString()
|
||||
&& !$context->inside_isset
|
||||
&& !$context->inside_unset
|
||||
) {
|
||||
$found_match = false;
|
||||
|
||||
foreach ($offset_type->getTypes() as $offset_type_part) {
|
||||
if ($array_var_id
|
||||
&& $offset_type_part instanceof TLiteralString
|
||||
&& 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
|
||||
}
|
||||
}
|
||||
self::checkLiteralArrayOffset(
|
||||
$offset_type,
|
||||
$expected_offset_type,
|
||||
$array_var_id,
|
||||
$stmt,
|
||||
$context,
|
||||
$statements_analyzer
|
||||
);
|
||||
}
|
||||
|
||||
if ((!$offset_type_contained_by_expected
|
||||
@ -690,6 +661,17 @@ class ArrayFetchAnalyzer
|
||||
} elseif ($type->previous_value_type) {
|
||||
$has_valid_offset = true;
|
||||
|
||||
if ($codebase->config->ensure_array_string_offsets_exist) {
|
||||
self::checkLiteralArrayOffset(
|
||||
$offset_type,
|
||||
$type->getGenericKeyType(),
|
||||
$array_var_id,
|
||||
$stmt,
|
||||
$context,
|
||||
$statements_analyzer
|
||||
);
|
||||
}
|
||||
|
||||
$type->properties[$key_value] = clone $type->previous_value_type;
|
||||
|
||||
$array_access_type = clone $type->previous_value_type;
|
||||
@ -1140,6 +1122,55 @@ class ArrayFetchAnalyzer
|
||||
return $array_access_type;
|
||||
}
|
||||
|
||||
private static function checkLiteralArrayOffset(
|
||||
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 ($offset_type->hasLiteralString()
|
||||
&& !$expected_offset_type->hasLiteralClassString()
|
||||
&& !$context->inside_isset
|
||||
&& !$context->inside_unset
|
||||
) {
|
||||
$found_match = false;
|
||||
|
||||
foreach ($offset_type->getTypes() as $offset_type_part) {
|
||||
if ($array_var_id
|
||||
&& $offset_type_part instanceof TLiteralString
|
||||
&& 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
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @return Type\Union
|
||||
*/
|
||||
|
@ -58,6 +58,7 @@ class BuildInfoCollector
|
||||
* "TRAVIS", "TRAVIS_JOB_ID" must be set.
|
||||
*
|
||||
* @return $this
|
||||
* @psalm-suppress PossiblyUndefinedArrayOffset
|
||||
*/
|
||||
protected function fillTravisCi() : self
|
||||
{
|
||||
@ -133,6 +134,7 @@ class BuildInfoCollector
|
||||
* "APPVEYOR", "APPVEYOR_BUILD_NUMBER" must be set.
|
||||
*
|
||||
* @return $this
|
||||
* @psalm-suppress PossiblyUndefinedArrayOffset
|
||||
*/
|
||||
protected function fillAppVeyor() : self
|
||||
{
|
||||
@ -204,6 +206,7 @@ class BuildInfoCollector
|
||||
* "JENKINS_URL", "BUILD_NUMBER" must be set.
|
||||
*
|
||||
* @return $this
|
||||
* @psalm-suppress PossiblyUndefinedArrayOffset
|
||||
*/
|
||||
protected function fillScrutinizer() : self
|
||||
{
|
||||
|
@ -481,7 +481,6 @@ class Reconciler
|
||||
if (!isset($array_properties[$key_parts_key])) {
|
||||
if ($existing_key_type_part->previous_value_type) {
|
||||
$new_base_type_candidate = clone $existing_key_type_part->previous_value_type;
|
||||
//$new_base_type_candidate->possibly_undefined = true;
|
||||
|
||||
return $new_base_type_candidate;
|
||||
}
|
||||
|
@ -95,7 +95,26 @@ class ArrayAccessTest extends TestCase
|
||||
$this->analyzeFile('somefile.php', new \Psalm\Context());
|
||||
}
|
||||
|
||||
/**
|
||||
* @return void
|
||||
*/
|
||||
public function testComplainAfterFirstIsset()
|
||||
{
|
||||
$this->expectException(\Psalm\Exception\CodeException::class);
|
||||
$this->expectExceptionMessage('PossiblyUndefinedArrayOffset');
|
||||
|
||||
\Psalm\Config::getInstance()->ensure_array_string_offsets_exist = true;
|
||||
|
||||
$this->addFile(
|
||||
'somefile.php',
|
||||
'<?php
|
||||
function foo(array $arr) : void {
|
||||
if (isset($arr["a"]) && $arr["b"]) {}
|
||||
}'
|
||||
);
|
||||
|
||||
$this->analyzeFile('somefile.php', new \Psalm\Context());
|
||||
}
|
||||
|
||||
/**
|
||||
* @return iterable<string,array{string,assertions?:array<string,string>,error_levels?:string[]}>
|
||||
|
Loading…
x
Reference in New Issue
Block a user