Do not mutate clean up functions

[exussum12]

Functions like Close and Free should not be removed by the function call
removals. Removing them does not change the tested behavior, but does
free the memory before the script finished / garbage collection is
invoked

This PR:

• Adds new feature ...
• Covered by tests
• Doc PR: https://github.com/infection/site/pull/XXX

[maks-rafalko]

Could you please run make cs to fix CS Fixer issues locally?

Saw a couple of such complains on Twitter about fclose and friends, so it makes sense for me.

[sanmai]

I'm not terribly excited about a special list for instance methods. I wish we could have this either configurable, or completed with the means supposed to exists under #1231

[exussum12]

If it was configurable you would need this everywhere ?
How about checking that all of these in the method versions take no argument ? eg $someClass->free() would be ignored but $someClass->free($someVariable) would be allowed to mutate ?

[theofidry]

I think doing so against native calls is ok but extending it to arbitrary classes is not: a user will likely be able to test that its $someClass->free() is called or not

[exussum12]

They return void though ? https://www.php.net/manual/en/mysqli-result.free.php or https://www.php.net/manual/en/event.free could check if void return and 0 arguments and called one of these things ?

[theofidry]

and how do you tell the difference between $mysqliResult->free() and $repository->free()? We do not have type inference at the moment so we cannot distinguish the two and the later may deserve to be covered by infection

[exussum12]

I agree there will be methods missed by doing this, I don't have the stats but most methods I have seen called one of these are methods which have no real observable behaviour.

Could have this as a user config option instead at project level instead of line level ?

[sanmai]

I don't think a user config option for this list is a bad idea.

[maks-rafalko]

Since we have different opinions on object's methods calls, here is what I suggest:

• leave only native functions call ignore list in this PR and merge it
• think about method calls in a separate issue. But: I recommend to read this thread: Allow adding mutation exclusions for specific calls #697. By implementing the feature that is being discussed there will cover your needs (method calls)

[exussum12]

Since we have different opinions on object's methods calls, here is what I suggest:

Sounds good to me, I have updated the PR to be just the native functions side of things here.

[maks-rafalko]

Thank you, @exussum12!

[lcobucci]

Does it make sense to add openssl_free_key() to the list too?

[maks-rafalko]

sounds good to me. Feel free to open a PR targeted to 0.17, so we will release it ASAP as 0.17.1