So I have a method inside my class, that does a GET request and then there are error handlers and various other components, but then if the response is good, it will be returned inside // THE 200 RESPONSE IS HERE.

How would I be able to make the method return the try/catch $response_body outside the try/catch if it's successful? So I have can return $response_body at the end of the method so that I have a valid return.

Update:

So I have the following method inside my class:

public function get_posts_via_rest_api(): array
    {
        $page = 1;
        try {
            $response = wp_remote_get(
                $this->global_endpoint . '?page=' . $page
            );
            if ((!is_wp_error($response)) && (200 === wp_remote_retrieve_response_code($response))) {
                $response_body = json_decode(
                    $response['body'],
                    false,
                    512,
                    JSON_THROW_ON_ERROR
                );
                if (empty($response_body)) {
                    return [];
                }
                if (json_last_error() === JSON_ERROR_NONE) {

                    // THE 200 RESPONSE IS HERE

                    $posts = $response_body;
                }
            } else {
                error_log(
                    print_r(
                        'Error: ' . $response->get_error_message()
                    )
                );

                return [];
            }
        } catch (Exception $e) {
            error_log(
                print_r(
                    'Error: ' . $e,
                    true
                )
            );

            return [];
        }

        // THIS SHOWS ERROR IN IDE
        return $posts;
    }

CodePudding user response：

if (json_last_error() === JSON_ERROR_NONE) {

PHPStorm doesn't know that this statement is redundant (you've already told json_decode() to throw exceptions) so all it sees is a potential logic path missing a return value because there's no else to this if.

You can simplify your code to use the following

$response_body = json_decode(
    $response['body'],
    false,
    512,
    JSON_THROW_ON_ERROR
);

return empty($response_body) ? [] : $response_body;

Now all your paths are covered.

You could also consolidate all your unhappy path return values into one statement at the end of your method.

The above can become

if (!empty($response_body)) {
    return $response_body;
}

and remove all the return []; and place a single return at the end.

CodePudding user response：

Per the comments, your code is missing an edge case that might not logically be possible but your static analysis cannot be absolutely certain of it. From its perspective, your code is effectively this however it needs to handle the edge case like this. (As I write this, Phil has responded, and I think his answer is more correct, but I’m going to post anyway.) In this answer, this first if block has a terminal return (with error logging)

public function get_posts_via_rest_api(): array
    {
        $page = 1;
        try {
            $response = wp_remote_get(
                $this->global_endpoint . '?page=' . $page
            );
            if ((!is_wp_error($response)) && (200 === wp_remote_retrieve_response_code($response))) {
                $response_body = json_decode(
                    $response['body'],
                    false,
                    512,
                    JSON_THROW_ON_ERROR
                );
                if (empty($response_body)) {
                    return [];
                }
                if (json_last_error() === JSON_ERROR_NONE) {

                    // THE 200 RESPONSE IS HERE

                    $posts = $response_body;
                }

                error_log(
                    print_r(
                        'Error: An unexpected condition happened'
                    )
                );

                return [];
            } else {
                error_log(
                    print_r(
                        'Error: ' . $response->get_error_message()
                    )
                );

                return [];
            }
        } catch (Exception $e) {
            error_log(
                print_r(
                    'Error: ' . $e,
                    true
                )
            );

            return [];
        }

        // THIS SHOWS ERROR IN IDE
        return $posts;
    }

EDIT

But, here’s how I would actually write it (untested, but should show the gist). First, try small, second, break out of your if blocks whenever possible, and third, return early.

public function get_posts_via_rest_api(): array
    {
        $page = 1;
        try {
            $response = wp_remote_get(
                $this->global_endpoint . '?page=' . $page
            );
        } catch (Exception $e) {
            error_log(
                print_r(
                    'Error: ' . $e,
                    true
                )
            );

            return [];
        }

        if (is_wp_error($response) || (200 !== wp_remote_retrieve_response_code($response)) {

            error_log(
                print_r(
                    'Either an error happened or a non-200 did',
                    true
                )
            );

            return [];
        }

        try{
            $response_body = json_decode(
              $response['body'],
              false,
              512,
                   JSON_THROW_ON_ERROR
                );
        } catch (Exception $e) {
            error_log(
                print_r(
                    'JSON Error: ' . $e,
                    true
                )
            );

            return [];
        }

        return $response_body;
    }