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

Improve type resolution of class constants. #5591

Merged
merged 1 commit into from
Apr 8, 2021
Merged

Improve type resolution of class constants. #5591

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions src/Psalm/Internal/Codebase/ClassLikes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1495,14 +1495,12 @@ public function getClassConstantType(
}

if ($constant_storage->unresolved_node) {
return new Type\Union([
ConstantTypeResolver::resolve(
$this,
$constant_storage->unresolved_node,
$statements_analyzer,
$visited_constant_ids
)
]);
$constant_storage->type = new Type\Union([ConstantTypeResolver::resolve(
$this,
$constant_storage->unresolved_node,
$statements_analyzer,
$visited_constant_ids
)]);
}

return $constant_storage->type;
Expand Down
64 changes: 55 additions & 9 deletions src/Psalm/Internal/Codebase/ConstantTypeResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Psalm\Internal\Analyzer\Statements\Expression\Fetch\ConstFetchAnalyzer;
use Psalm\Type;
use ReflectionProperty;
use function ctype_digit;

/**
* @internal
Expand Down Expand Up @@ -46,11 +47,17 @@ public static function resolve(
}

if ($c instanceof UnresolvedConstant\UnresolvedConcatOp) {
if ($left instanceof Type\Atomic\TLiteralString && $right instanceof Type\Atomic\TLiteralString) {
if (($left instanceof Type\Atomic\TLiteralString
|| $left instanceof Type\Atomic\TLiteralFloat
|| $left instanceof Type\Atomic\TLiteralInt)
&& ($right instanceof Type\Atomic\TLiteralString
|| $right instanceof Type\Atomic\TLiteralFloat
|| $right instanceof Type\Atomic\TLiteralInt)
) {
return new Type\Atomic\TLiteralString($left->value . $right->value);
}

return new Type\Atomic\TMixed;
return new Type\Atomic\TString();
}

if ($c instanceof UnresolvedConstant\UnresolvedAdditionOp
Expand Down Expand Up @@ -91,6 +98,10 @@ public static function resolve(
return self::getLiteralTypeFromScalarValue($left->value * $right->value);
}

if ($left instanceof Type\Atomic\TKeyedArray && $right instanceof Type\Atomic\TKeyedArray) {
return new Type\Atomic\TKeyedArray($left->properties + $right->properties);
}

return new Type\Atomic\TMixed;
}

Expand Down Expand Up @@ -133,14 +144,37 @@ public static function resolve(

if ($c instanceof UnresolvedConstant\ArrayValue) {
$properties = [];
$auto_key = 0;

if (!$c->entries) {
return new Type\Atomic\TArray([Type::getEmpty(), Type::getEmpty()]);
}

$is_list = true;

foreach ($c->entries as $i => $entry) {
foreach ($c->entries as $entry) {
if ($entry instanceof UnresolvedConstant\ArraySpread) {
$spread_array = self::resolve(
$classlikes,
$entry->array,
$statements_analyzer,
$visited_constant_ids + [$c_id => true]
);

if ($spread_array instanceof Type\Atomic\TArray && $spread_array->type_params[1]->isEmpty()) {
continue;
}

if (!$spread_array instanceof Type\Atomic\TKeyedArray) {
return new Type\Atomic\TArray([Type::getArrayKey(), Type::getMixed()]);
}

foreach ($spread_array->properties as $spread_array_type) {
$properties[$auto_key++] = $spread_array_type;
}
continue;
}

if ($entry->key) {
$key_type = self::resolve(
$classlikes,
Expand All @@ -150,18 +184,23 @@ public static function resolve(
);

if (!$key_type instanceof Type\Atomic\TLiteralInt
|| $key_type->value !== $i
|| $key_type->value !== $auto_key
) {
$is_list = false;
}
} else {
$key_type = new Type\Atomic\TLiteralInt($i);
$key_type = new Type\Atomic\TLiteralInt($auto_key);
}

if ($key_type instanceof Type\Atomic\TLiteralInt
|| $key_type instanceof Type\Atomic\TLiteralString
) {
$key_value = $key_type->value;
if ($key_type instanceof Type\Atomic\TLiteralInt) {
$auto_key = $key_type->value + 1;
} elseif (ctype_digit($key_type->value)) {
$auto_key = ((int) $key_type->value) + 1;
}
} else {
return new Type\Atomic\TArray([Type::getArrayKey(), Type::getMixed()]);
}
Expand All @@ -176,12 +215,19 @@ public static function resolve(
$properties[$key_value] = $value_type;
}

$objectlike = new Type\Atomic\TKeyedArray($properties);
if (empty($properties)) {
$resolved_type = new Type\Atomic\TArray([
new Type\Union([new Type\Atomic\TEmpty()]),
new Type\Union([new Type\Atomic\TEmpty()]),
]);
} else {
$resolved_type = new Type\Atomic\TKeyedArray($properties);

$objectlike->is_list = $is_list;
$objectlike->sealed = true;
$resolved_type->is_list = $is_list;
$resolved_type->sealed = true;
}

return $objectlike;
return $resolved_type;
}

if ($c instanceof UnresolvedConstant\ClassConstant) {
Expand Down
20 changes: 18 additions & 2 deletions src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@
use const PREG_SPLIT_DELIM_CAPTURE;
use const PREG_SPLIT_NO_EMPTY;
use function array_shift;
use function array_values;
use function get_class;

class ClassLikeNodeScanner
{
Expand Down Expand Up @@ -1161,17 +1163,31 @@ private function visitClassConstDeclaration(
$const
);

if ($const_type
&& $const->value instanceof \PhpParser\Node\Expr\BinaryOp\Concat
&& $const_type->isSingle()
&& get_class(array_values($const_type->getAtomicTypes())[0]) === Type\Atomic\TString::class
) {
// Prefer unresolved type over inferred string from concat, so that it can later be resolved to literal.
$const_type = null;
}

if ($const_type) {
$existing_constants[$const->name->name] = new \Psalm\Storage\ClassConstantStorage(
$const_type,
ClassLikeAnalyzer::VISIBILITY_PUBLIC,
$stmt->isProtected()
? ClassLikeAnalyzer::VISIBILITY_PROTECTED
: ($stmt->isPrivate()
? ClassLikeAnalyzer::VISIBILITY_PRIVATE
: ClassLikeAnalyzer::VISIBILITY_PUBLIC),
Comment on lines -1167 to +1182
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not quite sure if I'm fixing a mistake or introducing a bug here. I think it fixes a mistake, since it looks like these are existing constants for the current class, but none of the tests are affected either way, so I'm not sure exactly what affect this has.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is unnecessary — these constants are not actually used, but it's a harmless change so it's fine to keep.

null
);
} else {
$unresolved_const_expr = ExpressionResolver::getUnresolvedClassConstExpr(
$const->value,
$this->aliases,
$fq_classlike_name
$fq_classlike_name,
$storage->parent_class
);

if ($unresolved_const_expr) {
Expand Down
52 changes: 36 additions & 16 deletions src/Psalm/Internal/PhpVisitor/Reflector/ExpressionResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,30 @@
use Psalm\Internal\Analyzer\ClassLikeAnalyzer;
use Psalm\Internal\Scanner\UnresolvedConstant;
use Psalm\Internal\Scanner\UnresolvedConstantComponent;
use function assert;
use function strtolower;

class ExpressionResolver
{
public static function getUnresolvedClassConstExpr(
PhpParser\Node\Expr $stmt,
Aliases $aliases,
?string $fq_classlike_name
?string $fq_classlike_name,
?string $parent_fq_class_name = null
) : ?UnresolvedConstantComponent {
if ($stmt instanceof PhpParser\Node\Expr\BinaryOp) {
$left = self::getUnresolvedClassConstExpr(
$stmt->left,
$aliases,
$fq_classlike_name
$fq_classlike_name,
$parent_fq_class_name
);

$right = self::getUnresolvedClassConstExpr(
$stmt->right,
$aliases,
$fq_classlike_name
$fq_classlike_name,
$parent_fq_class_name
);

if (!$left || !$right) {
Expand Down Expand Up @@ -78,7 +82,8 @@ public static function getUnresolvedClassConstExpr(
$cond = self::getUnresolvedClassConstExpr(
$stmt->cond,
$aliases,
$fq_classlike_name
$fq_classlike_name,
$parent_fq_class_name
);

$if = null;
Expand All @@ -87,7 +92,8 @@ public static function getUnresolvedClassConstExpr(
$if = self::getUnresolvedClassConstExpr(
$stmt->if,
$aliases,
$fq_classlike_name
$fq_classlike_name,
$parent_fq_class_name
);

if ($if === null) {
Expand All @@ -98,7 +104,8 @@ public static function getUnresolvedClassConstExpr(
$else = self::getUnresolvedClassConstExpr(
$stmt->else,
$aliases,
$fq_classlike_name
$fq_classlike_name,
$parent_fq_class_name
);

if ($cond && $else && $if !== false) {
Expand Down Expand Up @@ -131,13 +138,15 @@ public static function getUnresolvedClassConstExpr(
$left = self::getUnresolvedClassConstExpr(
$stmt->var,
$aliases,
$fq_classlike_name
$fq_classlike_name,
$parent_fq_class_name
);

$right = self::getUnresolvedClassConstExpr(
$stmt->dim,
$aliases,
$fq_classlike_name
$fq_classlike_name,
$parent_fq_class_name
);

if ($left && $right) {
Expand All @@ -150,15 +159,20 @@ public static function getUnresolvedClassConstExpr(
&& $stmt->name instanceof PhpParser\Node\Identifier
&& $fq_classlike_name
&& $stmt->class->parts !== ['static']
&& $stmt->class->parts !== ['parent']
&& ($stmt->class->parts !== ['parent'] || $parent_fq_class_name !== null)
) {
if ($stmt->class->parts === ['self']) {
$const_fq_class_name = $fq_classlike_name;
} else {
$const_fq_class_name = ClassLikeAnalyzer::getFQCLNFromNameObject(
$stmt->class,
$aliases
);
if ($stmt->class->parts === ['parent']) {
assert($parent_fq_class_name !== null);
$const_fq_class_name = $parent_fq_class_name;
} else {
$const_fq_class_name = ClassLikeAnalyzer::getFQCLNFromNameObject(
$stmt->class,
$aliases
);
}
}

return new UnresolvedConstant\ClassConstant($const_fq_class_name, $stmt->name->name);
Expand Down Expand Up @@ -186,7 +200,8 @@ public static function getUnresolvedClassConstExpr(
$item_key_type = self::getUnresolvedClassConstExpr(
$item->key,
$aliases,
$fq_classlike_name
$fq_classlike_name,
$parent_fq_class_name
);

if (!$item_key_type) {
Expand All @@ -199,14 +214,19 @@ public static function getUnresolvedClassConstExpr(
$item_value_type = self::getUnresolvedClassConstExpr(
$item->value,
$aliases,
$fq_classlike_name
$fq_classlike_name,
$parent_fq_class_name
);

if (!$item_value_type) {
return null;
}

$items[] = new UnresolvedConstant\KeyValuePair($item_key_type, $item_value_type);
if ($item->unpack) {
$items[] = new UnresolvedConstant\ArraySpread($item_value_type);
} else {
$items[] = new UnresolvedConstant\KeyValuePair($item_key_type, $item_value_type);
}
}

return new UnresolvedConstant\ArrayValue($items);
Expand Down
19 changes: 19 additions & 0 deletions src/Psalm/Internal/Scanner/UnresolvedConstant/ArraySpread.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

namespace Psalm\Internal\Scanner\UnresolvedConstant;

use Psalm\Internal\Scanner\UnresolvedConstantComponent;

/**
* @psalm-immutable
*/
class ArraySpread extends UnresolvedConstantComponent
{
/** @var UnresolvedConstantComponent */
public $array;

public function __construct(UnresolvedConstantComponent $array)
{
$this->array = $array;
}
}
4 changes: 2 additions & 2 deletions src/Psalm/Internal/Scanner/UnresolvedConstant/ArrayValue.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@
*/
class ArrayValue extends UnresolvedConstantComponent
{
/** @var array<int, KeyValuePair> */
/** @var array<int, KeyValuePair|ArraySpread> */
public $entries;

/** @param list<KeyValuePair> $entries */
/** @param list<KeyValuePair|ArraySpread> $entries */
public function __construct(array $entries)
{
$this->entries = $entries;
Expand Down
Loading