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.

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

SimpleXMLElement - SimpleXMLIterator::var is not defined - After property_exists check #6686

Closed
lsv opened this issue Oct 18, 2021 · 8 comments · Fixed by #6690
Closed

SimpleXMLElement - SimpleXMLIterator::var is not defined - After property_exists check #6686

lsv opened this issue Oct 18, 2021 · 8 comments · Fixed by #6690

Comments

@lsv
Copy link

lsv commented Oct 18, 2021

With the following, I get a

Instance property SimpleXMLIterator::$projectFiles is not defined (see https://psalm.dev/039)

Now, I already check that $projectFiles is a property, with my property_exists method

<?php
$xml = <<<XML
<?xml version="1.0"?>
<psalm>
    <projectFiles>
        <directory name="src" />
        <ignoreFiles>
            <directory name="vendor" />
        </ignoreFiles>
    </projectFiles>
</psalm>
XML;

$xmlObject = simplexml_load_string($xml);
if ($xmlObject instanceof SimpleXMLElement) {
    $projectFiles = $xmlObject->children('projectFiles', true);
    if (property_exists($projectFiles, 'projectFiles')) {
        if ($projectFiles->projectFiles instanceof SimpleXMLElement) {
            return true;
        }
    }
}

Isnt this a false positive call, or is there a way to get rid of that check?

@lsv
Copy link
Author

lsv commented Oct 18, 2021

Reproduced: https://psalm.dev/r/ac48378869

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/ac48378869
<?php
$xml = <<<XML
<?xml version="1.0"?>
<psalm>
    <projectFiles>
        <directory name="src" />
        <ignoreFiles>
            <directory name="vendor" />
        </ignoreFiles>
    </projectFiles>
</psalm>
XML;

$xmlObject = simplexml_load_string($xml);
if ($xmlObject instanceof SimpleXMLElement) {
    $projectFiles = $xmlObject->children('projectFiles', true);
    if (property_exists($projectFiles, 'projectFiles')) {
        if ($projectFiles->projectFiles instanceof SimpleXMLElement) {
            return true;
        }
    }
}
Psalm output (using commit f7a6336):

ERROR: UndefinedPropertyFetch - 18:13 - Instance property SimpleXMLIterator::$projectFiles is not defined

@lsv lsv changed the title SimpleXMLElement false positive SimpleXMLElement - SimpleXMLIterator::var is not defined - After property_exists check Oct 18, 2021
@orklah
Copy link
Collaborator

orklah commented Oct 18, 2021

You can add

<universalObjectCrates>
        <class name="SimpleXMLIterator"/>
    </universalObjectCrates>

in your config file to resolve this (Psalm will consider any property as existing).

It should probably be something more integrated in Psalm though so I'll let this issue open

@weirdan
Copy link
Collaborator

weirdan commented Oct 18, 2021

is there a way to get rid of that check?

You can also use isset(): https://psalm.dev/r/4ef9699b4f

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/4ef9699b4f
<?php
$xml = <<<XML
<?xml version="1.0"?>
<psalm>
    <projectFiles>
        <directory name="src" />
        <ignoreFiles>
            <directory name="vendor" />
        </ignoreFiles>
    </projectFiles>
</psalm>
XML;

$xmlObject = simplexml_load_string($xml);
if ($xmlObject instanceof SimpleXMLElement) {
    $projectFiles = $xmlObject->children('projectFiles', true);
    if (isset($projectFiles->projectFiles)) {
        if ($projectFiles->projectFiles instanceof SimpleXMLElement) {
            return true;
        }
    }
}
Psalm output (using commit f7a6336):

No issues!

@lsv
Copy link
Author

lsv commented Oct 19, 2021

Thanks @weirdan
Though, shouldnt property_exists and isset give the same result?

@weirdan
Copy link
Collaborator

weirdan commented Oct 19, 2021

It should.

@orklah
Copy link
Collaborator

orklah commented Oct 19, 2021

The case for SimpleXMLIterator is fixed by PR above. The case for property_exists is a duplicate of #4182

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants