Skip to content
Permalink
Browse files

Catch circular references in constants

Fixes #2453
  • Loading branch information
muglug committed Dec 10, 2019
1 parent 20049eb commit b3cf9d395842ab915eb73c7d6678e17e87801134
@@ -0,0 +1,6 @@
<?php
namespace Psalm\Exception;

class CircularReferenceException extends \Exception
{
}
@@ -10,6 +10,7 @@
use Psalm\Codebase;
use Psalm\CodeLocation;
use Psalm\Context;
use Psalm\Issue\CircularReference;
use Psalm\Issue\DeprecatedClass;
use Psalm\Issue\DeprecatedConstant;
use Psalm\Issue\InaccessibleClassConstant;
@@ -206,6 +207,18 @@ public static function analyze(
$statements_analyzer
);
} catch (\InvalidArgumentException $_) {
return;
} catch (\Psalm\Exception\CircularReferenceException $e) {
if (IssueBuffer::accepts(
new CircularReference(
'Constant ' . $const_id . ' contains a circular reference',
new CodeLocation($statements_analyzer->getSource(), $stmt)
),
$statements_analyzer->getSuppressedIssues()
)) {
// fall through
}

This comment has been minimized.

Copy link
@Taluu

Taluu Dec 10, 2019

Contributor

why the if though ?

This comment has been minimized.

Copy link
@Taluu

Taluu Dec 10, 2019

Contributor

as nothing will be done, but it will return anyways, right ?

This comment has been minimized.

Copy link
@muglug

muglug Dec 10, 2019

Author Member

It’s a general pattern I use in the code - a lot of the time it’s extraneous


return;
}

@@ -1266,11 +1266,15 @@ private static function fleshOutAtomicType(
return new Type\Atomic\TLiteralClassString($return_type->fq_classlike_name);
}

$class_constant = $codebase->classlikes->getConstantForClass(
$return_type->fq_classlike_name,
$return_type->const_name,
\ReflectionProperty::IS_PRIVATE
);
try {
$class_constant = $codebase->classlikes->getConstantForClass(
$return_type->fq_classlike_name,
$return_type->const_name,
\ReflectionProperty::IS_PRIVATE
);
} catch (\Psalm\Exception\CircularReferenceException $e) {
$class_constant = null;
}

if ($class_constant) {
if ($class_constant->isSingle()) {
@@ -1292,11 +1296,15 @@ private static function fleshOutAtomicType(
}

if ($evaluate && $codebase->classOrInterfaceExists($return_type->fq_classlike_name)) {
$class_constant_type = $codebase->classlikes->getConstantForClass(
$return_type->fq_classlike_name,
$return_type->const_name,
\ReflectionProperty::IS_PRIVATE
);
try {
$class_constant_type = $codebase->classlikes->getConstantForClass(
$return_type->fq_classlike_name,
$return_type->const_name,
\ReflectionProperty::IS_PRIVATE
);
} catch (\Psalm\Exception\CircularReferenceException $e) {
$class_constant_type = null;
}

if ($class_constant_type) {
foreach ($class_constant_type->getTypes() as $const_type_atomic) {
@@ -1769,6 +1769,8 @@ public static function getSimpleType(
return null;
} catch (\InvalidArgumentException $e) {
return null;
} catch (\Psalm\Exception\CircularReferenceException $e) {
return null;
}
}
}
@@ -1440,10 +1440,10 @@ public function getConstantForClass(
string $class_name,
string $constant_name,
int $visibility,
?\Psalm\Internal\Analyzer\StatementsAnalyzer $statements_analyzer = null
?\Psalm\Internal\Analyzer\StatementsAnalyzer $statements_analyzer = null,
array $visited_constant_ids = []
) : ?Type\Union {
$class_name = strtolower($class_name);

$storage = $this->classlike_storage_provider->get($class_name);

if ($visibility === ReflectionProperty::IS_PUBLIC) {
@@ -1481,7 +1481,13 @@ public function getConstantForClass(
}

if (isset($fallbacks[$constant_name])) {
return new Type\Union([$this->resolveConstantType($fallbacks[$constant_name], $statements_analyzer)]);
return new Type\Union([
$this->resolveConstantType(
$fallbacks[$constant_name],
$statements_analyzer,
$visited_constant_ids
)
]);
}

return null;
@@ -1509,15 +1515,30 @@ private static function getLiteralTypeFromScalarValue($value) : Type\Atomic

private function resolveConstantType(
\Psalm\Internal\Scanner\UnresolvedConstantComponent $c,
\Psalm\Internal\Analyzer\StatementsAnalyzer $statements_analyzer = null
\Psalm\Internal\Analyzer\StatementsAnalyzer $statements_analyzer = null,
array $visited_constant_ids = []
) : Type\Atomic {
$c_id = \spl_object_id($c);

if (isset($visited_constant_ids[$c_id])) {
throw new \Psalm\Exception\CircularReferenceException('Found a circular reference');
}

if ($c instanceof UnresolvedConstant\ScalarValue) {
return self::getLiteralTypeFromScalarValue($c->value);
}

if ($c instanceof UnresolvedConstant\UnresolvedBinaryOp) {
$left = $this->resolveConstantType($c->left, $statements_analyzer);
$right = $this->resolveConstantType($c->right, $statements_analyzer);
$left = $this->resolveConstantType(
$c->left,
$statements_analyzer,
$visited_constant_ids + [$c_id => true]
);
$right = $this->resolveConstantType(
$c->right,
$statements_analyzer,
$visited_constant_ids + [$c_id => true]
);

if ($left instanceof Type\Atomic\TMixed || $right instanceof Type\Atomic\TMixed) {
return new Type\Atomic\TMixed;
@@ -1566,9 +1587,21 @@ private function resolveConstantType(
}

if ($c instanceof UnresolvedConstant\UnresolvedTernary) {
$cond = $this->resolveConstantType($c->cond, $statements_analyzer);
$if = $c->if ? $this->resolveConstantType($c->if, $statements_analyzer) : null;
$else = $this->resolveConstantType($c->else, $statements_analyzer);
$cond = $this->resolveConstantType(
$c->cond,
$statements_analyzer,
$visited_constant_ids + [$c_id => true]
);
$if = $c->if ? $this->resolveConstantType(
$c->if,
$statements_analyzer,
$visited_constant_ids + [$c_id => true]
) : null;
$else = $this->resolveConstantType(
$c->else,
$statements_analyzer,
$visited_constant_ids + [$c_id => true]
);

if ($cond instanceof Type\Atomic\TLiteralFloat
|| $cond instanceof Type\Atomic\TLiteralInt
@@ -1593,7 +1626,11 @@ private function resolveConstantType(

foreach ($c->entries as $i => $entry) {
if ($entry->key) {
$key_type = $this->resolveConstantType($entry->key, $statements_analyzer);
$key_type = $this->resolveConstantType(
$entry->key,
$statements_analyzer,
$visited_constant_ids + [$c_id => true]
);
} else {
$key_type = new Type\Atomic\TLiteralInt($i);
}
@@ -1606,7 +1643,11 @@ private function resolveConstantType(
return new Type\Atomic\TArray([Type::getArrayKey(), Type::getMixed()]);
}

$value_type = new Type\Union([$this->resolveConstantType($entry->value, $statements_analyzer)]);
$value_type = new Type\Union([$this->resolveConstantType(
$entry->value,
$statements_analyzer,
$visited_constant_ids + [$c_id => true]
)]);

$properties[$key_value] = $value_type;
}
@@ -1623,7 +1664,8 @@ private function resolveConstantType(
$c->fqcln,
$c->name,
ReflectionProperty::IS_PRIVATE,
$statements_analyzer
$statements_analyzer,
$visited_constant_ids + [$c_id => true]
);

if ($found_type) {
@@ -576,6 +576,21 @@ public function x() : void {
}',
'error_message' => 'UndefinedConstant',
],
'noCyclicConstReferences' => [
'<?php
class A {
const FOO = B::FOO;
}
class B {
const FOO = C::FOO;
}
class C {
const FOO = A::FOO;
}',
'error_message' => 'CircularReference'
],
];
}
}

0 comments on commit b3cf9d3

Please sign in to comment.
You can’t perform that action at this time.