Skip to content

Commit

Permalink
Simplify assertions generated by an array_key_exists check (#8763)
Browse files Browse the repository at this point in the history
  • Loading branch information
muglug committed Nov 25, 2022
1 parent 255a152 commit a0e4468
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 53 deletions.
108 changes: 56 additions & 52 deletions src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php
Expand Up @@ -3675,6 +3675,8 @@ private static function getArrayKeyExistsAssertions(

$literal_assertions = [];

$safe_to_track_literals = true;

if (isset($expr->getArgs()[0])
&& isset($expr->getArgs()[1])
&& $first_var_type
Expand Down Expand Up @@ -3710,75 +3712,77 @@ private static function getArrayKeyExistsAssertions(
foreach ($key_type->getLiteralInts() as $array_literal_type) {
$literal_assertions[] = new IsLooselyEqual($array_literal_type);
}
} else {
$safe_to_track_literals = false;
}
}
}
}

if ($literal_assertions && $first_var_name) {
if ($literal_assertions && $first_var_name && $safe_to_track_literals) {
$if_types[$first_var_name] = [$literal_assertions];
}

$array_root = isset($expr->getArgs()[1]->value)
? ExpressionIdentifier::getExtendedVarId(
$expr->getArgs()[1]->value,
$this_class_name,
$source
)
: null;
} else {
$array_root = isset($expr->getArgs()[1]->value)
? ExpressionIdentifier::getExtendedVarId(
$expr->getArgs()[1]->value,
$this_class_name,
$source
)
: null;

if ($array_root && isset($expr->getArgs()[0])) {
if ($first_var_name === null) {
$first_arg = $expr->getArgs()[0];
if ($array_root && isset($expr->getArgs()[0])) {
if ($first_var_name === null) {
$first_arg = $expr->getArgs()[0];

if ($first_arg->value instanceof PhpParser\Node\Scalar\String_) {
$first_var_name = '\'' . $first_arg->value->value . '\'';
} elseif ($first_arg->value instanceof PhpParser\Node\Scalar\LNumber) {
$first_var_name = (string)$first_arg->value->value;
if ($first_arg->value instanceof PhpParser\Node\Scalar\String_) {
$first_var_name = '\'' . $first_arg->value->value . '\'';
} elseif ($first_arg->value instanceof PhpParser\Node\Scalar\LNumber) {
$first_var_name = (string)$first_arg->value->value;
}
}
}

if ($expr->getArgs()[0]->value instanceof PhpParser\Node\Expr\ClassConstFetch
&& $expr->getArgs()[0]->value->name instanceof PhpParser\Node\Identifier
&& $expr->getArgs()[0]->value->name->name !== 'class'
) {
$const_type = null;
if ($expr->getArgs()[0]->value instanceof PhpParser\Node\Expr\ClassConstFetch
&& $expr->getArgs()[0]->value->name instanceof PhpParser\Node\Identifier
&& $expr->getArgs()[0]->value->name->name !== 'class'
) {
$const_type = null;

if ($source instanceof StatementsAnalyzer) {
$const_type = $source->node_data->getType($expr->getArgs()[0]->value);
}
if ($source instanceof StatementsAnalyzer) {
$const_type = $source->node_data->getType($expr->getArgs()[0]->value);
}

if ($const_type) {
if ($const_type->isSingleStringLiteral()) {
$first_var_name = $const_type->getSingleStringLiteral()->value;
} elseif ($const_type->isSingleIntLiteral()) {
$first_var_name = (string)$const_type->getSingleIntLiteral()->value;
if ($const_type) {
if ($const_type->isSingleStringLiteral()) {
$first_var_name = $const_type->getSingleStringLiteral()->value;
} elseif ($const_type->isSingleIntLiteral()) {
$first_var_name = (string)$const_type->getSingleIntLiteral()->value;
} else {
$first_var_name = null;
}
} else {
$first_var_name = null;
}
} else {
$first_var_name = null;
}
} elseif (($expr->getArgs()[0]->value instanceof PhpParser\Node\Expr\Variable
|| $expr->getArgs()[0]->value instanceof PhpParser\Node\Expr\PropertyFetch
|| $expr->getArgs()[0]->value instanceof PhpParser\Node\Expr\StaticPropertyFetch
)
&& $source instanceof StatementsAnalyzer
&& ($first_var_type = $source->node_data->getType($expr->getArgs()[0]->value))
) {
foreach ($first_var_type->getLiteralStrings() as $array_literal_type) {
$if_types[$array_root . "['" . $array_literal_type->value . "']"] = [[new ArrayKeyExists()]];
}
foreach ($first_var_type->getLiteralInts() as $array_literal_type) {
$if_types[$array_root . "[" . $array_literal_type->value . "]"] = [[new ArrayKeyExists()]];
} elseif (($expr->getArgs()[0]->value instanceof PhpParser\Node\Expr\Variable
|| $expr->getArgs()[0]->value instanceof PhpParser\Node\Expr\PropertyFetch
|| $expr->getArgs()[0]->value instanceof PhpParser\Node\Expr\StaticPropertyFetch
)
&& $source instanceof StatementsAnalyzer
&& ($first_var_type = $source->node_data->getType($expr->getArgs()[0]->value))
) {
foreach ($first_var_type->getLiteralStrings() as $array_literal_type) {
$if_types[$array_root . "['" . $array_literal_type->value . "']"] = [[new ArrayKeyExists()]];
}
foreach ($first_var_type->getLiteralInts() as $array_literal_type) {
$if_types[$array_root . "[" . $array_literal_type->value . "]"] = [[new ArrayKeyExists()]];
}
}
}

if ($first_var_name !== null
&& !strpos($first_var_name, '->')
&& !strpos($first_var_name, '[')
) {
$if_types[$array_root . '[' . $first_var_name . ']'] = [[new ArrayKeyExists()]];
if ($first_var_name !== null
&& !strpos($first_var_name, '->')
&& !strpos($first_var_name, '[')
) {
$if_types[$array_root . '[' . $first_var_name . ']'] = [[new ArrayKeyExists()]];
}
}
}

Expand Down
33 changes: 32 additions & 1 deletion tests/TypeReconciliation/ArrayKeyExistsTest.php
Expand Up @@ -456,7 +456,38 @@ public static function getStateLabelIf(int $state): string {
if ($cmp["administrative_area_level_1"] === "test") {
$cmp["administrative_area_level_1"] = "";
}'
]
],
'arrayKeyExistsPoorPerformance' => [
'code' => '<?php
class A {
private const CRITICAL_ERRORS = [
"category" => [],
"name" => [],
"geo" => [],
"city" => [],
"url" => [],
"comment_critical" => [],
"place" => [],
"price" => [],
"robot_error" => [],
"manual" => [],
"contacts" => [],
"not_confirmed_by_other_source" => [],
];
public function isCriticalError(int|string $key): bool {
if (!\array_key_exists($key, A::CRITICAL_ERRORS)) {
return false;
}
return true;
}
}',
'assertions' => [],
'ignored_issues' => [],
'php_version' => '8.0',
],
];
}

Expand Down

0 comments on commit a0e4468

Please sign in to comment.