From c9eafa15ad6656ac5ae392916aa261b7d47a43f3 Mon Sep 17 00:00:00 2001 From: Fabien Villepinte Date: Mon, 17 Jan 2022 22:52: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 | 4 ++ .../Internal/PluginManager/ConfigFile.php | 2 + tests/Config/ConfigTest.php | 15 ++--- tests/Config/PluginTest.php | 66 +++++++++---------- tests/MethodCallTest.php | 1 + 8 files changed, 65 insertions(+), 47 deletions(-) diff --git a/dictionaries/CallMap.php b/dictionaries/CallMap.php index 9f31a7a44..92219eda2 100644 --- a/dictionaries/CallMap.php +++ b/dictionaries/CallMap.php @@ -2109,9 +2109,9 @@ return [ '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 362a181db..813dffa7b 100644 --- a/dictionaries/CallMap_historical.php +++ b/dictionaries/CallMap_historical.php @@ -939,9 +939,9 @@ return [ '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 2e16c505e..6392d72d4 100644 --- a/src/Psalm/Config.php +++ b/src/Psalm/Config.php @@ -646,6 +646,10 @@ class Config 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 @@ class Config /** * 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 @@ class Config 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 @@ class Config } /** + * @param non-empty-string $file_contents + * * @throws ConfigException */ private static function validateXmlConfig(string $base_dir, string $file_contents): void @@ -731,7 +741,9 @@ class Config $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 @@ class Config } /** + * @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 36760726e..f67eb5cf3 100644 --- a/src/Psalm/Config/Creator.php +++ b/src/Psalm/Config/Creator.php @@ -52,6 +52,9 @@ class Creator '; + /** + * @return non-empty-string + */ public static function getContents( string $current_dir, ?string $suggested_dir, @@ -80,6 +83,7 @@ class Creator ); } + /** @var non-empty-string */ return str_replace( 'errorLevel="1"', 'errorLevel="' . $level . '"', diff --git a/src/Psalm/Internal/PluginManager/ConfigFile.php b/src/Psalm/Internal/PluginManager/ConfigFile.php index daab9d8a3..4fd025425 100644 --- a/src/Psalm/Internal/PluginManager/ConfigFile.php +++ b/src/Psalm/Internal/PluginManager/ConfigFile.php @@ -7,6 +7,7 @@ use DomElement; 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 @@ class ConfigFile } } + assert($file_contents !== ''); $doc->loadXML($file_contents); return $doc; diff --git a/tests/Config/ConfigTest.php b/tests/Config/ConfigTest.php index 806ea4e7d..6a15f41c9 100644 --- a/tests/Config/ConfigTest.php +++ b/tests/Config/ConfigTest.php @@ -1487,15 +1487,14 @@ class ConfigTest extends TestCase 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 0734f7e0f..6fd8b4976 100644 --- a/tests/Config/PluginTest.php +++ b/tests/Config/PluginTest.php @@ -982,24 +982,23 @@ class PluginTest extends TestCase 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 @@ class PluginTest extends TestCase $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 8676075fd..dde7c1523 100644 --- a/tests/MethodCallTest.php +++ b/tests/MethodCallTest.php @@ -387,6 +387,7 @@ class MethodCallTest extends TestCase ], 'domElementIteratorOrEmptyArray' => [ 'loadXML($XML);