Phpinspectionsea: Check for missing handler for return false from function

[4n70w4]

| Subject | Details |
| :------------- | :---------------------------------------------------------------------------- |
| Issue type | Feature request |
| Plugin | Php Inspections (EA Extended) |
| Language level | PHP 7.2 |

Current behaviour (description/screenshot:)

nothing

Expected behaviour

warning message

Notes

Show warning notifications for functions that can return false in the case of an error and is no handled of false-case in code. Example: file_put_contents and imagejpeg returns false on error, but lack of verification can cause errors in other parts of the code.

<?php

$image_file = 'test.jpg';

file_put_contents($image_file, 'some bites');

$image = imagecreatefromjpeg($image_file);

imagejpeg($image, $image_file);

Need warnings for file_put_contents($image_file, 'some bites'), imagejpeg($image, $image_file) and e.t.c. without check false case.

[kalessil]

So, if I getting this correctly, we are talking about un-checked file writes. Sounds like a good idea.

Here I'll request support by @4n70w4 , @rentalhost, @funivan: please prepare a list of such functions (similar to file_put_contents, imagejpeg) - I'm overloaded ATM.

[4n70w4]

I'm talking about any functions that can return false in case of error (instead of exception), and the programmer did not check the result false.

[funivan]

@4n70w4 functions json_encode, shuffle and preg_match should be checked ?

We can collect the list of functions and find how they are used in modern frameworks and tools (symfony, laravel, yii .....)

P.S. Google can help us:

1. site:php.net "FALSE on failure" inurl:function
2. site:php.net "FALSE if an error occurred" inurl:function

[kalessil]

Understood @4n70w4, but this is too big. We need to break it down to smaller pieces, as PHP APIs are not so well designed. Can we start with a practical example, preceding the FR then?

[4n70w4]

Hi!

Yes, the volume of such functions is great. You can split the task into parts, for example:

1) the functions of the standard library, taking the information from here:
C:\Program Files\JetBrains\PhpStorm 2018.2.1\plugins\php\lib\php.jar -> \stubs\
2) a detector of similar functions written in user code

Patterns to detect the similar functions:
1) «false on failure» in return phpdoс (found 635 matches in PhpStorm\plugins\php\lib\php.jar -> \stubs)
2) «<b>FALSE</b> on failure» in return phpdoс (found 718 matches in PhpStorm\plugins\php\lib\php.jar -> \stubs)
2) «FALSE if an error occurred» in return phpdoс (found 14 matches in PhpStorm\plugins\php\lib\php.jar -> \stubs)
3) «false otherwise» in return phpdoс (found 141 matches in PhpStorm\plugins\php\lib\php.jar -> \stubs)
4) «otherwise FALSE» in return phpdoс (found 45 matches in PhpStorm\plugins\php\lib\php.jar -> \stubs)
5) «Otherwise, false» in return phpdoс (found 7 matches in PhpStorm\plugins\php\lib\php.jar -> \stubs)
6) «FALSE on error» in return phpdoс (found 179 matches in PhpStorm\plugins\php\lib\php.jar -> \stubs)
7) ... and more more others: cat stubs// | grep 'return bool|\|boolean|\||bool' | grep -v '<b>FALSE</b> on failure' | grep -v 'FALSE on failure' | grep -v 'false otherwise' | grep -v '@return bool Success/Failure' | grep -v 'FALSE otherwise' | grep -v 'false on failure' | grep -v 'otherwise FALSE' | grep -v 'Otherwise, false' | grep -v 'FALSE on error' | sort | uniq

[kalessil]

Search criteria are clear, still breakdown is needed. Covering 100% of those cases will emit lots of false-positives, hence I need a practical breakdown to start with: like files operations, string manipulation, regular expressions and etc.

[4n70w4]

in my user case:

need notice/warning to check function return value:

// some code
function_return_false_on_error($param);
// another code

need notice/warning to check function return value:

// some code
$result = function_return_false_on_error($param);
// another code

need notice/warning to check function return FALSE value:

// some code
$result = function_return_false_on_error($param);

if(false !== $result) {
    // some code
}

// another code

need notice/warning to check function return FALSE value:

// some code
if(false !== function_return_false_on_error($param)) {
    // some code
}

// another code

need notice/warning to write phpdoc «return false on failure»:

// some code
return function_return_false_on_error($param);

need notice/warning to write phpdoc «return false on failure»:

// some code
$result = function_return_false_on_error($param);
// another code
return $result

correct code, notice/warning not need:

// some code
$result = function_return_false_on_error($param);

if(false === $result) {
    // some handler
}

// another code

correct code, notice/warning not need:

// some code
if(false === function_return_false_on_error($param) ) {
    // some handler
}

// another code

correct code, notice/warning not need:

// some code
if(false === sfunction_return_false_on_error($param) ) {
    // some handler
} else {
    //  another code
}

// another code

maybe something else...

[kalessil]

So, after multiple talks at UGs, we are going to skip this request types. PhpStan is doing a good job here, so I don't see much benefits in replicating its' functionality.