Problem/Motivation

A Drupal developer opening this module should be able to guess a name before reading it. Most of Yoyaku already reads that way, but an audit of all 1032 PHP files against core's own naming found several groups where the module invents a shape core already has a word for. Every candidate was counted against core itself, over core/lib and core/modules, so each finding is a difference from core rather than a matter of taste. A word core never uses is only a defect where core already has a word for the same thing.

Renames are free here: the module is pre-1.0 and reinstall-only, so there are no update hooks and no BC layer to carry.

What the audit found

  • 11 test traits do not end in Trait: StartsFromAnEmptySite, HoldsWithoutThrottling, RunsEveryScenario, SignsInOnce, RecordsQueryCounts, WaitsForTheNextPage, HoldsOneLine, AllowsForALoadedRunner, KnowsHowThisCoreGroupsAssets, DrawsAHall, FoldsTheMap. In core, 265 of 269 traits end in Trait, and one of the four exceptions is a typo. A sentence-shaped name in a class's use list gives no clue that it is a trait.
  • Words core has its own verb for. give, giveBackPlace, giveUpFirst and givenPartition against core's zero give-prefixed methods and 8 release-prefixed ones. activePinning, activeConfiguration and activeOrder against core's 16 distinct getActive methods and zero bare active. recomputeState and recomputeOnce against core's 9 compute methods and no recompute.
  • Bare-noun template methods on the overview and order forms. columns() and cells() do what core calls buildHeader() (35 uses) and buildRow() (37), and SelectableOverviewFormBase already declared a buildRow() beside a bare header(), so this is the module's own precedent as much as core's. The same applies to tabRoute, collectionRoute, emptyText, savedMessage, storage, tableKey, actionOptions, batchTitle, selectionWarning, activeChips and entityTypeId. Core's form template methods are consistently verb-prefixed: getCancelUrl() 52 uses, getQuestion() 50, getConfirmText() 42.
  • The of() readers. Core has no of-prefixed method anywhere. Four unrelated classes declared of() and three more declared ofSlot(), each meaning something different, so each is renamed for what it returns rather than for its argument.
  • British spelling, in 408 places: cancelled, cancelling, labelled, centre, colour, signalled, grey, catalogue. ResourceTypeCatalogue was British and also not core's word for "which of these are available here", so it becomes ResourceTypeRepository, which is what core's own jsonapi calls the same job.
  • Hook and subscriber class suffixes. 371 of the 442 classes in core's Hook directories end in Hooks; 46 of ours did and 7 did not. 106 of the 127 classes in core's EventSubscriber directories end in Subscriber; ten of ours did not.
  • camelCase variables. 29 parameter names and 57 local names were lowerCamel while the rest of the module already used snake_case, so the module disagreed with itself. Core uses snake_case for both.

What is deliberately not changed

Recorded here so a later audit does not "fix" them. Each was flagged by a first pass and then cleared by reading what core actually does.

  • The stored value cancelled stays a stored value. It is written into booking and transaction rows on a live site, into an order_retention config key, and into the orchestra workflow payloads that route on that literal. So the constant name is now STATE_CANCELED while its value is still the old string. Changing the value is a data migration and a cross-module change, not a rename.
  • kessai's constants keep kessai's spelling, both PaymentInterface::STATE_CANCELLED and PaymentEvents::CANCELLED.
  • Bare-noun readers on value objects stay. PolicyContext::lines(), PolicyOutcome::violations(), PolicyScope::host() and TransactionContext::transaction() read oddly, but core does the same in the same kind of class: jsonapi's EntityCondition has field(), operator() and value(), and workflows' State has id(), label() and weight(). On config entities, core's own NodeTypeInterface declares displaySubmitted(), which is the shape of autoConfirms().
  • Promoted constructor parameters stay lowerCamel, because they are properties. That distinction is what makes the variable sweep not a blind one.
  • Snake_case properties on config entities stay, because the property name is the config key, as core's NodeType declares $new_revision.
  • Abstract classes not ending in Base stay. 31 of the 110 abstract classes in core's lib are named that way, including DraggableListBuilder, which DraggableWeightedListBuilder deliberately mirrors.
  • PascalCase constraint plugin ids stay, because that is what core uses (AllowedValues, EntityBundleExists), unlike every other plugin type.
  • The service ids stay. The ones without a module prefix are core's own shapes: logger.channel, cache_context and plugin.manager. Only resource_type_catalogue moved, and only for its spelling.

Also verified clean, with nothing to change: PascalCase type names and file names matching the type they declare; the Interface and Exception suffixes; UPPER_SNAKE constants; route names, permission names, menu, task and action link ids; entity type ids and plugin ids; and test classes ending in Test with no two consecutive capitals in a test method name.

Remaining tasks

Three follow-ups came out of this and are not in the merge request, because each is a coherent group of its own and folding them in would leave a half-done rename visible in the diff.

  • The for prefix. 33 declarations across 12 names (forResource, forOrder, forSlot and so on) against exactly one in core, forUpdate(), which is the SQL clause rather than a getter.
  • BookingWorkflowStarter is in the wrong directory. It sits under EventSubscriber but implements WorkflowStarterInterface, not EventSubscriberInterface, so it is a misplaced file rather than a misnamed class, and moving it changes its namespace.
  • WaitsForTheNextPageTrait has no users. It solves a real problem, a stale element reference after a submit, and nothing calls it. It is the tool for the remaining click sites that still go through find(...)->click().

AI-Generated: Yes (Claude Code was used to help draft this issue summary and to write the code on the merge request. I reviewed and ran the work myself before posting it.)

Issue fork yoyaku-3618938

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mably created an issue. See original summary.

mably’s picture

Title: Name test traits and template methods the way core does, so a Drupal developer can guess them » Name things the way Drupal core does: the Trait and Hooks suffixes, core's verbs, American spelling and snake_case variables
Issue summary: View changes
Status: Active » Needs review

  • mably committed 6ce4d964 on 1.x
    task: #3618938 Name things the way Drupal core does: the Trait and Hooks...
mably’s picture

Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.