Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve signature of DOMDocument::loadXML() #7407

Merged
merged 1 commit into from Jan 17, 2022
Merged

Improve signature of DOMDocument::loadXML() #7407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions dictionaries/CallMap.php
View file
Open in desktop
Expand Up @@ -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'],
Expand Down
4 changes: 2 additions & 2 deletions dictionaries/CallMap_historical.php
View file
Open in desktop
Expand Up @@ -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'],
Expand Down
16 changes: 15 additions & 1 deletion src/Psalm/Config.php
View file
Open in desktop
Expand Up @@ -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);
Expand All @@ -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
*/
Expand All @@ -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();
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -857,6 +869,8 @@ private static function processConfigDeprecations(
}

/**
* @param non-empty-string $file_contents
*
* @psalm-suppress MixedMethodCall
* @psalm-suppress MixedAssignment
* @psalm-suppress MixedArgument
Expand Down
4 changes: 4 additions & 0 deletions src/Psalm/Config/Creator.php
View file
Open in desktop
Expand Up @@ -52,6 +52,9 @@ class Creator
</psalm>
';

/**
* @return non-empty-string
*/
public static function getContents(
string $current_dir,
?string $suggested_dir,
Expand Down Expand Up @@ -80,6 +83,7 @@ public static function getContents(
);
}

/** @var non-empty-string */
return str_replace(
'errorLevel="1"',
'errorLevel="' . $level . '"',
Expand Down
2 changes: 2 additions & 0 deletions src/Psalm/Internal/PluginManager/ConfigFile.php
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -120,6 +121,7 @@ private function readXml(): DOMDocument
}
}

assert($file_contents !== '');
$doc->loadXML($file_contents);

return $doc;
Expand Down
15 changes: 7 additions & 8 deletions tests/Config/ConfigTest.php
View file
Open in desktop
Expand Up @@ -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)
);


Expand Down
66 changes: 32 additions & 34 deletions tests/Config/PluginTest.php
View file
Open in desktop
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
1 change: 1 addition & 0 deletions tests/MethodCallTest.php
View file
Open in desktop
Expand Up @@ -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);
Expand Down