From 3eb902d0058f365413f3cb3381056c969d5efce4 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Sun, 1 Jan 2017 19:09:17 -0500 Subject: [PATCH] Fix #30 by consolidating properties into single storage array --- src/Psalm/Checker/ClassChecker.php | 60 +- src/Psalm/Checker/ClassLikeChecker.php | 567 ++++++++++++------ src/Psalm/Checker/InterfaceChecker.php | 26 +- src/Psalm/Checker/MethodChecker.php | 65 +- .../Expression/AssignmentChecker.php | 127 ++-- .../Statements/Expression/FetchChecker.php | 134 ++--- src/Psalm/Checker/TypeChecker.php | 15 +- src/Psalm/Storage/ClassLikeStorage.php | 71 +-- src/Psalm/Storage/PropertyStorage.php | 22 + tests/ArrayAssignmentTest.php | 16 +- tests/ClassScopeTest.php | 387 ++++++++++++ tests/InterfaceTest.php | 10 +- tests/PropertyTypeTest.php | 47 ++ tests/ScopeTest.php | 204 ------- tests/TraitTest.php | 30 +- 15 files changed, 1061 insertions(+), 720 deletions(-) create mode 100644 src/Psalm/Storage/PropertyStorage.php create mode 100644 tests/ClassScopeTest.php diff --git a/src/Psalm/Checker/ClassChecker.php b/src/Psalm/Checker/ClassChecker.php index eefe52d20d2..78bc0ae994b 100644 --- a/src/Psalm/Checker/ClassChecker.php +++ b/src/Psalm/Checker/ClassChecker.php @@ -11,27 +11,6 @@ class ClassChecker extends ClassLikeChecker */ protected $class; - /** - * A lookup table of existing classes - * - * @var array - */ - protected static $existing_classes = []; - - /** - * A lookup table of existing classes, all lowercased - * - * @var array - */ - protected static $existing_classes_ci = []; - - /** - * A lookup table used for caching the results of classExtends calls - * - * @var array> - */ - protected static $class_extends = []; - /** * @var integer */ @@ -59,14 +38,14 @@ public function __construct(PhpParser\Node\Stmt\ClassLike $class, StatementsSour self::$existing_classes[$fq_class_name] = true; self::$existing_classes_ci[strtolower($fq_class_name)] = true; + self::$class_extends[$this->fq_class_name] = []; + if ($this->class->extends) { $this->parent_class = self::getFQCLNFromNameObject( $this->class->extends, $this->namespace, $this->aliased_classes ); - - self::$class_extends[$this->fq_class_name][$this->parent_class] = true; } foreach ($class->implements as $interface_name) { @@ -100,27 +79,19 @@ public static function classExists($fq_class_name) return true; } - $old_level = error_reporting(); - error_reporting(0); - $class_exists = class_exists($fq_class_name); - error_reporting($old_level); - - if ($class_exists) { - $old_level = error_reporting(); - error_reporting(0); - $reflected_class = new \ReflectionClass($fq_class_name); - error_reporting($old_level); - - self::$existing_classes_ci[strtolower($fq_class_name)] = true; - self::$existing_classes[$reflected_class->getName()] = true; + if (parent::registerClassLike($fq_class_name) === false) { + self::$existing_classes[$fq_class_name] = false; - return true; + return false; } - // we can only be sure that the case-sensitive version does not exist - self::$existing_classes[$fq_class_name] = false; + if (!isset(self::$existing_classes_ci[strtolower($fq_class_name)])) { + // it exists, but it's not a class + self::$existing_classes_ci[strtolower($fq_class_name)] = false; + return false; + } - return false; + return true; } /** @@ -179,7 +150,7 @@ public static function classExtends($fq_class_name, $possible_parent) */ public static function getInterfacesForClass($fq_class_name) { - self::registerClass($fq_class_name); + self::registerClassLike($fq_class_name); return self::$storage[$fq_class_name]->class_implements; } @@ -203,7 +174,7 @@ public static function classImplements($fq_class_name, $interface) return false; } - if (self::registerClass($fq_class_name) === false) { + if (self::registerClassLike($fq_class_name) === false) { return false; } @@ -217,11 +188,6 @@ public static function classImplements($fq_class_name, $interface) */ public static function clearCache() { - self::$existing_classes = []; - self::$existing_classes_ci = []; - - self::$class_extends = []; - self::$anonymous_class_count = 0; MethodChecker::clearCache(); diff --git a/src/Psalm/Checker/ClassLikeChecker.php b/src/Psalm/Checker/ClassLikeChecker.php index 280cff7981d..b4d0fe4b5d7 100644 --- a/src/Psalm/Checker/ClassLikeChecker.php +++ b/src/Psalm/Checker/ClassLikeChecker.php @@ -8,6 +8,8 @@ use Psalm\Exception\DocblockParseException; use Psalm\Issue\InvalidClass; use Psalm\Issue\InvalidDocblock; +use Psalm\Issue\InaccessibleMethod; +use Psalm\Issue\InaccessibleProperty; use Psalm\Issue\MissingPropertyType; use Psalm\Issue\UndefinedClass; use Psalm\Issue\UndefinedTrait; @@ -15,6 +17,8 @@ use Psalm\IssueBuffer; use Psalm\StatementsSource; use Psalm\Storage\ClassLikeStorage; +use Psalm\Storage\MethodStorage; +use Psalm\Storage\PropertyStorage; use Psalm\Type; use ReflectionClass; use ReflectionMethod; @@ -22,6 +26,10 @@ abstract class ClassLikeChecker extends SourceChecker implements StatementsSource { + const VISIBILITY_PUBLIC = 1; + const VISIBILITY_PROTECTED = 2; + const VISIBILITY_PRIVATE = 3; + /** * @var array */ @@ -34,7 +42,9 @@ abstract class ClassLikeChecker extends SourceChecker implements StatementsSourc 'object', 'empty', 'callable', - 'array' + 'array', + 'null', + 'mixed', ]; /** @@ -106,6 +116,27 @@ abstract class ClassLikeChecker extends SourceChecker implements StatementsSourc */ public static $file_classes = []; + /** + * A lookup table of existing classes + * + * @var array + */ + protected static $existing_classes = []; + + /** + * A lookup table of existing classes, all lowercased + * + * @var array + */ + protected static $existing_classes_ci = []; + + /** + * A lookup table used for caching the results of classExtends calls + * + * @var array> + */ + protected static $class_extends = []; + /** * @param PhpParser\Node\Stmt\ClassLike $class * @param StatementsSource $source @@ -182,7 +213,7 @@ public function check($check_methods = true, Context $class_context = null, $upd } if ($this instanceof ClassChecker) { - if ($this->parent_class && $this->registerParentClassProperties($this->parent_class) === false) { + if ($this->parent_class && $this->registerParentClassInfo($this->parent_class) === false) { return false; } } @@ -261,25 +292,13 @@ public function check($check_methods = true, Context $class_context = null, $upd (new StatementsChecker($this))->check($leftover_stmts, $class_context); } - $all_instance_properties = array_merge( - $storage->public_class_properties, - $storage->protected_class_properties, - $storage->private_class_properties - ); - - foreach ($all_instance_properties as $property_name => $property_type) { - $class_context->vars_in_scope['$this->' . $property_name] = $property_type ?: Type::getMixed(); - } - - $all_static_properties = array_merge( - $storage->public_static_class_properties, - $storage->protected_static_class_properties, - $storage->private_static_class_properties - ); - - foreach ($all_static_properties as $property_name => $property_type) { - $class_context->vars_in_scope[$this->fq_class_name . '::$' . $property_name] = $property_type - ?: Type::getMixed(); + foreach ($storage->properties as $property_name => $property) { + if ($property->is_static) { + $class_context->vars_in_scope[$this->fq_class_name . '::$' . $property_name] = $property->type + ?: Type::getMixed(); + } else { + $class_context->vars_in_scope['$this->' . $property_name] = $property->type ?: Type::getMixed(); + } } $config = Config::getInstance(); @@ -290,26 +309,49 @@ public function check($check_methods = true, Context $class_context = null, $upd $storage->public_class_constants += $interface_storage->public_class_constants; - foreach ($interface_storage->public_class_methods as $method_name => $_) { - $mentioned_method_id = $interface_name . '::' . $method_name; - $implemented_method_id = $this->fq_class_name . '::' . $method_name; - - MethodChecker::setOverriddenMethodId($implemented_method_id, $mentioned_method_id); - - if (!isset($storage->public_class_methods[$method_name]) && !$this->class->isAbstract()) { - $cased_method_id = MethodChecker::getCasedMethodId($mentioned_method_id); - - if (IssueBuffer::accepts( - new UnimplementedInterfaceMethod( - 'Method ' . $cased_method_id . ' is not defined on class ' . $this->fq_class_name, - new CodeLocation($this, $this->class, true) - ), - $this->suppressed_issues - )) { - return false; + foreach ($interface_storage->methods as $method_name => $method) { + if ($method->visibility === self::VISIBILITY_PUBLIC) { + $mentioned_method_id = $interface_name . '::' . $method_name; + $implemented_method_id = $this->fq_class_name . '::' . $method_name; + + MethodChecker::setOverriddenMethodId($implemented_method_id, $mentioned_method_id); + + $declaring_method_id = MethodChecker::getDeclaringMethodId($implemented_method_id); + + $method_storage = $declaring_method_id + ? MethodChecker::getStorage($declaring_method_id) + : null; + + if (!$method_storage) { + $cased_method_id = MethodChecker::getCasedMethodId($mentioned_method_id); + + if (IssueBuffer::accepts( + new UnimplementedInterfaceMethod( + 'Method ' . $cased_method_id . ' is not defined on class ' . $this->fq_class_name, + new CodeLocation($this, $this->class, true) + ), + $this->suppressed_issues + )) { + return false; + } + + return null; + } elseif ($method_storage->visibility !== self::VISIBILITY_PUBLIC) { + $cased_method_id = MethodChecker::getCasedMethodId($mentioned_method_id); + + if (IssueBuffer::accepts( + new InaccessibleMethod( + 'Interface-defined method ' . $cased_method_id . + ' must be public in ' . $this->fq_class_name, + new CodeLocation($this, $this->class, true) + ), + $this->suppressed_issues + )) { + return false; + } + + return null; } - - return null; } } } @@ -357,7 +399,7 @@ public function check($check_methods = true, Context $class_context = null, $upd * @param string $parent_class * @return false|null */ - protected function registerParentClassProperties($parent_class) + protected function registerParentClassInfo($parent_class) { if (!$this->class instanceof PhpParser\Node\Stmt\Class_) { throw new \UnexpectedValueException('Cannot register parent class where none exists'); @@ -376,9 +418,13 @@ protected function registerParentClassProperties($parent_class) return false; } - self::registerClass($parent_class); + self::registerClassLike($parent_class); + + self::$class_extends[$this->fq_class_name] = self::$class_extends[$this->parent_class]; + self::$class_extends[$this->fq_class_name][$this->parent_class] = true; $this->registerInheritedMethods($parent_class); + $this->registerInheritedProperties($parent_class); $storage = self::$storage[$this->fq_class_name]; @@ -388,12 +434,6 @@ protected function registerParentClassProperties($parent_class) $storage->class_implements += $parent_storage->class_implements; - $storage->public_class_properties = $parent_storage->public_class_properties; - $storage->protected_class_properties = $parent_storage->protected_class_properties; - - $storage->public_static_class_properties = $parent_storage->public_static_class_properties; - $storage->protected_static_class_properties = $parent_storage->protected_static_class_properties; - $storage->public_class_constants = $parent_storage->public_class_constants; $storage->used_traits = $parent_storage->used_traits; @@ -443,12 +483,6 @@ protected function visitClassMethod( $this instanceof TraitChecker ? $implemented_method_id : $method_id ); } - - if ($stmt->isPublic()) { - $storage->public_class_methods[strtolower($stmt->name)] = true; - } elseif ($stmt->isProtected()) { - $storage->protected_class_methods[strtolower($stmt->name)] = true; - } } /** @@ -545,7 +579,7 @@ protected function visitPropertyDeclaration( ) { $comment = $stmt->getDocComment(); $type_in_comment = null; - $storage = self::$storage[$class_context->self]; + $storage = self::$storage[$this->fq_class_name]; if ($comment && $config->use_docblock_types) { try { @@ -586,23 +620,26 @@ protected function visitPropertyDeclaration( $property_type = count($stmt->props) === 1 ? $property_group_type : clone $property_group_type; } - if ($stmt->isStatic()) { - if ($stmt->isPublic()) { - $storage->public_static_class_properties[$property->name] = $property_type; - } elseif ($stmt->isProtected()) { - $storage->protected_static_class_properties[$property->name] = $property_type; - } elseif ($stmt->isPrivate()) { - $storage->private_static_class_properties[$property->name] = $property_type; - } - } else { - if ($stmt->isPublic()) { - $storage->public_class_properties[$property->name] = $property_type; - } elseif ($stmt->isProtected()) { - $storage->protected_class_properties[$property->name] = $property_type; - } elseif ($stmt->isPrivate()) { - $storage->private_class_properties[$property->name] = $property_type; - } + $storage->properties[$property->name] = new PropertyStorage(); + $storage->properties[$property->name]->is_static = (bool)$stmt->isStatic(); + $storage->properties[$property->name]->type = $property_type; + + if ($stmt->isPublic()) { + $storage->properties[$property->name]->visibility = self::VISIBILITY_PUBLIC; + } elseif ($stmt->isProtected()) { + $storage->properties[$property->name]->visibility = self::VISIBILITY_PROTECTED; + } elseif ($stmt->isPrivate()) { + $storage->properties[$property->name]->visibility = self::VISIBILITY_PRIVATE; } + + $property_id = $this->fq_class_name . '::$' . $property->name; + + $implemented_property_id = $class_context->self . '::$' . $property->name; + + $storage->declaring_property_ids[$property->name] = $property_id; + $storage->appearing_property_ids[$property->name] = $this instanceof TraitChecker + ? $implemented_property_id + : $property_id; } } @@ -645,7 +682,7 @@ public static function getMethodChecker($method_id) return self::$method_checkers[$method_id]; } - MethodChecker::registerClassMethod($method_id); + MethodChecker::registerClassLikeMethod($method_id); /** @var string */ $declaring_method_id = MethodChecker::getDeclaringMethodId($method_id); @@ -886,7 +923,7 @@ public function hasCustomGet() * @return boolean * @psalm-suppress MixedMethodCall due to Reflection class weirdness */ - public static function registerClass($class_name) + public static function registerClassLike($class_name) { if (isset(self::$storage[$class_name]) && self::$storage[$class_name]->registered) { return true; @@ -903,6 +940,7 @@ public static function registerClass($class_name) $reflected_class = new ReflectionClass($class_name); } catch (\ReflectionException $e) { error_reporting($old_level); + return false; } @@ -932,9 +970,15 @@ public static function registerClass($class_name) $public_mapped_properties = self::getPropertyMap()[strtolower($class_name)]; foreach ($public_mapped_properties as $property_name => $public_mapped_property) { - $storage->public_class_properties[$property_name] = Type::parseString( - $public_mapped_property - ); + $property_type = Type::parseString($public_mapped_property); + $storage->properties[$property_name] = new PropertyStorage(); + $storage->properties[$property_name]->type = $property_type; + $storage->properties[$property_name]->visibility = self::VISIBILITY_PUBLIC; + + $property_id = $class_name . '::$' . $property_name; + + $storage->declaring_property_ids[$property_name] = $property_id; + $storage->appearing_property_ids[$property_name] = $property_id; } } } else { @@ -957,58 +1001,70 @@ protected static function registerReflectedClass($class_name, ReflectionClass $r $parent_class = $reflected_class->getParentClass(); - $storage = self::$storage[$class_name] = new ClassLikeStorage(); + $cased_name = $reflected_class->getName(); + + if ($cased_name === 'LibXMLError') { + $cased_name = 'libXMLError'; + } + + $storage = self::$storage[$cased_name] = new ClassLikeStorage(); + + self::$existing_classes_ci[strtolower($class_name)] = true; + self::$existing_classes[$cased_name] = true; + + self::$class_extends[$cased_name] = []; if ($parent_class) { $parent_class_name = $parent_class->getName(); self::registerReflectedClass($parent_class_name, $parent_class); $parent_storage = self::$storage[$parent_class_name]; - - $storage->public_class_properties = $parent_storage->public_class_properties; - $storage->protected_class_properties = $parent_storage->protected_class_properties; - - $storage->public_static_class_properties = $parent_storage->public_static_class_properties; - $storage->protected_static_class_properties = $parent_storage->protected_static_class_properties; } $class_properties = $reflected_class->getProperties(); + $public_mapped_properties = self::inPropertyMap($class_name) + ? self::getPropertyMap()[strtolower($class_name)] + : []; + /** @var \ReflectionProperty $class_property */ foreach ($class_properties as $class_property) { + $property_name = $class_property->getName(); + $storage->properties[$property_name] = new PropertyStorage(); + + $storage->properties[$property_name]->type = Type::getMixed(); + if ($class_property->isStatic()) { - if ($class_property->isPublic()) { - $storage->public_static_class_properties[$class_property->getName()] = - Type::getMixed(); - } elseif ($class_property->isProtected()) { - $storage->protected_static_class_properties[$class_property->getName()] = - Type::getMixed(); - } elseif ($class_property->isPrivate()) { - $storage->private_static_class_properties[$class_property->getName()] = - Type::getMixed(); - } - } else { - if ($class_property->isPublic()) { - $storage->public_class_properties[$class_property->getName()] = - Type::getMixed(); - } elseif ($class_property->isProtected()) { - $storage->protected_class_properties[$class_property->getName()] = - Type::getMixed(); - } elseif ($class_property->isPrivate()) { - $storage->private_class_properties[$class_property->getName()] = - Type::getMixed(); - } + $storage->properties[$property_name]->is_static = true; } + + if ($class_property->isPublic()) { + $storage->properties[$property_name]->visibility = self::VISIBILITY_PUBLIC; + } elseif ($class_property->isProtected()) { + $storage->properties[$property_name]->visibility = self::VISIBILITY_PROTECTED; + } elseif ($class_property->isPrivate()) { + $storage->properties[$property_name]->visibility = self::VISIBILITY_PRIVATE; + } + + $property_id = (string)$class_property->class . '::$' . $property_name; + + $storage->declaring_property_ids[$property_name] = $property_id; + $storage->appearing_property_ids[$property_name] = $property_id; } - if (self::inPropertyMap($class_name)) { - $public_mapped_properties = self::getPropertyMap()[strtolower($class_name)]; + // have to do this separately as there can be new properties here + foreach ($public_mapped_properties as $property_name => $type) { + if (!isset($storage->properties[$property_name])) { + $storage->properties[$property_name] = new PropertyStorage(); + $storage->properties[$property_name]->visibility = self::VISIBILITY_PUBLIC; - foreach ($public_mapped_properties as $property_name => $public_mapped_property) { - $storage->public_class_properties[$property_name] = Type::parseString( - $public_mapped_property - ); + $property_id = $cased_name . '::$' . $property_name; + + $storage->declaring_property_ids[$property_name] = $property_id; + $storage->appearing_property_ids[$property_name] = $property_id; } + + $storage->properties[$property_name]->type = Type::parseString($type); } /** @var array */ @@ -1037,9 +1093,6 @@ protected static function registerReflectedClass($class_name, ReflectionClass $r $storage->class_implements[strtolower($interface_name)] = $interface_name; } - $storage->protected_class_methods = []; - $storage->public_class_methods = []; - /** @var \ReflectionMethod $reflection_method */ foreach ($reflection_methods as $reflection_method) { MethodChecker::extractReflectionMethodInfo($reflection_method); @@ -1050,13 +1103,12 @@ protected static function registerReflectedClass($class_name, ReflectionClass $r $reflection_method->class . '::' . strtolower($reflection_method->name) ); - $storage->public_class_methods[strtolower((string)$reflection_method->name)] = true; - } + MethodChecker::setAppearingMethodId( + $class_name . '::' . strtolower($reflection_method->name), + $reflection_method->class . '::' . strtolower($reflection_method->name) + ); - if (!$reflection_method->isAbstract() && - $reflection_method->getDeclaringClass()->getName() === $class_name - ) { - $storage->public_class_methods[strtolower((string)$reflection_method->getName())] = true; + continue; } } } @@ -1067,110 +1119,83 @@ protected static function registerReflectedClass($class_name, ReflectionClass $r */ protected function registerInheritedMethods($parent_class) { - $storage = self::$storage[$this->fq_class_name]; $parent_storage = self::$storage[$parent_class]; + $storage = self::$storage[$this->fq_class_name]; - foreach ($parent_storage->public_class_methods as $method_name => $_) { + // register where they appear (can never be in a trait) + foreach ($parent_storage->appearing_method_ids as $method_name => $appearing_method_id) { $parent_method_id = $parent_class . '::' . $method_name; - /** @var string */ - $declaring_method_id = MethodChecker::getDeclaringMethodId($parent_method_id); + /** @var string */ $appearing_method_id = MethodChecker::getAppearingMethodId($parent_method_id); $implemented_method_id = $this->fq_class_name . '::' . $method_name; - if (!isset($storage->public_class_methods[$method_name])) { - MethodChecker::setDeclaringMethodId($implemented_method_id, $declaring_method_id); - MethodChecker::setAppearingMethodId($implemented_method_id, $appearing_method_id); - $storage->public_class_methods[$method_name] = true; - MethodChecker::setOverriddenMethodId($implemented_method_id, $declaring_method_id); - } + $storage->appearing_method_ids[$method_name] = $appearing_method_id; } - foreach ($parent_storage->protected_class_methods as $method_name => $_) { + // register where they're declared + foreach ($parent_storage->declaring_method_ids as $method_name => $declaring_method_id) { $parent_method_id = $parent_class . '::' . $method_name; + /** @var string */ $declaring_method_id = MethodChecker::getDeclaringMethodId($parent_method_id); - /** @var string */ - $appearing_method_id = MethodChecker::getAppearingMethodId($parent_method_id); $implemented_method_id = $this->fq_class_name . '::' . $method_name; - if (!isset($storage->protected_class_methods[$method_name])) { - MethodChecker::setDeclaringMethodId($implemented_method_id, $declaring_method_id); - MethodChecker::setAppearingMethodId($implemented_method_id, $appearing_method_id); - $storage->protected_class_methods[$method_name] = true; - MethodChecker::setOverriddenMethodId($implemented_method_id, $declaring_method_id); - } + $storage->declaring_method_ids[$method_name] = $declaring_method_id; + + MethodChecker::setOverriddenMethodId($implemented_method_id, $declaring_method_id); } } /** - * @param string $class_name - * @param mixed $visibility - * @return array + * @param string $parent_class + * @return void */ - public static function getInstancePropertiesForClass($class_name, $visibility) + protected function registerInheritedProperties($parent_class) { - if (self::registerClass($class_name) === false) { - return []; - } - - $storage = self::$storage[$class_name]; - - if ($visibility === ReflectionProperty::IS_PUBLIC) { - return $storage->public_class_properties; - } + $parent_storage = self::$storage[$parent_class]; + $storage = self::$storage[$this->fq_class_name]; - if ($visibility === ReflectionProperty::IS_PROTECTED) { - return array_merge( - $storage->public_class_properties, - $storage->protected_class_properties - ); + // register where they appear (can never be in a trait) + foreach ($parent_storage->appearing_property_ids as $property_name => $appearing_property_id) { + $storage->appearing_property_ids[$property_name] = $appearing_property_id; } - if ($visibility === ReflectionProperty::IS_PRIVATE) { - return array_merge( - $storage->public_class_properties, - $storage->protected_class_properties, - $storage->private_class_properties - ); + // register where they're declared + foreach ($parent_storage->declaring_property_ids as $property_name => $declaring_property_id) { + $storage->declaring_property_ids[$property_name] = $declaring_property_id; } - - throw new \InvalidArgumentException('Must specify $visibility'); } /** * @param string $class_name * @param mixed $visibility - * @return array + * @param bool $is_static + * @return array */ - public static function getStaticPropertiesForClass($class_name, $visibility) + public static function getPropertiesForClass($class_name, $visibility, $is_static) { - if (self::registerClass($class_name) === false) { + if (self::registerClassLike($class_name) === false) { return []; } $storage = self::$storage[$class_name]; - if ($visibility === ReflectionProperty::IS_PUBLIC) { - return $storage->public_static_class_properties; - } + $properties = []; - if ($visibility === ReflectionProperty::IS_PROTECTED) { - return array_merge( - $storage->public_static_class_properties, - $storage->protected_static_class_properties - ); - } - - if ($visibility === ReflectionProperty::IS_PRIVATE) { - return array_merge( - $storage->public_static_class_properties, - $storage->protected_static_class_properties, - $storage->private_static_class_properties - ); + foreach ($storage->properties as $property_name => $property) { + if (!$property->is_static) { + if ($visibility === ReflectionProperty::IS_PRIVATE || + $property->visibility === ClassLikeChecker::VISIBILITY_PUBLIC || + ($property->visibility === ClassLikeChecker::VISIBILITY_PROTECTED && + $visibility === ReflectionProperty::IS_PROTECTED) + ) { + $properties[$property_name] = $property; + } + } } - throw new \InvalidArgumentException('Must specify $visibility'); + return $properties; } /** @@ -1215,7 +1240,7 @@ public static function getConstantsForClass($class_name, $visibility) // remove for PHP 7.1 support $visibility = ReflectionProperty::IS_PUBLIC; - if (self::registerClass($class_name) === false) { + if (self::registerClassLike($class_name) === false) { return []; } @@ -1241,6 +1266,171 @@ public static function setConstantType($class_name, $const_name, Type\Union $typ $storage->public_class_constants[$const_name] = $type; } + /** + * Whether or not a given property exists + * + * @param string $property_id + * @return bool + */ + public static function propertyExists($property_id) + { + // remove trailing backslash if it exists + $property_id = preg_replace('/^\\\\/', '', $property_id); + + list($fq_class_name, $property_name) = explode('::$', $property_id); + + $old_property_id = null; + + if (ClassLikeChecker::registerClassLike($fq_class_name) === false) { + return false; + } + + $class_storage = self::$storage[$fq_class_name]; + + if (isset($class_storage->declaring_property_ids[$property_name])) { + return true; + } + + return false; + } + + /** + * @param string $property_id + * @param string|null $calling_context + * @param StatementsSource $source + * @param CodeLocation $code_location + * @param array $suppressed_issues + * @return false|null + */ + public static function checkPropertyVisibility( + $property_id, + $calling_context, + StatementsSource $source, + CodeLocation $code_location, + array $suppressed_issues + ) { + $declaring_property_class = self::getDeclaringClassForProperty($property_id); + $appearing_property_class = self::getAppearingClassForProperty($property_id); + + if (!$declaring_property_class || !$appearing_property_class) { + throw new \UnexpectedValueException( + 'Appearing/Declaring classes are not defined for ' . $property_id + ); + } + + list($property_class, $property_name) = explode('::$', (string)$property_id); + + // if the calling class is the same, we know the property exists, so it must be visible + if ($appearing_property_class === $calling_context) { + return null; + } + + if ($source->getSource() instanceof TraitChecker && $declaring_property_class === $source->getFQCLN()) { + return null; + } + + $class_storage = self::$storage[$declaring_property_class]; + + if (!$class_storage) { + throw new \UnexpectedValueException('$class_storage should not be null for ' . $declaring_property_class); + } + + $storage = $class_storage->properties[$property_name]; + + if (!$storage) { + throw new \UnexpectedValueException('$storage should not be null for ' . $property_id); + } + + switch ($storage->visibility) { + case self::VISIBILITY_PUBLIC: + return null; + + case self::VISIBILITY_PRIVATE: + if (!$calling_context || $appearing_property_class !== $calling_context) { + if (IssueBuffer::accepts( + new InaccessibleProperty( + 'Cannot access private property ' . $property_id . ' from context ' . $calling_context, + $code_location + ), + $suppressed_issues + )) { + return false; + } + } + + return null; + + case self::VISIBILITY_PROTECTED: + if ($appearing_property_class === $calling_context) { + return null; + } + + if (!$calling_context) { + if (IssueBuffer::accepts( + new InaccessibleProperty( + 'Cannot access protected property ' . $property_id, + $code_location + ), + $suppressed_issues + )) { + return false; + } + + return null; + } + + if (ClassChecker::classExtends($appearing_property_class, $calling_context)) { + return null; + } + + if (!ClassChecker::classExtends($calling_context, $appearing_property_class)) { + if (IssueBuffer::accepts( + new InaccessibleProperty( + 'Cannot access protected property ' . $property_id . ' from context ' . $calling_context, + $code_location + ), + $suppressed_issues + )) { + return false; + } + } + } + + return null; + } + + /** + * @param string $property_id + * @return string|null + */ + public static function getDeclaringClassForProperty($property_id) + { + list($fq_class_name, $property_name) = explode('::$', $property_id); + + if (isset(ClassLikeChecker::$storage[$fq_class_name]->declaring_property_ids[$property_name])) { + $declaring_property_id = ClassLikeChecker::$storage[$fq_class_name]->declaring_property_ids[$property_name]; + + return explode('::$', $declaring_property_id)[0]; + } + } + + /** + * Get the class this property appears in (vs is declared in, which could give a trait) + * + * @param string $property_id + * @return string|null + */ + public static function getAppearingClassForProperty($property_id) + { + list($fq_class_name, $property_name) = explode('::$', $property_id); + + if (isset(ClassLikeChecker::$storage[$fq_class_name]->appearing_property_ids[$property_name])) { + $appearing_property_id = ClassLikeChecker::$storage[$fq_class_name]->appearing_property_ids[$property_name]; + + return explode('::$', $appearing_property_id)[0]; + } + } + /** * @param string|null $this_class * @return void @@ -1284,7 +1474,7 @@ public static function getClassesForFile($file_name) */ public static function isUserDefined($fq_class_name) { - self::registerClass($fq_class_name); + self::registerClassLike($fq_class_name); $storage = self::$storage[$fq_class_name]; return $storage->user_defined; } @@ -1362,6 +1552,11 @@ public static function clearCache() self::$storage = []; + self::$existing_classes = []; + self::$existing_classes_ci = []; + + self::$class_extends = []; + ClassChecker::clearCache(); InterfaceChecker::clearCache(); TraitChecker::clearCache(); diff --git a/src/Psalm/Checker/InterfaceChecker.php b/src/Psalm/Checker/InterfaceChecker.php index 7c4cbfc3a12..308014c630f 100644 --- a/src/Psalm/Checker/InterfaceChecker.php +++ b/src/Psalm/Checker/InterfaceChecker.php @@ -61,25 +61,19 @@ public static function interfaceExists($interface) return false; } - $old_level = error_reporting(); - error_reporting(0); + if (self::registerClassLike($interface) === false) { + self::$existing_interfaces_ci[strtolower($interface)] = false; - $interface_exists = interface_exists($interface, true); - - error_reporting($old_level); - - if ($interface_exists) { - $reflected_interface = new \ReflectionClass($interface); - - self::$existing_interfaces_ci[strtolower($interface)] = true; - self::$existing_interfaces[$reflected_interface->getName()] = true; - return true; + return false; } - self::$existing_interfaces_ci[strtolower($interface)] = false; - self::$existing_interfaces_ci[$interface] = false; + if (!isset(self::$existing_interfaces_ci[strtolower($interface)])) { + // it exists, but it's not an interface + self::$existing_interfaces_ci[strtolower($interface)] = false; + return false; + } - return false; + return true; } /** @@ -111,7 +105,7 @@ public static function interfaceExtends($interface_name, $possible_parent) */ public static function getParentInterfaces($interface_name) { - if (self::registerClass($interface_name) === false) { + if (self::registerClassLike($interface_name) === false) { throw new \UnexpectedValueException('Cannot deal with unfound file'); } diff --git a/src/Psalm/Checker/MethodChecker.php b/src/Psalm/Checker/MethodChecker.php index fe69abf7e35..44895443c93 100644 --- a/src/Psalm/Checker/MethodChecker.php +++ b/src/Psalm/Checker/MethodChecker.php @@ -18,10 +18,6 @@ class MethodChecker extends FunctionLikeChecker { - const VISIBILITY_PUBLIC = 1; - const VISIBILITY_PROTECTED = 2; - const VISIBILITY_PRIVATE = 3; - /** * @param PhpParser\Node\FunctionLike $function * @param StatementsSource $source @@ -46,7 +42,7 @@ public function __construct($function, StatementsSource $source, array $this_var */ public static function getMethodParams($method_id) { - self::registerClassMethod($method_id); + self::registerClassLikeMethod($method_id); if ($method_id = self::getDeclaringMethodId($method_id)) { $storage = self::getStorage($method_id); @@ -63,7 +59,7 @@ public static function getMethodParams($method_id) */ public static function isVariadic($method_id) { - self::registerClassMethod($method_id); + self::registerClassLikeMethod($method_id); $method_id = (string)self::getDeclaringMethodId($method_id); @@ -121,7 +117,7 @@ public static function getMethodReturnType($method_id) */ public static function getMethodReturnTypeLocation($method_id, CodeLocation &$defined_location = null) { - self::registerClassMethod($method_id); + self::registerClassLikeMethod($method_id); /** @var string */ $method_id = self::getDeclaringMethodId($method_id); @@ -185,8 +181,8 @@ public static function extractReflectionMethodInfo(\ReflectionMethod $method) $class_storage->overridden_method_ids[$method_name] = []; $storage->visibility = $method->isPrivate() - ? self::VISIBILITY_PRIVATE - : ($method->isProtected() ? self::VISIBILITY_PROTECTED : self::VISIBILITY_PUBLIC); + ? ClassLikeChecker::VISIBILITY_PRIVATE + : ($method->isProtected() ? ClassLikeChecker::VISIBILITY_PROTECTED : ClassLikeChecker::VISIBILITY_PUBLIC); $params = $method->getParameters(); @@ -228,7 +224,7 @@ public static function checkMethodStatic( CodeLocation $code_location, array $suppressed_issues ) { - self::registerClassMethod($method_id); + self::registerClassLikeMethod($method_id); /** @var string */ $method_id = self::getDeclaringMethodId($method_id); @@ -308,11 +304,11 @@ protected function registerMethod(PhpParser\Node\Stmt\ClassMethod $method) $storage->file_name = $this->file_name; if ($method->isPrivate()) { - $storage->visibility = self::VISIBILITY_PRIVATE; + $storage->visibility = ClassLikeChecker::VISIBILITY_PRIVATE; } elseif ($method->isProtected()) { - $storage->visibility = self::VISIBILITY_PROTECTED; + $storage->visibility = ClassLikeChecker::VISIBILITY_PROTECTED; } else { - $storage->visibility = self::VISIBILITY_PUBLIC; + $storage->visibility = ClassLikeChecker::VISIBILITY_PUBLIC; } $method_param_names = []; @@ -522,7 +518,7 @@ public static function methodExists($method_id) $old_method_id = null; - if (ClassLikeChecker::registerClass($method_parts[0]) === false) { + if (ClassLikeChecker::registerClassLike($method_parts[0]) === false) { return false; } @@ -550,9 +546,9 @@ public static function methodExists($method_id) * @param string $method_id * @return void */ - public static function registerClassMethod($method_id) + public static function registerClassLikeMethod($method_id) { - ClassLikeChecker::registerClass(explode('::', $method_id)[0]); + ClassLikeChecker::registerClassLike(explode('::', $method_id)[0]); } /** @@ -563,7 +559,7 @@ public static function getStorage($method_id) { list($fq_class_name, $method_name) = explode('::', $method_id); - ClassLikeChecker::registerClass($fq_class_name); + ClassLikeChecker::registerClassLike($fq_class_name); $class_storage = ClassLikeChecker::$storage[$fq_class_name]; @@ -619,20 +615,17 @@ public static function checkMethodVisibility( CodeLocation $code_location, array $suppressed_issues ) { - self::registerClassMethod($method_id); - - $declared_method_id = self::getDeclaringMethodId($method_id); + self::registerClassLikeMethod($method_id); - $method_class = explode('::', (string)$method_id)[0]; - $declaring_method_class = explode('::', (string)$declared_method_id)[0]; - $method_name = explode('::', $method_id)[1]; + $declaring_method_id = self::getDeclaringMethodId($method_id); + $appearing_method_id = self::getAppearingMethodId($method_id); - if (TraitChecker::traitExists($declaring_method_class) && ClassLikeChecker::classUsesTrait($method_class, $declaring_method_class)) { - return null; - } + list($method_class, $method_name) = explode('::', (string)$method_id); + list($declaring_method_class) = explode('::', (string)$declaring_method_id); + list($appearing_method_class) = explode('::', (string)$appearing_method_id); // if the calling class is the same, we know the method exists, so it must be visible - if ($method_class === $calling_context) { + if ($appearing_method_class === $calling_context) { return null; } @@ -640,18 +633,18 @@ public static function checkMethodVisibility( return null; } - $storage = self::getStorage((string)$declared_method_id); + $storage = self::getStorage((string)$declaring_method_id); if (!$storage) { throw new \UnexpectedValueException('$storage should not be null'); } switch ($storage->visibility) { - case self::VISIBILITY_PUBLIC: + case ClassLikeChecker::VISIBILITY_PUBLIC: return null; - case self::VISIBILITY_PRIVATE: - if (!$calling_context || $declaring_method_class !== $calling_context) { + case ClassLikeChecker::VISIBILITY_PRIVATE: + if (!$calling_context || $appearing_method_class !== $calling_context) { if (IssueBuffer::accepts( new InaccessibleMethod( 'Cannot access private method ' . MethodChecker::getCasedMethodId($method_id) . @@ -666,8 +659,8 @@ public static function checkMethodVisibility( return null; - case self::VISIBILITY_PROTECTED: - if ($declaring_method_class === $calling_context) { + case ClassLikeChecker::VISIBILITY_PROTECTED: + if ($appearing_method_class === $calling_context) { return null; } @@ -685,13 +678,11 @@ public static function checkMethodVisibility( return null; } - if (ClassChecker::classExtends($declaring_method_class, $calling_context) && - MethodChecker::methodExists($calling_context . '::' . $method_name) - ) { + if (ClassChecker::classExtends($appearing_method_class, $calling_context)) { return null; } - if (!ClassChecker::classExtends($calling_context, $declaring_method_class)) { + if (!ClassChecker::classExtends($calling_context, $appearing_method_class)) { if (IssueBuffer::accepts( new InaccessibleMethod( 'Cannot access protected method ' . MethodChecker::getCasedMethodId($method_id) . diff --git a/src/Psalm/Checker/Statements/Expression/AssignmentChecker.php b/src/Psalm/Checker/Statements/Expression/AssignmentChecker.php index 3a3cce90dd6..58059541cd6 100644 --- a/src/Psalm/Checker/Statements/Expression/AssignmentChecker.php +++ b/src/Psalm/Checker/Statements/Expression/AssignmentChecker.php @@ -309,12 +309,17 @@ public static function checkPropertyAssignment( return null; } - $class_properties = ClassLikeChecker::getInstancePropertiesForClass( - $context->self, - \ReflectionProperty::IS_PRIVATE - ); + $property_id = $context->self . '::$' . $prop_name; + + if (!ClassLikeChecker::propertyExists($property_id)) { + return null; + } - $class_property_type = $class_properties[$prop_name]; + $declaring_property_class = ClassLikeChecker::getDeclaringClassForProperty($property_id); + + $class_storage = ClassLikeChecker::$storage[$declaring_property_class]; + + $class_property_type = $class_storage->properties[$prop_name]->type; $class_property_types[] = $class_property_type ? clone $class_property_type : Type::getMixed(); @@ -429,12 +434,18 @@ public static function checkPropertyAssignment( ['stdclass', 'simplexmlelement', 'dateinterval', 'domdocument', 'domnode'] ) ) { - $context->vars_in_scope[$var_id] = Type::getMixed(); + if (strtolower($lhs_type_part->value) === 'stdclass') { + $context->vars_in_scope[$var_id] = $assignment_value_type; + } else { + $context->vars_in_scope[$var_id] = Type::getMixed(); + } + return null; } if (ExpressionChecker::isMock($lhs_type_part->value)) { $context->vars_in_scope[$var_id] = Type::getMixed(); + return null; } @@ -476,16 +487,13 @@ public static function checkPropertyAssignment( return null; } - $class_properties = ClassLikeChecker::getInstancePropertiesForClass( - $lhs_type_part->value, - $class_visibility - ); + $property_id = $lhs_type_part->value . '::$' . $prop_name; - if (!isset($class_properties[$prop_name])) { + if (!ClassLikeChecker::propertyExists($property_id)) { if ($stmt->var instanceof PhpParser\Node\Expr\Variable && $stmt->var->name === 'this') { if (IssueBuffer::accepts( new UndefinedThisPropertyAssignment( - 'Instance property ' . $lhs_type_part->value . '::$' . $prop_name . ' is not defined', + 'Instance property ' . $property_id . ' is not defined', new CodeLocation($statements_checker->getSource(), $stmt) ), $statements_checker->getSuppressedIssues() @@ -495,7 +503,7 @@ public static function checkPropertyAssignment( } else { if (IssueBuffer::accepts( new UndefinedPropertyAssignment( - 'Instance property ' . $lhs_type_part->value . '::$' . $prop_name . ' is not defined', + 'Instance property ' . $property_id . ' is not defined', new CodeLocation($statements_checker->getSource(), $stmt) ), $statements_checker->getSuppressedIssues() @@ -507,7 +515,23 @@ public static function checkPropertyAssignment( continue; } - $class_property_type = $class_properties[$prop_name]; + if (ClassLikeChecker::checkPropertyVisibility( + $property_id, + $context->self, + $statements_checker->getSource(), + new CodeLocation($statements_checker->getSource(), $stmt), + $statements_checker->getSuppressedIssues() + ) === false) { + return false; + } + + $declaring_property_class = ClassLikeChecker::getDeclaringClassForProperty( + $lhs_type_part->value . '::$' . $prop_name + ); + + $property_storage = ClassLikeChecker::$storage[$declaring_property_class]->properties[$stmt->name]; + + $class_property_type = $property_storage->type; if ($class_property_type === false) { if (IssueBuffer::accepts( @@ -625,36 +649,19 @@ protected static function checkStaticPropertyAssignment( $class_visibility = \ReflectionProperty::IS_PUBLIC; } - $class_properties = ClassLikeChecker::getStaticPropertiesForClass( - $fq_class_name, - $class_visibility - ); - - $all_class_properties = ClassLikeChecker::getStaticPropertiesForClass( - $fq_class_name, - $class_visibility - ); - $prop_name = $stmt->name; if (!is_string($prop_name)) { return; } - if (!isset($class_properties[$prop_name])) { - $all_class_properties = null; - - if ($class_visibility !== \ReflectionProperty::IS_PRIVATE) { - $all_class_properties = ClassLikeChecker::getStaticPropertiesForClass( - $fq_class_name, - \ReflectionProperty::IS_PRIVATE - ); - } + $property_id = $fq_class_name . '::$' . $prop_name; - if (isset($all_class_properties[$prop_name])) { + if (!ClassLikeChecker::propertyExists($property_id)) { + if ($stmt->class instanceof PhpParser\Node\Name && $stmt->class->parts[0] === 'this') { if (IssueBuffer::accepts( - new InaccessibleProperty( - 'Static property ' . $var_id . ' is not visible in this context', + new UndefinedThisPropertyAssignment( + 'Static property ' . $property_id . ' is not defined', new CodeLocation($statements_checker->getSource(), $stmt) ), $statements_checker->getSuppressedIssues() @@ -662,35 +669,39 @@ protected static function checkStaticPropertyAssignment( return false; } } else { - if ($stmt->class instanceof PhpParser\Node\Name && $stmt->class->parts[0] === 'this') { - if (IssueBuffer::accepts( - new UndefinedThisPropertyAssignment( - 'Static property ' . $var_id . ' is not defined', - new CodeLocation($statements_checker->getSource(), $stmt) - ), - $statements_checker->getSuppressedIssues() - )) { - return false; - } - } else { - if (IssueBuffer::accepts( - new UndefinedPropertyAssignment( - 'Static property ' . $var_id . ' is not defined', - new CodeLocation($statements_checker->getSource(), $stmt) - ), - $statements_checker->getSuppressedIssues() - )) { - return false; - } + if (IssueBuffer::accepts( + new UndefinedPropertyAssignment( + 'Static property ' . $property_id . ' is not defined', + new CodeLocation($statements_checker->getSource(), $stmt) + ), + $statements_checker->getSuppressedIssues() + )) { + return false; } } - return null; + return; + } + + if (ClassLikeChecker::checkPropertyVisibility( + $property_id, + $context->self, + $statements_checker->getSource(), + new CodeLocation($statements_checker->getSource(), $stmt), + $statements_checker->getSuppressedIssues() + ) === false) { + return false; } + $declaring_property_class = ClassLikeChecker::getDeclaringClassForProperty( + $fq_class_name . '::$' . $prop_name + ); + + $property_storage = ClassLikeChecker::$storage[$declaring_property_class]->properties[$stmt->name]; + $context->vars_in_scope[$var_id] = $assignment_value_type; - $class_property_type = $class_properties[$prop_name]; + $class_property_type = $property_storage->type; if ($class_property_type === false) { if (IssueBuffer::accepts( diff --git a/src/Psalm/Checker/Statements/Expression/FetchChecker.php b/src/Psalm/Checker/Statements/Expression/FetchChecker.php index 63e267e3751..645aa96d5e3 100644 --- a/src/Psalm/Checker/Statements/Expression/FetchChecker.php +++ b/src/Psalm/Checker/Statements/Expression/FetchChecker.php @@ -138,7 +138,7 @@ public static function checkPropertyFetch( if ($stmt_var_type->isNullable()) { if (IssueBuffer::accepts( new NullPropertyFetch( - 'Cannot get property on possibly null variable ' . $stmt_var_id, + 'Cannot get property on possibly null variable ' . $stmt_var_id . ' of type ' . $stmt_var_type, new CodeLocation($statements_checker->getSource(), $stmt) ), $statements_checker->getSuppressedIssues() @@ -222,48 +222,13 @@ public static function checkPropertyFetch( continue; } - if ($var_name === 'this' - || $lhs_type_part->value === $context->self - || ( - $statements_checker->getSource()->getSource() instanceof TraitChecker && - $lhs_type_part->value === $statements_checker->getSource()->getFQCLN() - ) - ) { - $class_visibility = \ReflectionProperty::IS_PRIVATE; - } elseif ($context->self && ClassChecker::classExtends($lhs_type_part->value, $context->self)) { - $class_visibility = \ReflectionProperty::IS_PROTECTED; - } else { - $class_visibility = \ReflectionProperty::IS_PUBLIC; - } - - $class_properties = ClassLikeChecker::getInstancePropertiesForClass( - $lhs_type_part->value, - $class_visibility - ); - - if (!$class_properties || !isset($class_properties[$stmt->name])) { - $stmt->inferredType = Type::getMixed(); - - $all_class_properties = ClassLikeChecker::getInstancePropertiesForClass( - $lhs_type_part->value, - \ReflectionProperty::IS_PRIVATE - ); - - if ($var_id) { - $context->vars_in_scope[$var_id] = Type::getMixed(); - } + $property_id = $lhs_type_part->value . '::$' . $stmt->name; - if ($all_class_properties && isset($all_class_properties[$stmt->name])) { - IssueBuffer::add( - new InaccessibleProperty( - 'Property ' . $var_id . ' is not visible in this context', - new CodeLocation($statements_checker->getSource(), $stmt) - ) - ); - } elseif ($stmt_var_id === '$this') { + if (!ClassLikeChecker::propertyExists($property_id)) { + if ($stmt_var_id === '$this') { if (IssueBuffer::accepts( new UndefinedThisPropertyFetch( - 'Instance property ' . $lhs_type_part->value .'::$' . $stmt->name . ' is not defined', + 'Instance property ' . $property_id . ' is not defined', new CodeLocation($statements_checker->getSource(), $stmt) ), $statements_checker->getSuppressedIssues() @@ -273,7 +238,7 @@ public static function checkPropertyFetch( } else { if (IssueBuffer::accepts( new UndefinedPropertyFetch( - 'Instance property ' . $lhs_type_part->value .'::$' . $stmt->name . ' is not defined', + 'Instance property ' . $property_id . ' is not defined', new CodeLocation($statements_checker->getSource(), $stmt) ), $statements_checker->getSuppressedIssues() @@ -282,10 +247,24 @@ public static function checkPropertyFetch( } } - return null; + return; } - $class_property_type = $class_properties[$stmt->name]; + if (ClassLikeChecker::checkPropertyVisibility( + $property_id, + $context->self, + $statements_checker->getSource(), + new CodeLocation($statements_checker->getSource(), $stmt), + $statements_checker->getSuppressedIssues() + ) === false) { + return false; + } + + $declaring_property_class = ClassLikeChecker::getDeclaringClassForProperty($property_id); + + $property_storage = ClassLikeChecker::$storage[$declaring_property_class]->properties[$stmt->name]; + + $class_property_type = $property_storage->type; if ($class_property_type === false) { if (IssueBuffer::accepts( @@ -538,57 +517,40 @@ public static function checkStaticPropertyFetch( return null; } - if ($fq_class_name === $context->self - || ( - $statements_checker->getSource()->getSource() instanceof TraitChecker && - $fq_class_name === $statements_checker->getSource()->getFQCLN() - ) - ) { - $class_visibility = \ReflectionProperty::IS_PRIVATE; - } elseif ($context->self && ClassChecker::classExtends($context->self, $fq_class_name)) { - $class_visibility = \ReflectionProperty::IS_PROTECTED; - } else { - $class_visibility = \ReflectionProperty::IS_PUBLIC; - } - - $visible_class_properties = ClassLikeChecker::getStaticPropertiesForClass( - $fq_class_name, - $class_visibility - ); + $property_id = $fq_class_name . '::$' . $stmt->name; - if (!isset($visible_class_properties[$stmt->name])) { - $all_class_properties = []; - - if ($fq_class_name !== $context->self) { - $all_class_properties = ClassLikeChecker::getStaticPropertiesForClass( - $fq_class_name, - \ReflectionProperty::IS_PRIVATE - ); + if (!ClassLikeChecker::propertyExists($property_id)) { + if (IssueBuffer::accepts( + new UndefinedPropertyFetch( + 'Static property ' . $property_id . ' is not defined', + new CodeLocation($statements_checker->getSource(), $stmt) + ), + $statements_checker->getSuppressedIssues() + )) { + return false; } - if ($all_class_properties && isset($all_class_properties[$stmt->name])) { - IssueBuffer::add( - new InaccessibleProperty( - 'Static property ' . $var_id . ' is not visible in this context', - new CodeLocation($statements_checker->getSource(), $stmt) - ) - ); - } else { - IssueBuffer::add( - new UndefinedPropertyFetch( - 'Static property ' . $var_id . ' does not exist', - new CodeLocation($statements_checker->getSource(), $stmt) - ) - ); - } + return; + } + if (ClassLikeChecker::checkPropertyVisibility( + $property_id, + $context->self, + $statements_checker->getSource(), + new CodeLocation($statements_checker->getSource(), $stmt), + $statements_checker->getSuppressedIssues() + ) === false) { return false; } - $visible_class_property = $visible_class_properties[$stmt->name]; + $declaring_property_class = ClassLikeChecker::getDeclaringClassForProperty( + $fq_class_name . '::$' . $stmt->name + ); + + $property = ClassLikeChecker::$storage[$declaring_property_class]->properties[$stmt->name]; - $context->vars_in_scope[$var_id] = $visible_class_property - ? clone $visible_class_property + $context->vars_in_scope[$var_id] = $property->type + ? clone $property->type : Type::getMixed(); $stmt->inferredType = clone $context->vars_in_scope[$var_id]; diff --git a/src/Psalm/Checker/TypeChecker.php b/src/Psalm/Checker/TypeChecker.php index 684c3cbeb25..dfbcdc063a1 100644 --- a/src/Psalm/Checker/TypeChecker.php +++ b/src/Psalm/Checker/TypeChecker.php @@ -816,17 +816,20 @@ protected static function getValueForKey($key, array &$existing_keys) foreach ($existing_keys[$base_key]->types as $existing_key_type_part) { if ($existing_key_type_part->isNull()) { $class_property_type = Type::getNull(); + } elseif ($existing_key_type_part->isMixed()) { + $class_property_type = Type::getMixed(); } else { - $class_properties = ClassLikeChecker::getInstancePropertiesForClass( - $existing_key_type_part->value, - \ReflectionProperty::IS_PRIVATE - ); + $property_id = $existing_key_type_part->value . '::$' . $key_parts[$i]; - if (!isset($class_properties[$key_parts[$i]])) { + if (!ClassLikeChecker::propertyExists($property_id)) { return null; } - $class_property_type = $class_properties[$key_parts[$i]]; + $declaring_property_class = ClassLikeChecker::getDeclaringClassForProperty($property_id); + + $class_storage = ClassLikeChecker::$storage[$declaring_property_class]; + + $class_property_type = $class_storage->properties[$key_parts[$i]]->type; $class_property_type = $class_property_type ? clone $class_property_type : Type::getMixed(); } diff --git a/src/Psalm/Storage/ClassLikeStorage.php b/src/Psalm/Storage/ClassLikeStorage.php index ce48ea0695c..6da72adc708 100644 --- a/src/Psalm/Storage/ClassLikeStorage.php +++ b/src/Psalm/Storage/ClassLikeStorage.php @@ -5,62 +5,6 @@ class ClassLikeStorage { - /** - * A lookup table of all public methods in this class - * - * @var array - */ - public $public_class_methods = []; - - /** - * A lookup table of all protected methods in this class - * - * @var array - */ - public $protected_class_methods = []; - - /** - * A lookup table for public class properties - * - * @var array - */ - public $public_class_properties = []; - - /** - * A lookup table for protected class properties - * - * @var array - */ - public $protected_class_properties = []; - - /** - * A lookup table for protected class properties - * - * @var array - */ - public $private_class_properties = []; - - /** - * A lookup table for public class properties - * - * @var array - */ - public $public_static_class_properties = []; - - /** - * A lookup table for protected class properties - * - * @var array - */ - public $protected_static_class_properties = []; - - /** - * A lookup table for private class properties - * - * @var array - */ - public $private_static_class_properties = []; - /** * A lookup table for public class constants * @@ -147,4 +91,19 @@ class ClassLikeStorage * @var array> */ public $overridden_method_ids = []; + + /** + * @var array + */ + public $properties = []; + + /** + * @var array + */ + public $declaring_property_ids = []; + + /** + * @var array + */ + public $appearing_property_ids = []; } diff --git a/src/Psalm/Storage/PropertyStorage.php b/src/Psalm/Storage/PropertyStorage.php new file mode 100644 index 00000000000..7f76341a2c7 --- /dev/null +++ b/src/Psalm/Storage/PropertyStorage.php @@ -0,0 +1,22 @@ +parse('create(ParserFactory::PREFER_PHP7); + + $config = new TestConfig(); + } + + public function setUp() + { + FileChecker::clearCache(); + } + + /** + * @expectedException \Psalm\Exception\CodeException + * @expectedExceptionMessage InaccessibleMethod + */ + public function testInaccessiblePrivateMethod() + { + $stmts = self::$parser->parse('fooFoo(); + '); + + $file_checker = new FileChecker('somefile.php', $stmts); + $file_checker->check(); + } + + /** + * @expectedException \Psalm\Exception\CodeException + * @expectedExceptionMessage InaccessibleMethod + */ + public function testInaccessibleProtectedMethod() + { + $stmts = self::$parser->parse('fooFoo(); + '); + + $file_checker = new FileChecker('somefile.php', $stmts); + $file_checker->check(); + } + + public function testAccessiblePrivateMethodFromSubclass() + { + $stmts = self::$parser->parse('fooFoo(); + } + } + '); + + $file_checker = new FileChecker('somefile.php', $stmts); + $file_checker->check(); + } + + /** + * @expectedException \Psalm\Exception\CodeException + * @expectedExceptionMessage InaccessibleMethod + */ + public function testInaccessiblePrivateMethodFromSubclass() + { + $stmts = self::$parser->parse('fooFoo(); + } + } + '); + + $file_checker = new FileChecker('somefile.php', $stmts); + $file_checker->check(); + } + + public function testAccessibleProtectedMethodFromSubclass() + { + $stmts = self::$parser->parse('fooFoo(); + } + } + '); + + $file_checker = new FileChecker('somefile.php', $stmts); + $file_checker->check(); + } + + public function testAccessibleProtectedMethodFromOtherSubclass() + { + $stmts = self::$parser->parse('fooFoo(); + } + } + '); + + $file_checker = new FileChecker('somefile.php', $stmts); + $file_checker->check(); + } + + /** + * @expectedException \Psalm\Exception\CodeException + * @expectedExceptionMessage InaccessibleMethod + */ + public function testInaccessibleProtectedMethodFromOtherSubclass() + { + $stmts = self::$parser->parse('fooFoo(); + } + } + '); + + $file_checker = new FileChecker('somefile.php', $stmts); + $file_checker->check(); + } + + /** + * @expectedException \Psalm\Exception\CodeException + * @expectedExceptionMessage InaccessibleProperty + */ + public function testInaccessiblePrivateProperty() + { + $stmts = self::$parser->parse('fooFoo; + '); + + $file_checker = new FileChecker('somefile.php', $stmts); + $file_checker->check(); + } + + /** + * @expectedException \Psalm\Exception\CodeException + * @expectedExceptionMessage InaccessibleProperty + */ + public function testInaccessibleProtectedProperty() + { + $stmts = self::$parser->parse('fooFoo; + '); + + $file_checker = new FileChecker('somefile.php', $stmts); + $file_checker->check(); + } + + /** + * @expectedException \Psalm\Exception\CodeException + * @expectedExceptionMessage InaccessibleProperty + */ + public function testInaccessiblePrivatePropertyFromSubclass() + { + $stmts = self::$parser->parse('fooFoo; + } + } + '); + + $file_checker = new FileChecker('somefile.php', $stmts); + $file_checker->check(); + } + + /** + * @expectedException \Psalm\Exception\CodeException + * @expectedExceptionMessage InaccessibleProperty + */ + public function testInaccessibleStaticPrivateProperty() + { + $stmts = self::$parser->parse('check(); + } + + /** + * @expectedException \Psalm\Exception\CodeException + * @expectedExceptionMessage InaccessibleProperty + */ + public function testInaccessibleStaticProtectedProperty() + { + $stmts = self::$parser->parse('check(); + } + + /** + * @expectedException \Psalm\Exception\CodeException + * @expectedExceptionMessage InaccessibleProperty + */ + public function testInaccessibleStaticPrivatePropertyFromSubclass() + { + $stmts = self::$parser->parse('check(); + } + + public function testAccessibleProtectedPropertyFromSubclass() + { + $stmts = self::$parser->parse('fooFoo; + } + } + '); + + $file_checker = new FileChecker('somefile.php', $stmts); + $file_checker->check(); + } + + public function testAccessibleProtectedPropertyFromGreatGrandparent() + { + $stmts = self::$parser->parse('fooFoo; + } + } + '); + + $file_checker = new FileChecker('somefile.php', $stmts); + $file_checker->check(); + } + + public function testAccessibleProtectedPropertyFromOtherSubclass() + { + $stmts = self::$parser->parse('fooFoo = "hello"; + } + } + '); + + $file_checker = new FileChecker('somefile.php', $stmts); + $file_checker->check(); + } + + public function testAccessibleStaticPropertyFromSubclass() + { + $stmts = self::$parser->parse('check(); + } +} diff --git a/tests/InterfaceTest.php b/tests/InterfaceTest.php index 2642f1ea71e..8d66b47c181 100644 --- a/tests/InterfaceTest.php +++ b/tests/InterfaceTest.php @@ -243,26 +243,20 @@ public function fooFoo(int $a) : void { $file_checker->check(true, true, $context); } - public function testInterfaceMethodImplementedInParentAndTrait() + public function testInterfaceMethodImplementedInParent() { $stmts = self::$parser->parse(' '); diff --git a/tests/PropertyTypeTest.php b/tests/PropertyTypeTest.php index 64332b9236b..17ab5fa073d 100644 --- a/tests/PropertyTypeTest.php +++ b/tests/PropertyTypeTest.php @@ -378,6 +378,53 @@ class Foo { $file_checker->check(true, true, $context); } + public function testNullablePropertyCheck() + { + $stmts = self::$parser->parse('bb) && $b->bb->aa === "aa") { + echo $b->bb->aa; + } + '); + + $file_checker = new FileChecker('somefile.php', $stmts); + $context = new Context('somefile.php'); + $file_checker->check(true, true, $context); + } + + public function testNullableStaticPropertyWithIfCheck() + { + $stmts = self::$parser->parse('check(true, true, $context); + } + public function testReflectionProperties() { $stmts = self::$parser->parse('check(); } - /** - * @expectedException \Psalm\Exception\CodeException - * @expectedExceptionMessage InaccessibleMethod - */ - public function testInaccessiblePrivateMethod() - { - $stmts = self::$parser->parse('fooFoo(); - '); - - $file_checker = new FileChecker('somefile.php', $stmts); - $file_checker->check(); - } - - /** - * @expectedException \Psalm\Exception\CodeException - * @expectedExceptionMessage InaccessibleMethod - */ - public function testInaccessibleProtectedMethod() - { - $stmts = self::$parser->parse('fooFoo(); - '); - - $file_checker = new FileChecker('somefile.php', $stmts); - $file_checker->check(); - } - - public function testAccessiblePrivateMethodFromSubclass() - { - $stmts = self::$parser->parse('fooFoo(); - } - } - '); - - $file_checker = new FileChecker('somefile.php', $stmts); - $file_checker->check(); - } - - /** - * @expectedException \Psalm\Exception\CodeException - * @expectedExceptionMessage UndefinedMethod - */ - public function testInaccessiblePrivateMethodFromSubclass() - { - $stmts = self::$parser->parse('fooFoo(); - } - } - '); - - $file_checker = new FileChecker('somefile.php', $stmts); - $file_checker->check(); - } - - public function testAccessibleProtectedMethodFromSubclass() - { - $stmts = self::$parser->parse('fooFoo(); - } - } - '); - - $file_checker = new FileChecker('somefile.php', $stmts); - $file_checker->check(); - } - - /** - * @expectedException \Psalm\Exception\CodeException - * @expectedExceptionMessage InaccessibleProperty - */ - public function testInaccessiblePrivateProperty() - { - $stmts = self::$parser->parse('foo; - '); - - $file_checker = new FileChecker('somefile.php', $stmts); - $file_checker->check(); - } - - /** - * @expectedException \Psalm\Exception\CodeException - * @expectedExceptionMessage InaccessibleProperty - */ - public function testInaccessibleProtectedProperty() - { - $stmts = self::$parser->parse('foo; - '); - - $file_checker = new FileChecker('somefile.php', $stmts); - $file_checker->check(); - } - - /** - * @expectedException \Psalm\Exception\CodeException - * @expectedExceptionMessage UndefinedThisPropertyFetch - */ - public function testInaccessiblePrivatePropertyFromSubclass() - { - $stmts = self::$parser->parse('foo; - } - } - '); - - $file_checker = new FileChecker('somefile.php', $stmts); - $file_checker->check(); - } - - public function testAccessibleProtectedPropertyFromSubclass() - { - $stmts = self::$parser->parse('foo; - } - } - '); - - $file_checker = new FileChecker('somefile.php', $stmts); - $file_checker->check(); - } - - public function testAccessibleStaticPropertyFromSubclass() - { - $stmts = self::$parser->parse('check(); - } - /** * @expectedException \Psalm\Exception\CodeException * @expectedExceptionMessage InvalidGlobal diff --git a/tests/TraitTest.php b/tests/TraitTest.php index b04aa3d6b53..8fbf9dcf8fa 100644 --- a/tests/TraitTest.php +++ b/tests/TraitTest.php @@ -26,13 +26,13 @@ public function setUp() public function testAccessiblePrivateMethodFromTrait() { $stmts = self::$parser->parse('fooFoo(); @@ -47,13 +47,13 @@ public function doFoo() : void { public function testAccessibleProtectedMethodFromTrait() { $stmts = self::$parser->parse('fooFoo(); @@ -68,13 +68,13 @@ public function doFoo() : void { public function testAccessiblePublicMethodFromTrait() { $stmts = self::$parser->parse('fooFoo(); @@ -88,18 +88,18 @@ public function doFoo() : void { /** * @expectedException \Psalm\Exception\CodeException - * @expectedExceptionMessage UndefinedMethod + * @expectedExceptionMessage InaccessibleMethod */ public function testInccessiblePrivateMethodFromInheritedTrait() { $stmts = self::$parser->parse('parse('parse('parse('