From b3cf9d395842ab915eb73c7d6678e17e87801134 Mon Sep 17 00:00:00 2001 From: Brown Date: Tue, 10 Dec 2019 16:16:44 -0500 Subject: [PATCH] Catch circular references in constants Fixes #2453 --- .../Exception/CircularReferenceException.php | 6 ++ .../Fetch/ClassConstFetchAnalyzer.php | 13 ++++ .../Statements/ExpressionAnalyzer.php | 28 +++++--- .../Internal/Analyzer/StatementsAnalyzer.php | 2 + src/Psalm/Internal/Codebase/ClassLikes.php | 66 +++++++++++++++---- tests/ConstantTest.php | 15 +++++ 6 files changed, 108 insertions(+), 22 deletions(-) create mode 100644 src/Psalm/Exception/CircularReferenceException.php diff --git a/src/Psalm/Exception/CircularReferenceException.php b/src/Psalm/Exception/CircularReferenceException.php new file mode 100644 index 000000000..991557ee8 --- /dev/null +++ b/src/Psalm/Exception/CircularReferenceException.php @@ -0,0 +1,6 @@ +getSource(), $stmt) + ), + $statements_analyzer->getSuppressedIssues() + )) { + // fall through + } + return; } diff --git a/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php index 36fcf8bcd..fde598291 100644 --- a/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php @@ -1266,11 +1266,15 @@ class ExpressionAnalyzer 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 @@ class ExpressionAnalyzer } 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) { diff --git a/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php b/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php index 51babcec1..0763e3481 100644 --- a/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php @@ -1769,6 +1769,8 @@ class StatementsAnalyzer extends SourceAnalyzer implements StatementsSource return null; } catch (\InvalidArgumentException $e) { return null; + } catch (\Psalm\Exception\CircularReferenceException $e) { + return null; } } } diff --git a/src/Psalm/Internal/Codebase/ClassLikes.php b/src/Psalm/Internal/Codebase/ClassLikes.php index ed1ef4bae..799874ec9 100644 --- a/src/Psalm/Internal/Codebase/ClassLikes.php +++ b/src/Psalm/Internal/Codebase/ClassLikes.php @@ -1440,10 +1440,10 @@ class ClassLikes 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 @@ class ClassLikes } 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 @@ class ClassLikes 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 @@ class ClassLikes } 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 @@ class ClassLikes 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 @@ class ClassLikes 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 @@ class ClassLikes $c->fqcln, $c->name, ReflectionProperty::IS_PRIVATE, - $statements_analyzer + $statements_analyzer, + $visited_constant_ids + [$c_id => true] ); if ($found_type) { diff --git a/tests/ConstantTest.php b/tests/ConstantTest.php index 93492f089..f5f25986f 100644 --- a/tests/ConstantTest.php +++ b/tests/ConstantTest.php @@ -576,6 +576,21 @@ class ConstantTest extends TestCase }', 'error_message' => 'UndefinedConstant', ], + 'noCyclicConstReferences' => [ + ' 'CircularReference' + ], ]; } }