False positives with NamespaceSniff

[schmengler]

In #13e5071 the "NamespaceSniff" sniffer was introduced, with the comment "When catching an exception inside a namespace it is important that you escape to the global space."

I assume that this is meant to prevent accidental catch (Exception $e) statements where Exception is not imported.

However it is also triggered by code like this:

namespace N1;

class CustomException extends \Exception {}

try {
    throw CustomException;
} catch (CustomException) {
}

which is valid good code. Using the FQN instead even triggers a warning by PHP Inspections in PhpStorm.

If you do not want to remove this check (which I would prefer), I suggest to limit it to "Exception" and "Mage_*Exception" classes. But note that this would still give a false positive with N1\Exception.

But it should definitely be a warning, and not an error.

[zhooravlik]

Hi @schmengler , Thanks for noticing possible issues, and thanks for the long-awaiting response :) The issue here - is not properly described purpose of the NamespaceSniff,- that's needed only for the non-imported Classes, see an example: Error is the appropriate type of sniffer results for the case.

For all other cases, this just will work (e.g. if you're using full class path like \N1\Exception or \RuntimeException or \Exception). Closing the issue.

[schmengler]

Thanks for the response, I totally understand the purpose but the issue was that it did not check if classes were imported and was triggered on all not fully qualified class names.

but since we're not useing magento-ecg anymore, I'm fine with closing the issue personally ;)