Issue #3618938: Name things the way Drupal core does: a verb and an object on every reader, every hook method named after its hook, the Trait and Hooks suffixes, American spelling, snake_case variables, and Drupal's own words for block, note and chip

This is the merge request for [#3618938].

Every candidate was counted against core itself, over core/lib and core/modules, so a finding here is a difference from core rather than a difference from my taste. A word core never uses is only a defect where core already has a word for the same thing.

What changed, commit by commit

• Test traits. 11 traits did not end in Trait, against 265 of core's 269 that do (one of the four exceptions is a typo, LinkInputValuesTraits). A sentence-shaped name in a use list gives no clue that it is a trait.
• Core's verbs on the readers. give becomes release (core has no give-prefixed method and 8 release-prefixed ones). activePinning(), activeConfiguration() and activeOrder() become getActive*, matching core's 16 and its zero bare active. recomputeState()/recomputeOnce() become computeState()/computeOnce(), core having 9 compute* and no recompute. giveUpFirst() is sortForRelease(), which says that it returns an ordering rather than performing one.
• Form template methods. 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 rest take a verb: getTabRoute(), getCollectionRoute(), getEmptyText(), getSavedMessage(), getStorage(), getTableKey(), getActionOptions(), getBatchTitle(), getSelectionWarning(), getActiveChips(), getEntityTypeId(). Core's form template methods are consistently verb-prefixed: getCancelUrl() 52, getQuestion() 50, getConfirmText() 42.
• of() readers named for what they return. Core has no of-prefixed method at all. LineFields::of() is fromValues(), SubsectionNumber::of() is parse(), ResourceFieldGroups::of() is getGroup(), FieldTables::of() is get(), MapLayoutSpacing::ofPitch() is fromPitch(), and the three unrelated ofSlot() methods become getTotal(), getAllocations() and getSlotTariffs(). Naming them for the return value rather than for the argument also keeps them clear of the separate for* question below.
• American spelling. canceled, canceling, labeled, center, color, signaled, gray. ResourceTypeCatalogue was British and also not core's word for "which of these are available here", so it is ResourceTypeRepository, which is what core's own jsonapi calls the same job.
• Hook and subscriber classes. 371 of the 442 classes in core's Hook/ directories end in Hooks; 46 of ours did and 7 did not, so all 53 do now. 106 of the 127 classes in core's EventSubscriber/ directories end in Subscriber; ten of ours did not and now do.
• Names that leaned on a preposition. 101 declarations, the family an earlier draft of this summary deferred. Deferring it was wrong: these are the names that actually stop a reader. The rule is the call site, not the declaration - getTotal(1) does not say that 1 is a slot id, and a variable is no better than a literal - so a method whose subject is a scalar names it (getSlotTotal(), getSlotTaken(), getSlotVenue(), getMessageLabel()), while one taking a typed object only needs its verb (getCloseness(), getStretches(), getPartitions()). A trailing preposition survives only behind a verb: applyTo(), refuseWith() and inheritsFrom() are whole sentences, getScopesFor() and getTotalFor() left the reader asking "for what?". Two were not stylistic at all: SessionPlaceRecord::setUnits() accumulates a signed delta and drops the row when it cancels to nothing, so the name said the opposite of the body and is now adjustUnits(); and domainsOf() was one name over two subjects, now getTenantDomains() and getChannelDomains().
• Words that named nothing. Four vocabulary problems, each raised by reading the names aloud rather than by counting core. getActiveChips() and its chip_ element keys carried a Material Design word for what the page calls a filter summary. blocksOf(), freeCountsInBlocks() and their neighbours used block, which is Drupal's own noun for something else entirely, so they say section. note was banned outright as too vague to survive a reader: noteReleased()/noteGone() are setReleased()/setGone(), and the payload keys, render keys and CSS classes say description. And piece was a second name for a thing already named: a party that cannot sit together sits in several runs of adjacent places, which is what the geometry has always called them, so pieceSizes() is getRunSizes() and fewestPieces() is fewestRuns(). The ordinary English senses stay - a piece of work, a piece of equipment, the pieces an SVG path is cut into.
• snake_case variables. 29 parameter names and 57 local names were lowerCamel while the rest of the module already used snake_case. Promoted constructor parameters are properties, so those stay lowerCamel: that distinction is what makes this not a blind sweep, and phpcs caught four @param lines where an early pass got it wrong.

Documentation and the French translations move in the same commits.

• Every hook implementation named after its hook. Core names 1158 of its 1252 #[Hook] methods exactly after the hook (92%); this module had 52 of 146 and has 141 now. It also settles disagreements the old names hid: three classes answered yoyaku_booking_delete with bookingDelete() while a fourth called it bookingDeleted(), and yoyaku_slot_tariff_insert was slotTariffInsert() in one class and slotTariffWritten() in another. The five left descriptive are core's own exception, a class implementing one hook several times: PaymentUiHooks answers yoyaku_ui_entity_table_alter three times and PlacementHooks twice, so addPriceColumn() and addGradesColumn() cannot both take the hook's name.

• The fee arithmetic moved out of the hook class. FeeSummaryHooks held three private methods doing money maths; core's 429 Hook/ classes hold 19 private helpers between them, because a hook class answers a hook rather than keeping a module's arithmetic. It is FeeTotals now and the hook class is 171 lines down to 84. Not a duplication fix: FeeRecordSubscriber writes the fee fields and splits charged from absorbed, FeeTotals reads them back or prices a basket that never reached a checkout.

• Say policy where the name only said attachment. An attachment here is a constraint policy paired with the host it hangs on, and four readers never mentioned the policy: getAttachedPoliciesForLine(), getAttachedPoliciesInContext(), getPolicyLabel() and isPolicyAttachableTo(). Attached rather than plain policies, because the return is pairs of policy and scope while getPoliciesForResource() and its siblings hand back bare policy entries. Ten more came out of the same question, every one hidden by the audit's verb list admitting entity, field, form and node as verbs - entityPolicies(), entity(), fieldValue(), formKey(), node() and field() twice over, which was the cookie field a contributor owns in one class and the query path for a host entity type in another.

• Two words the glossary leaned on without defining. concepts.md never said what an attachment is, while constraint-policies.md used it from its middle onward; the word is also Drupal's for a render array's assets and Orchestra's for a file on a notification. Both are pinned there now.

Deliberately not changed

Recorded on the issue 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. It moved too. The first pass kept the value and renamed only the constant, on the grounds that a stored value is a migration rather than a rename. That left the codebase saying STATE_CANCELED = 'cancelled', which is the worst of both spellings. The project is pre-1.0 and reinstall-only, so there is no migration to write: the value, the order_retention config key, its schema, and the workflow payloads that route on the literal all say canceled now. What stays British is only what belongs to another project, below.
• kessai's constants are kessai's. PaymentInterface::STATE_CANCELLED and PaymentEvents::CANCELLED keep their spelling. phpstan caught the second one after a sweep reached it.
• 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().
• Snake_case properties on config entities stay, because the property name is the config key, as core's NodeType::$new_revision is.
• Abstract classes not ending in Base stay. 31 of the 110 abstract classes in core's lib are named that way, DraggableListBuilder among them.
• PascalCase constraint plugin ids stay, which is what core uses.
• The service ids stay. The ones without a module prefix are core's own shapes (logger.channel.*, cache_context.*, plugin.manager.*). Only resource_type_catalogue moved, for its spelling.
• BookingWorkflowStarter keeps its name. It sits in an EventSubscriber directory but implements WorkflowStarterInterface, not EventSubscriberInterface, so it is a misplaced file rather than a misnamed class. Worth a follow-up, not a rename here.

What the audit got wrong

Recorded because the next audit should not repeat it.

• Its verb list was learned by frequency, counting any word that leads at least eight distinct core method names. That admitted eleven plain nouns as verbs - entity on 60 core methods, field on 38, form on 36, theme on 25, block on 23, and user, node, view, menu, module, comment - so the 64 methods here that start with one were never examined. TicketReference::entityTypeId() was among them.

• Renaming by family left the same name meaning two things. cells() moved in five form classes and stayed in TariffSummaries; entityTypeId() moved in four and stayed in TicketReference; candidates() moved in ResourceManagers and stayed in BookerMessages. All three are finished now.

• A tree-wide replace crossed two unrelated classes. offersByTariff() was both a route access callback and a predicate on the resource saying bookings must name a tariff. One replace renamed both, so twelve call sites read as permission checks on an entity. The predicate keeps its name and the callback is checkAccess().

• A hook matcher that could not read its own target. The pattern for #[Hook('name')] could not match an attribute carrying an order: argument, because the brackets inside it close the pattern early. Three implementations kept their old names while every other one took its hook's, and it surfaced only when unrelated work opened that file.

• A promoted-property test that failed both ways. It missed readonly and nullable types, so three promoted properties read as plain parameters; the guard added to compensate then protected two real parameters whose constructors are one-liners, renaming their docblocks away from their arguments. phpcs is what caught that, since it checks every @param name against the signature.

• A container fetched by string is invisible to static analysis. CI's Kernel job caught TransactionDeleteCascadeTest calling the transaction access hook by its old name. Nothing local could have: the test fetches the class with a string rather than ::class, so container->get() returns mixed and phpstan has no type to check the method against, while the same call written with ::class elsewhere was caught before the push. The local Kernel run that covered the rename filtered on hook-shaped class names, and that test is not one. The seventy removed names have since been grepped across every call site; those three were the only ones left.

Also carries Orchestra's renamed API

Orchestra merged its own naming issues while this was open and renamed part of the API this module implements. yoyaku requires orchestra: 1.x-dev, so CI installs the tip and every one of these would have failed there.

phpstan caught the loud half: AssignmentInterface::getCandidates() and getViewerTokens(), DeadlineProviderInterface::getDeadlineFor() and shouldRearm(), RecipientInterface::getAccount(), CheckoutEvent::getRefusals() and InteractionContext::getSignalUrl().

It could not catch the quiet half. AudienceBase::getRecipients() and InteractionBase::getSignalOutcomes() are concrete, so our recipients() and signalOutcomes() went on compiling while overriding nothing at all. The same silence is why notifyByDefault() has its name back: this MR's own sweep read it as a local predicate and made it shouldNotifyByDefault(), which orphaned the override and quietly defaulted resource managers to not being notified, since AudienceBase::notifyByDefault() answers FALSE.

Verification

• phpcs --standard=Drupal,DrupalPractice over the module root: exit 0, 0 errors and 0 warnings. Six of those warnings were mine, comment lines that the longer identifiers pushed past 80 characters; they are rewrapped by hand.
• phpstan level 3: [OK] No errors. It found a real cross-module break that phpcs could not see: the spelling sweep had rewritten PaymentEvents::CANCELLED, which is kessai's constant.
• cspell over the changed files: 13 words unknown to core's dictionary and absent from .cspell-project-words.txt, none of them on a line this MR adds.
• Kernel classes covering the renamed constants, readers, hook classes and subscribers run green against MySQL: 8 classes.
• eslint with core's .eslintrc.passing.json and .prettierrc.json: clean.

The full suite is left to CI, and it caught two things no linter could. Both are the same mistake in two languages: a rename that collided with a name already in scope.

• Renaming $chip to $filter in the bookings form gave the summary loop the name of the transaction filter read from the query string above it, so the filtered listing answered 500 while the unfiltered one rendered.
• Renaming the calendar's note to validity gave its live region the name of the validity() method that works out what to say, so building the region replaced the method with a div and the first call to it threw. Every calendar scenario then sat waiting for a grid that could not arrive, which is why the failure looked like ten unrelated timeouts.

Both are fixed, and both classes of collision are now swept for across the module rather than waited on: no foreach value shadows an assignment it is later read against, and no this.x = clobbers a method x().

AI-Generated: Yes (Claude Code ran the audit and wrote these renames. I reviewed the work before posting it.)

Also carries [#3618956]

kessai merged its own naming issue ([#3618948]) while this was open, renaming part of its public API. That broke this project's blocking jobs, so !307 (closed) is folded in here rather than landing separately: PaymentEvents::CANCELLED to CANCELED, PaymentInterface::STATE_CANCELLED to STATE_CANCELED, refunds() to getRefunds(), claims() to getClaims(), forgetCard() to deleteStoredCard(), and the service reference @kessai.gateway_manager to @plugin.manager.kessai_payment_gateway. That last one is resolved from the container by name, so a stale reference fails compilation rather than a test. #3618956 can be closed against this merge request.