Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
- make SimpleXmlElement and SimpleXmlIterator not a universal object crate
- added typed magic __get method to SimpleXmlElement
- 'fixed' code translating `null` to `TNamedObject('null')` instead of `TNull`
  • Loading branch information
ygottschalk committed Jul 25, 2023
1 parent be82c3a commit 313b54f
Show file tree
Hide file tree
Showing 13 changed files with 58 additions and 100 deletions.
48 changes: 22 additions & 26 deletions src/Psalm/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -728,8 +728,6 @@ protected function __construct()
$this->eventDispatcher = new EventDispatcher();
$this->universal_object_crates = [
strtolower(stdClass::class),
strtolower(SimpleXMLElement::class),
strtolower(SimpleXMLIterator::class),
];
}

Expand Down Expand Up @@ -1030,7 +1028,6 @@ private static function processConfigDeprecations(

/**
* @param non-empty-string $file_contents
* @psalm-suppress MixedMethodCall
* @psalm-suppress MixedAssignment
* @psalm-suppress MixedArgument
* @psalm-suppress MixedPropertyFetch
Expand Down Expand Up @@ -1161,12 +1158,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)) {
Expand Down Expand Up @@ -1312,7 +1310,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);
Expand All @@ -1336,7 +1334,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'];
Expand All @@ -1347,7 +1344,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'];
Expand Down Expand Up @@ -1401,7 +1397,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'];

Expand All @@ -1413,7 +1408,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
Expand All @@ -1429,21 +1423,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,
);
}
}
}
}
Expand Down
9 changes: 0 additions & 9 deletions src/Psalm/Config/FileFilter.php
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,6 @@ public static function loadFromXMLElement(

if ($e->directory) {
$config['directory'] = [];
/** @var SimpleXMLElement $directory */
foreach ($e->directory as $directory) {
$config['directory'][] = [
'name' => (string) $directory['name'],
Expand All @@ -401,56 +400,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']);
}
Expand Down
7 changes: 4 additions & 3 deletions src/Psalm/Config/IssueHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion src/Psalm/Config/ProjectFileFilter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
14 changes: 0 additions & 14 deletions src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 0 additions & 2 deletions src/Psalm/Internal/Provider/MethodReturnTypeProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down

This file was deleted.

15 changes: 9 additions & 6 deletions src/Psalm/Type/Atomic.php
Original file line number Diff line number Diff line change
Expand Up @@ -323,11 +323,7 @@ private static function createInner(
return $analysis_php_version_id !== null ? new TNamedObject($value) : new TScalar();

case 'null':
if ($analysis_php_version_id === null || $analysis_php_version_id >= 8_00_00) {
return new TNull();
}

return new TNamedObject($value);
return new TNull();

case 'mixed':
if ($analysis_php_version_id === null || $analysis_php_version_id >= 8_00_00) {
Expand Down Expand Up @@ -577,7 +573,14 @@ public function isArrayAccessibleWithStringKey(Codebase $codebase): bool
|| $this instanceof TKeyedArray
|| $this instanceof TClassStringMap
|| $this->hasArrayAccessInterface($codebase)
|| ($this instanceof TNamedObject && $this->value === 'SimpleXMLElement');
|| ($this instanceof TNamedObject
&& ($this->value === 'SimpleXMLElement'
|| $codebase->classExtendsOrImplements(
$this->value,
'SimpleXMLElement',
)
)
);
}

public function isArrayAccessibleWithIntOrStringKey(Codebase $codebase): bool
Expand Down
3 changes: 3 additions & 0 deletions stubs/extensions/simplexml.phpstub
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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 {}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/ArrayAccessTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1013,7 +1013,7 @@ public function pop(): void {
],
'simpleXmlArrayFetch' => [
'code' => '<?php
function foo(SimpleXMLElement $s) : SimpleXMLElement {
function foo(SimpleXMLElement $s) : ?SimpleXMLElement {
return $s["a"];
}',
],
Expand Down
10 changes: 10 additions & 0 deletions tests/PropertyTypeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1389,6 +1389,16 @@ public function __construct() {
'$d->e' => 'mixed',
],
],
'getPropertiesOfSimpleXmlElement' => [
'code' => '<?php
$a = new SimpleXMLElement("<person><child role=\"son\"></child></person>");
$b = $a->b;',
'assertions' => [
'$a' => 'SimpleXMLElement',
'$a->b' => 'SimpleXMLElement|null',
'$b' => 'SimpleXMLElement|null',
],
],
'allowLessSpecificReturnTypeForOverriddenMethod' => [
'code' => '<?php
class A {
Expand Down
7 changes: 5 additions & 2 deletions tests/TypeReconciliation/ConditionalTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2817,7 +2817,6 @@ function getIntOrFalse() {return false;}
$lilstring = "";
$n = new SimpleXMLElement($lilstring);
/** @psalm-suppress MixedAssignment */
$n = $n->b;
if (!$n instanceof SimpleXMLElement) {
Expand Down Expand Up @@ -2905,7 +2904,11 @@ function b(B $_b): void {
$lilstring = "";
$n = new SimpleXMLElement($lilstring);
$n = $n->children();
$n = $n->b;
if (!$n instanceof SimpleXMLIterator) {
return;
}
if (!$n) {
echo "false";
Expand Down

0 comments on commit 313b54f

Please sign in to comment.