When running the code style sniffer upon commit, I'm getting errors like this: ```FOUND 1 ERROR AFFE

U01LKKBK97T

When running the code style sniffer upon commit, I'm getting errors like this:

FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 173 | ERROR | [x] @throw annotation `\Exception` superfluous and
     |       |     needs to be removed
----------------------------------------------------------------------

Indeed, my code might throw an exception at this very place. Thus, having a @throws annotation in the function's docblock should be fully correct. How would the calling function know that it needs to catch it otherwise? Am I missing something here or is the code sniffer behaving wrongly here?

U013EK8RD7A

Just a guess, but perhaps it’s because You’re using a generic Exception class. If You’d throw “SomeSpecificException” exception this error would go away.

U01LKKBK97T

Just wanted to give you a most basic example.
It also appears with custom exceptions, unfortunately.

U01LKKBK97T

Interesting enough that the sniffer complains about the @throw annotation while I'm using @throws, actually. Would that make the difference maybe?

Unknown

Yes, needs to be throws as per standards afaik.

U01LKKBK97T

My assumption is that the sniffer will complain if the annotated function doesn't explicitly throw this type of exception. If e. g. it is thrown in a nested function, it won't recognize it.

vasily.rodin

As far as I remember, this code sniffer rule checks only exceptions, which you explicitly define inside the method body. So if exception is not directly in the method body, annotation is not needed

U01LKKBK97T

Yes, looks like it. But how would you get around this then? Catching and rethrowing the exception as a workaround doesn't feel right.

vasily.rodin

https://github.com/spryker/code-sniffer/blob/07022098a526f7d64a04da148c95d86e93360111/Spryker/Sniffs/Commenting/DocBlockThrowsSniff.php#L18
Looks like it’s intentional:

We only ever declare them for the exceptions inside the own method.

So you can extend this sniff, or disable it. Or maybe @UQK3ZPJEN can suggest better, since he’s author of this sniff

Unknown

yes, we only use primary level exceptions, as nested starts to lie soon (as it is almost impossible to assert by sniffer).

Unknown

you can always remove this on project level - or replace it.

Unknown

What is the concrete code in your method?

U01LKKBK97T

It's basically this:

/**
 * @throws \Exception
 */
protected function foo()
{
    // this will throw an exception that should be propagated to the caller of foo()
    $this->nestedFoo();
}

Unknown

In that case you could only extend/overwrite/silence the core sniffer rule.
I wonder if a bang-overwrite could make sense

* @throws \Exception !

with the ! as comment it could ignore the "removal" requirement.

I use a similar approach for IDE helper based annotations to be auto-managed.

U01LKKBK97T

I was hoping for something like that. But doesn't help, unfortunately.

U01LKKBK97T

/**
 * @param array $tourTime
 * @param string $localeTransfer
 *
 * @throws \Exception !
 *
 * @return string
 */
protected function getDeliveryDateText(array $tourTime, string $localeTransfer): string
{
    ...
}

still gives

----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 174 | ERROR | [x] @throw annotation `\Exception` superfluous and
     |       |     needs to be removed
----------------------------------------------------------------------

Unknown

"wonder" + "could make sense" => I was only spitballing ideas here, none of this is of course implemented.

U01LKKBK97T

Sure, I appreciate any idea. Didn't expect it to be final, but hey, we could have been lucky.

Unknown

I added the idea here for further feedback from everyone: https://github.com/spryker/code-sniffer/issues/275
Also, if someone wants, they can do a PR here to support this.

Unknown

@U01LKKBK97T Can you check if https://github.com/spryker/code-sniffer/pull/278 would work here as expected?
I am planning a release for end of this week, so this fit in with almost no work needed.

U01LKKBK97T

Seems to work, great job!