RemoveUnusedVariableAssignRector: calls with side-effects should not be removed

[staabm]

Bug Report

Subject	Details
Rector version	last dev-main
Installed as	composer dependency

Minimal PHP Code Causing Issue

See https://getrector.com/demo/a56b4a20-48c9-4a49-8316-ba6873ba3c76

<?php

final class DemoFile
{
    public function run(bool $param)
    {
        $return = file_get_contents("http://example.com");
    }
}

Responsible rules

RemoveUnusedVariableAssignRector

Expected Behavior

calling file_get_contents has side-effects and therefore this line should not be removed, but just the assigned variable (or just leave the line as is)

[samsonasik]

What the side effect of file_get_contents?

Unless it used along with ob_* function call, it seems no side effect

[staabm]

one local side-effect is, that it spawns a $http_response_header variable.

another one: the call itself could trigger whatever side-effect at the remote system beeing called via the url.

[samsonasik]

I see, that seems need to be registered to PureFunctionDetector service, could you add it? Thank you.

[samsonasik]

After I re-check, it seems only spawn header when it called in singular process, not different process, because there is no next direct usage, after it is make null headers. Calling:

get_contents();
var_dump($http_response_header);

will get :

Undefined variable $http_response_header

except, we have real repository to test real use case for re-used after removed.

Could you create a simple repo use case for it which header can be re-used?

[staabm]

in my testing, the example behaves identical to what is described on the php.net manual:

[staabm]

will get :

it seems you missed the point of the code example. I just sent a php-docs improvement PR to make it more clear

[samsonasik]

no, that on singular process, the process must be:

call method via another method call which variable is only scoped there

var_dump($http_response_header)

eg:

call_method_that_call_has_inner_file_get_contents_inside_it();
var_dump($http_response_header);

[staabm]

no, that on singular process, the process must be:

how did you came to this conclusion? I can't find anything in the docs which reads like what you describe

---

the docs mention

$http_response_header will be created in the local scope

which means it will be created in the local scope of the function triggering the http-call - in our example file_get_contents

[samsonasik]

I see, I created PR rectorphp/rector-src#5162 for it.