Fix vimeo#9997

- make SimpleXmlElement and SimpleXmlIterator not a universal object crate - added typed magic __get method to SimpleXmlElement - adjusted tests to reflect new behaviour of SimpleXmlElement - added tests
ygottschalk · Aug 5, 2023 · 9b77760 · 9b77760
1 parent 96a61fc
commit 9b77760
Show file tree

Hide file tree

Showing 15 changed files with 71 additions and 105 deletions.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -446,9 +446,6 @@
   </file>
   <file src="src/Psalm/Type/Atomic.php">
     <ImpureMethodCall>
-      <code>classExtendsOrImplements</code>
-      <code>classExtendsOrImplements</code>
-      <code>classExtendsOrImplements</code>
       <code>classOrInterfaceExists</code>
       <code>classOrInterfaceExists</code>
       <code>classOrInterfaceExists</code>

diff --git a/src/Psalm/Codebase.php b/src/Psalm/Codebase.php
@@ -699,6 +699,7 @@ public function classOrInterfaceOrEnumExists(
         );
     }
 
+    /** @psalm-mutation-free */
     public function classExtendsOrImplements(string $fq_class_name, string $possible_parent): bool
     {
         return $this->classlikes->classExtends($fq_class_name, $possible_parent)

diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php
@@ -42,7 +42,6 @@
 use Psalm\Progress\VoidProgress;
 use RuntimeException;
 use SimpleXMLElement;
-use SimpleXMLIterator;
 use Symfony\Component\Filesystem\Path;
 use Throwable;
 use UnexpectedValueException;
@@ -728,8 +727,6 @@ protected function __construct()
         $this->eventDispatcher = new EventDispatcher();
         $this->universal_object_crates = [
             strtolower(stdClass::class),
-            strtolower(SimpleXMLElement::class),
-            strtolower(SimpleXMLIterator::class),
         ];
     }
 
@@ -1030,7 +1027,6 @@ private static function processConfigDeprecations(
 
     /**
      * @param non-empty-string $file_contents
-     * @psalm-suppress MixedMethodCall
      * @psalm-suppress MixedAssignment
      * @psalm-suppress MixedArgument
      * @psalm-suppress MixedPropertyFetch
@@ -1161,12 +1157,13 @@ private static function fromXmlAndPaths(
         }
 
         if (isset($config_xml['autoloader'])) {
-            $autoloader_path = $config->base_dir . DIRECTORY_SEPARATOR . $config_xml['autoloader'];
+            $autoloader = (string) $config_xml['autoloader'];
+            $autoloader_path = $config->base_dir . DIRECTORY_SEPARATOR . $autoloader;
 
             if (!file_exists($autoloader_path)) {
                 // in here for legacy reasons where people put absolute paths but psalm resolved it relative
-                if ($config_xml['autoloader']->__toString()[0] === '/') {
-                    $autoloader_path = $config_xml['autoloader']->__toString();
+                if ($autoloader[0] === '/') {
+                    $autoloader_path = $autoloader;
                 }
 
                 if (!file_exists($autoloader_path)) {
@@ -1312,7 +1309,7 @@ private static function fromXmlAndPaths(
             );
         }
 
-        if (isset($config_xml->fileExtensions)) {
+        if (isset($config_xml->fileExtensions->extension)) {
             $config->file_extensions = [];
 
             $config->loadFileExtensions($config_xml->fileExtensions->extension);
@@ -1336,7 +1333,6 @@ private static function fromXmlAndPaths(
 
         if (isset($config_xml->ignoreExceptions)) {
             if (isset($config_xml->ignoreExceptions->class)) {
-                /** @var SimpleXMLElement $exception_class */
                 foreach ($config_xml->ignoreExceptions->class as $exception_class) {
                     $exception_name = (string) $exception_class['name'];
                     $global_attribute_text = (string) $exception_class['onlyGlobalScope'];
@@ -1347,7 +1343,6 @@ private static function fromXmlAndPaths(
                 }
             }
             if (isset($config_xml->ignoreExceptions->classAndDescendants)) {
-                /** @var SimpleXMLElement $exception_class */
                 foreach ($config_xml->ignoreExceptions->classAndDescendants as $exception_class) {
                     $exception_name = (string) $exception_class['name'];
                     $global_attribute_text = (string) $exception_class['onlyGlobalScope'];
@@ -1401,7 +1396,6 @@ private static function fromXmlAndPaths(
         // this plugin loading system borrows heavily from etsy/phan
         if (isset($config_xml->plugins)) {
             if (isset($config_xml->plugins->plugin)) {
-                /** @var SimpleXMLElement $plugin */
                 foreach ($config_xml->plugins->plugin as $plugin) {
                     $plugin_file_name = (string) $plugin['filename'];
 
@@ -1413,7 +1407,6 @@ private static function fromXmlAndPaths(
                 }
             }
             if (isset($config_xml->plugins->pluginClass)) {
-                /** @var SimpleXMLElement $plugin */
                 foreach ($config_xml->plugins->pluginClass as $plugin) {
                     $plugin_class_name = $plugin['class'];
                     // any child elements are used as plugin configuration
@@ -1429,21 +1422,23 @@ private static function fromXmlAndPaths(
 
         if (isset($config_xml->issueHandlers)) {
             foreach ($config_xml->issueHandlers as $issue_handlers) {
-                /** @var SimpleXMLElement $issue_handler */
-                foreach ($issue_handlers->children() as $key => $issue_handler) {
-                    if ($key === 'PluginIssue') {
-                        $custom_class_name = (string) $issue_handler['name'];
-                        /** @var string $key */
-                        $config->issue_handlers[$custom_class_name] = IssueHandler::loadFromXMLElement(
-                            $issue_handler,
-                            $base_dir,
-                        );
-                    } else {
-                        /** @var string $key */
-                        $config->issue_handlers[$key] = IssueHandler::loadFromXMLElement(
-                            $issue_handler,
-                            $base_dir,
-                        );
+                $issue_handler_children = $issue_handlers->children();
+                if ($issue_handler_children) {
+                    foreach ($issue_handler_children as $key => $issue_handler) {
+                        if ($key === 'PluginIssue') {
+                            $custom_class_name = (string)$issue_handler['name'];
+                            /** @var string $key */
+                            $config->issue_handlers[$custom_class_name] = IssueHandler::loadFromXMLElement(
+                                $issue_handler,
+                                $base_dir,
+                            );
+                        } else {
+                            /** @var string $key */
+                            $config->issue_handlers[$key] = IssueHandler::loadFromXMLElement(
+                                $issue_handler,
+                                $base_dir,
+                            );
+                        }
                     }
                 }
             }

diff --git a/src/Psalm/Config/FileFilter.php b/src/Psalm/Config/FileFilter.php
@@ -389,7 +389,6 @@ public static function loadFromXMLElement(
 
         if ($e->directory) {
             $config['directory'] = [];
-            /** @var SimpleXMLElement $directory */
             foreach ($e->directory as $directory) {
                 $config['directory'][] = [
                     'name' => (string) $directory['name'],
@@ -402,56 +401,48 @@ public static function loadFromXMLElement(
 
         if ($e->file) {
             $config['file'] = [];
-            /** @var SimpleXMLElement $file */
             foreach ($e->file as $file) {
                 $config['file'][]['name'] = (string) $file['name'];
             }
         }
 
         if ($e->referencedClass) {
             $config['referencedClass'] = [];
-            /** @var SimpleXMLElement $referenced_class */
             foreach ($e->referencedClass as $referenced_class) {
                 $config['referencedClass'][]['name'] = strtolower((string)$referenced_class['name']);
             }
         }
 
         if ($e->referencedMethod) {
             $config['referencedMethod'] = [];
-            /** @var SimpleXMLElement $referenced_method */
             foreach ($e->referencedMethod as $referenced_method) {
                 $config['referencedMethod'][]['name'] = (string)$referenced_method['name'];
             }
         }
 
         if ($e->referencedFunction) {
             $config['referencedFunction'] = [];
-            /** @var SimpleXMLElement $referenced_function */
             foreach ($e->referencedFunction as $referenced_function) {
                 $config['referencedFunction'][]['name'] = strtolower((string)$referenced_function['name']);
             }
         }
 
         if ($e->referencedProperty) {
             $config['referencedProperty'] = [];
-            /** @var SimpleXMLElement $referenced_property */
             foreach ($e->referencedProperty as $referenced_property) {
                 $config['referencedProperty'][]['name'] = strtolower((string)$referenced_property['name']);
             }
         }
 
         if ($e->referencedConstant) {
             $config['referencedConstant'] = [];
-            /** @var SimpleXMLElement $referenced_constant */
             foreach ($e->referencedConstant as $referenced_constant) {
                 $config['referencedConstant'][]['name'] = strtolower((string)$referenced_constant['name']);
             }
         }
 
         if ($e->referencedVariable) {
             $config['referencedVariable'] = [];
-
-            /** @var SimpleXMLElement $referenced_variable */
             foreach ($e->referencedVariable as $referenced_variable) {
                 $config['referencedVariable'][]['name'] = strtolower((string)$referenced_variable['name']);
             }

diff --git a/src/Psalm/Config/IssueHandler.php b/src/Psalm/Config/IssueHandler.php
@@ -38,9 +38,10 @@ public static function loadFromXMLElement(SimpleXMLElement $e, string $base_dir)
             }
         }
 
-        /** @var SimpleXMLElement $error_level */
-        foreach ($e->errorLevel as $error_level) {
-            $handler->custom_levels[] = ErrorLevelFileFilter::loadFromXMLElement($error_level, $base_dir, true);
+        if (isset($e->errorLevel)) {
+            foreach ($e->errorLevel as $error_level) {
+                $handler->custom_levels[] = ErrorLevelFileFilter::loadFromXMLElement($error_level, $base_dir, true);
+            }
         }
 
         return $handler;

diff --git a/src/Psalm/Config/ProjectFileFilter.php b/src/Psalm/Config/ProjectFileFilter.php
@@ -28,7 +28,6 @@ public static function loadFromXMLElement(
                 throw new ConfigException('Cannot nest ignoreFiles inside itself');
             }
 
-            /** @var SimpleXMLElement $e->ignoreFiles */
             $filter->file_filter = static::loadFromXMLElement($e->ignoreFiles, $base_dir, false);
         }
 

diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php
@@ -755,20 +755,6 @@ public static function handleIterable(
 
             $has_valid_iterator = true;
 
-            if ($iterator_atomic_type instanceof TNamedObject
-                && strtolower($iterator_atomic_type->value) === 'simplexmlelement'
-            ) {
-                $value_type = Type::combineUnionTypes(
-                    $value_type,
-                    new Union([$iterator_atomic_type]),
-                );
-
-                $key_type = Type::combineUnionTypes(
-                    $key_type,
-                    Type::getString(),
-                );
-            }
-
             if ($iterator_atomic_type instanceof TIterable
                 || (strtolower($iterator_atomic_type->value) === 'traversable'
                     || $codebase->classImplements(

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php
@@ -1737,8 +1737,10 @@ private static function handleArrayAccessOnNamedObject(
         ?Union &$array_access_type,
         bool &$has_array_access
     ): void {
-        if (strtolower($type->value) === 'simplexmlelement') {
-            $call_array_access_type = new Union([new TNamedObject('SimpleXMLElement')]);
+        if (strtolower($type->value) === 'simplexmlelement'
+            || $statements_analyzer->getCodebase()->classExtendsOrImplements($type->value, 'SimpleXMLElement')
+        ) {
+            $call_array_access_type = new Union([new TNull(), new TNamedObject('SimpleXMLElement')]);
         } elseif (strtolower($type->value) === 'domnodelist' && $stmt->dim) {
             $old_data_provider = $statements_analyzer->node_data;
 

diff --git a/src/Psalm/Internal/Codebase/ClassLikes.php b/src/Psalm/Internal/Codebase/ClassLikes.php
@@ -597,6 +597,7 @@ public function classExists(
     /**
      * Determine whether or not a class extends a parent
      *
+     * @psalm-mutation-free
      * @throws UnpopulatedClasslikeException when called on unpopulated class
      * @throws InvalidArgumentException when class does not exist
      */
@@ -620,6 +621,8 @@ public function classExtends(string $fq_class_name, string $possible_parent, boo
 
     /**
      * Check whether a class implements an interface
+     *
+     * @psalm-mutation-free
      */
     public function classImplements(string $fq_class_name, string $interface): bool
     {

diff --git a/src/Psalm/Internal/Provider/MethodReturnTypeProvider.php b/src/Psalm/Internal/Provider/MethodReturnTypeProvider.php
@@ -11,7 +11,6 @@
 use Psalm\Internal\Provider\ReturnTypeProvider\DomNodeAppendChild;
 use Psalm\Internal\Provider\ReturnTypeProvider\ImagickPixelColorReturnTypeProvider;
 use Psalm\Internal\Provider\ReturnTypeProvider\PdoStatementReturnTypeProvider;
-use Psalm\Internal\Provider\ReturnTypeProvider\SimpleXmlElementAsXml;
 use Psalm\Plugin\EventHandler\Event\MethodReturnTypeProviderEvent;
 use Psalm\Plugin\EventHandler\MethodReturnTypeProviderInterface;
 use Psalm\StatementsSource;
@@ -39,7 +38,6 @@ public function __construct()
 
         $this->registerClass(DomNodeAppendChild::class);
         $this->registerClass(ImagickPixelColorReturnTypeProvider::class);
-        $this->registerClass(SimpleXmlElementAsXml::class);
         $this->registerClass(PdoStatementReturnTypeProvider::class);
         $this->registerClass(ClosureFromCallableReturnTypeProvider::class);
         $this->registerClass(DateTimeModifyReturnTypeProvider::class);

diff --git a/src/Psalm/Internal/Provider/ReturnTypeProvider/SimpleXmlElementAsXml.php b/src/Psalm/Internal/Provider/ReturnTypeProvider/SimpleXmlElementAsXml.php
diff --git a/stubs/extensions/simplexml.phpstub b/stubs/extensions/simplexml.phpstub
@@ -25,6 +25,7 @@ function simplexml_import_dom(SimpleXMLElement|DOMNode $node, ?string $class_nam
 
 /**
  * @implements Traversable<string, SimpleXMLElement>
+ * @psalm-no-seal-properties
  */
 class SimpleXMLElement implements Traversable, Countable
 {
@@ -63,6 +64,8 @@ class SimpleXMLElement implements Traversable, Countable
     public function __toString(): string {}
 
     public function count(): int {}
+
+    public function __get(string $name): SimpleXMLElement|SimpleXMLIterator|null {}
 }
 
 /**

diff --git a/tests/ArrayAccessTest.php b/tests/ArrayAccessTest.php
@@ -1013,7 +1013,7 @@ public function pop(): void {
             ],
             'simpleXmlArrayFetch' => [
                 'code' => '<?php
-                    function foo(SimpleXMLElement $s) : SimpleXMLElement {
+                    function foo(SimpleXMLElement $s) : ?SimpleXMLElement {
                         return $s["a"];
                     }',
             ],