Add the No Bag Name Rule

README.md

@@ -71,6 +71,16 @@ Guarantee that a class is no more than 200 lines long, to clearly separate roles
 
 Guarantee that a method is no more than 40 lines long, to separate roles between classes and encourage specification.
 
+### No Bag Name Rule
+
+*Identifier : xefi.noBagName*
+
+Forbid bag names — words that name nothing and attract unrelated code. A class, interface, trait or enum may not end in `Service`, `Repository`, `Manager`, `Helper`, `Handler`, `Util` or `Utility`, singular or plural, nor be declared in a namespace with a `Services`, `Repositories`, `Managers`, `Helpers`, `Handlers`, `Utils`, `Utilities`, `Support`, `Common` or `Misc` segment. A well-named class in a `Services` namespace is still in a bag: the folder attracts classes the way the suffix attracts methods.
+
+A suffix is allowed only when a third-party class or interface that the declaration directly extends or implements ends in the same word, a trailing `Interface` aside: `RefusingDatabaseManager extends MySQLDatabaseManager`, `DatabaseSessionHandler implements SessionHandlerInterface`. A `*ServiceProvider` ends in `Provider` and is never reported.
+
+To adopt the rule in a code base that already has bags, generate a baseline (`vendor/bin/phpstan analyse --generate-baseline`): the existing offences stay listed, any new one fails, and an entry left behind once its bag is dissolved is reported as unmatched.
+
 ### No Basic Exception Rule
 
 *Identifier : xefi.noBasicException*

extension.neon

@@ -3,6 +3,7 @@ rules:
     - Xefi\PHPStanRules\Rules\BooleanPropertyNamingRule
     - Xefi\PHPStanRules\Rules\MaxLinePerClassRule
     - Xefi\PHPStanRules\Rules\MaxLinePerMethodRule
+    - Xefi\PHPStanRules\Rules\NoBagNameRule
     - Xefi\PHPStanRules\Rules\NoBasicExceptionRule
     - Xefi\PHPStanRules\Rules\NoDeleteCascadeLaravelRule
     - Xefi\PHPStanRules\Rules\NoGenericWordRule

src/Rules/NoBagNameRule.php

@@ -0,0 +1,127 @@
+<?php
+
+namespace Xefi\PHPStanRules\Rules;
+
+use PhpParser\Node;
+use PhpParser\Node\Name;
+use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\ClassLike;
+use PhpParser\Node\Stmt\Enum_;
+use PhpParser\Node\Stmt\Interface_;
+use PHPStan\Analyser\Scope;
+use PHPStan\Reflection\ReflectionProvider;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+
+/**
+ * @implements Rule<ClassLike>
+ */
+class NoBagNameRule implements Rule
+{
+    protected static string $ruleIdentifier = 'xefi.noBagName';
+
+    private const BAG_WORDS = [
+        'Service', 'Services', 'Repository', 'Repositories', 'Manager', 'Managers', 'Helper', 'Helpers',
+        'Handler', 'Handlers', 'Util', 'Utils', 'Utility', 'Utilities',
+    ];
+
+    private const CATCH_ALL_SEGMENTS = ['Support', 'Common', 'Misc'];
+
+    public function __construct(private ReflectionProvider $reflectionProvider)
+    {
+    }
+
+    public function getNodeType(): string
+    {
+        return ClassLike::class;
+    }
+
+    public function processNode(Node $node, Scope $scope): array
+    {
+        if ($node->namespacedName === null) {
+            return [];
+        }
+
+        $fullName = $node->namespacedName->toString();
+        $namespaceSegments = explode('\\', $fullName);
+        $shortName = array_pop($namespaceSegments);
+        $errors = [];
+
+        foreach (array_intersect($namespaceSegments, array_merge(self::BAG_WORDS, self::CATCH_ALL_SEGMENTS)) as $segment) {
+            $errors[] = RuleErrorBuilder::message(sprintf('%s sits in a "%s" namespace, a bag that attracts unrelated classes. Please move it to a namespace named after its domain.', $fullName, $segment))
+                ->identifier(self::$ruleIdentifier)
+                ->build();
+        }
+
+        $suffix = $this->bagSuffixOf($shortName);
+
+        if ($suffix !== null && !$this->takesTheSuffixOfAThirdPartyParent($node, $suffix)) {
+            $errors[] = RuleErrorBuilder::message(sprintf('%s ends in "%s", a bag name that attracts unrelated methods. Please name it after its single action, question or role.', $fullName, $suffix))
+                ->identifier(self::$ruleIdentifier)
+                ->build();
+        }
+
+        return $errors;
+    }
+
+    private function bagSuffixOf(string $shortName): ?string
+    {
+        foreach (self::BAG_WORDS as $word) {
+            if (str_ends_with($shortName, $word)) {
+                return $word;
+            }
+        }
+
+        return null;
+    }
+
+    private function takesTheSuffixOfAThirdPartyParent(ClassLike $node, string $suffix): bool
+    {
+        foreach ($this->parentsOf($node) as $parent) {
+            $parentName = (string) preg_replace('/Interface$/', '', $parent->getLast());
+
+            if (str_ends_with($parentName, $suffix) && $this->isThirdParty($parent->toString())) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * @return list<Name>
+     */
+    private function parentsOf(ClassLike $node): array
+    {
+        if ($node instanceof Class_) {
+            return array_values(array_filter(array_merge([$node->extends], $node->implements)));
+        }
+
+        if ($node instanceof Interface_) {
+            return array_values($node->extends);
+        }
+
+        if ($node instanceof Enum_) {
+            return array_values($node->implements);
+        }
+
+        return [];
+    }
+
+    private function isThirdParty(string $className): bool
+    {
+        if (!$this->reflectionProvider->hasClass($className)) {
+            return false;
+        }
+
+        $fileName = $this->reflectionProvider->getClass($className)->getFileName();
+
+        if ($fileName === null) {
+            return true;
+        }
+
+        $path = realpath($fileName) ?: $fileName;
+
+        return str_contains(str_replace('\\', '/', $path), '/vendor/');
+    }
+}

tests/Rules/NoBagNameRuleTest.php

@@ -0,0 +1,102 @@
+<?php declare(strict_types = 1);
+
+namespace Xefi\PHPStanRules\Tests\Rules;
+
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use Xefi\PHPStanRules\Rules\NoBagNameRule;
+
+/**
+ * @extends RuleTestCase<NoBagNameRule>
+ */
+class NoBagNameRuleTest extends RuleTestCase
+{
+    private const NAMESPACE = 'Xefi\PHPStanRules\Tests\Rules\data\NoBagNameRule';
+
+    protected function getRule(): Rule
+    {
+        return new NoBagNameRule($this->createReflectionProvider());
+    }
+
+    public function testRuleDumpAnExceptionInABagNamespace(): void
+    {
+        $this->analyse([__DIR__ . '/data/NoBagNameRule/BagNamespace.php'], [
+            [$this->inABagNamespace(self::NAMESPACE . '\Services\OrderTotals', 'Services'), 5],
+        ]);
+    }
+
+    public function testRuleDumpAnExceptionInACatchAllNamespace(): void
+    {
+        $this->analyse([__DIR__ . '/data/NoBagNameRule/CatchAllNamespaceOnATrait.php'], [
+            [$this->inABagNamespace(self::NAMESPACE . '\Support\ChannelLabels', 'Support'), 5],
+        ]);
+    }
+
+    public function testRuleDumpAnExceptionOnABagSuffix(): void
+    {
+        $this->analyse([__DIR__ . '/data/NoBagNameRule/BagSuffix.php'], [
+            [$this->endingInABagWord(self::NAMESPACE . '\OrderRepository', 'Repository'), 5],
+        ]);
+    }
+
+    public function testRuleDumpAnExceptionOnABagSuffixOfATrait(): void
+    {
+        $this->analyse([__DIR__ . '/data/NoBagNameRule/BagSuffixOnATrait.php'], [
+            [$this->endingInABagWord(self::NAMESPACE . '\OrderHelpers', 'Helpers'), 5],
+        ]);
+    }
+
+    public function testRuleDumpAnExceptionOnTheSuffixOfAProjectParent(): void
+    {
+        $this->analyse([__DIR__ . '/data/NoBagNameRule/SuffixOfAProjectParent.php'], [
+            [$this->endingInABagWord(self::NAMESPACE . '\InvoiceManager', 'Manager'), 5],
+        ]);
+    }
+
+    public function testRuleDoesNotDumpExceptionOnTheSuffixOfAThirdPartyParent(): void
+    {
+        $this->analyse([__DIR__ . '/data/NoBagNameRule/SuffixOfAThirdPartyParent.php'], []);
+    }
+
+    public function testRuleDoesNotDumpExceptionOnTheSuffixOfAPhpInterface(): void
+    {
+        $this->analyse([__DIR__ . '/data/NoBagNameRule/SuffixOfAPhpInterface.php'], []);
+    }
+
+    public function testRuleDumpAnExceptionInABagNamespaceUnderAThirdPartyParent(): void
+    {
+        $this->analyse([__DIR__ . '/data/NoBagNameRule/BagNamespaceUnderAThirdPartyParent.php'], [
+            [$this->inABagNamespace(self::NAMESPACE . '\Handlers\RefusingErrorHandler', 'Handlers'), 8],
+        ]);
+    }
+
+    public function testRuleDoesNotDumpExceptionOnAContractNamedAfterItsAnswer(): void
+    {
+        $this->analyse([__DIR__ . '/data/NoBagNameRule/ContractNamedAfterItsAnswer.php'], []);
+    }
+
+    public function testRuleDoesNotDumpExceptionOnAServiceProvider(): void
+    {
+        $this->analyse([__DIR__ . '/data/NoBagNameRule/ServiceProvider.php'], []);
+    }
+
+    public function testRuleDoesNotDumpExceptionOnABagWordInsideAName(): void
+    {
+        $this->analyse([__DIR__ . '/data/NoBagNameRule/BagWordInsideAName.php'], []);
+    }
+
+    public function testRuleDoesNotDumpExceptionOnAnAnonymousClass(): void
+    {
+        $this->analyse([__DIR__ . '/data/NoBagNameRule/AnonymousClass.php'], []);
+    }
+
+    private function inABagNamespace(string $className, string $segment): string
+    {
+        return sprintf('%s sits in a "%s" namespace, a bag that attracts unrelated classes. Please move it to a namespace named after its domain.', $className, $segment);
+    }
+
+    private function endingInABagWord(string $className, string $suffix): string
+    {
+        return sprintf('%s ends in "%s", a bag name that attracts unrelated methods. Please name it after its single action, question or role.', $className, $suffix);
+    }
+}

tests/Rules/data/NoBagNameRule/AnonymousClass.php

@@ -0,0 +1,5 @@
+<?php
+
+namespace Xefi\PHPStanRules\Tests\Rules\data\NoBagNameRule;
+
+$probe = new class extends LegacyManager {};

tests/Rules/data/NoBagNameRule/BagNamespace.php

@@ -0,0 +1,6 @@
+<?php
+
+namespace Xefi\PHPStanRules\Tests\Rules\data\NoBagNameRule\Services;
+
+class OrderTotals
+{}

tests/Rules/data/NoBagNameRule/BagNamespaceUnderAThirdPartyParent.php

@@ -0,0 +1,13 @@
+<?php
+
+namespace Xefi\PHPStanRules\Tests\Rules\data\NoBagNameRule\Handlers;
+
+use PhpParser\Error;
+use PhpParser\ErrorHandler;
+
+class RefusingErrorHandler implements ErrorHandler
+{
+    public function handleError(Error $error): void
+    {
+    }
+}

tests/Rules/data/NoBagNameRule/BagSuffix.php

@@ -0,0 +1,6 @@
+<?php
+
+namespace Xefi\PHPStanRules\Tests\Rules\data\NoBagNameRule;
+
+interface OrderRepository
+{}

tests/Rules/data/NoBagNameRule/BagSuffixOnATrait.php

@@ -0,0 +1,6 @@
+<?php
+
+namespace Xefi\PHPStanRules\Tests\Rules\data\NoBagNameRule;
+
+trait OrderHelpers
+{}

tests/Rules/data/NoBagNameRule/BagWordInsideAName.php

@@ -0,0 +1,6 @@
+<?php
+
+namespace Xefi\PHPStanRules\Tests\Rules\data\NoBagNameRule\Servicing;
+
+class ServiceArea
+{}

tests/Rules/data/NoBagNameRule/CatchAllNamespaceOnATrait.php

@@ -0,0 +1,6 @@
+<?php
+
+namespace Xefi\PHPStanRules\Tests\Rules\data\NoBagNameRule\Support;
+
+trait ChannelLabels
+{}

tests/Rules/data/NoBagNameRule/ContractNamedAfterItsAnswer.php

@@ -0,0 +1,6 @@
+<?php
+
+namespace Xefi\PHPStanRules\Tests\Rules\data\NoBagNameRule\Contracts;
+
+interface ExchangeRates
+{}

tests/Rules/data/NoBagNameRule/LegacyManager.php

@@ -0,0 +1,6 @@
+<?php
+
+namespace Xefi\PHPStanRules\Tests\Rules\data\NoBagNameRule;
+
+class LegacyManager
+{}

tests/Rules/data/NoBagNameRule/ServiceProvider.php

@@ -0,0 +1,6 @@
+<?php
+
+namespace Xefi\PHPStanRules\Tests\Rules\data\NoBagNameRule\Providers;
+
+class BillingServiceProvider
+{}

tests/Rules/data/NoBagNameRule/SuffixOfAPhpInterface.php

@@ -0,0 +1,38 @@
+<?php
+
+namespace Xefi\PHPStanRules\Tests\Rules\data\NoBagNameRule;
+
+use SessionHandlerInterface;
+
+class DatabaseSessionHandler implements SessionHandlerInterface
+{
+    public function open(string $path, string $name): bool
+    {
+        return true;
+    }
+
+    public function close(): bool
+    {
+        return true;
+    }
+
+    public function read(string $id): string|false
+    {
+        return '';
+    }
+
+    public function write(string $id, string $sessionData): bool
+    {
+        return true;
+    }
+
+    public function destroy(string $id): bool
+    {
+        return true;
+    }
+
+    public function gc(int $maxLifetime): int|false
+    {
+        return 0;
+    }
+}

tests/Rules/data/NoBagNameRule/SuffixOfAProjectParent.php

@@ -0,0 +1,6 @@
+<?php
+
+namespace Xefi\PHPStanRules\Tests\Rules\data\NoBagNameRule;
+
+class InvoiceManager extends LegacyManager
+{}

tests/Rules/data/NoBagNameRule/SuffixOfAThirdPartyParent.php

@@ -0,0 +1,13 @@
+<?php
+
+namespace Xefi\PHPStanRules\Tests\Rules\data\NoBagNameRule;
+
+use PhpParser\Error;
+use PhpParser\ErrorHandler;
+
+class RefusingErrorHandler implements ErrorHandler
+{
+    public function handleError(Error $error): void
+    {
+    }
+}