Skip to content

Commit

Permalink
Rework Exception Naming Conventions
Browse files Browse the repository at this point in the history
Until now the SuperfluousExceptionNaming sniff was enabled in Doctrine
Coding Standard, but every of our projects disabled this sniff in their
local phpcs.xml rule.

Doctrine does not actually follow this rule, instead we provide the
context of the exception in the name as a "prefix" similar to PHPs own
RuntimeException and so on.

This is done, because exceptions are used soley in the context of code
that reads like:

    } catch (DBALException $e) {
    }

If we allow classes named "Exception", then we introduce a developer
experience problem, because it potentially requires the user
to make an alias/renaming decision, increasing their mental load:

    use OtherLibrary\Exception as OtherException;
    use Doctrine\DBAL\Exception as DBALException;
    use Exception;

    } catch (OtherException $e) {
    } catch (DBALException $e) {
    } catch (Exception $e) {
    }

Additionally it makes it hard for developers understanding catch clauses
when they cannot rely on the fact that "Exception" is the global one.

    } catch (Exception $e) { // don't mess with user expectations
  • Loading branch information
beberlei committed Sep 9, 2020
1 parent 637003f commit e5101ef
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 7 deletions.
56 changes: 56 additions & 0 deletions lib/Doctrine/Sniffs/NamingConventions/ExceptionNamingSniff.php
@@ -0,0 +1,56 @@
<?php

declare(strict_types=1);

namespace Doctrine\Sniffs\NamingConventions;

use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Sniffs\Sniff;

use function trim;
use function ucfirst;

use const T_CLASS;
use const T_STRING;

class ExceptionNamingSniff implements Sniff
{
/**
* @return array<int, (int|string)>
*
* @phpcsSuppress SlevomatCodingStandard.TypeHints.ParameterTypeHint.MissingNativeTypeHint
* @phpcsSuppress SlevomatCodingStandard.TypeHints.ReturnTypeHint.MissingNativeTypeHint
*/
public function register()
{
return [T_CLASS];
}

/**
* @param int $stackPtr
*
* @return void
*
* @phpcsSuppress SlevomatCodingStandard.TypeHints.ParameterTypeHint.MissingNativeTypeHint
* @phpcsSuppress SlevomatCodingStandard.TypeHints.ReturnTypeHint.MissingNativeTypeHint
*/
public function process(File $phpcsFile, $stackPtr)
{
$tokens = $phpcsFile->getTokens();

$className = $phpcsFile->findNext(T_STRING, $stackPtr);
$name = trim($tokens[$className]['content']);
$errorData = [ucfirst($tokens[$stackPtr]['content'])];

switch ($name) {
case 'Exception':
$phpcsFile->addError(
'Using Exception as a short class name is not allowed.',
$stackPtr,
'Invalid',
$errorData
);
break;
}
}
}
4 changes: 2 additions & 2 deletions lib/Doctrine/ruleset.xml
Expand Up @@ -147,8 +147,6 @@
<rule ref="SlevomatCodingStandard.Classes.UnusedPrivateElements"/>
<!-- Forbid prefix and suffix "Abstract" for abstract classes -->
<rule ref="SlevomatCodingStandard.Classes.SuperfluousAbstractClassNaming"/>
<!-- Forbid prefix and suffix "Exception" for exception classes -->
<rule ref="SlevomatCodingStandard.Classes.SuperfluousExceptionNaming"/>
<!-- Forbid prefix and suffix "Interface" for interfaces -->
<rule ref="SlevomatCodingStandard.Classes.SuperfluousInterfaceNaming"/>
<!-- Forbid suffix "Trait" for traits -->
Expand Down Expand Up @@ -557,4 +555,6 @@
<!-- turned off by PSR-12 -> turning back on -->
<severity>5</severity>
</rule>

<rule ref="Doctrine.NamingConventions.ExceptionNaming"/>
</ruleset>
5 changes: 3 additions & 2 deletions tests/expected_report.txt
Expand Up @@ -16,6 +16,7 @@ tests/input/doc-comment-spacing.php 10 0
tests/input/duplicate-assignment-variable.php 1 0
tests/input/EarlyReturn.php 6 0
tests/input/example-class.php 36 0
tests/input/exception-naming.php 2 0
tests/input/forbidden-comments.php 14 0
tests/input/forbidden-functions.php 6 0
tests/input/inline_type_hint_assertions.php 7 0
Expand All @@ -35,7 +36,7 @@ tests/input/semicolon_spacing.php 3 0
tests/input/single-line-array-spacing.php 5 0
tests/input/spread-operator.php 6 0
tests/input/static-closures.php 1 0
tests/input/superfluous-naming.php 11 0
tests/input/superfluous-naming.php 10 0
tests/input/test-case.php 8 0
tests/input/trailing_comma_on_array.php 1 0
tests/input/traits-uses.php 11 0
Expand All @@ -45,7 +46,7 @@ tests/input/use-ordering.php 1 0
tests/input/useless-semicolon.php 2 0
tests/input/UselessConditions.php 20 0
----------------------------------------------------------------------
A TOTAL OF 375 ERRORS AND 0 WARNINGS WERE FOUND IN 41 FILES
A TOTAL OF 376 ERRORS AND 0 WARNINGS WERE FOUND IN 42 FILES
----------------------------------------------------------------------
PHPCBF CAN FIX 310 OF THESE SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
Expand Down
10 changes: 10 additions & 0 deletions tests/fixed/exception-naming.php
@@ -0,0 +1,10 @@
<?php

declare(strict_types=1);

namespace Test;

class Exception
{
}

9 changes: 9 additions & 0 deletions tests/input/exception-naming.php
@@ -0,0 +1,9 @@
<?php

declare(strict_types=1);

namespace Test;

class Exception
{
}
6 changes: 3 additions & 3 deletions tests/php-compatibility.patch
Expand Up @@ -8,9 +8,9 @@ index fd5432c..233e24d 100644
tests/input/EarlyReturn.php 6 0
-tests/input/example-class.php 36 0
+tests/input/example-class.php 39 0
tests/input/exception-naming.php 2 0
tests/input/forbidden-comments.php 14 0
tests/input/forbidden-functions.php 6 0
tests/input/inline_type_hint_assertions.php 7 0
@@ -23,9 +23,9 @@ tests/input/LowCaseTypes.php 2 0
tests/input/namespaces-spacing.php 7 0
tests/input/NamingCamelCase.php 7 0
Expand All @@ -34,8 +34,8 @@ index fd5432c..233e24d 100644
tests/input/useless-semicolon.php 2 0
tests/input/UselessConditions.php 20 0
----------------------------------------------------------------------
-A TOTAL OF 375 ERRORS AND 0 WARNINGS WERE FOUND IN 41 FILES
+A TOTAL OF 384 ERRORS AND 0 WARNINGS WERE FOUND IN 41 FILES
-A TOTAL OF 376 ERRORS AND 0 WARNINGS WERE FOUND IN 42 FILES
+A TOTAL OF 385 ERRORS AND 0 WARNINGS WERE FOUND IN 42 FILES
----------------------------------------------------------------------
-PHPCBF CAN FIX 310 OF THESE SNIFF VIOLATIONS AUTOMATICALLY
+PHPCBF CAN FIX 319 OF THESE SNIFF VIOLATIONS AUTOMATICALLY
Expand Down

0 comments on commit e5101ef

Please sign in to comment.