Improve signature of DOMDocument::loadXML()

vimeo · Jan 17, 2022 · 7d4c65c · 7d4c65c
1 parent a99c433
commit 7d4c65c
Show file tree

Hide file tree

Showing 8 changed files with 67 additions and 47 deletions.
diff --git a/dictionaries/CallMap.php b/dictionaries/CallMap.php
@@ -2109,9 +2109,9 @@
 'DOMDocument::getElementsByTagNameNS' => ['DOMNodeList', 'namespaceuri'=>'string', 'localname'=>'string'],
 'DOMDocument::importNode' => ['DOMNode|false', 'importednode'=>'DOMNode', 'deep='=>'bool'],
 'DOMDocument::load' => ['DOMDocument|bool', 'filename'=>'string', 'options='=>'int'],
-'DOMDocument::loadHTML' => ['bool', 'source'=>'string', 'options='=>'int'],
+'DOMDocument::loadHTML' => ['bool', 'source'=>'non-empty-string', 'options='=>'int'],
 'DOMDocument::loadHTMLFile' => ['bool', 'filename'=>'string', 'options='=>'int'],
-'DOMDocument::loadXML' => ['DOMDocument|bool', 'source'=>'string', 'options='=>'int'],
+'DOMDocument::loadXML' => ['DOMDocument|bool', 'source'=>'non-empty-string', 'options='=>'int'],
 'DOMDocument::normalizeDocument' => ['void'],
 'DOMDocument::registerNodeClass' => ['bool', 'baseclass'=>'string', 'extendedclass'=>'string'],
 'DOMDocument::relaxNGValidate' => ['bool', 'filename'=>'string'],

diff --git a/dictionaries/CallMap_historical.php b/dictionaries/CallMap_historical.php
@@ -939,9 +939,9 @@
     'DOMDocument::getElementsByTagNameNS' => ['DOMNodeList', 'namespaceuri'=>'string', 'localname'=>'string'],
     'DOMDocument::importNode' => ['DOMNode|false', 'importednode'=>'DOMNode', 'deep='=>'bool'],
     'DOMDocument::load' => ['DOMDocument|bool', 'filename'=>'string', 'options='=>'int'],
-    'DOMDocument::loadHTML' => ['bool', 'source'=>'string', 'options='=>'int'],
+    'DOMDocument::loadHTML' => ['bool', 'source'=>'non-empty-string', 'options='=>'int'],
     'DOMDocument::loadHTMLFile' => ['bool', 'filename'=>'string', 'options='=>'int'],
-    'DOMDocument::loadXML' => ['DOMDocument|bool', 'source'=>'string', 'options='=>'int'],
+    'DOMDocument::loadXML' => ['DOMDocument|bool', 'source'=>'non-empty-string', 'options='=>'int'],
     'DOMDocument::normalizeDocument' => ['void'],
     'DOMDocument::registerNodeClass' => ['bool', 'baseclass'=>'string', 'extendedclass'=>'string'],
     'DOMDocument::relaxNGValidate' => ['bool', 'filename'=>'string'],

diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php
@@ -646,6 +646,10 @@ public static function loadFromXMLFile(string $file_path, string $current_dir):
             throw new InvalidArgumentException('Cannot open ' . $file_path);
         }
 
+        if ($file_contents === '') {
+            throw new InvalidArgumentException('Invalid empty file ' . $file_path);
+        }
+
         try {
             $config = self::loadFromXML($base_dir, $file_contents, $current_dir, $file_path);
             $config->hash = sha1($file_contents . PSALM_VERSION);
@@ -669,6 +673,7 @@ public function computeHash(): string
     /**
      * Creates a new config object from an XML string
      * @param  string|null      $current_dir Current working directory, if different to $base_dir
+     * @param  non-empty-string $file_contents
      *
      * @throws ConfigException
      */
@@ -687,6 +692,9 @@ public static function loadFromXML(
         return self::fromXmlAndPaths($base_dir, $file_contents, $current_dir, $file_path);
     }
 
+    /**
+     * @param non-empty-string $file_contents
+     */
     private static function loadDomDocument(string $base_dir, string $file_contents): DOMDocument
     {
         $dom_document = new DOMDocument();
@@ -704,6 +712,8 @@ private static function loadDomDocument(string $base_dir, string $file_contents)
     }
 
     /**
+     * @param non-empty-string $file_contents
+     *
      * @throws ConfigException
      */
     private static function validateXmlConfig(string $base_dir, string $file_contents): void
@@ -731,7 +741,9 @@ private static function validateXmlConfig(string $base_dir, string $file_content
             $psalm_node->setAttribute('xmlns', self::CONFIG_NAMESPACE);
 
             $old_dom_document = $dom_document;
-            $dom_document = self::loadDomDocument($base_dir, $old_dom_document->saveXML());
+            $old_file_contents = $old_dom_document->saveXML();
+            assert($old_file_contents !== false && $old_file_contents !== '');
+            $dom_document = self::loadDomDocument($base_dir, $old_file_contents);
         }
 
         // Enable user error handling
@@ -857,6 +869,8 @@ private static function processConfigDeprecations(
     }
 
     /**
+     * @param non-empty-string $file_contents
+     *
      * @psalm-suppress MixedMethodCall
      * @psalm-suppress MixedAssignment
      * @psalm-suppress MixedArgument

diff --git a/src/Psalm/Config/Creator.php b/src/Psalm/Config/Creator.php
@@ -52,6 +52,12 @@ class Creator
 </psalm>
 ';
 
+    /**
+     * @psalm-suppress MoreSpecificReturnType
+     * @psalm-suppress LessSpecificReturnStatement
+     *
+     * @return non-empty-string
+     */
     public static function getContents(
         string $current_dir,
         ?string $suggested_dir,

diff --git a/src/Psalm/Internal/PluginManager/ConfigFile.php b/src/Psalm/Internal/PluginManager/ConfigFile.php
@@ -7,6 +7,7 @@
 use Psalm\Config;
 use RuntimeException;
 
+use function assert;
 use function file_get_contents;
 use function file_put_contents;
 use function strpos;
@@ -120,6 +121,7 @@ private function readXml(): DOMDocument
             }
         }
 
+        assert($file_contents !== '');
         $doc->loadXML($file_contents);
 
         return $doc;

diff --git a/tests/Config/ConfigTest.php b/tests/Config/ConfigTest.php
@@ -1487,15 +1487,14 @@ public function pluginRegistersScannerAndAnalyzer(int $flags, ?int $expectedExce
         FileTypeSelfRegisteringPlugin::$names = $names;
         FileTypeSelfRegisteringPlugin::$flags = $flags;
 
+        /** @var non-empty-string $xml */
+        $xml = sprintf(
+            '<?xml version="1.0"?>
+            <psalm><plugins><pluginClass class="%s"/></plugins></psalm>',
+            FileTypeSelfRegisteringPlugin::class
+        );
         $projectAnalyzer = $this->getProjectAnalyzerWithConfig(
-            TestConfig::loadFromXML(
-                dirname(__DIR__, 2),
-                sprintf(
-                    '<?xml version="1.0"?>
-                    <psalm><plugins><pluginClass class="%s"/></plugins></psalm>',
-                    FileTypeSelfRegisteringPlugin::class
-                )
-            )
+            TestConfig::loadFromXML(dirname(__DIR__, 2), $xml)
         );
 
 

diff --git a/tests/Config/PluginTest.php b/tests/Config/PluginTest.php
@@ -982,24 +982,23 @@ public function testAfterAnalysisHooks(): void
 
     public function testPluginFilenameCanBeAbsolute(): void
     {
+        /** @var non-empty-string $xml */
+        $xml = sprintf(
+            '<?xml version="1.0"?>
+            <psalm
+            errorLevel="1"
+        >
+                <projectFiles>
+                    <directory name="src" />
+                </projectFiles>
+                <plugins>
+                    <plugin filename="%s/examples/plugins/StringChecker.php" />
+                </plugins>
+            </psalm>',
+            __DIR__ . '/../..'
+        );
         $this->project_analyzer = $this->getProjectAnalyzerWithConfig(
-            TestConfig::loadFromXML(
-                dirname(__DIR__, 2) . DIRECTORY_SEPARATOR,
-                sprintf(
-                    '<?xml version="1.0"?>
-                    <psalm
-                    errorLevel="1"
-                >
-                        <projectFiles>
-                            <directory name="src" />
-                        </projectFiles>
-                        <plugins>
-                            <plugin filename="%s/examples/plugins/StringChecker.php" />
-                        </plugins>
-                    </psalm>',
-                    __DIR__ . '/../..'
-                )
-            )
+            TestConfig::loadFromXML(dirname(__DIR__, 2) . DIRECTORY_SEPARATOR, $xml)
         );
 
         $this->project_analyzer->getCodebase()->config->initializePlugins($this->project_analyzer);
@@ -1010,24 +1009,23 @@ public function testPluginInvalidAbsoluteFilenameThrowsException(): void
         $this->expectException(InvalidArgumentException::class);
         $this->expectExceptionMessage('does-not-exist/plugins/StringChecker.php');
 
+        /** @var non-empty-string $xml */
+        $xml = sprintf(
+            '<?xml version="1.0"?>
+            <psalm
+            errorLevel="1"
+        >
+                <projectFiles>
+                    <directory name="src" />
+                </projectFiles>
+                <plugins>
+                    <plugin filename="%s/does-not-exist/plugins/StringChecker.php" />
+                </plugins>
+            </psalm>',
+            __DIR__ . '/..'
+        );
         $this->project_analyzer = $this->getProjectAnalyzerWithConfig(
-            TestConfig::loadFromXML(
-                dirname(__DIR__, 2) . DIRECTORY_SEPARATOR,
-                sprintf(
-                    '<?xml version="1.0"?>
-                    <psalm
-                    errorLevel="1"
-                >
-                        <projectFiles>
-                            <directory name="src" />
-                        </projectFiles>
-                        <plugins>
-                            <plugin filename="%s/does-not-exist/plugins/StringChecker.php" />
-                        </plugins>
-                    </psalm>',
-                    __DIR__ . '/..'
-                )
-            )
+            TestConfig::loadFromXML(dirname(__DIR__, 2) . DIRECTORY_SEPARATOR, $xml)
         );
 
         $this->project_analyzer->getCodebase()->config->initializePlugins($this->project_analyzer);

diff --git a/tests/MethodCallTest.php b/tests/MethodCallTest.php
@@ -387,6 +387,7 @@ function foo(DOMElement $e) : ?string {
             ],
             'domElementIteratorOrEmptyArray' => [
                 '<?php
+                    /** @param non-empty-string $XML */
                     function foo(string $XML) : void {
                         $dom = new DOMDocument();
                         $dom->loadXML($XML);