From 7d4c65ccdcaa438f27d5cf096d54a265d5f92ce4 Mon Sep 17 00:00:00 2001 From: Fabien Villepinte Date: Mon, 17 Jan 2022 22:44:58 +0000 Subject: [PATCH] Improve signature of DOMDocument::loadXML() --- dictionaries/CallMap.php | 4 +- dictionaries/CallMap_historical.php | 4 +- src/Psalm/Config.php | 16 ++++- src/Psalm/Config/Creator.php | 6 ++ .../Internal/PluginManager/ConfigFile.php | 2 + tests/Config/ConfigTest.php | 15 ++--- tests/Config/PluginTest.php | 66 +++++++++---------- tests/MethodCallTest.php | 1 + 8 files changed, 67 insertions(+), 47 deletions(-) diff --git a/dictionaries/CallMap.php b/dictionaries/CallMap.php index 9f31a7a44d4..92219eda252 100644 --- 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 index 362a181db8c..813dffa7bd0 100644 --- 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 index 2e16c505ec4..6392d72d449 100644 --- 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 index 36760726e1c..6574d19f09c 100644 --- a/src/Psalm/Config/Creator.php +++ b/src/Psalm/Config/Creator.php @@ -52,6 +52,12 @@ class Creator '; + /** + * @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 index daab9d8a320..4fd02542575 100644 --- 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 index 806ea4e7d5e..6a15f41c9ac 100644 --- 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( + ' + ', + FileTypeSelfRegisteringPlugin::class + ); $projectAnalyzer = $this->getProjectAnalyzerWithConfig( - TestConfig::loadFromXML( - dirname(__DIR__, 2), - sprintf( - ' - ', - FileTypeSelfRegisteringPlugin::class - ) - ) + TestConfig::loadFromXML(dirname(__DIR__, 2), $xml) ); diff --git a/tests/Config/PluginTest.php b/tests/Config/PluginTest.php index 0734f7e0f06..6fd8b4976f8 100644 --- 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( + ' + + + + + + + + ', + __DIR__ . '/../..' + ); $this->project_analyzer = $this->getProjectAnalyzerWithConfig( - TestConfig::loadFromXML( - dirname(__DIR__, 2) . DIRECTORY_SEPARATOR, - sprintf( - ' - - - - - - - - ', - __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( + ' + + + + + + + + ', + __DIR__ . '/..' + ); $this->project_analyzer = $this->getProjectAnalyzerWithConfig( - TestConfig::loadFromXML( - dirname(__DIR__, 2) . DIRECTORY_SEPARATOR, - sprintf( - ' - - - - - - - - ', - __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 index 8676075fd61..dde7c15239e 100644 --- a/tests/MethodCallTest.php +++ b/tests/MethodCallTest.php @@ -387,6 +387,7 @@ function foo(DOMElement $e) : ?string { ], 'domElementIteratorOrEmptyArray' => [ 'loadXML($XML);