>>> [!note] Migrated issue <!-- Drupal.org comment --> <!-- Migrated from issue #3618947. --> Reported by: [mably](https://www.drupal.org/user/3375160) Related to !78 >>> <p>A naming audit of pdv and its eleven submodules, checked against measured Drupal core practice rather than against the linters. The mechanical layer is already clean: phpcs with Drupal and DrupalPractice over the whole module exits 0 with no messages, all 256 class declarations match their filename and PSR-4 path, all 1692 method declarations are lowerCamelCase, every interface, trait, abstract base and exception carries its suffix, and every constant is UPPER_SNAKE. Everything below sits above what a sniff can see: whether a name still describes what the code does, and whether one concept is spelled one way.</p> <h3>Two vocabularies have drifted</h3> <p><strong>document and file are two words for one payload type.</strong> The stored value is file: PdvItemKindInterface::TYPE_FILE is 'file'. But PdvItemKind documents the field as "Payload nature: 'document' (a file) or 'record'", naming a value the code never writes. The synonym then reaches stored data and a public accessor: PdvGrantRequest has a base field document_uuid read back through getDocumentUuid(), and what it holds is an item uuid, for record kinds as much as file kinds. Vault::findReplaceableUniqueDocument() is the clearest case: it returns PdvItemInterface and its own docblock says "Finds the item a unique-kind store would replace". There are 41 document and 24 documents identifier occurrences sitting beside readFile(), saveFile() and TYPE_FILE.</p> <p><strong>The item-kind vocabulary has three spellings.</strong> Entity type id pdv_item_kind, class PdvItemKind, entity label "Item kind", permission "administer pdv item kinds" and menu link pdv.item_kinds all agree. The config prefix says kind (files pdv.kind.passport.yml), the registry class is ItemKindCatalogue and its test is PdvKindCatalogueTest. The admin path says something different again: /admin/config/pdv/document-kinds, which is also the one spelling that is factually wrong, because the vocabulary holds record kinds too.</p> <h3>Where the module diverges from core, measured</h3> <ul> <li>Hook classes: 9 end in Hook, 1 in Hooks, 1 has no suffix. Core has 376 ending in Hooks against 10 in Hook. A requirements-only class is named for the module in core (UserRequirements, FileRequirements), not RequirementsHook.</li> <li>Two hook methods are named for their intent rather than their hook. Core names every form_FORM_ID_alter method formSomethingAlter, without exception.</li> <li>Three event subscribers do not end in Subscriber, beside four that do. Core: 62 of 74.</li> <li>Five settings forms, three naming schemes: PdvSettingsForm, PdvClientSettingsForm, PdvServerApiSettingsForm and two plain SettingsForm. Core's module Form classes are 242 unprefixed against 93 prefixed, and the prefixed ones name the entity they act on, not the module. Across pdv, 36 production classes take the Pdv prefix and 140 do not, with no rule separating them.</li> <li>One settings route in six is pdv_audit_trail.settings_form. No route name in core ends in settings_form.</li> <li>Config prefixes strip a word too many. Core's default is the provider plus the full entity type id, which is why core always overrides it; the idiomatic override is the id with the module prefix removed and nothing else. pdv_tenant to pdv.tenant is right; pdv_item_kind to pdv.kind drops item, and pdv_vault_connection to pdv_client.connection drops vault. pdv_vault_connection is also the only entity type whose class is not the CamelCase of its id.</li> <li>15 methods take a noun where the module's other 150 getter declarations take get, most of them on published interfaces. kindAccess() is the one that actively misleads: in Drupal, access() is an access check returning AccessResultInterface, not a data reader.</li> <li>Four verb pairs make a real distinction that only their docblocks state: revoke() beside revokeGrant(), revoke() beside revokeTrust(), purge() beside purgeAll(), lock() beside lockCurrent(). Core puts the axis in the name: load(), loadByProperties(), loadMultiple().</li> <li>Test groups: pdv_eca, pdv_mail and pdv_message group their tests as pdv while the other four submodules use their own machine name, so running one submodule's tests by group is impossible and the CI split cannot see them.</li> <li>Two test classes do not name the class under test: PdvKindCatalogueTest covers ItemKindCatalogue, PdvElementFormAlterTest covers ElementFormHook.</li> <li>Nouns with no precedent in core: ItemRef abbreviates where its neighbours SealedItem and DeferredConsent do not, and Catalogue is a British spelling in a codebase where core has none. Three names cover one keyed in-request store: UnlockedSubjectKeys, HandleStore, RemoteReadCache and VaultItemCache.</li> <li>Three names describe the wrong thing: PdvFileValue is a stateless resolver, GrantRequestCallback builds a redirect and is not a callback, and PdvWebformSettings answers whether vault features are switched off while sitting beside that module's real SettingsForm.</li> <li>pdv_test_console ships in modules/ as a real, permission-gated developer tool. Core ships 602 modules whose name contains test and none of them outside a tests directory, so the name tells both a reader and any tooling that filters fixtures by name the wrong thing.</li> <li>src/Entity/TenantField is not an entity. It is one static returning the shared tenant base field definition, and its name reads as a field plugin.</li> </ul> <h3>Deliberately not changed</h3> <ul> <li>HandleCodec and RecordCodec keep their names. Core has no class containing Codec, but both genuinely encode and decode, and Encoder would be less accurate than the name it replaced.</li> <li>ItemKindRepository keeps all(). Core has five bare all() methods, so the noun-shaped getter rule does not reach it.</li> <li>ReadController::kindAccess() and kindLabels() keep their names. They are route callbacks named after their routes, which is core's convention for a controller method.</li> <li>UnlockedSubjectKeys was plural, which is not itself a fault: core has Actions, AttachedAssets and CalculatedPermissions. It is renamed for the three-way inconsistency with HandleStore and the two caches, not for the plural.</li> <li>TenantField stays a static in src/Entity. Core's shape for a shared base field is a trait, but converting one static that five entity classes call into a trait they each use is more code for no gain; only the name changes.</li> <li>The seven entity classes, their interfaces, their forms and list builders keep the Pdv prefix, as do the pdv_file element and pdv_record_map handler. That is core's rule: prefix when the class is named after its entity type or plugin id, and the namespace carries the module otherwise.</li> </ul> <h3>Upgrade path</h3> <p>pdv is pre-1.0 and carries no update hooks by policy, so renames are free and reinstall is the upgrade path. This one moves a base field name, two config prefixes with their installed files and French counterparts, a submodule machine name and methods on three published interfaces, so a site running an alpha needs to uninstall and reinstall rather than update in place.</p> <h3>Dependencies patched so the Drupal 12 lane can run this</h3> <p>The next-major lane was red before this branch, and red silently: the 1.x badge is green over a SKIPPED lane, because composer (next major) sits manual and neither next-major job had run since 2026-08-23. Played on unmodified 1.x at this branch's base it fails identically, 376 errors across 64 classes, every one of them Drupal\key\Plugin\KeyPluginManager only supports annotation-based discovery. Core 11.2 inserted a plugin attribute argument ahead of the annotation argument on DefaultPluginManager, and core 12 refuses a manager that offers only an annotation class.</p> <p>Rather than skip the tests that boot those projects, the lane patches them, scoped to the next-major composer job only. key is covered by two upstream merge requests: #3483550 adds its three Attribute classes and passes them alongside the annotation classes, and #3599449 takes the cacheability argument core 12 added to getOperations(). Both were applied against the released key 8.x-1.22 before being wired in. That alone took the lane from 60 passing tests to 386, and from 64 erroring classes to 3.</p> <p>webform has the same defect in five plugin managers and no upstream issue for it, so the last entry is a patch file carried in this repository rather than a diff URL. It adds Attribute classes for WebformElement, WebformHandler, WebformVariant, WebformExporter and WebformSourceEntity, and passes each alongside its existing Annotation class, so webform's own annotated plugins and this module's two keep being discovered. No plugin is converted, because the annotation fallback makes that unnecessary. It inserts the attribute argument unconditionally, which is correct on a next-major-only lane and is exactly why it is not an upstream patch yet: webform still supports core 10.3, where that argument position is the annotation. Filing it upstream is follow-up work.</p> <p>Offering an attribute class is then what makes core deprecate every plugin still discovered by annotation, and the lane runs with the fail-on-deprecation flag, so 347 of those are ignored by two patterns in .deprecation-ignore.txt. Four of them are this module's own plugins rather than a dependency's, and the file says so: pdv_vault_transit and pdv_test_wrap on key, pdv_file and pdv_record_map on webform. They stay annotations because the current lane installs the released key and webform, where the Attribute classes do not exist, so declaring them with attributes would make this module's own plugins undiscoverable there. The comment names the condition for converting them and deleting the lines.</p> <p>With all of that in place both lanes are green: phpunit and phpunit (next major) each report 436 passing tests, 0 failures and 0 exceptions.</p> <p>AI-Generated: Yes (Claude Code was used to run this audit, draft this issue summary, and write the renames and test updates on the merge request. I reviewed and ran the work myself before posting it.)</p>