Array deconstructing reports new variables as "avoid using undefined variables"

spaceemotion commented 4 years ago

PHPMD version: 2.8.1
PHP Version: 7.4.5
Installation type: composer
Operating System / Distribution & Version: Mac OSX 10.14.2

Current Behavior

[$foo, $bar] = split();

returns with errors like Avoid using undefined variables such as $foo which will lead to PHP notices.

I tried this with a SCCE that only called a simple method, but curiously that one did not fail. Only after I put the method in a class i was able to reproduce it.

Expected Behavior

Since it's a new variable from array deconstruction I would expect no error.

Steps To Reproduce:

Create test.php:

<?php

class Test
{
   private static function split($string)
   {
       return [$string, $string];
   }

   public function deconstruct()
   {
       [$foo, $bar] = self::split('foo bar');
   }
}

Run phpmd: vendor/bin/phpmd test.php text phpmd.xml Here's my config:

<?xml version="1.0"?>
<ruleset name="phpcstd" xmlns="https://pmd.sourceforge.io/ruleset_xml_schema.xsd">
   <rule ref="rulesets/codesize.xml"/>
   <rule ref="rulesets/design.xml"/>
   <rule ref="rulesets/naming.xml"/>
   <rule ref="rulesets/cleancode.xml"/>
</ruleset>

See the following errors:

/xxxx/test.php:12    Avoid using undefined variables such as '$foo' which will lead to PHP notices.
/xxxx/test.php:12    Avoid using undefined variables such as '$bar' which will lead to PHP notices.

Checks before submitting

[x] Be sure that there isn't already an issue about this. See: Issues list
[x] Be sure that there isn't already a pull request about this. See: Pull requests
[x] I have added every step to reproduce the bug.
[x] If possible I added relevant code examples.
[x] This issue is about 1 bug and nothing more.
[x] The issue has a descriptive title. For example: "JSON rendering failed on Windows for filenames with space".

kylekatarnls commented 4 years ago

$foo and $bar are created in deconstruct() but not used so I would be tempted to say the rule just does its job.

It actually (implicitly) suggests to just call the method without storing what you don't use:

    public function deconstruct()
    {
        self::split('foo bar');
    }

Would be equivalent and consume less memory (considering "split" is doing some other required processes).

spaceemotion commented 4 years ago

Ah so the thing is, I actually used them later down the line. Just not in this SSCE.

If I use this instead:

<?php

class Test
{
    private static function split($string)
    {
        return [$string, $string];
    }

    public function deconstruct()
    {
        [$foo, $bar] = self::split('foo bar');
        echo "$foo $bar";
    }
}

and run phpmd again, I get this result:

/xxxx/test.php:12   Avoid using undefined variables such as '$foo' which will lead to PHP notices.
/xxxx/test.php:12   Avoid using undefined variables such as '$bar' which will lead to PHP notices.
/xxxx/test.php:13   Avoid using undefined variables such as '$foo' which will lead to PHP notices.
/xxxx/test.php:13   Avoid using undefined variables such as '$bar' which will lead to PHP notices.

kylekatarnls commented 4 years ago

Oh, yes that's an other known issue. It's not related to deconstruct at all. PDepend, our engine is not able to find variables interpolated in "" strings.

kylekatarnls commented 4 years ago

Or maybe not, when looking at the first code, I though it were about "unused", but I see your error is about using "undefined". I will check this.

spaceemotion commented 4 years ago

Hm, this one fails as well though:

<?php

class Test
{
    private static function split($string)
    {
        return [$string, $string];
    }

    public function deconstruct()
    {
        [$foo, $bar] = self::split('foo bar');

        $this->analyze($foo);
        $this->analyze($bar);
    }

    private function analyze($foo) {
        echo $foo;
    }
}

Nothing going on with strings, same errors (line 12,14,15)

spaceemotion commented 4 years ago

Oh, just found out that if I use list() instead of the short array syntax it works. But then my code style tools want to revert it back.

phpmd / phpmd