Skip to content

Add the No Bag Name Rule - #3

Open
GautierDele wants to merge 1 commit into
mainfrom
feat/no-bag-name-rule
Open

GautierDele wants to merge 1 commit into
mainfrom
feat/no-bag-name-rule

Conversation

@GautierDele

Copy link
Copy Markdown
Member

What

A tenth rule, NoBagNameRule (identifier xefi.noBagName). It refuses a class, interface, trait or enum that:

  • ends in a bag word — Service, Repository, Manager, Helper, Handler, Util or Utility, singular or plural;
  • or sits in a bag namespace — a segment made of one of those words, or of Support, Common or Misc.

A well-named class in a Services namespace is still in a bag: the folder attracts classes the way the suffix attracts methods, so the namespace counts as much as the name.

Not reported:

  • A suffix a third-party parent ends in too. When a class or interface that the declaration directly extends or implements ends in the same word — a trailing Interface aside — and comes from vendor/ or from PHP itself: RefusingDatabaseManager extends MySQLDatabaseManager, DatabaseSessionHandler implements SessionHandlerInterface. A project parent never excuses it (InvoiceManager extends LegacyManager is reported), and a bag namespace is never excused.
  • *ServiceProvider, which ends in Provider.
  • Anonymous classes.

Why

It is the tool behind the no-god-classes skill of xefi/claude-skills (xefi/claude-skills#229), which says the check belongs in this package: a loaded skill still lets a well-named class land in Services/, and a review reads the class, not its folder. A code base that already has bags adopts the rule with a baseline (vendor/bin/phpstan analyse --generate-baseline): existing offences stay listed, any new one fails, and an entry left behind once its bag is dissolved is reported as unmatched. The README documents this.

Please review

  • Upgrade impact. Like the other rules in extension.neon, it is on by default, so a project that upgrades gets one error per existing bag until it generates a baseline. Worth a minor release and a line in the release notes.
  • Symfony. Doctrine's default App\Repository namespace is reported (the *Repository suffix itself is excused, since ServiceEntityRepository is a third-party parent). Should a Repository namespace segment be exempt, or left to the baseline?

Tests

  • NoBagNameRuleTest covers 12 cases: both offences, traits, a project parent, a vendor interface, a PHP interface, a bag namespace under a third-party parent, a contract named after its answer, a service provider, a bag word inside a name, and an anonymous class. Disabling the vendor check makes the three exemption cases fail.
  • The whole suite passes on PHP 8.0 and 8.4 (38 tests). The rule sticks to PHP 8.0 syntax, as the CI matrix requires.
  • On a real OSDD Laravel project it reports exactly the six classes of a layer's Services/ before that folder was dissolved, and nothing after.
  • The package's own phpstan analyse still reports 13 errors, all in BooleanPropertyNamingRule, NoDeleteCascadeLaravelRule and CascadeDeleteVisitor, the files Close detection gaps in logic rules #1 touches. None comes from the new rule.

Refuse a class, interface, trait or enum whose name ends in a bag word
(Service, Repository, Manager, Helper, Handler, Util or Utility, singular
or plural), or that sits 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 excused only when a third-party class or interface that the
declaration directly extends or implements ends in the same word, a
trailing Interface aside. A project parent never excuses it, and a bag
namespace is never excused. A *ServiceProvider ends in Provider and is
never reported. A code base that already has bags adopts the rule with a
PHPStan baseline.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant