From d5e2cb70545799839382335c96a391a54274e295 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Wed, 9 Sep 2020 11:07:43 +0200 Subject: [PATCH] Rework Exception Naming Conventions 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 --- .../ExceptionNamingSniff.php | 56 +++++++++++++++++++ lib/Doctrine/ruleset.xml | 4 +- tests/expected_report.txt | 5 +- tests/fixed/exception-naming.php | 9 +++ tests/input/exception-naming.php | 9 +++ tests/php-compatibility.patch | 6 +- 6 files changed, 82 insertions(+), 7 deletions(-) create mode 100644 lib/Doctrine/Sniffs/NamingConventions/ExceptionNamingSniff.php create mode 100644 tests/fixed/exception-naming.php create mode 100644 tests/input/exception-naming.php diff --git a/lib/Doctrine/Sniffs/NamingConventions/ExceptionNamingSniff.php b/lib/Doctrine/Sniffs/NamingConventions/ExceptionNamingSniff.php new file mode 100644 index 00000000..e6d97060 --- /dev/null +++ b/lib/Doctrine/Sniffs/NamingConventions/ExceptionNamingSniff.php @@ -0,0 +1,56 @@ + + * + * @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; + } + } +} diff --git a/lib/Doctrine/ruleset.xml b/lib/Doctrine/ruleset.xml index c0bf70bd..648553ef 100644 --- a/lib/Doctrine/ruleset.xml +++ b/lib/Doctrine/ruleset.xml @@ -148,8 +148,6 @@ - - @@ -558,4 +556,6 @@ 5 + + diff --git a/tests/expected_report.txt b/tests/expected_report.txt index fd5432cc..8d60f99c 100644 --- a/tests/expected_report.txt +++ b/tests/expected_report.txt @@ -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 @@ -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 @@ -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 ---------------------------------------------------------------------- diff --git a/tests/fixed/exception-naming.php b/tests/fixed/exception-naming.php new file mode 100644 index 00000000..085c85e2 --- /dev/null +++ b/tests/fixed/exception-naming.php @@ -0,0 +1,9 @@ +