[RFC] Improvements suggestions for AnnotateThrowablesRector and problems related to their implementation

[Aerendir]

Summary

Currently the rector AnnotateThrowablesRector only supports a direct thrown \Throwable like:

throw new \Exception

The statement throw is analyzed both in methods and in functions.

However, this is not sufficient to cover the full range of concrete possibilities that may lead to the throw of a \Throwable.

Following there are the possible kinds of \Throwables and the places in which is possible to find them.

1. Kinds of \Throwables (direct use of throw)

There are basically three types of possible thrown \Throwables:

1. Direct class instantiation;
2. Use of a variable;
3. Use of a factory method.

1.1 - Direct class instantiation

throw new \Exception();

How to proceed

This is the current covered type of \Throwable.

1.2 - Use of a variable

Example from a real world application that uses Guzzle and has to manage its exceptions

class VariableThrow
{
    /**
     * @param Throwable $e
     *
     * @throws Throwable
     * @throws \InvalidArgumentException
     *
     * @return string
     */
    public static function guessCode(Throwable $e): string
    {
        if ( ! $e instanceof Exception) {
            throw new InvalidArgumentException('Only an \Exception can be passed.');
        }

        // This is thrown if there is an error in connection and relates to problem of the app itself
        if ($e instanceof ConnectException) {
            switch ($e->getHandlerContext()['errno']) {
                case CURLE_OPERATION_TIMEOUTED:
                    return self::ERROR_CODE_OPERATION_TIMEOUT;
                case CURLE_COULDNT_RESOLVE_HOST:
                    return self::ERROR_CODE_COULDNT_RESOLVE_HOST;
                case CURLE_COULDNT_CONNECT:
                    return self::ERROR_CODE_COULDNT_CONNECT;
                case CURLE_SSL_CONNECT_ERROR:
                    return self::ERROR_CODE_SSL_CONNECT_ERROR;
                case CURLE_GOT_NOTHING:
                    return self::ERROR_CODE_GOT_NOTHING;
            }
        } elseif ($e instanceof TooManyRedirectsException) {
            return self::ERROR_CODE_TOO_MANY_REDIRECTS;
        } elseif ($e instanceof ClientException) {
            return self::ERROR_CODE_CLIENT_4XX;
        } elseif ($e instanceof ServerException) {
            return self::ERROR_CODE_CLIENT_5XX;
        }
        // This is reached if the Exception kind is not handled: it has to be handled
        elseif ($e instanceof GuzzleException) {
            return self::ERROR_CODE_GUZZLE_ERROR;
        }

        // As last chance, rethrow the exception as it doesn't come from Guzzle
        throw $e;
    }
}

How to proceed

1. Find all the possible values of $e
2. If they are classes, add them to the Docblock of method VariableThrow::guessCode()

1.3 - Use of a factory method

<?php

class ThrowWithAFactoryMethod
{
    /**
     * @throws Exception
     */
    private function createException()
    {
        return new \Exception();
    }

    /**
     * @throws Exception
     */
    public function callingMethod()
    {
        throw $this->createException();
    }
}

How to proceed

1. Access the Dockblock of ThrowWithAFactoryMethod::createException()
2. Get the @return tags of the method
3. Add the found @return tags to Docblock of method MethodCall::callingMethod(), BUT as @throws tags

2. Scenarios in which is possible to find a throw of a \Throwable

1. Method call
2. Function call

2.1 - Method call

<?php

class MethodCall
{
    /**
     * @throws Exception
     */
    private function methodThatThrows()
    {
        throw new \Exception();
    }

    /**
     * @throws Exception
     */
    public function callingMethod()
    {
        $this->methodThatThrows();
    }
}

2.2 - Function call

This case can be easily illustrated using thecodingmachine/safe library:

<?php

use function Safe\sprintf;

class UseOfAPossibleThrow
{
    /**
     * @return string
     * @throws \Safe\Exceptions\StringsException
     */
    public function callingMethod()
    {
        return sprintf('On error, this call will throw an exception of kind %s', \Safe\Exceptions\StringsException::class);
    }
}

How to proceed

The process is the same for both methods and functions.

Given a code like this:

public function aFunction(string $aParam)
{
    if (false === $aParam) {
        throw new \Exception();
    }

    $defaultException = new \RuntimeException();
    
    switch($param) {
        case 1:
            return a_function_that_may_throw_an_exception();
        case 2:
            return $object->aMethodThatMayThrowAnException();
        case 3:
            $defaultException = new AnotherException();
    }
    
    throw $defaultException;
}

1. Analyze all throws
2. If they are a class, annotate them
3. If they are a variable ($defaultException), collect all the possible values it can assume
   1. For each collected value, if is a class, annotate it
4. Create a list of all function calls and for each of them
5. Access the Dockblock of function sprintf
6. Get the @throws tags of the function
7. Add the found @throws tags to Docblock of method UseOfAPossibleThrow::callingMethod()
8. Create a list of all method calls and for each of them
9. Access the Dockblock of function sprintf
10. Get the @throws tags of the function
11. Add the found @throws tags to Docblock of method UseOfAPossibleThrow::callingMethod()

Implementation problems

• 1.1 - Direct class instantiation: Already implemented
• 1.2 - Use of a variable: Not yet analyzed in depth Implemented
• 1.3 - Use of a factory method: How to access the Dockblock of the factory method from the Throw_ node? Implemented

The main problem is that it seems that currently it is not possible to access the Docblock of a method or of a function from the Throw_ PhpParser node.

Without being able to access the Docblock, it is not possible to annotate the calling method.

Maybe we need a new NodeVisitor or we need to improve someone of the already existing?
https://github.com/nikic/PHP-Parser/blob/master/doc/2_Usage_of_basic_components.markdown#node-traversation

@TomasVotruba , please, RFC.

[TomasVotruba]

Could you add per feature checklist? Link 1.1 to already finished PR
And send PR with first feature of 1.2

[Aerendir]

Yes, for sure: tomorrow I’ll add the checklist.

In the mean time, any ideas about the way to access the docblocks of methods and functions from the Throws_ node?
I haven’t one...

[TomasVotruba]

In the mean time, any ideas about the way to access the docblocks of methods and functions from the Throws_ node?

I'm not sure what you mean Could you share a code?

[Aerendir]

Read the 1.3: to know what is thrown we need to access the dock lock of the invoked method or function.

Currently it is not available from the Throw_ object (or, at least, I cannot find a reliable path).

[Aerendir]

From theTvrow path is available the name of the method and the name of the class, but they are strings and not objects, so it is not possible to access the dockblock.

[TomasVotruba]

There is too much text and issues at once for me to understand. That's why I prefer to take one case at a time, with deep focus and low cognitive complexity.

[Aerendir]

It is one case 😅

How to access the docblock of a statement like

Throw $this->aMethod()

We need to access the node that represents aMethod().

This is the only one issue to address now: without solving this, nothing more is possible.

[TomasVotruba]

I'm not the author of this issues, so me it is really very hard to read :)

Is is docblock or PHP code?

Demo link would be better: https://getrector.org/demo

[Aerendir]

I’m from the phone.

‘’’php
throw ExceptionFactory::createException();
‘’’
From inside AnnotateThrowablesRector, How would you know which is the class returned by ‘createException()’?

You have to read the dockblock of ‘createException()’.

How would you access its Docblock? I don’t know how.

[TomasVotruba]

I'd use:

$reflectionMethod = new ReflectionMethod(...);
$reflectionMethod::getDocComment();

[Aerendir]

I'd use:

$reflectionMethod = new ReflectionMethod(...);
$reflectionMethod::getDocComment();

Yes, for sure it is possible to use the reflection.

But the reflection only returns a string like this:

    /**\n
     * @param int $code\n
     *\n
     * @return TheException|TheExceptionTheSecond|TheExceptionTheThird|\RuntimeException\n
     */

It hasn't the power that Rector\BetterPhpDocParser\PhpDocInfo has.

But to create a Rector\BetterPhpDocParser\PhpDocInfo object it is required a code similar to this:

$classFqn = implode('\\', $node->expr->class->parts);
$method = $node->expr->name->name;
$reflectionMethod = new \ReflectionMethod($classFqn, $method);
$docComment = $reflectionMethod->getDocComment();
$phpDocInfoFactory = new \Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory(...);
$phpDocInfo = $phpDocInfoFactory->createFromString($docComment);

The problem is that PhpDocInfoFactory has this signature:

final class PhpDocInfoFactory
{
    ...

    public function __construct(
        PhpDocParser $phpDocParser,
        Lexer $lexer,
        CurrentNodeProvider $currentNodeProvider,
        StaticTypeMapper $staticTypeMapper,
        TypeComparator $typeComparator
    ) ...

It has a lot of dependencies!

Or am I missing somthing? Are we sure there isn't an easier way of getting the Docblock comment?

[TomasVotruba]

Reflection and PhpDocInfo cannot be used together.

Look for ReflectionMethod in the rest of Rector code in relation with doc blocks