Replacement for ignoredExceptions

[xificurk]

I've been thinking a bit more about the problem discussed under 461c803

I believe the class inheritance is not the real problem for configuring checked/unchecked exceptions. All the exceptions can be arranged in tree graph, and you can unambiguously decide whether a given class is checked or unchecked - the original implementation using checkedExceptions and ignoredExceptions handled this correctly.

Let's take a look at the "problematic" case you mentioned:

when ho have @throws LogicException as unchecked, it could throws SomeMyCheckedLogicException and you have a problem - analysis will process it as unchecked.

• Current configuration options (even after removal of possibility to combine whitelist/blacklist) allow you to get into the exact same situation - simply use: checkedExceptions: [SomeMyCheckedLogicException]
• In your own code you should never have @throws LogicException. All @throws annotations with unchecked exception class should be reported as an error.
• For too generic @throws annotations in 3rd party code, you can either use methodThrowTypeDeclarations parameter in phpstan config, or you can catch and rewrap/rethrow specific exceptions in your code.

The real problem comes from interfaces - any class can implement multiple interfaces, and even worse any interface may inherit from multiple interfaces. Even with a pretty strict rules that you suggested in 461c803 it's still really easy to construct an exception with ambiguous resolution of checked/unchecked, e.g.

interface CheckedFoo {}
interface UncheckedFoo extends CheckedFoo {}
interface CheckedBar {}
class AmICheckedOrUncheckedException extends Exception implements UncheckedFoo, CheckedBar {}

checkedExceptions: [CheckedFoo, CheckedBar]
uncheckedExceptions: [UncheckedFoo]

I'm not really sure how to solve this. Maybe the checkedExceptions and uncheckedExceptions parameters should always contain only class names, but not interfaces?

[pepakriz]

Thank you for your effort, really nice research.

You are right, interfaces cause a similar problem, which does not have been mentioned. So this is the main reason why I don't like checkedExceptions options because everything should be "checked" by default ideally. But in real PHP ecosystem there is 3rd party code which uses one generic Exception for everything, or doesn't respect logic vs runtime, or uses @throws annotation for LogicExceptions (see Doctrine\ORM\EntityManager::find). So marking only LogicException (or some other) as unchecked will not work across the whole project.

But... If will be possible to define different unchecked exceptions for different namespaces, it could be a workable solution. Let's say:

parameters:
	exceptionRules:
		uncheckedExceptions:
			'\': # Root namespace
				- LogicException
			FooUglyLibrary: # Library which uses `Exception` for all situations
				- Exception
			Doctrine:
				- Doctrine\Common\Persistence\Mapping\MappingException # I'm really right about doctrine mapping in runtime

Do you see some downsides except more complex configuration?

BTW PhpStorm uses unchecked exception list too.

[xificurk]

Well, this is not an easy issue to solve :/

I don't really like the configuration by namespace you're describing. I think it brings similar sorts of problems that lead to the removal of ignoredExceptions with much less readable configuration. I mean - would the configuration allow only top-level namespaces? What would be the rules regarding priority? All exceptions inherit from exceptions of root namespace, how would the rules for root namespace and vendor namespace interact? ...

I have thought about this a lot and came to the conclusion that the originally implemented resolution algorithm was the only one that really makes sense. The configuration was easy to read and the algorithm does not have any unnecessary complexity built in.

I would love to see it re-introduced in some future version, possibly hidden behind some config flag and disabled by default. I do think there are real life use cases where this kind of advanced more complex config is needed/useful. On the other hand, as you mentioned, e.g. PhpStorm has only unchecked exceptions list, so we (as php community) probably should fix our shit code instead of inventing workarounds for these idiosyncrasies.