From 042df00ecf72be19fcbda1dd4ec71bc3c0d04dce Mon Sep 17 00:00:00 2001 From: Nikolay Strikhar Date: Tue, 25 Aug 2026 13:04:34 +0200 Subject: [PATCH 01/10] Say each thing once, where it is hardest to get wrong Comments only -- every changed file is token-identical outside its comments, and every @param, @return, @var and @throws survives with its type, which level 9 reads. The rationale had been written twice: once in AGENTS.md, which is the durable document, and again in the file it described. Boot/Scheduler shared 52 phrases with it, Provider 35, Loader 25. Within src/ the same arguments repeated too -- the reader-drains-the-buffer note in four files, the hook-prefix stand-down in three, the Throwable catch in three. Each now has one home and a clause everywhere else. Prose in src/ goes 14,876 words to 6,873. --- src/Absorber.php | 105 ++++------- src/Activator.php | 26 +-- src/Boot/Scheduler.php | 124 ++++--------- src/Config.php | 48 ++--- src/Conflict/Contracts/Resolver_Interface.php | 11 +- src/Conflict/Detector.php | 41 ++--- src/Conflict/Gatekeeper.php | 132 ++++---------- src/Conflict/Redirector.php | 129 +++---------- src/Conflict/Resolver.php | 101 +++------- src/Conflict/Rewriter.php | 67 ++----- src/Conflict_Policy.php | 20 +- src/Contracts/Activator_Interface.php | 3 +- src/Contracts/Provider_Interface.php | 8 +- src/Loader.php | 67 ++----- src/Notices/Contracts/Writer_Interface.php | 30 ++- src/Notices/Presenter.php | 34 +--- src/Notices/Renderer.php | 41 ++--- src/Notices/Store.php | 30 +-- src/Notices/Writer.php | 37 +--- src/Plugin/Checker.php | 7 +- src/Plugin/Contracts/Checker_Interface.php | 27 +-- .../Contracts/Deactivator_Interface.php | 9 +- src/Plugin/Deactivator.php | 19 +- src/Plugin/Loads_Plugin_Functions.php | 12 +- src/Provider.php | 45 ++--- src/Registry/Reader.php | 47 ++--- src/Registry/Registrar.php | 9 +- src/Sub_Plugin.php | 172 ++++++------------ src/Traits/Guards_Hook_Prefix.php | 10 +- src/Traits/Guards_Plugin_Capability.php | 26 +-- 30 files changed, 402 insertions(+), 1035 deletions(-) diff --git a/src/Absorber.php b/src/Absorber.php index 4b9fc85..090d146 100644 --- a/src/Absorber.php +++ b/src/Absorber.php @@ -22,14 +22,8 @@ /** * Static facade: registration, and the one call that starts everything. * - * What a host touches, and deliberately little else. How collaborators are built belongs to - * `Provider`, when they run to `Boot\Scheduler`, and the load pass itself to `Loader` — so - * the only reason to open this file is to change what a host may say to the library. - * - * `final` because it cannot usefully be extended: every member is static, the one property and the - * one helper behind the API are private, and every internal call is `self::`, so a subclass would - * inherit the API, be unable to change what any of it does, and change nothing — which is the silent - * no-op this class reports on everywhere else. + * `final` because every member is static and every internal call is `self::` — a subclass could + * override nothing, and would silently change nothing. * * @since 1.0.0 */ @@ -37,8 +31,6 @@ final class Absorber { use Guards_Hook_Prefix; /** - * Whether the hooks have been wired. - * * @var bool */ private static $booted = false; @@ -67,7 +59,7 @@ final class Absorber { * * @since 1.0.0 * - * @throws Config_Exception When no container has been set, or its binding is unusable. + * @throws Config_Exception When the container is unset, or its binding unusable. * * @return Registrar_Interface */ @@ -84,7 +76,7 @@ public static function registrar(): Registrar_Interface { /** * @since 1.0.0 * - * @throws Config_Exception When no container has been set, or its binding is unusable. + * @throws Config_Exception When the container is unset, or its binding unusable. * * @return Writer_Interface */ @@ -95,7 +87,7 @@ public static function notices(): Writer_Interface { /** * @since 1.0.0 * - * @throws Config_Exception When no container has been set, or its binding is unusable. + * @throws Config_Exception When the container is unset, or its binding unusable. * * @return Resolver_Interface */ @@ -106,16 +98,9 @@ public static function resolver(): Resolver_Interface { /** * Register one bundled sub-plugin. Call once per sub-plugin, before boot(). * - * The sub-plugin is buffered rather than handed straight to the registrar, so that registering - * resolves nothing — not even the container. A host that registers before it calls - * Config::set_container() would otherwise fail on a call that has nothing to do with the - * container. The buffer belongs to `Registry\Reader`, which is where it is read back out: this - * class hands its collaborators no work and holds none of their state. - * - * The configuration is still validated here: building the Sub_Plugin is what rejects it, and - * that happens at the call the host can see in its own stack trace. It is built rather than - * resolved because it is a value object — a container asked for one would need the config - * passed through it, and there is nothing about it to rebind. + * Buffered rather than handed to the registrar, so registration resolves nothing — not even the + * container, which a host may set after this call. `Sub_Plugin` still validates the config here, + * in the host's own stack trace. * * @since 1.0.0 * @@ -132,10 +117,8 @@ public static function register( array $config ): void { /** * Every registered sub-plugin, keyed by slug, in registration order. * - * A delegation like the accessors above it, and for the same reason: what a host calls is here, - * what it does is the collaborator's. The passes that read the registry are handed that - * collaborator directly rather than calling back through this method — a facade sits in front of - * its collaborators, never underneath them. + * The passes read the registry through the reader they were handed, never back through here: a + * facade sits in front of its collaborators, never underneath them. * * @since 1.0.0 * @@ -148,20 +131,11 @@ public static function all(): array { } /** - * Bind the collaborators, then let the scheduler decide when they run. Idempotent — safe to - * call from more than one code path. - * - * Idempotent means the first call wins outright, and the container is part of what it wins. - * A Config::set_container() after this has returned binds nothing: the scheduler keeps the - * container it closed over, while the accessors and the notice trampolines resolve from - * whatever Config holds when they are called, so the two halves would answer to different - * containers and the accessors would ask an unbound one. Set the container first — the - * recommended slot is plugins_loaded priority 0 — and do not replace it afterwards. + * Bind the collaborators, then let the scheduler decide when they run. Idempotent. * - * The provider is constructed rather than resolved: it is what teaches the container about this - * library, so the container cannot be asked to build it first. It is bound afterwards, and only - * when nothing answers to `Provider_Interface` already, so a host may replace the whole set of - * bindings with one of its own. + * Set the container before booting: the scheduler closes over the one it finds here. The provider + * is bound only when nothing answers to `Provider_Interface` already, so a host may replace the + * whole set of bindings. * * @since 1.0.0 * @@ -183,9 +157,8 @@ public static function boot(): void { $container->get( Provider_Interface::class )->register(); $container->get( Scheduler::class )->wire(); - // Last, not first. A boot that threw on its way through -- no container, a binding that - // cannot be built -- has wired nothing, and a host that fixes the mistake and calls again - // should get a working library rather than a silent no-op. + // Last, not first: a boot that threw part-way through wired nothing, so calling again after + // the fix must give a working library rather than a no-op. self::$booted = true; } @@ -201,10 +174,8 @@ public static function render_notices(): void { return; } - // The messages a presenter draws were worded by host callables, so rendering runs somebody - // else's code -- on all_admin_notices, which every admin screen fires. A throw out of here - // would white-screen wp-admin, which is exactly where a site owner would go to undo whatever - // caused it. The notice is worth less than the screen it would be read on. + // Notice wording comes from host callables, on a hook every admin screen fires: the notice is + // worth less than the wp-admin a throw would white-screen. try { self::collaborator( Presenter::class )->render(); } catch ( Throwable $thrown ) { @@ -219,9 +190,9 @@ public static function render_notices(): void { /** * Rewrite the activation-error notice for a standalone this library has absorbed. * - * The parameter is untyped because a filter receives whatever the filter before it returned, - * and a `string` declaration would turn another plugin's sloppy return into a TypeError raised - * from here. + * The parameter is untyped deliberately: a filter receives whatever the filter before it + * returned, and a `string` declaration would raise a TypeError from the screen least able to + * afford one. * * @since 1.0.0 * @@ -236,10 +207,8 @@ public static function filter_activation_error_markup( $markup ): string { return $markup; } - // Guarded like render_notices, and for a sharper version of the same reason: this runs while - // WordPress is drawing the screen that reports a fatal, so a throw out of here would replace - // the error the admin came to read with one of ours. The markup goes back as it arrived and - // core's wording stands. + // This draws the screen that reports a fatal: a throw would replace the error the admin came + // to read with one of ours. try { return self::collaborator( Rewriter::class )->rewrite( $markup ); } catch ( Throwable $thrown ) { @@ -256,12 +225,9 @@ public static function filter_activation_error_markup( $markup ): string { /** * The object bound to a collaborator interface, checked before it is handed on. * - * The container's own return type promises nothing, so a host that bound the wrong class -- a - * typo'd class name, an interface it forgot to implement -- would otherwise surface as a - * TypeError raised inside this library, naming this library's method. That reads as a bug here - * rather than a mistake in the host's own bindings, and it happens inside `plugins_loaded`, - * where nobody is looking. Naming the interface and the class that failed it turns the same - * failure into an instruction. + * A host that bound the wrong class would otherwise surface as a TypeError inside + * `plugins_loaded` reading as a bug here; naming the interface and the class that failed it + * makes it an instruction instead. * * @since 1.0.0 * @@ -269,24 +235,19 @@ public static function filter_activation_error_markup( $markup ): string { * * @param class-string $interface Collaborator interface to resolve. * - * @throws Config_Exception When no container has been set, when it throws while building the - * binding, or when the binding does not implement the interface it was - * bound to. + * @throws Config_Exception When no container has been set, when building the binding throws, or + * when the binding does not implement the interface. * * @return T */ private static function collaborator( string $interface ): object { - // Resolved outside the try: a missing container is this library's own configuration error - // already, reported in its own words, and re-wrapping it would bury that sentence one - // exception deeper for no gain. + // Outside the try: a missing container is already this library's own error, in its own words, + // and re-wrapping would bury that sentence an exception deeper. $container = Config::get_container(); - // A host factory closure is free to throw, and a container asked for a binding with an - // unsatisfiable dependency -- or for an interface nothing has bound yet, which is every - // interface before boot() runs the provider -- throws its own exception type. Uncaught, - // either one leaves the host's plugins_loaded with a fatal from a vendor namespace that - // names neither this library nor the binding at fault, so both are reported the same way as - // a binding of the wrong type. The original failure is kept as the previous exception. + // A host factory may throw, and so does the container asked for an interface nothing has + // bound yet. Uncaught, either fatals from a vendor namespace naming neither this library + // nor the binding at fault. try { $collaborator = $container->get( $interface ); } catch ( Throwable $thrown ) { diff --git a/src/Activator.php b/src/Activator.php index d78943a..9d341e6 100644 --- a/src/Activator.php +++ b/src/Activator.php @@ -39,9 +39,8 @@ public function maybe_run( Sub_Plugin $sub_plugin ): void { return; } - // Recorded after the callback rather than before it. A callback that fatals halfway leaves - // the site half-migrated either way, but recording first would also mean the next request - // skips it, so the half-finished state becomes permanent and invisible. + // Recorded after the callback, never before: recording first would freeze a callback that + // throws halfway, since the next request would skip it rather than retry. $callback( $sub_plugin ); $done[ $slug ] = true; @@ -52,14 +51,9 @@ public function maybe_run( Sub_Plugin $sub_plugin ): void { /** * Slugs whose activation callback has already run, keyed by slug. * - * A site option, not a plain one: `deactivate_plugins()` is network-wide on multisite, so an - * activation that follows a network-wide merge has to be recorded network-wide too, or every - * site in the network runs the callback again. - * - * The other end of that is the accepted cost: a site created on the network afterwards finds the - * record already set and never runs the callback at all. One run for the network is what this - * default promises, and per-site work belongs on a host's own new-site hook, or in an - * `Activator_Interface` of its own that records per site. + * A site option, not a plain one: `deactivate_plugins()` is network-wide on multisite, so the + * record is too. The cost is that a site created afterwards finds it set and never runs the + * callback; per-site work belongs in an `Activator_Interface` of the host's own. * * @since 1.0.0 * @@ -70,19 +64,15 @@ public function maybe_run( Sub_Plugin $sub_plugin ): void { private function completed(): array { $done = get_site_option( $this->option_name(), [] ); - // Anything that is not an array is replaced rather than trusted. A corrupted option would - // otherwise fatal on the array read, inside plugins_loaded, on every request. + // A corrupted option would otherwise fatal on the array read, on every request. return is_array( $done ) ? $done : []; } /** * The option every slug's activation record lives in. * - * Private, and not static, unlike `Notices\Store::option_name()`: that one is public because - * `docs/notices.md` tells a host to read the queue option itself, and this record has no such - * reader. Nothing outside this class composes the name, so nothing outside it needs to be told - * the name, and a host that wants the bookkeeping elsewhere binds `Activator_Interface` rather - * than reading around this one. + * Private, unlike `Notices\Store::option_name()`: nothing outside reads it, and a host wanting + * the bookkeeping elsewhere binds `Activator_Interface` instead. * * @since 1.0.0 * diff --git a/src/Boot/Scheduler.php b/src/Boot/Scheduler.php index 310fc59..4fde07f 100644 --- a/src/Boot/Scheduler.php +++ b/src/Boot/Scheduler.php @@ -17,13 +17,9 @@ use WP_Hook; /** - * When this library's work happens: which hook, at which priority, and what to do when it is - * already too late to say so. + * Which hook this library's work runs on, at which priority, and what to do when it is too late. * - * Its own class because timing is the part of booting that changes for its own reasons — a step - * added, a priority moved, a hook that fires too late to wire — and none of those are reasons to - * touch registration or the load pass. The provider says how collaborators are built and this says - * when they run, so neither has to be read to change the other. + * Its own class because the provider says how collaborators are built and this says when they run. * * @since 1.0.0 */ @@ -31,16 +27,8 @@ class Scheduler { /** * plugins_loaded priority the load pass runs at. * - * Ahead of the default priority, so a bundled plugin is in memory before the plugins that - * expect it start their own work. - * - * Every priority below this one is a band of the bundled plugin's *own* plugins_loaded - * callbacks that silently never fire: a standalone copy is included by wp-settings.php before - * the action is dispatched at all and keeps every callback it registers, while a bundled copy - * required from a callback here only keeps the ones above the priority that required it. - * Hooking plugins_loaded below the default is already a special case, so the band this gives - * up is a narrow one — but it is given up silently, which is why the number moves down when - * there is any doubt and not up. + * Every priority below this one is a band of the bundled plugin's *own* plugins_loaded callbacks + * that silently never fire, which is why this number moves down when in doubt and not up. * * @since 1.0.0 * @@ -49,18 +37,11 @@ class Scheduler { private const LOAD_PRIORITY = 6; /** - * plugins_loaded priority conflict resolution runs at, ahead of the load pass. - * - * A standalone that survives the conflict defines the guard constant as it loads, and the load - * pass has to see that, so resolution cannot share a priority with it. + * plugins_loaded priority conflict resolution runs at, one ahead of the load pass. * - * This is the number a host is measured against, not the load: it is the first step in the - * sequence, so it is the priority `set_container()` and `boot()` have to beat. At 1 the only - * slot left was 0, which made a documented convention a hard requirement — and both LearnDash - * and MemberDash wire Harbor's `set_container()` at priority 1, so a host copying the habit it - * already has landed exactly on the barrier and got the inline fallback instead of the hooks. - * Five slots ahead of it covers both habits with room over, and costs the load pass four - * priorities it was not using. + * A surviving standalone defines the guard constant as it loads and the load pass has to see + * that. Also the number a host is measured against, being the first step in the sequence — 5 + * rather than 1 because hosts already wire Harbor's `set_container()` at priority 1. * * @since 1.0.0 * @@ -78,7 +59,7 @@ class Scheduler { /** * @since 1.0.0 * - * @param ContainerInterface $container Container every step resolves from, when it runs. + * @param ContainerInterface $container Container every step resolves from when it runs. */ public function __construct( ContainerInterface $container ) { $this->container = $container; @@ -87,13 +68,9 @@ public function __construct( ContainerInterface $container ) { /** * Wire the WordPress hooks. * - * Nothing is resolved here. Each step is a closure over the container that asks for its - * collaborator when the hook fires, so a host may still rebind one after boot() and up until - * plugins_loaded, and a binding nothing reaches is never built at all. - * - * Called too late, the steps run inline instead of being wired — and conflict resolution can - * end the request, so on an admin page load this call may not return. Boot before plugins_loaded - * priority 5, as documented, and it always does. + * Nothing is resolved here: every callback asks the container for its collaborator when the hook + * fires, so a host may still rebind one after boot() and a binding nothing reaches is never + * built. Called too late, the steps run inline instead — and may then not return. * * @since 1.0.0 * @@ -101,25 +78,16 @@ public function __construct( ContainerInterface $container ) { */ public function wire(): void { if ( is_admin() ) { - // all_admin_notices, not admin_notices. WordPress dispatches admin_notices, - // network_admin_notices and user_admin_notices as mutually exclusive branches, so a - // superadmin working in the network admin -- exactly where a network-wide - // deactivation gets noticed -- would never see the queue rendered. + // all_admin_notices, not admin_notices: the latter's three branches are mutually exclusive, + // so a superadmin in the network admin would never see the queue rendered. add_action( 'all_admin_notices', [ Absorber::class, 'render_notices' ] ); - // A named static trampoline like the notice step above, not a closure. Both resolve - // their collaborator when they fire, so neither builds anything at boot, but a named - // callback can also be taken back with remove_filter() -- which matters most here, on - // the one hook that rewrites a screen WordPress drew rather than adding one of our own. - // The closures below are shaped that way for a reason these two do not share: the - // plugins_loaded sequence has to be runnable inline as well as wirable. + // Named rather than a closure: remove_filter() is a host's only way back to core's wording. add_filter( 'wp_admin_notice_markup', [ Absorber::class, 'filter_activation_error_markup' ] ); } - // Adding an action at a priority the current dispatch has already passed is accepted and - // then never fires. Booting from plugins_loaded at the default priority -- the commonest - // hook mistake there is -- would otherwise mean nothing loads at all, with no warning and - // a site that looks entirely healthy. + // A callback added at a priority the dispatch has already passed is accepted and never fires, + // so a host that boots too late would otherwise get a healthy-looking site that loads nothing. if ( $this->wiring_window_has_closed() ) { _doing_it_wrong( Absorber::class . '::boot', @@ -185,13 +153,8 @@ public static function registration_window_has_closed(): bool { /** * The plugins_loaded steps, in run order, as priority and callback. * - * Stated once because wire() expresses this order twice — as hook priorities when it can still - * wire, and as straight calls when it is too late to and has to run them inline. Those two are - * the same sequence, and a comment is the only thing that could hold them in agreement. - * Iterating one list cannot drift. - * - * A method rather than a constant because a step is a closure now: naming the step and giving - * its priority in two separate lists would put the drift straight back, one list deep. + * Stated once because wire() expresses this order twice — as hook priorities, and as straight + * calls when it is too late to wire — and iterating one list cannot drift. * * @since 1.0.0 * @@ -219,9 +182,6 @@ private function sequence(): array { /** * The conflict step: the two gates, the probe between them, and the resolve behind all three. * - * Static, and handed the container rather than reading one, so the closure in sequence() stays a - * closure over the container like the load step beside it. - * * @since 1.0.0 * * @param ContainerInterface $container Container each collaborator is resolved from. @@ -232,20 +192,15 @@ private static function resolve_conflicts( ContainerInterface $container ): void try { $gatekeeper = $container->get( Gatekeeper::class ); - // The shape of the request first. It reads the request and nothing else, so cron, WP-CLI, a - // POST and every front-end view are turned away having resolved no user and built no - // resolver. + // Request shape first: cron, WP-CLI, POSTs and front-end views turn back with no user + // resolved and no resolver built. if ( ! $gatekeeper->request_may_resolve() ) { return; } - // Then whether there is a conflict at all, before anyone asks who is signed in. - // current_user_can() resolves and caches the current user, and this step runs at - // plugins_loaded priority 5 -- ahead of the plugins that add their determine_current_user - // filter from a plugins_loaded callback of their own. Ask on every admin GET and an SSO or - // JWT visitor is pinned as logged out for the rest of the request, on requests with nothing - // to resolve. The detector reports and changes nothing, so the capability is only asked for - // where its answer decides something. + // Then whether there is a conflict at all, before anyone asks who is signed in: + // current_user_can() caches the current user, and at priority 5 that would settle it ahead + // of an SSO plugin's own plugins_loaded filter, on requests with nothing to resolve. if ( ! $container->get( Detector::class )->has_conflict() ) { return; } @@ -254,16 +209,13 @@ private static function resolve_conflicts( ContainerInterface $container ): void return; } - // Both gates and the probe live here rather than inside the resolver, so a host binding - // its own cannot drop one by omission -- and asking them first means a resolver is built - // only on the request that goes on to use it. + // Both gates and the probe live here, not in the resolver, so a host binding its own cannot + // drop one by omission. $container->get( Resolver_Interface::class )->resolve_all(); } catch ( Throwable $thrown ) { - // The backstop, and the promise the whole library rests on: plugins_loaded fires on every - // request a site serves, so a throw out of a step is a white screen on all of them. What - // reaches here is a collaborator a host's factory could not build, a gate, the probe, or a - // resolver a host bound itself -- the passes guard their own per-sub-plugin loops, and this - // guards everything the step touches on the way to them. + // Nothing reached from a hook may throw: plugins_loaded fires on every request a site + // serves, so a throw out of a step is a white screen on all of them. Report it and abandon + // this step alone -- the passes guard their own per-sub-plugin loops. self::report_a_step_that_threw( 'conflict pass', 'no conflict was resolved', $thrown ); } } @@ -278,10 +230,8 @@ private static function resolve_conflicts( ContainerInterface $container ): void * @return void */ private static function load( ContainerInterface $container ): void { - // Guarded like the conflict step, and for the same reason. `Loader::load_all()` already reports - // per sub-plugin and carries on, so what is left for this to catch is the pass itself -- a - // container that cannot build it above all, which is the shape a host's own broken binding - // takes. + // Guarded like the conflict step; `Loader::load_all()` already reports per sub-plugin, so what + // is left to catch is a container that cannot build the pass at all. try { $container->get( Loader::class )->load_all(); } catch ( Throwable $thrown ) { @@ -311,15 +261,9 @@ private static function report_a_step_that_threw( string $step, string $conseque /** * Whether it is already too late to wire the first step of the sequence. * - * Measured against the earliest priority in sequence(), read rather than restated, because a - * boot that can still wire a later step but has missed an earlier one has missed something — - * and with resolution at 5 and the load at 6, booting between the two is a real window. - * - * The comparison is inclusive. A callback added to the priority currently being dispatched is - * accepted and never reached either: WP_Hook::apply_filters() walks `$this->callbacks[$priority]` - * with a by-value foreach, so the append lands on an array the running loop has already copied. - * Booting from plugins_loaded at that priority is the case a host is likeliest to hit by - * accident, and an exclusive comparison would let exactly that one through unreported. + * Measured against the earliest priority in sequence(), read rather than restated: with resolution + * at 5 and the load at 6, booting between the two would otherwise wire the load and silently lose + * the conflict pass. Inclusive, because a callback added at the priority dispatching never runs. * * @since 1.0.0 * diff --git a/src/Config.php b/src/Config.php index b18edbe..46a39e2 100644 --- a/src/Config.php +++ b/src/Config.php @@ -34,8 +34,7 @@ class Config { /** * Set the unique per-host slug that keys this library's hooks and options. * - * It is stored exactly as given: hook names repeat it verbatim, and only `get_option_name()` - * folds it. + * Stored exactly as given: hook names repeat it verbatim, and only `get_option_name()` folds it. * * @since 1.0.0 * @@ -79,9 +78,7 @@ public static function get_hook_prefix(): string { /** * Build the name of one of this library's filters. * - * The prefix lives here, so everything derived from it is built here too. A collaborator that - * assembled its own name would be repeating the segment between the host's prefix and the - * hook's own name, and would have to be found and corrected if it ever changed. + * Nothing else assembles the segment between the host's prefix and the key's own name. * * @since 1.0.0 * @@ -98,17 +95,10 @@ public static function get_hook_name( string $name ): string { /** * Build the name of one of this library's options. * - * The prefix is a hook-naming value: `set_hook_prefix()` takes anything WordPress will accept - * inside a filter name, mixed case and hyphens included, and `get_hook_name()` repeats it byte - * for byte — a host that passed `Give-Core` must be able to hook - * `Give-Core/plugin_absorber/should_load` and have it fire. A storage key answers to a - * narrower convention, so the folding happens here and nowhere else: the same prefix produces + * Deliberately does not share `get_hook_name()`'s normalisation, and must not be collapsed into + * it: a host that passed `Give-Core` hooks that verbatim, while the option folds to * `give_core_plugin_absorber_notices`. * - * The `_plugin_absorber_` segment lives here for the reason `get_hook_name()` gives for its - * own: the notice queue is not the only option keyed this way, and a segment each caller - * assembled would have to be found in every one of them if it ever changed. - * * @since 1.0.0 * * @param string $name Option name, less the prefix and this library's namespace. @@ -124,10 +114,8 @@ public static function get_option_name( string $name ): string { /** * Share the host's container. Required, and required before boot(). * - * Every collaborator this library uses is resolved from it, which is what makes each of them - * replaceable by binding an interface. There is no second, container-less path to keep working - * beside that one: two ways to reach a collaborator means two sets of behaviour to reason - * about, and the one nobody runs is the one that rots. + * Every collaborator resolves from it, which is what makes each replaceable by binding an + * interface. There is deliberately no container-less path. * * @since 1.0.0 * @@ -166,15 +154,11 @@ public static function has_container(): bool { } /** - * Tell the library the host plugin's own basename, so a standalone's activation scope can be - * compared against the host's on multisite. + * Tell the library the host plugin's own basename, for the multisite stranding guard. * - * Optional, and read by one thing: the conflict resolver's guard against deactivating a - * network-active standalone whose bundled replacement ships in a host that is not itself - * network-active -- a deactivation that would strand the sites the host never reached. Left - * unset, that guard stands down and deactivation behaves exactly as it always has. Stored - * exactly as given: a basename is a path, `directory/file.php`, not a hook-naming value, so the - * hook-prefix validator's character rules deliberately do not apply here. + * Optional, and read by one thing: the guard that declines to deactivate a network-active + * standalone whose host is not. Left unset, it stands down. Stored exactly as given -- a + * basename is a path, not a hook-naming value, so the prefix rules do not apply. * * @since 1.0.0 * @@ -189,9 +173,8 @@ public static function set_host_plugin_basename( string $basename ): void { /** * The host plugin basename, or an empty string when none was set. * - * Does not throw the way `get_hook_prefix()` and `get_container()` do. Those name a step the host - * must take before boot; this is optional, and an empty string is the honest answer for a host - * that did not opt into the multisite guard -- the same value that stands the guard down. + * Does not throw the way `get_hook_prefix()` and `get_container()` do: an empty string is the + * honest answer for a host that did not opt in. * * @since 1.0.0 * @@ -204,10 +187,9 @@ public static function get_host_plugin_basename(): string { /** * The hook prefix folded into the shape a storage key takes. * - * Only option names are folded, and only on the way out — the stored prefix keeps whatever the - * host passed, because that is what its hook names are made of. Two prefixes differing only in - * case or in hyphens against underscores would land on the same option, which is the price of - * asking a host for one prefix rather than two; no host runs both `Give-Core` and `give_core`. + * Folded on the way out only — the stored prefix keeps whatever the host's hook names are made + * of. Two prefixes differing only in case or hyphens collide here, the price of asking a host + * for one prefix rather than two. * * @since 1.0.0 * diff --git a/src/Conflict/Contracts/Resolver_Interface.php b/src/Conflict/Contracts/Resolver_Interface.php index a1d130a..4ead43e 100644 --- a/src/Conflict/Contracts/Resolver_Interface.php +++ b/src/Conflict/Contracts/Resolver_Interface.php @@ -12,14 +12,9 @@ /** * Decides what happens when a bundled sub-plugin's standalone counterpart is still active. * - * Bind a replacement to change what happens about a conflict. One method, because the policy branch - * is all this contract promises: finding the conflict belongs to `Conflict\Detector` and deciding - * who may have one resolved to `Conflict\Gatekeeper`. - * - * Both of those are the caller's to ask, and the conflict step asks them before it builds an - * implementation of this — so a replacement cannot drop a guard by omission, is never asked to - * resolve on a request that fails one, and is not built at all on a request with nothing to - * resolve. + * Bind a replacement to change what happens about a conflict. One method, because finding the + * conflict belongs to `Conflict\Detector` and deciding who may have one resolved to + * `Conflict\Gatekeeper` -- both asked before an implementation of this is built. * * @since 1.0.0 */ diff --git a/src/Conflict/Detector.php b/src/Conflict/Detector.php index 7a74840..d69b2b6 100644 --- a/src/Conflict/Detector.php +++ b/src/Conflict/Detector.php @@ -17,19 +17,9 @@ /** * Whether a bundled sub-plugin's standalone counterpart is still active. * - * Its own class because detection is asked for on its own: the conflict step needs something cheap - * to ask between the request gate and the capability gate, so that `current_user_can()` — which - * pins the current user for the rest of the request — is never reached on a request with nothing to - * resolve. Answering that from `Resolver_Interface` would put a detection query on the contract of - * every host that binds its own resolver, and give the resolver two reasons to change: how a - * conflict is found, and what to do about one. - * - * Nothing here leaves a mark on the request. Nothing resolves a user, - * deactivates a plugin or queues a notice — an answer is all a caller gets, and the acting is the - * resolver's. The one thing either of them writes is a report to the developer about a bootstrap - * that cannot be right, which changes nothing on the site and is addressed to nobody on it. - * - * Not `final`: it is bound by class name, which is the seam a host rebinds and a test subclasses. + * Its own class so the conflict step has something cheap to ask before `current_user_can()`, which + * pins the current user for the rest of the request. Everything here only reports. Not `final`: it + * is bound by class name, the seam a host rebinds and a test subclasses. * * @since 1.0.0 */ @@ -73,8 +63,8 @@ public function __construct( Reader $registry, Checker_Interface $plugin_checker /** * Whether any registered sub-plugin's standalone counterpart is currently active. * - * Short-circuits on the first one found: the caller only needs to know whether the rest of the - * conflict step is worth entering, and the resolver walks the whole registry again anyway. + * Short-circuits: the caller only needs to know whether the rest of the conflict step is worth + * entering. * * @since 1.0.0 * @@ -83,9 +73,7 @@ public function __construct( Reader $registry, Checker_Interface $plugin_checker * @return bool */ public function has_conflict(): bool { - // The reader rather than a registrar of our own: it drains the registrations still buffered - // on the facade before it reads, and a registrar asked directly would miss anything - // registered since the last read. + // The reader rather than a registrar of our own: it drains the buffered registrations first. foreach ( $this->registry->all() as $sub_plugin ) { if ( $this->is_in_conflict( $sub_plugin ) ) { return true; @@ -98,8 +86,8 @@ public function has_conflict(): bool { /** * Whether this sub-plugin's standalone counterpart is active and ours to act on. * - * Policy is not consulted: a sub-plugin set to defer is still in conflict, and the resolver is - * where the decision to leave it alone belongs. + * Policy is not consulted: a sub-plugin set to defer is still in conflict, and leaving it alone + * is the resolver's decision. * * The `should_load` filter is, and it is the one piece of host code this class reads. It decides * whether the bundled copy will be in memory at all, one priority behind this — so a sub-plugin @@ -143,16 +131,9 @@ public function is_in_conflict( Sub_Plugin $sub_plugin ): bool { /** * Whether deactivating this standalone would strand sites the bundled copy will never reach. * - * `deactivate_plugins()` takes a network-active standalone out of *every* site's plugins, but the - * bundled copy only loads where the host plugin runs. So on the one topology of a network-active - * standalone whose host is not itself network-active, a network-wide deactivation removes it from - * the sites the host never loads on, where nothing stands in for it. The resolver reads this and - * declines, leaving the load guard to defer the bundled copy network-wide instead. - * - * Opt-in, and cheap in the common case: with no host basename configured the guard stands down on - * a single string compare, before any option is read. It needs no `is_multisite()` test either -- - * `Checker_Interface::is_network_active()` is `false` off a network, so the whole predicate is - * `false` on a single site. + * `deactivate_plugins()` pulls a network-active standalone from *every* site, while the bundled + * copy loads only where the host does -- so a network-active standalone under a host that is not + * network-active leaves those sites with nothing. False off a network: `is_network_active()` is. * * The basename itself is checked against the installed plugins on the way past, because a name no * plugin answers to answers "not network-active" for ever, and that is indistinguishable from the diff --git a/src/Conflict/Gatekeeper.php b/src/Conflict/Gatekeeper.php index a57ca7b..27804c6 100644 --- a/src/Conflict/Gatekeeper.php +++ b/src/Conflict/Gatekeeper.php @@ -13,20 +13,9 @@ /** * Whether this request may have a conflict resolved at all. * - * Separate from the resolver, and resolved ahead of it, because the two answer different questions: - * a resolver decides what a conflict means and what to do about it, while this decides who is - * allowed to have one resolved. That split is what makes the guarantee survive rebinding — the - * conflict step asks the gatekeeper first and only then builds a `Resolver_Interface`, so a host - * that binds its own resolver cannot drop a gate by omission, and equally is never asked to resolve - * on a request that fails one. - * - * The two gates are asked at different moments, which is why they are two methods rather than one. - * The request gate is cheap and reads nothing but the request, so it runs first and keeps a resolver - * from being built at all. The user gate is asked last, once a conflict is known to exist, because - * `current_user_can()` resolves the current user and caches it: called from plugins_loaded priority - * 5 it would settle who is signed in before an SSO or JWT plugin that hooks `determine_current_user` - * from its own plugins_loaded callback has been given the chance to, and that plugin's users would - * then be treated as logged out for the rest of the request. A site with no conflict never pays it. + * Separate from the resolver, and asked ahead of it, so a host binding its own `Resolver_Interface` + * cannot drop a gate by omission. Two methods because the cheap request gate runs first, while the + * user gate waits: `current_user_can()` pins the current user ahead of an SSO or JWT plugin. * * @since 1.0.0 */ @@ -37,8 +26,8 @@ class Gatekeeper { /** * Admin scripts that exist only to perform work. * - * Every one of them does its job and then redirects or prints a result, so there is no page here - * to resolve a conflict on — only work to interrupt. + * Each redirects or prints a result, so there is no page here to resolve on — only work to + * interrupt. * * @since 1.0.0 * @@ -54,8 +43,8 @@ class Gatekeeper { /** * Query args that name an action for the current screen to perform. * - * Both, because core's list tables put a bulk selector above the table and a second one below it, - * and the lower one submits as `action2` with `action` left at its empty value. + * Both, because core's list tables carry a second bulk selector that submits as `action2`, + * leaving `action` at its empty value. * * @since 1.0.0 * @@ -67,7 +56,7 @@ class Gatekeeper { * The value core gives an action arg when no bulk action is selected. * * A list table submits its selector whether or not anything was chosen, so this arrives on - * ordinary paging and search requests and means nothing is being asked for. + * ordinary paging and search requests. * * @since 1.0.0 * @@ -78,14 +67,9 @@ class Gatekeeper { /** * Whether this request is the kind conflict resolution may run on. * - * Reads nothing about who is making it: the capability lives in user_may_resolve(), so that - * asking this question does not decide the current user before the plugins that have an opinion - * about it have loaded. - * - * The hook prefix is checked last. It is the only one of these that reports to the developer, and - * resolution runs from plugins_loaded, so checking it first would put a _doing_it_wrong() in the - * log of every front-end request rather than of the admin request that was about to resolve - * something. + * The hook prefix is checked last because it is the only one of these that reports to the + * developer: checked first, it would log a _doing_it_wrong() from every front-end request rather + * than from the admin request that was about to resolve something. * * @since 1.0.0 * @@ -100,26 +84,9 @@ public function request_may_resolve(): bool { /** * Whether the current user may have a conflict resolved on their request. * - * Reaching conflict resolution does not mean anyone is signed in. wp-admin/admin.php loads - * wp-load.php -- which dispatches plugins_loaded -- well before it calls auth_redirect(), so an - * unauthenticated GET of any admin URL gets this far. Without this check a stranger could turn - * the standalone off site-wide by requesting a page they are about to be bounced off. - * - * Which capability that is belongs to Traits\Guards_Plugin_Capability, because the notice - * presenter has to ask the identical question: rendering the queue clears it for everybody, so a - * user who may not have a conflict resolved may not consume the report of one either. Spelling - * the capability here as well is what let the two answers drift apart. - * - * Here rather than inside the default resolver, because it is the one thing about conflict - * resolution that must survive a host binding its own: whoever cannot activate a plugin must not - * be able to deactivate one, and a replacement that forgot to re-check would reopen exactly that. - * - * It gates every policy, not only the destructive one, and that costs nothing. The other - * policies queue a notice, and Notices\Presenter::render() will not render -- or clear -- for a - * user this same guard turns away. Queuing on a request that cannot act only parks the notice - * until an administrator who can act arrives, which is the request this gate lets resolution run - * on anyway. Nothing is consumed or suppressed by waiting: the standalone is still there to - * detect. + * plugins_loaded dispatches well before auth_redirect(), so an unauthenticated GET of any admin + * URL gets this far. It gates every policy, not only the destructive one, and that costs nothing: + * Notices\Presenter::render() refuses to render or clear for a user this same guard turns away. * * @since 1.0.0 * @@ -132,13 +99,9 @@ public function user_may_resolve(): bool { /** * Whether this request is one a person is watching in wp-admin. * - * Conflict resolution deactivates a plugin and ends the request, so it must only run where - * someone is there to see the result. Unguarded it fires at plugins_loaded on every request: - * a visitor's checkout POST becomes a 302 that drops the order, a login POST bounces back to - * a blank form, wp-cron never reaches its event loop, and a WP-CLI command exits 0 having - * printed nothing, because header() is a no-op under the CLI SAPI. - * - * is_admin() alone is not enough: admin-ajax.php and admin-post.php both define WP_ADMIN. + * Resolution deactivates a plugin and ends the request, so unguarded it turns a visitor's + * checkout POST into a 302 that drops the order. is_admin() alone is not enough: admin-ajax.php + * and admin-post.php both define WP_ADMIN. * * @since 1.0.0 * @@ -153,11 +116,9 @@ private function is_interactive_admin_request(): bool { return false; } - // Only a GET. A redirect discards the request, and the browser follows it with a GET, so - // anything submitted is gone -- which is exactly what would happen to a form posted to - // admin-post.php or options.php, both of which define WP_ADMIN and neither of which - // wp_doing_ajax() catches. Core draws the same line in wp_cron(). Deferring resolution to - // the next page view costs nothing: the standalone is still there to detect. + // Only a GET: a redirect discards whatever was submitted -- as it would for a form posted to + // options.php, which defines WP_ADMIN and which wp_doing_ajax() does not catch. Deferring to + // the next page view costs nothing, since the standalone is still there to detect. if ( ( $_SERVER['REQUEST_METHOD'] ?? 'GET' ) !== 'GET' ) { return false; } @@ -168,20 +129,9 @@ private function is_interactive_admin_request(): bool { /** * Whether this GET is asking wp-admin to do something rather than to draw something. * - * A GET is not automatically safe to discard. `update.php?action=upgrade-plugin`, - * `plugins.php?action=activate` and an `admin-post.php` link are all admin GETs that perform - * work, and resolving on one deactivates, redirects and exits before core reaches the work: the - * user clicks "Update Now" and lands on a list screen with nothing updated. That is the same - * silent discard of a submitted action the POST branch exists to prevent, one verb over. - * - * `plugins.php?action=activate` matters twice, because it is the request - * plugin_sandbox_scrape() replays while activating a plugin -- an exit there aborts the - * activation itself and core reports the plugin as fatal. - * - * The test is deliberately blunt: any action arg at all, not a list of the dangerous ones. Half - * the screens in wp-admin take an `action`, plugins define their own, and a list of known-safe - * values would have to be right about every one of them forever. Refusing them all costs a - * deferral to the next plain page view, and the standalone is still there to be detected then. + * A GET is not automatically safe to discard: resolving on `plugins.php?action=activate` exits + * inside the request plugin_sandbox_scrape() replays, and core reports the plugin as fatal. Blunt + * on purpose -- any action arg at all, since a known-safe list would have to stay right forever. * * @since 1.0.0 * @@ -195,27 +145,17 @@ private function is_action_request(): bool { foreach ( self::ACTION_ARGS as $arg ) { $raw = $_GET[ $arg ] ?? null; - // Anything that is not a string names no action core could dispatch on: an array never - // matches one of core's action names, and a missing arg asks for nothing at all. There is - // no work on either to interrupt. + // Anything but a string names no action core could dispatch on. if ( ! is_string( $raw ) ) { continue; } - // Compared as it arrived rather than through sanitize_key(): admin.php dispatches - // admin_action_{$action} on the raw value, so an action named outside a-z0-9_- -- a - // non-Latin script, a bare '+' -- is work a plugin can be hooked to, and sanitizing - // first would empty it and admit the very request this gate exists to refuse. - // - // Not through wp_unslash() either, for the reason Conflict\Resolver reads the request URI - // raw: core adds its slashes in wp_magic_quotes(), which wp-settings.php calls *after* - // do_action( 'plugins_loaded' ), so there are none on this value yet and unslashing is a - // plain stripslashes() over the query string. '\' would strip to '' and '-\1' to '-1' -- - // both of them values this gate reads as asking for nothing -- and the request would be - // admitted and resolved out from under work core is still going to dispatch. Should this - // ever be reached after the slashing, from the inline fallback a too-late boot reports, - // a slashed value only ever reads as more of an action than it is, which is the - // direction a gate may safely be wrong in. + // Compared as it arrived: admin.php dispatches admin_action_{$action} on the raw value, so + // sanitize_key() would empty an action named outside a-z0-9_- and admit that request. Not + // unslashed either, for the reason Conflict\Resolver reads the request URI raw -- core + // slashes in wp_magic_quotes(), after plugins_loaded, so stripslashes() would only damage + // the value, turning '\' into '' and '-\1' into '-1', both of which read here as no action + // at all. if ( $raw !== '' && $raw !== self::NO_ACTION ) { return true; } @@ -227,15 +167,9 @@ private function is_action_request(): bool { /** * The admin script this request is running, as a bare file name. * - * $pagenow is what core's own admin code branches on, and wp-settings.php requires the file that - * sets it before it loads a single plugin, so it is already populated when plugins_loaded - * dispatches. It is also the value that has been reduced to a bare file name for all three - * admins -- site, network and user -- where matching on a path would have to know about each of - * those prefixes. - * - * The fallback is not superstition: core derives $pagenow from PHP_SELF, which some SAPI and - * proxy configurations leave empty, and a host is free to have unset the global. SCRIPT_NAME is - * consulted first there because it is the one every FastCGI SAPI fills in. + * $pagenow is set before any plugin loads and is already a bare name in all three admins, where + * matching on a path would have to know each prefix. The fallback is not superstition: core + * derives $pagenow from PHP_SELF, which some SAPI and proxy configurations leave empty. * * @since 1.0.0 * diff --git a/src/Conflict/Redirector.php b/src/Conflict/Redirector.php index 1e98165..14e964c 100644 --- a/src/Conflict/Redirector.php +++ b/src/Conflict/Redirector.php @@ -10,18 +10,9 @@ /** * Where the user lands after a standalone has been deactivated. * - * The point of the redirect is to re-render whatever the user asked for, now that the standalone is - * gone -- so the destination is derived from the *current* request, not from the referrer. The - * referrer is the page before this one: an admin who clicks through from a public post, or who - * opens an admin screen from a bookmark, carries a referrer that names something other than what - * they are looking at, and following it sends them somewhere they did not ask to go. - * - * The request URI is never trusted as a URL. Only the screen and the query survive it, and the - * destination is assembled inside the admin URL space from those two parts. - * - * Decides where, and never goes there: `wp_safe_redirect()` and the `exit` after it stay in - * Resolver. That is what lets every destination be asserted directly, without a test having to - * stand in for the end of a request. + * The destination comes from the *current* request, not the referrer, which for an admin arriving + * from a bookmark names somewhere they never asked to go. The URI is never trusted as a URL -- only + * the screen and the query survive it. `wp_safe_redirect()` and the `exit` stay in Resolver. * * @since 1.0.0 */ @@ -29,20 +20,9 @@ class Redirector { /** * Where to send the user after deactivating. * - * Re-requesting the screen the user is already on is the point rather than a waste: the - * standalone's code is in memory for this request and only a fresh one sheds it. That includes - * the plugins list, which is why there is no "stay put" answer. It cannot loop, either -- - * the next request finds no active standalone, so nothing resolves and nothing redirects. - * - * The update screens are the exception, because reloading either of them re-runs an update. - * - * The parameter is documented `string` and declared as nothing, and the two say different things - * on purpose. `string` is the contract: Resolver reads $_SERVER['REQUEST_URI'] and makes a string - * of whatever it finds there, so a caller that hands over anything else has a bug static analysis - * should name. Declaring it would turn that same bug into a TypeError raised under strict_types - * from inside plugins_loaded -- on the one path whose job is to get an admin back to a working - * screen -- for a value the SAPI supplies and any plugin may have filtered on the way. So the - * type is a promise to callers, and the guard below is what happens when the promise is broken. + * Re-requesting the screen the user is already on is the point, not a waste: only a fresh request + * sheds the standalone's code from memory, and it cannot loop. The parameter is documented + * `string` and declared as nothing, so a filtered $_SERVER value cannot TypeError from here. * * @since 1.0.0 * @@ -51,8 +31,7 @@ class Redirector { * @return string Absolute admin URL to send the user to. */ public function after_deactivation( $request_uri ): string { - // Both halves earn their place: the type check is the only refusal a non-string ever meets, - // and an empty string names no screen to go back to. + // A non-string meets no other refusal; an empty string names no screen. if ( ! is_string( $request_uri ) || $request_uri === '' ) { return $this->admin_url_for( 'plugins.php' ); } @@ -61,13 +40,12 @@ public function after_deactivation( $request_uri ): string { $screen = $this->screen_from_path( is_string( $path ) ? $path : '' ); - // Nothing that names an admin screen, so there is nothing to re-render: a front-end - // permalink, a directory that is not an admin root, a php file this admin does not serve, a - // traversal attempt. Those take the same route as no request URI at all. + // Names no admin screen -- a front-end permalink, a traversal attempt -- nothing to re-render. if ( $screen === '' ) { return $this->admin_url_for( 'plugins.php' ); } + // The exception to re-requesting: reloading either update screen re-runs an update. if ( $screen === 'update.php' || $screen === 'update-core.php' ) { return $this->admin_url_for( 'plugins.php' ); } @@ -78,13 +56,9 @@ public function after_deactivation( $request_uri ): string { /** * The admin screen a request path names, or an empty string if it names none. * - * Read from the path's basename rather than from the URI, and returned only once it is a screen: - * the name has to be well formed, and this admin has to have a file of that name to serve. The - * request URI is a path on a site that may live in a subdirectory, may be behind a - * TLS-terminating proxy whose scheme disagrees with admin_url(), and on multisite may sit under - * the network or user admin -- so nothing built from admin_url() would recognise it. Taking the - * basename is also what keeps a crafted URI out of the destination: only a validated screen name - * leaves here, and admin_url_for() supplies everything in front of it. + * Read from the path's basename rather than the URI: the request URI is a bare path on a site + * that may sit in a subdirectory, behind a TLS-terminating proxy, or under the network or user + * admin, so nothing built from admin_url() would recognise it. It also keeps a crafted URI out. * * @since 1.0.0 * @@ -95,21 +69,16 @@ public function after_deactivation( $request_uri ): string { private function screen_from_path( string $path ): string { $screen = basename( $path ); - // Two questions, and both have to be yes. Anchored with \z rather than $, which in PCRE also - // matches immediately before a trailing newline -- so "edit.php\n" would satisfy $ and a line - // break would leave here inside the one value this class promises is validated. The pattern - // runs first because it is also what makes the name safe to put after a directory below. + // Anchored with \z rather than $, which in PCRE also matches before a trailing newline -- so + // "edit.php\n" would satisfy $ and leave here inside the one value this class promises is + // validated. The pattern runs first: it is what makes the name safe to put after a directory. if ( (bool) preg_match( '/^[A-Za-z0-9_-]+\.php\z/', $screen ) && $this->is_admin_screen( $screen ) ) { return $screen; } - // The admin roots name the dashboard by leaving it out, the way core's own /wp-admin/ link - // does -- so a path that resolves to one is not a nameless directory, it is index.php. The - // network and user admins have roots of their own, and admin_url_for() picks the base to put - // in front of the screen. - // - // Matched against the end of the path rather than against its last segment alone, because a - // front-end permalink ending in /network/ would otherwise read as the network admin's root. + // The admin roots name the dashboard by leaving it out, as core's own /wp-admin/ link does. + // Matched against the end of the path rather than its last segment alone, or a front-end + // permalink ending in /network/ would read as the network admin's root. $trimmed = rtrim( $path, '/' ); foreach ( [ '/wp-admin', '/wp-admin/network', '/wp-admin/user' ] as $root ) { @@ -124,26 +93,9 @@ private function screen_from_path( string $path ): string { /** * Whether the admin this request belongs to has a screen of that name to be sent back to. * - * Well formed is not the same as naming a screen. `wp-login.php`, `wp-cron.php` and a plugin's - * own bootstrap file all satisfy the pattern above, and each of them would be rebuilt as an admin - * URL for a file that is not there -- the web server's own 404, in place of the plugins list this - * class documents for a request that names no admin screen. - * - * Asked of the filesystem rather than of a list of core's screens, because a list would be wrong - * the first time a plugin registered a top-level page: a host's screens are `admin.php`, - * `edit.php`, `options-general.php` or `tools.php` with a `page` argument on them, so the file is - * core's however many screens are hung off it and every one of them still comes back intact. What - * `is_file()` cannot tell apart is a screen from one of the admin's own includes -- but a browser - * is only ever on the first kind, and both are inside wp-admin either way. - * - * The three branches are admin_url_for()'s, in the same order and for the same reason: the answer - * has to be about the directory the destination will be built in. The network and user admins - * serve only the files core gives them -- there is no `wp-admin/network/options-general.php` -- - * and a request under one of them cannot have been on a screen it does not hold. - * - * The name reaching here has already matched the pattern above, so it is [A-Za-z0-9_-] and a - * '.php' and nothing else: there is no way for it to climb out of the directory being asked - * about, and no readability question either, since the web server is what serves the file, not us. + * Well formed is not the same as naming a screen: `wp-login.php` satisfies the pattern above and + * would be rebuilt as an admin URL for a file that is not there. Asked of the filesystem rather + * than a list of core's screens, since a plugin's screens are core's files with a `page` arg. * * @since 1.0.0 * @@ -166,15 +118,9 @@ private function is_admin_screen( string $screen ): bool { /** * The current request's query, re-encoded, ready to append to a screen name. * - * The query carries which list, which page and which filter the user was looking at, so - * dropping it would re-render the screen showing something else. It is taken apart and rebuilt - * rather than carried over verbatim, because it arrives from the URL bar and nothing about it - * has been checked. - * - * http_build_query() rather than add_query_arg(), which is the usual answer: add_query_arg() - * writes the array it is given straight into the result -- build_query() passes $urlencode as - * false -- so a value holding an '&' or a '#' would go on to add a parameter or a fragment of - * its own. Here both halves of every pair are encoded, which is the whole reason for rebuilding. + * The query carries which list, page and filter the user was looking at, so dropping it would + * re-render something else. Rebuilt with http_build_query() rather than add_query_arg(), which + * passes $urlencode as false -- a value holding an '&' would add a parameter of its own. * * @since 1.0.0 * @@ -206,17 +152,9 @@ private function query_string( string $request_uri ): string { /** * The parsed query with CR, LF and NUL taken out of every string in it, and nothing else. * - * The property being protected is that the destination cannot end a header: it is handed to - * wp_safe_redirect(), which puts it in a Location. That is all that is being protected, because - * it is all that is left to protect -- http_build_query() re-encodes both halves of every pair - * with PHP_QUERY_RFC3986, so no value can add a parameter, open a fragment or arrive as markup, - * whatever it holds. - * - * Deliberately not sanitize_text_field(). wp_parse_str() has already url-decoded these values, - * and _sanitize_text_fields() deletes every '%xx' sequence it can find and entity-encodes a bare - * '<' -- so a search for '100%ab' would be re-run as '100', and one for 'aregistry->all() as $sub_plugin ) { - // The policy may be a host callable behind a filter any plugin on the site may have - // hooked, the notice message is another callable, and deactivate_plugins() runs the - // standalone's own deactivation hook -- so this loop calls arbitrary code, from inside - // plugins_loaded, on an admin page view. A throw out of here would take away the screen - // the site owner would have used to undo whatever caused it, and would leave a second - // standalone running with nothing said about it. Reported per sub-plugin, and the next - // one is still resolved. + // The policy, the notice message and the standalone's own deactivation hook are all + // arbitrary code. Caught per sub-plugin, so one throw does not take the rest with it. try { if ( ! $this->detector->is_in_conflict( $sub_plugin ) ) { continue; @@ -149,15 +128,9 @@ public function resolve_all(): void { } } - // After the loop, never inside it. A site bundling two sub-plugins can have both standalones - // active, and an `exit` on the first would leave the second's standalone running with no - // notice raised about it — and would take the load pass at the next priority with it, so - // nothing bundled loaded on the request that was supposed to fix the conflict. - // - // And only where a standalone really did go away. A request that deactivated nothing has - // nothing to shed from memory, so re-requesting the screen would arrive at the same conflict - // — while the notices this pass queued are waiting on a request that reaches - // `all_admin_notices`, which a redirect never does. + // After the loop, never inside it: an `exit` on the first of two active standalones leaves the + // second running and takes the load pass at the next priority with it. And only where one + // really went away — a request that deactivated nothing would redirect into the same conflict. if ( $standalone_gone ) { $this->redirect(); } @@ -175,9 +148,8 @@ public function resolve_all(): void { protected function resolve( Sub_Plugin $sub_plugin ): bool { $policy = $sub_plugin->get_conflict_policy(); - // A host may persist a policy in an option and a filter may return anything. Falling - // through to deactivate() would turn off a plugin the site owner deliberately activated - // on the strength of a typo, so an unrecognised policy takes the conservative branch. + // A filter may return anything, and falling through to deactivate() would turn a plugin off + // on the strength of a typo. if ( ! Conflict_Policy::is_valid( $policy ) ) { $policy = Conflict_Policy::NOTICE_ONLY; } @@ -188,12 +160,9 @@ protected function resolve( Sub_Plugin $sub_plugin ): bool { return false; case Conflict_Policy::DEACTIVATE: - // A network-active standalone whose bundled replacement ships in a host that is not - // itself network-active: deactivating it network-wide would pull it from the sites the - // host never reached, where nothing loads the bundled copy. Leave it -- the load guard - // defers the bundled copy network-wide instead -- and say why. Opt-in and - // single-site-safe: false whenever no host basename is configured, and false off a - // network, so this is the ordinary deactivation in every other case. + // Deactivating network-wide would pull the standalone from sites the host never + // reaches, where nothing loads the bundled copy. Leave it -- the load guard defers + // the bundled copy network-wide instead -- and say why. if ( $this->detector->deactivation_would_strand_sites( $sub_plugin ) ) { $this->notices->queue_stranding_notice( $sub_plugin ); @@ -202,9 +171,8 @@ protected function resolve( Sub_Plugin $sub_plugin ): bool { return $this->deactivate( $sub_plugin ); - // NOTICE_ONLY, and anything is_valid() would accept that this switch has grown no - // branch for. The default sits on the branch that only talks, never on the one that - // deactivates: a policy nobody wrote must not be read as consent to turn a plugin off. + // NOTICE_ONLY, and any valid policy this switch has grown no branch for. The default only + // talks: a policy nobody wrote must never be read as consent to turn a plugin off. default: $this->notices->queue_conflict_notice( $sub_plugin ); @@ -231,25 +199,17 @@ protected function resolve( Sub_Plugin $sub_plugin ): bool { protected function deactivate( Sub_Plugin $sub_plugin ): bool { $this->plugin_deactivator->deactivate( $sub_plugin->get_standalone_plugin_basename() ); - // Asked again rather than assumed, because turning the standalone off is not the same event as - // the standalone being off. A site or mu-plugin filtering `option_active_plugins` puts it - // straight back, a host may have rebound `Plugin\Contracts\Deactivator_Interface` to something - // that does nothing, and a rebound `Plugin\Contracts\Checker_Interface` may mean by "active" - // something `deactivate_plugins()` never touches. + // Asked again rather than assumed: an mu-plugin filtering `option_active_plugins` puts it + // straight back, and either plugin seam may have been rebound to something that never turns + // it off. if ( $this->detector->is_in_conflict( $sub_plugin ) ) { - // Nothing is said and nothing is reported. The merge notice would tell the owner a plugin - // they can watch still running had been deactivated, and would say it again on every admin - // GET for as long as the site kept putting it back; and this is the DEFER outcome reached - // by another route, so there is no failure to announce either -- the standalone is running, - // which means its own guard constant stands the bundled copy down and nothing re-declares. - // Every way of arriving here is a site's own configuration rather than a mistake in the - // host's code, and `Plugin\Contracts\Deactivator_Interface` invites one of them by name. + // Said nothing about, and reported nowhere. This is the DEFER outcome by another route -- + // the standalone is running, so its own guard constant stands the bundled copy down -- and + // every way of arriving here is a site's configuration rather than the host's mistake. return false; } - // Queued as each deactivation is confirmed rather than once at the end, so the explanation is - // durable whether or not the request goes on to redirect — and so a site with two standalones - // gets one notice per plugin it lost. + // Queued per deactivation, so a site with two standalones gets one notice per plugin it lost. $this->notices->queue_merge_notice( $sub_plugin ); return true; @@ -263,11 +223,9 @@ protected function deactivate( Sub_Plugin $sub_plugin ): bool { * @return void */ protected function redirect(): void { - // Boot\Scheduler runs this whole sequence inline when a host boots too late, immediately - // after a _doing_it_wrong() that prints on a debugging site — so the headers are gone before - // we get here, wp_safe_redirect() would warn and set no Location, and the exit behind it - // would end the request on a blank page. Falling through instead lets the page finish - // rendering, which is where the merge notice queued above is waiting to be read. + // Boot\Scheduler runs this sequence inline when a host boots too late, behind a + // _doing_it_wrong() that prints on a debugging site — so wp_safe_redirect() would set no + // Location and the exit would end the request on a blank page. Falling through renders it. if ( headers_sent() ) { return; } @@ -281,8 +239,7 @@ protected function redirect(): void { // cost is a stray backslash in a re-encoded query arg rather than a deleted one. $request_uri = $_SERVER['REQUEST_URI'] ?? ''; - // $_SERVER carries whatever the SAPI put there, so the string the redirector is promised is - // made one here rather than assumed. + // $_SERVER carries whatever the SAPI put there, so the promised string is made one here. if ( ! is_string( $request_uri ) ) { $request_uri = ''; } diff --git a/src/Conflict/Rewriter.php b/src/Conflict/Rewriter.php index a0a6bb0..e0d9af7 100644 --- a/src/Conflict/Rewriter.php +++ b/src/Conflict/Rewriter.php @@ -14,23 +14,9 @@ /** * Rewrites the fatal-error screen an absorbed standalone earns when someone activates it. * - * This is the one conflict the load guard cannot prevent. WordPress includes the plugin being - * activated *after* the bundled copy is in memory, so the re-declaration is a real fatal; core - * catches it in its activation sandbox and reports "the plugin triggered a fatal error" — true, and - * useless to whoever pressed the button. All this library gets to do about it is reword that one - * sentence, and the wording is the sub-plugin's own `conflict_notice_message`. - * - * Its own class rather than a method on the notice writer, though the message is shared with the - * merge notice. Nothing here is stored, drawn or queued: it reads the request, checks the screen, - * verifies a nonce and edits markup core wrote. The writer would have needed a registry to find out - * which sub-plugin the screen is even about — a collaborator only that one method used — and every - * host binding its own `Notices\Contracts\Writer_Interface` would have had to implement an error - * screen to get its notices worded. - * - * In `Conflict\` rather than `Notices\` for the same reason: what it is about is the standalone - * conflict, and it changes when that story does, alongside `Detector`, `Resolver` and `Redirector`. - * - * Not `final`: it is bound by class name, which is the seam a host rebinds and a test subclasses. + * The one conflict the load guard cannot prevent: core includes the plugin being activated *after* + * the bundled copy, so the re-declaration really does fatal and all this can do is reword core's + * sentence, with the sub-plugin's own `conflict_notice_message`. Not `final`: bound by class name. * * @since 1.0.0 */ @@ -54,8 +40,8 @@ public function __construct( Reader $registry ) { /** * The markup to print in place of the one WordPress was about to. * - * Handed back untouched unless the request really is a nonce-verified activation error, on a - * plugins screen, for a standalone this library has registered. + * Handed back untouched unless the request is a nonce-verified activation error, on a plugins + * screen, for a standalone this library has registered. * * @since 1.0.0 * @@ -72,37 +58,25 @@ public function rewrite( string $markup ): string { $screen = get_current_screen(); - // Both plugin lists, because wp-admin/network/plugins.php is a one-line require of - // wp-admin/plugins.php and draws the identical activation error -- but WP_Screen appends - // `-network` to the id there. On a default multisite the subsite has no plugins UI at all, - // so the network screen is the *only* place a super admin can reactivate the standalone, - // and matching 'plugins' alone would decline on the one screen that matters most. + // Both plugin lists: wp-admin/network/plugins.php is a one-line require of the other, but + // WP_Screen appends `-network` to the id there -- and on a default multisite it is the only + // screen an absorbed standalone can be reactivated from. if ( $screen === null || ! in_array( $screen->id, [ 'plugins', 'plugins-network' ], true ) ) { return $markup; } - // phpcs:disable WordPress.Security.NonceVerification.Recommended -- verified below, once - // the plugin named turns out to be one this library owns. Nothing is acted on until then. + // phpcs:disable WordPress.Security.NonceVerification.Recommended -- verified below, before + // anything is acted on. $basename = isset( $_GET['plugin'] ) ? wp_unslash( $_GET['plugin'] ) : ''; - // Unslashed and no further. Core mints the activation-error nonce from - // wp_unslash( $_REQUEST['plugin'] ) verbatim (wp-admin/plugins.php), so sanitizing here - // would verify an action core never signed: a plugin whose folder name holds a '%xx' - // sequence, a '<' or a leading space comes back changed from sanitize_text_field(), and - // both the nonce check and the registry lookup below would then miss -- silently, and on - // the one screen this class exists to improve. Nothing sanitizing would remove is needed - // here either: the value is compared against a basename the host configured and hashed into - // a nonce action, and never reaches the page. What does reach it is the sub-plugin's message. - // - // is_string() because sanitize_text_field() was doing that job: '?plugin[]=x' arrives as an - // array, and an array reaching wp_verify_nonce() is a string conversion, not a refusal. + // Unslashed and no further: core mints the nonce from the unslashed value verbatim, so + // sanitizing a folder name holding a '%xx' sequence would make both the nonce check and the + // registry lookup miss, silently. is_string() because '?plugin[]=x' arrives as an array. if ( ! is_string( $basename ) || $basename === '' ) { return $markup; } - // Looked up before the nonce is checked, deliberately: there is no nonce work to do for a - // plugin this library does not own, and the nonce is still verified before any markup is - // touched. + // Looked up first: there is no nonce work to do for a plugin this library does not own. $sub_plugin = $this->find_by_standalone_basename( $basename ); if ( $sub_plugin === null ) { @@ -126,11 +100,8 @@ public function rewrite( string $markup ): string { ) ); - // Sanitised before the emptiness check rather than after. wp_kses_post( '' ) - // is the empty string, and swapping WordPress's wording for nothing leaves a blank notice - // box where the explanation should be. wp_kses_post() and not esc_html(), for the reason - // the renderer uses it: these messages come from the host's own config, and a link to a - // knowledge-base article has to survive. + // Sanitised before the emptiness check rather than after: wp_kses_post( '' ) + // is the empty string, and swapping core's wording for nothing leaves a blank notice box. $message = trim( wp_kses_post( $message ) ); if ( $message === '' ) { @@ -146,10 +117,8 @@ public function rewrite( string $markup ): string { /** * The registered sub-plugin a standalone basename belongs to, if any. * - * Read through the reader this class was handed rather than a registrar of its own, for the - * reason the conflict and load passes do: the reader drains the registrations still buffered - * before it reads, and a registrar asked directly would miss anything registered since the last - * read. + * Read through the reader rather than a registrar of its own: it drains the buffered + * registrations first. * * @since 1.0.0 * diff --git a/src/Conflict_Policy.php b/src/Conflict_Policy.php index 0b44339..1602484 100644 --- a/src/Conflict_Policy.php +++ b/src/Conflict_Policy.php @@ -14,8 +14,8 @@ */ final class Conflict_Policy { /** - * Deactivate the standalone, notify, and redirect. The bundled copy loads on the next - * request, since the standalone has already defined the guard constant on this one. + * Deactivate the standalone, notify, and redirect. The bundled copy loads next request, since the + * standalone has already defined the guard constant on this one. * * @since 1.0.0 * @@ -44,13 +44,9 @@ final class Conflict_Policy { /** * The policy that applies when a sub-plugin configures none. * - * Deactivating is the default because two copies of the same plugin are the failure this - * library exists to prevent, and a sub-plugin that has not thought about the question wants - * the outcome where its bundled copy ends up running. - * - * Distinct from the branch a caller takes for a policy it does not recognise: not configuring - * one is a choice to accept the default, whereas an unrecognised value is a value nobody - * chose, and reading it as consent to deactivate would act on a typo. + * Deactivating, because two copies of the same plugin are the failure this library exists to + * prevent. Configuring nothing accepts that; an unrecognised policy does not, so callers must + * not reach this default for one — it would act on a typo. * * @since 1.0.0 * @@ -63,10 +59,8 @@ public static function default(): string { /** * Whether a policy string is one this library understands. * - * Hosts may persist a policy in an option and filters may return anything, so callers that - * dispatch on a policy should reject unknown values here rather than letting them fall - * through to a default branch — deactivating a plugin the site owner deliberately turned on - * is the most surprising of the three outcomes to arrive at by accident. + * A filter may return anything, and an unknown value must not fall through a `switch` into a + * deactivation nobody asked for. * * @since 1.0.0 * diff --git a/src/Contracts/Activator_Interface.php b/src/Contracts/Activator_Interface.php index 437d707..0aaba55 100644 --- a/src/Contracts/Activator_Interface.php +++ b/src/Contracts/Activator_Interface.php @@ -14,8 +14,7 @@ * Runs a sub-plugin's activation routine once, ever. * * A bundled plugin is `require_once`d rather than activated, so `register_activation_hook()` never - * fires for it and whatever that hook would have done — creating a table, seeding options — never - * happens. Bind a replacement to change how "once, ever" is recorded. + * fires for it. Bind a replacement to change how "once, ever" is recorded. * * @since 1.0.0 */ diff --git a/src/Contracts/Provider_Interface.php b/src/Contracts/Provider_Interface.php index d5e3ac9..f3f5b2a 100644 --- a/src/Contracts/Provider_Interface.php +++ b/src/Contracts/Provider_Interface.php @@ -10,9 +10,8 @@ /** * Teaches a container how to build this library's collaborators. * - * One method, and it binds — it wires no hooks and resolves nothing. When things run is - * `Boot\Scheduler`'s subject, and a provider that also hooked would have to be replaced wholesale - * by a host that only wanted a different implementation of one binding. + * It binds and nothing else: when things run is `Boot\Scheduler`'s subject, and a provider that + * also hooked would have to be replaced wholesale by a host wanting one binding changed. * * @since 1.0.0 */ @@ -20,8 +19,7 @@ interface Provider_Interface { /** * Bind this library's collaborators. * - * Called once, from `Absorber::boot()`, and expected to be safe to call again: boot() is - * idempotent and a host may bind its own provider that another code path also registers. + * Must be safe to call again: `boot()` is idempotent, and another code path may register too. * * @since 1.0.0 * diff --git a/src/Loader.php b/src/Loader.php index 4238983..d7808f7 100644 --- a/src/Loader.php +++ b/src/Loader.php @@ -17,9 +17,7 @@ /** * The load pass: every registered sub-plugin, in registration order, gated one at a time. * - * Separate from `Absorber` because it is the one thing here that runs rather than configures. It is - * reached from a hook, it needs the notice queue and the activator, and it is the piece a host is - * likeliest to want to watch or replace — none of which is true of registration. + * Separate from `Absorber` because it runs rather than configures. * * @since 1.0.0 */ @@ -70,9 +68,7 @@ public function __construct( * @return void */ public function load_all(): void { - // The load path needs the prefix for the should_load filter and for the notice store. - // Throwing out of a core action would take the whole site down over a bootstrap mistake, - // so it is reported where a developer will see it and the load is abandoned instead. + // No prefix, no should_load filter and no notice store; the trait reports it. if ( ! self::has_hook_prefix() ) { return; } @@ -135,10 +131,9 @@ private function load( Sub_Plugin $sub_plugin ): void { return; } - // Ahead of the dependency check, which calls an arbitrary host callable. This is one - // defined(), it carries the whole re-declaration guarantee, and it is the only gate that - // means "the plugin is already running" -- warning that requirements are unmet for a - // plugin the admin can see working would be worse than useless. + // Ahead of the dependency check, never after: it carries the whole re-declaration guarantee, + // and warning of unmet requirements for a plugin already running sends the admin after the + // wrong problem. if ( $sub_plugin->is_already_loaded() ) { $this->announce_skip( $sub_plugin, Skip_Reason::ALREADY_LOADED ); @@ -152,11 +147,8 @@ private function load( Sub_Plugin $sub_plugin ): void { return; } - // Not file_exists(): that is true for a directory and for a file with no read permission, - // and require_once fatals on both. A missing file is a broken build in the host plugin - // rather than anything a site owner can act on, so it goes to the developer instead of - // into the notice queue, where it would have displayed the host's own - // dependency_notice_message and sent the owner after the wrong problem entirely. + // Not file_exists(): that is true for a directory and for an unreadable file, and + // require_once fatals on both. A broken build reports to the developer, not a site owner. $file = $sub_plugin->get_bundled_plugin_file(); if ( ! is_file( $file ) || ! is_readable( $file ) ) { @@ -179,9 +171,8 @@ private function load( Sub_Plugin $sub_plugin ): void { return; } - // No type guard on the return, unlike the conflict_policy filter: there is no cast here, - // and every unexpected value is falsy-or-truthy without fataling. Anything odd skips the - // load, which is the safe direction. + // No type guard on the return: there is no cast here, so anything odd is merely falsy and + // skips the load, which is the safe direction. $should_load = apply_filters( Config::get_hook_name( 'should_load' ), true, $sub_plugin ); if ( ! $should_load ) { @@ -190,32 +181,16 @@ private function load( Sub_Plugin $sub_plugin ): void { return; } - // An include takes the scope of the line it sits on, and this one is inside a method, where - // wp-settings.php includes plugins at global scope. Top-level assignments in the bundled - // file are function-local as a result -- documented for hosts, because no amount of - // wrapping here can hand a required file the global scope it would have had. + // An include takes the scope of the line it sits on, so top-level assignments in the bundled + // file are function-local where wp-settings.php would have made them global. Not fixable. require_once $file; - // The same defined() the second gate asked, on the other side of the require -- asked of the - // constant table here rather than through `Sub_Plugin::is_already_loaded()`, because it is no - // longer the same question. In front of the require that predicate means "the code is already - // here, from whichever copy"; behind it, the only thing worth asking is whether this require - // did what the guard promises. `Sub_Plugin` still owns the name, so nothing here spells a guard - // constant for Strauss's constant_prefix to rewrite at build time. - // - // Nothing else checks: a typo in `plugin_loaded_constant`, or a bundled plugin that defines its - // constant from its own plugins_loaded callback rather than at file scope, leaves the code in - // memory with nothing standing a standalone copy down -- and a re-declaration fatal, which PHP - // does not raise as a Throwable and nothing here can catch, is then one activation away while - // every counter and every action says the load went perfectly. - // - // Reported rather than skipped, and the load carries on. The require happened and cannot be - // undone: the file's code is in memory whatever the guard says, so the activation callback - // behind this still has to run and `loaded` still has to fire -- announcing a skip would tell - // a host that code which is running is not there, and withholding the callback would leave - // the sub-plugin loaded with the tables it expects never created. What is broken is the - // host's build, which is a developer's to fix and nothing a site owner's screen can help with -- - // so it goes to _doing_it_wrong() and to no notice. + // The same defined() the second gate asked, on the other side of the require: in front of it + // the question is whether any copy is loaded, behind it whether this require kept the guard's + // promise. Reported and not skipped -- the require cannot be undone, so the callback and + // `loaded` still have to happen, and a broken host build is nothing a site owner's screen can + // fix. `Sub_Plugin` still owns the name, so nothing here spells a constant for Strauss to + // rewrite. $guard_constant = $sub_plugin->get_plugin_loaded_constant(); if ( ! defined( $guard_constant ) ) { @@ -232,11 +207,9 @@ private function load( Sub_Plugin $sub_plugin ): void { ); } - // Only after a require that actually happened. A bundled plugin is included rather than - // activated, so register_activation_hook() never fires for it and whatever that hook would - // have done -- creating a table, seeding options -- would never happen at all. Running it - // for a sub-plugin that was skipped would be worse: the schema would appear for a plugin - // that is not loaded. + // Last, and only after a require that happened: register_activation_hook() never fires for + // an included plugin, so this stands in for it with the sub-plugin's code in memory, and a + // skipped load would spend the once-ever record for good. $this->activator->maybe_run( $sub_plugin ); // Behind the activation callback, not in front of it. A listener here is host code that will diff --git a/src/Notices/Contracts/Writer_Interface.php b/src/Notices/Contracts/Writer_Interface.php index 4a6009d..3978a07 100644 --- a/src/Notices/Contracts/Writer_Interface.php +++ b/src/Notices/Contracts/Writer_Interface.php @@ -11,13 +11,11 @@ use Nexcess\PluginAbsorber\Sub_Plugin; /** - * What the absorber has to say about a sub-plugin. Bind a replacement to word or keep it your own way. + * What the absorber has to say about a sub-plugin. Bind a replacement to word or keep it your own + * way. * - * The seam, and the only one in this folder: what a notice says is the thing a host has an opinion - * about, and a host already running `stellarwp/admin-notices` binds its own here. How a pending - * notice reaches the screen is `Notices\Presenter`'s, which is a class rather than a contract because - * nothing in the library dispatches on it — the trampoline on `all_admin_notices` is the only caller, - * and a host that wants it gone takes the callback off. + * The only seam in this folder, because nothing dispatches on how a notice reaches the screen: a + * host that wants no rendering of ours takes the `all_admin_notices` callback off instead. * * @since 1.0.0 */ @@ -25,12 +23,8 @@ interface Writer_Interface { /** * Queue the "we deactivated the standalone for you" notice. * - * Queued after the deactivation has already happened, and raised exactly once — nothing - * re-queues it on a later request, so an implementation that drops it drops the only warning - * the site owner gets. - * - * Whatever an implementation writes has to survive the request that wrote it: the resolver - * redirects, so the notice is almost never read by the request that raised it. + * Raised exactly once, so an implementation that drops it drops the only warning the site owner + * gets — and it must survive the request that wrote it, since the resolver then redirects. * * @since 1.0.0 * @@ -58,9 +52,9 @@ public function queue_conflict_notice( Sub_Plugin $sub_plugin ): void; /** * Queue the "we left the standalone active to avoid stranding sites" notice. * - * Raised in one topology only: on multisite, a network-active standalone whose host plugin is not - * network-activated, where deactivating it network-wide would remove it from the sites the host - * never reached. Its wording must not tell the user to deactivate the standalone. + * Raised in one topology only: a network-active standalone whose host is not network-activated, + * where a network-wide deactivation would remove it from the sites the host never reached. So + * its wording must not tell the user to deactivate the standalone. * * @since 1.0.0 * @@ -88,10 +82,8 @@ public function queue_dependency_notice( Sub_Plugin $sub_plugin ): void; /** * Where these notices are kept, so a host can render them itself without replacing the writer. * - * On the contract rather than on the default implementation, and an instance method rather than - * a static one, because the honest answer depends on which writer a site is running: an - * implementation bound in place of the default keeps its notices where it likes, and a host - * reading a name off the default class would read an option nothing writes to. + * An instance method on the contract because the answer depends on which writer a site runs: a + * name read off the default class would name an option nothing writes to. * * @since 1.0.0 * diff --git a/src/Notices/Presenter.php b/src/Notices/Presenter.php index 977ad1f..1cec960 100644 --- a/src/Notices/Presenter.php +++ b/src/Notices/Presenter.php @@ -13,26 +13,9 @@ /** * Who may consume the queue, and what happens when they do. * - * Separate from `Writer` because the two change for different reasons: one is asked "what does this - * notice say", the other "may this user see the pending set, and is it gone once they have". Nothing - * here words a notice, and nothing in the writer decides who reads one. - * - * The capability check lives here rather than in `Renderer` because it guards the clearing as much as - * the drawing: the two have to be decided together, or a user who may not see the queue could still - * destroy it. Which capability is `Traits\Guards_Plugin_Capability`'s answer, shared with the gate - * that decides who may have a conflict resolved at all — the conflict notices report what that gate - * let happen, so consuming one is the same authority as causing it. The dependency notice is queued - * off the load path, behind no gate at all, and is held to the same capability for the reason above: - * whoever consumes the queue consumes all of it. - * - * The queue is single-consumer. Rendering consumes it for everybody, so the first eligible - * administrator to load any admin screen is the only person who ever sees a given notice — - * network-wide on multisite, where the queue is one network option. A host that wants every - * administrator to see it has to track consumption per user itself. - * - * Bound by class name rather than behind a contract: the trampoline on `all_admin_notices` is the - * only caller, so nothing in the library dispatches on this, and a host that wants no rendering of - * ours takes that callback off rather than binding an implementation that does nothing. + * The queue is single-consumer: rendering consumes it for everybody, network-wide on multisite. The + * capability check lives here rather than in `Renderer` because it guards the clearing as much as + * the drawing — decided apart, a user who may not see the queue could still destroy it. * * @since 1.0.0 */ @@ -67,9 +50,8 @@ public function __construct( Store $store, Renderer $renderer ) { /** * Draw the queue, then consume it. * - * The store rather than the bound writer, deliberately: this draws what the *default* writer - * kept, and a host that binds a writer of its own has taken over where notices live along with - * what they say. + * Reads the store, not the bound writer: a host binding its own writer has taken over where + * notices live along with what they say. * * @since 1.0.0 * @@ -78,10 +60,8 @@ public function __construct( Store $store, Renderer $renderer ) { * @return void */ public function render(): void { - // Rendering clears the queue, so a user who cannot act on a notice must not be shown one: a - // subscriber loading their profile page would otherwise silently swallow the only warning an - // administrator was ever going to get, and on multisite the queue they swallowed it out of is - // shared by every site on the network. + // Rendering clears the queue, so a subscriber loading their profile page would otherwise + // swallow the only warning an administrator was ever going to get. if ( ! self::user_may_manage_plugins() ) { return; } diff --git a/src/Notices/Renderer.php b/src/Notices/Renderer.php index cb14651..1064e17 100644 --- a/src/Notices/Renderer.php +++ b/src/Notices/Renderer.php @@ -10,12 +10,8 @@ /** * How a queued notice is drawn. * - * Deliberately minimal markup so the library stays dependency-free, and deliberately its own class: - * a host that only wants different markup — one already using stellarwp/admin-notices, say — has - * one small thing to replace rather than the whole queue. - * - * Knows nothing about where the queue came from or who is allowed to see it. It is handed messages - * and prints them. + * Minimal markup so the library stays dependency-free, and its own class so a host wanting other + * markup has one small thing to replace. * * @since 1.0.0 */ @@ -23,9 +19,8 @@ class Renderer { /** * The `notice-*` class each notice type renders with. * - * A dependency notice reports a plugin that did not load at all, which is `notice-error` by - * WordPress convention. The other three report a conflict the library has already handled or held - * off — the site works, so they are warnings. + * A dependency notice reports a plugin that did not load at all, `notice-error` by WordPress + * convention. The other three report a conflict already handled: the site works, so they warn. * * @since 1.0.0 * @@ -41,17 +36,9 @@ class Renderer { /** * Print every message in the queue. * - * Messages go through `wp_kses_post()`, the standard WordPress post-content allowlist, so a - * link to a knowledge-base article, emphasis or a list survives, while a script or an event - * handler attribute does not. - * - * The paragraph comes from `wpautop()` rather than a literal `

` around the message. That - * same allowlist keeps a `

`, so a host may well send one already wrapped, and a hard wrap - * around it is markup no browser can honour: the parser closes the outer paragraph at the - * inner one and leaves a stray `

`, which draws an empty line above the notice. A `
    ` - * fares worse — it cannot legally sit in a `

    ` at all, so the list the allowlist just - * preserved would break straight back out of it. `wpautop()` wraps only what needs wrapping, - * and turns the blank line a plain translated string uses as a break into a real one. + * `wp_kses_post()` rather than `esc_html()`, so a host's knowledge-base link or list survives + * while a script does not; and `wpautop()` rather than a literal `

    ` around the message, + * because a `

      ` cannot legally sit inside one and would break straight back out of it. * * @since 1.0.0 * @@ -61,10 +48,8 @@ class Renderer { */ public function render( array $queue ): void { foreach ( $queue as $key => $message ) { - // Filtered before the emptiness check rather than after it, because filtering can - // empty a message on its own: `wp_kses_post( '' )` is the empty - // string. Either that or a whitespace-only message would print an empty notice box, - // which reads as a bug. + // Filtered before the emptiness check: `wp_kses_post( '' )` is the + // empty string, and an empty notice box reads as a bug. $message = trim( wp_kses_post( $message ) ); if ( $message === '' ) { @@ -74,9 +59,7 @@ public function render( array $queue ): void { printf( '
      %s
      ', esc_attr( $this->notice_class( (string) $key ) ), - // Already filtered: escaping it again here would undo the whole point and print a - // link as literal angle brackets. Trimmed because `wpautop()` leaves a trailing - // newline, which would otherwise sit inside the div on every notice. + // Already filtered: escaping again would print a link as literal angle brackets. trim( wpautop( $message ) ) ); } @@ -95,8 +78,8 @@ private function notice_class( string $key ): string { $parts = explode( ':', $key ); $type = (string) end( $parts ); - // An entry written by an older version, or by a host reading and rewriting the option, is - // shown rather than dropped: a warning is the safe severity for something unrecognised. + // An unrecognised type — an older version's entry, or a host's own — is shown at the safe + // severity rather than dropped. return self::CLASSES[ $type ] ?? 'notice-warning'; } } diff --git a/src/Notices/Store.php b/src/Notices/Store.php index a8cf5fd..4c2686d 100644 --- a/src/Notices/Store.php +++ b/src/Notices/Store.php @@ -13,14 +13,9 @@ /** * Where the notice queue lives: one option, keyed `slug:type`. * - * An option rather than a transient. With an external object cache, `set_transient()` never - * touches the database at all — the queue would live only in Redis or Memcached, where a routine - * `wp_cache_flush()` from a deploy script or a "purge cache" button destroys it. The merge notice - * is raised once and never again, so losing it means a site owner is never told their plugin was - * deactivated. This queue is not a cache. - * - * Separate from Writer so that changing where notices are kept does not mean touching how they are - * worded or drawn. + * An option rather than a transient: with an external object cache a transient never reaches the + * database, so a routine `wp_cache_flush()` destroys it — and the merge notice is raised exactly + * once, so losing it means the site owner is never told. * * @since 1.0.0 */ @@ -29,11 +24,6 @@ class Store { * The option name backing the queue. Read it from here rather than composing it, since the hook * prefix is normalised on its way into a storage key. * - * An instance method, not a static one: it is the answer for *this* store, and a host that binds - * a queue keeping its notices somewhere else has to be able to give a different one. A static - * would answer for the default implementation whatever the site actually uses, which is the - * wrong answer stated with confidence. - * * @since 1.0.0 * * @throws Config_Exception When no hook prefix has been set. @@ -54,16 +44,15 @@ public function option_name(): string { * @return array */ public function all(): array { - // Outside multisite `get_site_option()` is `get_option()`, so this reads back whatever - // put() wrote on either install type. + // Outside multisite the `*_site_option()` family is the `*_option()` one, so the reads and + // writes here cover both. $queue = get_site_option( $this->option_name(), [] ); if ( ! is_array( $queue ) ) { return []; } - // Anything that is not a string message is dropped rather than printed. put() writes the - // filtered array back, so a corrupted entry heals itself. + // put() writes the filtered array back, so a corrupted entry heals itself. return array_filter( $queue, 'is_string' ); } @@ -84,10 +73,8 @@ public function put( string $key, string $message ): void { $queue[ $key ] = $message; - // One call covers both install types. Outside multisite `update_site_option()` ends in - // `update_option( $option, $value, false )`, or `add_option( $option, $value, '', false )` - // the first time — either way autoload is off, which is exactly what this queue wants: it - // is empty on almost every request and only ever read in the admin. + // Autoload off, which is what this queue wants: empty on almost every request, read only in + // admin. update_site_option( $this->option_name(), $queue ); } @@ -99,7 +86,6 @@ public function put( string $key, string $message ): void { * @return void */ public function clear(): void { - // Outside multisite `delete_site_option()` is `delete_option()`. delete_site_option( $this->option_name() ); } } diff --git a/src/Notices/Writer.php b/src/Notices/Writer.php index 1b5dfd2..a951be0 100644 --- a/src/Notices/Writer.php +++ b/src/Notices/Writer.php @@ -14,27 +14,14 @@ /** * What a notice says, and under which key it is kept. * - * One reason to change: the wording. Where the queue is kept is `Store`'s, how a pending notice is - * drawn is `Renderer`'s, and who may consume the queue is `Presenter`'s — so a host can reword a - * sentence without reading any of them, and none of them has to be understood to answer "what does - * the merge notice say". - * - * Option-backed through `Store`, so what is written here survives the resolver's redirect: the - * request that raises a notice is almost never the one that shows it. - * - * The collaborator is a required constructor argument, and `Provider` is what hands it over. No - * defaults: a class that can build its own dependencies has a second way to be constructed that - * bypasses every binding a host made, and it is the one a test or a stray `new` reaches for. - * - * A host already using stellarwp/admin-notices can bind its own implementation of Writer_Interface - * and read the same option, whose name is `option_name()`. + * Wording only: `Store` keeps the queue, `Renderer` draws it, `Presenter` decides who consumes it. * * @since 1.0.0 */ class Writer implements Writer_Interface { /** - * Notice types. Public because they are the second half of a queue key — an entry is stored - * under `slug:type`, and reading the queue yourself means matching against these. + * Notice types. Public because an entry is stored under `slug:type`, so reading the queue + * yourself means matching against these. * * @since 1.0.0 * @@ -102,9 +89,6 @@ public function queue_merge_notice( Sub_Plugin $sub_plugin ): void { } /** - * The default differs from the merge notice's on purpose: this one asks the user to act, - * where that one reports something already done. - * * @since 1.0.0 * * @param Sub_Plugin $sub_plugin Sub-plugin concerned. @@ -127,12 +111,8 @@ public function queue_conflict_notice( Sub_Plugin $sub_plugin ): void { } /** - * The "we left the standalone active to avoid stranding sites" notice. - * - * Its own type and its own default because it must not carry the conflict notice's "you can - * safely deactivate the standalone": on the topology it fires for -- a network-active standalone - * whose host is not network-activated -- a network-wide deactivation is the very thing that would - * strand the sites the host never reached. The wording lives on `Sub_Plugin` with the others. + * Its own type and default because it must not carry the conflict notice's "you can safely + * deactivate the standalone" -- here that deactivation is what would strand the sites. * * @since 1.0.0 * @@ -171,11 +151,10 @@ public function option_name(): string { } /** - * Store one notice, keyed by slug and type so different types can coexist. + * Store one notice, keyed `slug:type`. * - * A sub-plugin can legitimately earn a merge notice while the conflict is resolved and a - * dependency notice while the load is attempted, in the same request. Keying by slug alone - * would silently drop one of them. + * One sub-plugin can earn a merge notice and a dependency notice in the same request; keying by + * slug alone would silently drop one of them. * * @since 1.0.0 * diff --git a/src/Plugin/Checker.php b/src/Plugin/Checker.php index db136f8..c343da1 100644 --- a/src/Plugin/Checker.php +++ b/src/Plugin/Checker.php @@ -27,9 +27,7 @@ class Checker implements Checker_Interface { public function is_active( string $basename ): bool { $this->load_plugin_functions(); - // WordPress's own is_plugin_active() already ORs in the network check, so asking - // is_plugin_active_for_network() as well would only buy a second get_site_option() per - // sub-plugin per request. + // is_plugin_active() already ORs in the network check. return is_plugin_active( $basename ); } @@ -43,8 +41,7 @@ public function is_active( string $basename ): bool { public function is_network_active( string $basename ): bool { $this->load_plugin_functions(); - // is_plugin_active_for_network() checks is_multisite() itself and returns false off a - // network, so the stranding guard that reads this needs no is_multisite() test of its own. + // Returns false off a network on its own, so callers need no is_multisite() test. return is_plugin_active_for_network( $basename ); } } diff --git a/src/Plugin/Contracts/Checker_Interface.php b/src/Plugin/Contracts/Checker_Interface.php index 14ce389..0d91b6b 100644 --- a/src/Plugin/Contracts/Checker_Interface.php +++ b/src/Plugin/Contracts/Checker_Interface.php @@ -8,17 +8,12 @@ namespace Nexcess\PluginAbsorber\Plugin\Contracts; /** - * The library's one way of asking WordPress about a plugin. + * The library's one way of asking WordPress about a plugin. Bind a replacement to answer from + * somewhere other than the active-plugins option. * - * Stated in terms of a plugin basename — "give-recurring/give-recurring.php" — because that is the - * only identifier WordPress itself accepts. Bind a replacement to answer from somewhere other than - * the active-plugins option. - * - * Separate from `Plugin\Contracts\Deactivator_Interface` because the two are asked for by different code for - * different reasons: reading plugin state is a question anything may ask, while turning a plugin - * off is an action exactly one policy branch takes. A host that wants deactivation to be a no-op — - * plugin state managed outside WordPress, say — should not have to reimplement the reading half - * to say so. + * Separate from `Deactivator_Interface` because reading plugin state is a question anything may + * ask, while turning a plugin off is an action one policy branch takes: a host making deactivation + * a no-op should not have to reimplement the reading half to say so. * * @since 1.0.0 */ @@ -26,8 +21,8 @@ interface Checker_Interface { /** * Whether the plugin is active, in either scope. * - * Site-wide and network-wide both count. The conflict this answers for is "the standalone's - * code is going to run this request", and a network activation runs it just as surely. + * The question is "is the standalone's code going to run this request", which a network + * activation answers yes just as surely as a site one. * * @since 1.0.0 * @@ -40,12 +35,8 @@ public function is_active( string $basename ): bool; /** * Whether the plugin is active across the whole network. * - * Network scope only, unlike `is_active()`, which counts either scope. The two are asked for - * different reasons: `is_active()` answers "the standalone's code is going to run this request", - * while this answers "deactivating it reaches every site" -- the question the conflict resolver's - * stranding guard turns on. Returns `false` whenever the site is not multisite, because - * WordPress's own `is_plugin_active_for_network()` does, so a caller needs no `is_multisite()` - * guard of its own and the stranding guard is a no-op off a network. + * Network scope only, where `is_active()` counts either. Must return `false` off a network, as + * core's `is_plugin_active_for_network()` does, so no caller needs an `is_multisite()` guard. * * @since 1.0.0 * diff --git a/src/Plugin/Contracts/Deactivator_Interface.php b/src/Plugin/Contracts/Deactivator_Interface.php index 0cc9f18..7214029 100644 --- a/src/Plugin/Contracts/Deactivator_Interface.php +++ b/src/Plugin/Contracts/Deactivator_Interface.php @@ -10,9 +10,7 @@ /** * The library's one way of turning a plugin off. * - * Stated in terms of a plugin basename — "give-recurring/give-recurring.php" — because that is - * the only identifier WordPress itself accepts. Bind a replacement to make deactivation a no-op - * in an environment where plugin state is managed outside WordPress. + * Bind a replacement to make deactivation a no-op where plugin state is managed outside WordPress. * * @since 1.0.0 */ @@ -20,9 +18,8 @@ interface Deactivator_Interface { /** * Deactivate the plugin in every scope it is active in. * - * Called unattended, during plugins_loaded, on behalf of a user who did not ask for it. An - * implementation that reports success without deactivating leaves two copies of the same - * plugin to load, which is the failure this library exists to prevent. + * Called unattended, during plugins_loaded. An implementation that returns without deactivating + * leaves two copies of the same plugin to load, which is the failure this library prevents. * * @since 1.0.0 * diff --git a/src/Plugin/Deactivator.php b/src/Plugin/Deactivator.php index 2cbff93..987e20f 100644 --- a/src/Plugin/Deactivator.php +++ b/src/Plugin/Deactivator.php @@ -27,20 +27,11 @@ class Deactivator implements Deactivator_Interface { public function deactivate( string $basename ): void { $this->load_plugin_functions(); - // Silent, and with no $network_wide argument. - // - // Silent because this is an unattended deactivation, and the standalone's own deactivation - // hook has already been registered this request. Running it at plugins_loaded means a - // routine flush_rewrite_rules() in that callback regenerates the rules before init has - // registered a single post type, and every custom permalink on the site starts 404ing. - // WordPress core makes the same call: its interactive paths are noisy, its automatic ones - // -- validate_active_plugins(), the plugin upgrader -- are silent. - // - // The $network_wide default is null, not false, and null is the value that handles both - // scopes. WordPress core enters the network branch on `false !== $network_wide` and the - // blog branch on `true !== $network_wide`, so null takes both. A computed true would skip - // the blog branch, stranding an entry for a plugin that is active in both, which then takes - // a second request and a second deactivation hook to clear. + // Silent, because a flush_rewrite_rules() in the standalone's deactivation hook runs here + // at plugins_loaded and regenerates the rules before init has registered a post type, + // 404ing every custom permalink. And no $network_wide argument: its null default enters + // both of core's branches, where a computed true would skip the blog one and strand an + // entry for a plugin active in both. deactivate_plugins( $basename, true ); } } diff --git a/src/Plugin/Loads_Plugin_Functions.php b/src/Plugin/Loads_Plugin_Functions.php index 5b5fb74..d23e338 100644 --- a/src/Plugin/Loads_Plugin_Functions.php +++ b/src/Plugin/Loads_Plugin_Functions.php @@ -10,10 +10,8 @@ /** * Makes WordPress's plugin functions available to the class using it. * - * A trait rather than a fifth collaborator: there is nothing here to replace. It has no state, no - * decision and no alternative implementation — the file either is loaded or it is not — so an - * interface and a binding would buy a seam nobody would ever bind to, and both classes that touch - * plugin functions would then have to be handed it. + * A trait rather than a collaborator: no state, no decision and no alternative implementation, so + * an interface and a binding would buy a seam nobody would bind to. * * @since 1.0.0 */ @@ -21,9 +19,9 @@ trait Loads_Plugin_Functions { /** * WordPress only loads these in the admin, and we run at plugins_loaded on every request. * - * Guarded on deactivate_plugins() rather than is_plugin_active(), because the latter is a - * common third-party shim: something else defining it would short-circuit this and leave the - * rest of the file unloaded, so the first call that needs a function nobody shimmed fatals. + * Guarded on deactivate_plugins() rather than is_plugin_active(), because the latter is a common + * third-party shim: a shimmed copy would leave the rest of the file unloaded, and the first call + * to a function nobody shimmed then fatals. * * @since 1.0.0 * diff --git a/src/Provider.php b/src/Provider.php index 789f8f1..3281d34 100644 --- a/src/Provider.php +++ b/src/Provider.php @@ -34,12 +34,9 @@ /** * Teaches the host's container how to build every collaborator this library uses. * - * Binding only. Nothing here hooks, and nothing here resolves: `register()` runs at boot, when a - * host may still be binding, and building an object at that point would pin whichever - * implementation happened to be bound first. - * - * `final` because it is a list of bindings, and the way to change one is to bind it yourself - * before boot rather than to inherit the list. + * Binding only, and nothing here resolves: `register()` runs at boot, when a host may still be + * binding, so building an object then would pin whichever implementation was bound first. `final` + * because a binding is changed by binding it yourself before boot, not by inheriting the list. * * @since 1.0.0 */ @@ -68,9 +65,8 @@ public function __construct( ContainerInterface $container ) { public function register(): void { $container = $this->container; - // The container under its own contract, so that a container which builds unbound classes - // reflectively can still satisfy the two collaborators that take one. Bound first because - // everything below it may be resolved that way. + // The container under its own contract, first of all, so that a container which builds unbound + // classes reflectively can still satisfy the collaborators below that take one. $this->bind_once( ContainerInterface::class, $container ); $this->bind_once( Registrar_Interface::class, Registrar::class ); @@ -82,9 +78,8 @@ public function register(): void { $this->bind_once( Redirector::class ); $this->bind_once( Gatekeeper::class ); - // Explicit factories rather than a class name for everything with a constructor argument: - // container-contract promises `bind`, `get`, `has` and `singleton` and nothing about - // autowiring, so a container that resolves nothing by reflection has to be told. + // Explicit factories for everything with a constructor argument: container-contract promises + // nothing about autowiring, so a container that resolves nothing by reflection has to be told. $this->bind_once( Writer_Interface::class, static function () use ( $container ): Writer { @@ -156,29 +151,11 @@ static function () use ( $container ): Scheduler { } /** - * Bind as a singleton, unless the host already bound something. - * - * The host binds first and wins: this library's defaults are what a container has when nobody - * said otherwise, and a provider that overwrote a binding would make the order in which a host - * calls `set_container()` and `boot()` decide which implementation it gets. It is also what - * keeps a second `register()` harmless, instead of swapping in a fresh registrar and losing - * every sub-plugin registered so far. - * - * The `class_exists()` half is what makes that question answerable at all. `has()` means "can - * return an entry", not "the host bound this" -- di52 answers it with `isBound() || - * class_exists()`, so for a class id it is true before anything has been bound. Asked alone it - * stands down every binding above whose id is a class, the explicit factories included, leaving - * those collaborators autowired where the container autowires, broken where it does not, and - * singletons nowhere. An interface no container can build unprompted, so there the same call - * answers exactly what is being asked. - * - * What that costs is a host rebinding one of the concrete workers, which has to happen after - * boot: nothing here can tell that binding apart from the container's own willingness to build - * the class. The interface seams -- the ones a host is invited to replace -- are unaffected. + * Bind as a singleton -- every binding here is one -- unless the host bound this id first. * - * Singletons throughout. Every one of these is either a registry whose contents are the point - * or a stateless worker, and a second registrar would hold a second, emptier list of - * sub-plugins. + * The guard stands down on interface ids only: `has()` means "can return an entry", not "the host + * bound this" -- di52 answers it with `isBound() || class_exists()` -- so dropping that half would + * stand down every concrete binding above. A host replacing a concrete worker binds after boot. * * @since 1.0.0 * diff --git a/src/Registry/Reader.php b/src/Registry/Reader.php index 8e0ec6e..6e31ee4 100644 --- a/src/Registry/Reader.php +++ b/src/Registry/Reader.php @@ -15,19 +15,9 @@ /** * Every registered sub-plugin, as something a pass can be handed rather than reach for. * - * Registration is buffered before it is stored, and the buffer is static because it has to be: - * `Absorber::register()` is a static call a host makes at plugin-file scope, before there is a - * container to resolve a registrar from. What has to be decided is which class that costs — and it - * is this one, not the facade. Everything that reads the registry (`Conflict\Detector`, - * `Conflict\Resolver`, `Loader`, and `Conflict\Rewriter`) declares this - * object in its constructor, so no collaborator reaches the registry through `Absorber`, and the - * dependency between the facade and the collaborators runs one way. - * - * The buffer is deliberately shared across instances. It is one process's registrations, and a second - * reader holding a second, emptier list is the bug `Provider` binds every collaborator as a singleton - * to avoid. - * - * Not `final`: it is bound by class name, which is the seam a host rebinds and a test subclasses. + * The buffer registrations land in is static because it has to be: `Absorber::register()` is a + * static call a host makes at plugin-file scope, before there is a container to resolve a registrar + * from. It lives here, not on the facade, so the dependency on `Absorber` runs one way. * * @since 1.0.0 */ @@ -60,10 +50,9 @@ public function __construct( Registrar_Interface $registrar ) { /** * Hold a registration until something reads. * - * Static, and it stores rather than registers, because `Absorber::register()` must resolve - * nothing: the host container LearnDash hands us is *replaced* at `plugins_loaded` priority 0, so - * a registration that reached a registrar before that point would go into the container being - * thrown away. Buffering is what lets the container arrive at any point before boot. + * It stores rather than registers because `Absorber::register()` must resolve nothing: a host + * container may still be *replaced* at `plugins_loaded` priority 0, so a registration that + * reached a registrar earlier would land in the one being thrown away. * * A registration that arrives after the load pass has gone by is buffered like any other and * reported, because a buffer nothing reads again leaves next to nothing behind to go on: no @@ -117,8 +106,7 @@ public static function buffer( Sub_Plugin $sub_plugin ): void { * * The buffer is drained on the way past, which is why a pass is handed this rather than the * registrar it could resolve for itself: a registrar asked directly would miss everything - * registered since the last read, a host registering from its own `plugins_loaded` callback - * included. + * registered since the last read. * * A read always answers with what the registrar legitimately holds. A duplicate slug is refused * and reported as it drains, never raised out of here: every caller is inside `plugins_loaded`, @@ -132,12 +120,8 @@ public static function buffer( Sub_Plugin $sub_plugin ): void { public function all(): array { $this->flush(); - // Registrar_Interface::all() can only declare `array` -- PHP 7.4 has no way to say - // array in a signature -- so a host binding its own registrar may return - // anything at all. Narrowed once here, where the untrusted value crosses into the library, - // rather than at each call site: a consumer that forgot the check would fatal inside - // plugins_loaded on its first predicate call, which is the exact failure this library - // exists to prevent, and every future consumer would have to remember it too. + // A host may bind a registrar returning anything, and PHP 7.4 cannot say array + // in the interface signature -- so narrow once here, where the untrusted value enters. return array_filter( $this->registrar->all(), static function ( $sub_plugin ): bool { @@ -147,17 +131,10 @@ static function ( $sub_plugin ): bool { } /** - * Hand every buffered registration to the registrar. - * - * Public because the drain is wanted without the read. `Absorber::registrar()` hands a host the - * registrar itself, which is empty until something drains into it, and reading `all()` there for - * its effect and discarding the list said nothing about why the line was there. The two are the - * whole of this class's surface and a rebound reader owes both: `all()` is this method plus the - * registrar's contents, narrowed. + * Hand every buffered registration to the registrar, which stays the single source of truth. * - * The registrar stays the single source of truth: the buffer is a pre-store that needs no - * container, and duplicate-slug detection and ordering remain the registrar's alone rather than - * being restated here in a second dialect. + * Public because the drain is wanted without the read: `Absorber::registrar()` hands back a + * registrar that is empty until something drains into it, and a rebound reader owes both halves. * * The buffer is emptied before the loop, so a second read cannot re-register what the registrar * already holds and trip its duplicate-slug guard. Nothing has to empty it *after* a failure diff --git a/src/Registry/Registrar.php b/src/Registry/Registrar.php index a832585..a0fd64f 100644 --- a/src/Registry/Registrar.php +++ b/src/Registry/Registrar.php @@ -26,11 +26,8 @@ class Registrar implements Registrar_Interface { * Register a sub-plugin. * * A slug is an identity, not a key this map happens to use: it also names the sub-plugin's - * notices and its once-ever activation record. Letting a second registration win would drop - * the first sub-plugin from the load silently and hand its activation record to the winner, so - * the collision is refused instead. There is no legitimate second registration to protect — - * a decision the host cannot make up front belongs in the `enabled` callable, which is - * re-evaluated on every load, not in a second call to this method. + * notices and its once-ever activation record, so letting a second registration win would + * silently drop the first from the load and hand it that record. * * @since 1.0.0 * @@ -45,7 +42,7 @@ public function register( Sub_Plugin $sub_plugin ): void { if ( isset( $this->sub_plugins[ $slug ] ) ) { // Both files, because the two registrations routinely come from different host plugins - // and the stack trace only shows the one that lost. + // and the stack trace shows only the one that lost. throw new Config_Exception( sprintf( 'Two sub-plugins are registered under the slug "%1$s": %2$s and %3$s.' diff --git a/src/Sub_Plugin.php b/src/Sub_Plugin.php index 1376926..32aad71 100644 --- a/src/Sub_Plugin.php +++ b/src/Sub_Plugin.php @@ -13,12 +13,9 @@ * One registered sub-plugin: its configuration, and every answer it can give without a * container-bound collaborator. * - * Not the same as "configuration alone". is_already_loaded() reads the global constant table, and a - * callable under `enabled` may query whatever the host likes. The line drawn here is dependency - * direction: asking whether the standalone counterpart is active needs Checker_Interface, and - * resolving one would put a container read in front of Absorber::register(), which deliberately - * performs none so that the container may arrive at any point before boot. This object only names - * the plugin to ask about. + * The line is dependency direction, not "configuration alone": resolving a Checker_Interface to ask + * whether the standalone is active would put a container read in front of Absorber::register(), + * which performs none so the container may arrive any time before boot. * * @since 1.0.0 * @@ -37,8 +34,6 @@ */ class Sub_Plugin { /** - * Keys without which this object cannot do its job. - * * @since 1.0.0 * * @var string[] @@ -46,8 +41,6 @@ class Sub_Plugin { private const REQUIRED_KEYS = [ 'slug', 'bundled_plugin_file', 'plugin_loaded_constant' ]; /** - * Keys that are only ever a callable, never a value. - * * @since 1.0.0 * * @var string[] @@ -55,8 +48,6 @@ class Sub_Plugin { private const CALLABLE_KEYS = [ 'dependency_check', 'activation_callback' ]; /** - * Optional keys that are only ever a string, never a callable. - * * @since 1.0.0 * * @var string[] @@ -64,7 +55,7 @@ class Sub_Plugin { private const STRING_KEYS = [ 'standalone_plugin_basename' ]; /** - * Keys that take a string, or something to call for one when it is wanted. + * Keys taking a string, or something to call for one. * * @since 1.0.0 * @@ -73,7 +64,7 @@ class Sub_Plugin { private const DEFERRABLE_KEYS = [ 'conflict_policy' ]; /** - * Keys that carry human-readable text, and so only ever hold something to call for it. + * Keys carrying user-facing text, which hold something to call for it, never the text. * * @since 1.0.0 * @@ -95,9 +86,8 @@ class Sub_Plugin { * * @param array $config Sub-plugin configuration. * - * @throws Config_Exception When a required key is missing, empty, or not a string; when a - * callable-only key holds something that cannot be called; or when a - * message key holds a string rather than something to call for one. + * @throws Config_Exception When a required key is missing, empty or not a string, a callable-only + * key cannot be called, or a message key holds a string. */ public function __construct( array $config ) { foreach ( self::REQUIRED_KEYS as $required ) { @@ -111,9 +101,8 @@ public function __construct( array $config ) { ); } - // Not just a truthiness check. An array survives one of those and then casts to the - // string "Array", which every sub-plugin with the same mistake would share as its - // registry key, its activation-tracking key, and its notice id. + // is_string(), not truthiness: an array passes truthiness, then casts to the string + // "Array", which every sub-plugin making the mistake would share as its key. if ( ! is_string( $config[ $required ] ) || $config[ $required ] === '' ) { throw new Config_Exception( sprintf( @@ -126,10 +115,8 @@ public function __construct( array $config ) { } } - // Rejected here rather than ignored at read time, where "not configured" and "configured - // but uncallable" would collapse into the same answer. A dependency_check that is a - // private method or a typo'd function name would otherwise report dependencies met and - // let the load proceed into the fatal it exists to prevent. + // Rejected at registration, not at read time, where "not configured" and "configured but + // uncallable" collapse and a typo'd dependency_check would report dependencies met. foreach ( self::CALLABLE_KEYS as $key ) { if ( isset( $config[ $key ] ) && ! is_callable( $config[ $key ] ) ) { throw new Config_Exception( @@ -143,9 +130,7 @@ public function __construct( array $config ) { } } - // A basename names a file already on disk; nothing about it waits on anything. A closure - // under it would reach deactivate_plugins() as the string "Closure", so it is worth a loud - // failure at registration rather than a puzzling one much later. + // A basename names a file already on disk, so there is nothing here to defer. foreach ( self::STRING_KEYS as $key ) { if ( isset( $config[ $key ] ) && ! is_string( $config[ $key ] ) ) { throw new Config_Exception( @@ -159,9 +144,7 @@ public function __construct( array $config ) { } } - // Anything that is neither a string nor a callable is rejected where the mistake was made. - // At read time an array would cast to the string "Array", and a [ class, method ] pair - // naming a method that does not exist would become the value itself. + // Either form: a policy is usually a constant with nothing to defer, and is never text. foreach ( self::DEFERRABLE_KEYS as $key ) { if ( ! isset( $config[ $key ] ) ) { continue; @@ -180,11 +163,8 @@ public function __construct( array $config ) { } } - // A string is refused outright here, unlike everywhere else. Text a host has already - // translated cannot be told from text it has not, and a config array is built before init - // -- so accepting the one shape that can only have been produced too early would leave the - // just-in-time textdomain notice these keys exist to prevent. One `static fn()` at the call - // site fails here the first time the code runs, rather than in someone else's log. + // Any string is refused, not just a string callable: a config array is built before init, so + // text here can only have been translated too early, and the value cannot say it was not. foreach ( self::MESSAGE_KEYS as $key ) { if ( ! isset( $config[ $key ] ) ) { continue; @@ -205,8 +185,8 @@ public function __construct( array $config ) { } } - // The loops above have proved every key this class reads. Only `enabled` is read without a - // type behind it, and its two forms -- a bool and a callable -- cannot be confused. + // `enabled` is the one key with no type behind it: it is read as a bool when it is not + // callable, so an array or an object there counts as enabled. /** @var Sub_Plugin_Config $config */ $this->config = $config; } @@ -245,9 +225,8 @@ public function get_plugin_loaded_constant(): string { /** * The configured policy, after the filter has had the final say. * - * The result is not checked against the known policies here: a filter may legitimately - * return anything, and rejecting it at this boundary would hide the override rather than - * report it. Callers that dispatch on the value check it with Conflict_Policy::is_valid(). + * The result is deliberately not validated here — rejecting a filter's return would hide the + * override rather than report it. Callers that dispatch on it use Conflict_Policy::is_valid(). * * @since 1.0.0 * @@ -262,12 +241,10 @@ public function get_conflict_policy(): string { /** * Filters the policy applied when a standalone copy of a sub-plugin is found active. * - * The dynamic portion of the hook name, `$hook_prefix`, is the prefix given to - * Config::set_hook_prefix(). + * The dynamic portion, `$hook_prefix`, is the prefix given to Config::set_hook_prefix(). * - * Fires after the configured value and its default, so a host can decide per request - * rather than at registration. Returning a value that is not one of the Conflict_Policy - * constants is not consent to deactivate — callers leave the standalone alone instead. + * Fires last, after the configured value and its default, so a host can decide per request. A + * return that is not a Conflict_Policy constant is not consent to deactivate. * * @since 1.0.0 * @@ -276,8 +253,7 @@ public function get_conflict_policy(): string { */ $policy = apply_filters( Config::get_hook_name( 'conflict_policy' ), $policy, $this ); - // An empty string matches no known policy, so a filter that returned something uncastable - // lands at the conservative branch rather than at a deactivation. + // An uncastable filter return becomes '', which matches no policy, so callers stay put. return $this->as_string( $policy ); } @@ -289,17 +265,15 @@ public function get_conflict_policy(): string { public function is_enabled(): bool { $enabled = $this->config['enabled'] ?? true; - // A bool is never callable, so nothing here is guessing at intent the way it would if this - // key also took a string. Every callable form works, a plain function name included, and a - // callable is re-evaluated on each call rather than resolved once at registration. + // No string form here, so a plain function name is called too, deliberately, on every read. return (bool) ( is_callable( $enabled ) ? $enabled( $this ) : $enabled ); } /** * True when the plugin's code is already present, from either copy. The fatal guard. * - * Only sound when the constant is defined at file scope. A standalone that defines it from a - * bootstrap hooked at plugins_loaded or later has not defined it yet when this is asked. + * Only sound when the constant is defined at file scope: a standalone defining it from a hook at + * plugins_loaded or later has not defined it yet when this is asked. * * @since 1.0.0 * @@ -345,11 +319,7 @@ public function are_dependencies_met(): bool { /** * Shown when the standalone is auto-deactivated, and when the user tries to re-activate it. * - * The fallback is a parameter because the two contexts want different wording, and because - * a caller with no fallback of its own would otherwise render nothing at all. - * - * Configured as a callable and never as a string, so that __() cannot run in the config array, - * where it would run before the textdomain is loaded. + * The fallback is a parameter because those two contexts want different wording. * * @since 1.0.0 * @@ -371,11 +341,10 @@ public function get_conflict_notice_message( string $default = '' ): string { * Filters the notice shown when a standalone copy is deactivated, and when the user tries * to activate it again. * - * The dynamic portion of the hook name, `$hook_prefix`, is the prefix given to - * Config::set_hook_prefix(). + * The dynamic portion, `$hook_prefix`, is the prefix given to Config::set_hook_prefix(). * - * Fires when the message is asked for rather than when the sub-plugin is registered, which - * is what makes this the place to translate it: the textdomain is loaded by then. + * Fires last, when the message is asked for rather than at registration, so the textdomain + * is loaded and the text can be translated here. * * @since 1.0.0 * @@ -384,15 +353,11 @@ public function get_conflict_notice_message( string $default = '' ): string { */ $message = apply_filters( Config::get_hook_name( 'conflict_notice_message' ), $message, $this ); - // An empty string renders no notice, which is where a filter returning an array or an - // object lands rather than in a fatal cast. return $this->as_string( $message ); } /** - * Shown when a dependency_check fails. Falls back to a generic, untranslated sentence — the key - * takes a callable, and never a string, so that a host's own __() cannot run in its config - * array, before the textdomain is loaded. + * Shown when a dependency_check fails, falling back to a generic, untranslated sentence. * * @since 1.0.0 * @@ -414,12 +379,9 @@ public function get_dependency_notice_message(): string { /** * Filters the notice shown when a sub-plugin's dependency_check fails. * - * The dynamic portion of the hook name, `$hook_prefix`, is the prefix given to - * Config::set_hook_prefix(). + * The dynamic portion, `$hook_prefix`, is the prefix given to Config::set_hook_prefix(). * - * Fires when the message is asked for rather than when the sub-plugin is registered, which - * is what makes this the place to translate the generic default: the textdomain is loaded - * by then. + * Fires when the message is asked for, so the default can be translated here. * * @since 1.0.0 * @@ -428,16 +390,14 @@ public function get_dependency_notice_message(): string { */ $message = apply_filters( Config::get_hook_name( 'dependency_notice_message' ), $message, $this ); - // An empty string renders no notice, which is where a filter returning an array or an - // object lands rather than in a fatal cast. return $this->as_string( $message ); } /** - * Shown on multisite when a network-active standalone is left active because the host plugin is - * not itself network-activated -- deactivating it network-wide would strand the sites the host - * never reached. Self-contained, with no config key: the text has no per-host variant worth a - * registration-time value, and the filter below is the seam for rewording or translating it. + * Shown when a network-active standalone is left active to avoid stranding sites. + * + * Deactivating it network-wide would take it from the sites a host that is not network-activated + * never reaches. There is no config key -- the filter below is the seam for rewording it. * * @since 1.0.0 * @@ -456,14 +416,11 @@ public function get_stranding_notice_message(): string { ); /** - * Filters the notice shown when a network-active standalone is left active to avoid stranding - * the sites a host that is not network-activated does not reach. + * Filters the notice shown when a network-active standalone is left active. * - * The dynamic portion of the hook name, `$hook_prefix`, is the prefix given to - * Config::set_hook_prefix(). + * The dynamic portion, `$hook_prefix`, is the prefix given to Config::set_hook_prefix(). * - * Fires when the message is asked for rather than when the sub-plugin is registered, which is - * what makes this the place to translate the default: the textdomain is loaded by then. + * Fires when the message is asked for, so the default can be translated here. * * @since 1.0.0 * @@ -472,8 +429,6 @@ public function get_stranding_notice_message(): string { */ $message = apply_filters( Config::get_hook_name( 'stranding_notice_message' ), $message, $this ); - // An empty string renders no notice, which is where a filter returning an array or an - // object lands rather than in a fatal cast. return $this->as_string( $message ); } @@ -491,19 +446,9 @@ public function get_activation_callback(): ?callable { /** * Take a configured value as it stands, or call it for one. * - * A string is always the value itself, however real a function of that name happens to be. - * `date`, `flush` and `key` are all existing functions and all plausible configured values, so - * honouring a string as a call would make the answer depend on what else the site had loaded -- - * and calling one of those with a Sub_Plugin argument is a fatal, not a value. Only the policy - * arrives here as one; the message keys take no string at all. - * - * Every other callable form says "call me" and nothing else: a closure, a [ class, method ] - * pair, an invokable object, a container callback. That is what lets a host defer __() to the - * moment the text is wanted, rather than translating while it builds its config array -- which - * happens before init, and is what raises WordPress's just-in-time textdomain notice. - * - * Called on every read. Resolving once in the constructor would move the too-early call from - * the config array to the line after it. + * A string is always the value itself, however real a function of that name happens to be: + * `date`, `flush` and `key` are all functions and all plausible configured values. Every other + * callable form says "call me", which is what lets a host defer __() until the text is wanted. * * @since 1.0.0 * @@ -512,8 +457,7 @@ public function get_activation_callback(): ?callable { * @return mixed */ private function resolve_deferred( $value ) { - // The constructor has already made this good. It stays because reaching a call on something - // uncallable would be a fatal where the wrong shape is merely a notice that says so. + // Proved by the constructor already, but kept: calling something uncallable is a fatal. if ( is_string( $value ) || ! is_callable( $value ) ) { return $value; } @@ -524,13 +468,9 @@ private function resolve_deferred( $value ) { /** * Reduce whatever a filter or a configured callable returned to a string. * - * Only the cast is shared. Each filter is applied at the method that owns it rather than - * through a common helper, so that every hook keeps a documented call site a reader and a hook - * scanner can both find, and a version of its own. - * - * Both run at plugins_loaded, where casting an array or an object would be a fatal. Anything - * uncastable is treated as though nothing had come back at all; what nothing means is for each - * caller to say. + * Only the cast is shared -- each filter stays applied at the method that owns it, so every hook + * keeps a documented call site. An array or an object would fatal on the cast, so it reads as + * nothing come back, and what nothing means is each caller's to say. * * @since 1.0.0 * @@ -545,16 +485,8 @@ private function as_string( $value ): string { /** * Name the entry a rejection is about, for a host registering several from one loop. * - * A host calling Absorber::register() inside a loop -- over a manifest array of five entries, - * for example -- gets the same file and line in the stack trace whichever entry was rejected, - * because there is only the one call. The rest of the message names the key at fault; only this - * names the entry that key belongs to. - * - * The slug first, because it is what every other message about a sub-plugin names, and the - * bundled file when the slug is itself the key at fault -- it identifies the entry just as well, - * and it names the file the host has to go and look at. Both are read defensively, since either - * may be the missing or malformed key being reported; when neither can be read, saying so is - * still better than a message that quietly names nothing. + * A host calling Absorber::register() in a loop gets the same file and line in the stack trace + * whichever entry was rejected. Both are read defensively: either may be the key at fault. * * @since 1.0.0 * @@ -577,9 +509,9 @@ private function describe_config_entry( array $config ): string { /** * Name the type a key was given, so that a stray value can be found rather than hunted for. * - * An object reports its class: "object given" leaves a Closure and a WP_Error looking the same, - * and they are very different mistakes. The empty string is named as one, because "string given" - * under a key that must be a non-empty string says nothing the host does not already know. + * An object reports its class, since "object given" leaves a Closure and a WP_Error looking the + * same, and the empty string is named as one, since "string given" under a key that must be a + * non-empty string says nothing. * * @since 1.0.0 * diff --git a/src/Traits/Guards_Hook_Prefix.php b/src/Traits/Guards_Hook_Prefix.php index 735bfb6..e9f8c66 100644 --- a/src/Traits/Guards_Hook_Prefix.php +++ b/src/Traits/Guards_Hook_Prefix.php @@ -13,13 +13,9 @@ /** * Turns "no hook prefix has been set" into something a developer sees and a request survives. * - * Every entry point this library puts on a hook needs the prefix — it names the should_load filter, - * the conflict and notice filters, and the option the notice queue lives in — and every one of them - * is reached from a core hook, where throwing would take the whole site down over a bootstrap - * mistake. So the load pass, the conflict pass, the gatekeeper and both of the facade's admin - * trampolines ask this first and stand down when the answer is no. A trait rather than a shared - * collaborator because the answer comes from `Config` either way; all that is shared is how the - * mistake is reported. + * Every entry point needing the prefix is reached from a core hook, where throwing would take the + * whole site down over a bootstrap mistake — so each asks this first and stands down on a no. A + * trait, not a collaborator: the answer comes from `Config` either way, only the report is shared. * * @since 1.0.0 */ diff --git a/src/Traits/Guards_Plugin_Capability.php b/src/Traits/Guards_Plugin_Capability.php index 695918d..3fceb29 100644 --- a/src/Traits/Guards_Plugin_Capability.php +++ b/src/Traits/Guards_Plugin_Capability.php @@ -10,27 +10,13 @@ /** * The one answer to "may this user act on a plugin, at the reach this library's acts have". * - * Two places ask it about the same act. `Conflict\Gatekeeper` asks before a standalone is - * deactivated, and `Notices\Presenter` asks before the queue reporting that deactivation is printed - * — which also clears it, for everybody. So the two have to agree by construction: a consume gate - * looser than the resolve gate hands the only copy of a notice to somebody who could not have caused - * what it reports and cannot undo it. + * `Conflict\Gatekeeper` asks before a standalone is deactivated, `Notices\Presenter` before the + * queue reporting it is printed and cleared for everybody. They have to agree by construction: a + * looser consume gate hands the only copy of a notice to somebody who cannot act on it. * - * The capability is network-scoped wherever a network exists, because that is how far the act - * reaches. `Plugin\Deactivator` leaves `deactivate_plugins()`'s `$network_wide` at its default, which - * core reads as both scopes, so the standalone comes out of the network's active plugins whichever - * site the request arrived on; and the notice queue is one network option, so consuming it consumes - * it for every site. `activate_plugins` does not establish that authority and cannot be relied on to - * imply it: core widens it into the network capability only while the `menu_items` site option keeps - * the Plugins menu off, so on a network that has turned that menu on, every site administrator holds - * the site capability outright and none of them holds the network one. - * - * A trait rather than a shared collaborator, for the same reason `Guards_Hook_Prefix` is one: the - * answer comes from `current_user_can()` either way, and all that is shared is which capability to - * name. A collaborator would want a container binding, a constructor argument on both classes and an - * interface nothing dispatches on, to carry one boolean. What it may not be is a literal in each - * class with a docblock in each pointing at the other — that is what the two had, and they drifted - * apart without a single test noticing. + * Network-scoped wherever a network exists, because that is how far the act reaches. + * `activate_plugins` cannot be relied on to imply the network capability: core widens it only while + * the `menu_items` site option keeps the Plugins menu off, and that menu is usually on. * * @since 1.0.0 */ From c50ca5c4bb930bb7404543861f2896f9bb2c2d6c Mon Sep 17 00:00:00 2001 From: Nikolay Strikhar Date: Tue, 25 Aug 2026 13:23:20 +0200 Subject: [PATCH 02/10] Let the docs point at the fact instead of restating it The how-to file had been re-explaining the mechanisms the reference files own. recipes.md restated conflict-handling.md's "a policy is only reached for a sub-plugin that is enabled..." at 93% overlap, and configuration.md's activation record semantics again; conflict-handling.md restated notices.md's render-then-clear. Each fact now sits in the file whose subject it is, and the links already in the text carry the reader. Every heading and every link is byte-identical, so the anchors other files target still resolve. Code blocks, the mermaid diagram and table rows are untouched. 8,084 words to 6,047. filters.md and the README are already dense and keep their length. --- README.md | 21 +++-- docs/configuration.md | 154 ++++++++++++++-------------------- docs/conflict-handling.md | 169 ++++++++++++++++---------------------- docs/extending.md | 154 ++++++++++++++-------------------- docs/filters.md | 33 ++++---- docs/installing.md | 13 ++- docs/notices.md | 54 ++++++------ docs/recipes.md | 107 +++++++++--------------- 8 files changed, 285 insertions(+), 420 deletions(-) diff --git a/README.md b/README.md index 5240a83..59db757 100644 --- a/README.md +++ b/README.md @@ -41,15 +41,15 @@ you already hand to Telemetry or Uplink. If your plugin can run on multisite, add `Config::set_host_plugin_basename( plugin_basename( __FILE__ ) )`. It is a no-op off a network, so set -it unconditionally: where it matters is the one topology the library must not deactivate a standalone -in — a network-active standalone whose host plugin is not itself network-activated, where a -network-wide deactivation would leave the network's other sites with no copy of it at all. +it unconditionally: it stops the library deactivating a network-active standalone when your host +plugin is not itself network-activated, which would leave the network's other sites with no copy of +it at all. -**Keep the `, 0`.** Anything below `plugins_loaded` priority 5 wires cleanly, but priority 0 is the -recommendation, in the block that owns your container rather than in a service provider. Booting at 5 -or later still works and is reported through `_doing_it_wrong()`, with the whole sequence running -inline instead. [Configuration][configuration] explains both, and closes with a complete bootstrap — -two sub-plugins, every optional key. +**Keep the `, 0`.** Anything below `plugins_loaded` priority 5 wires cleanly; priority 0, in the +block that owns your container rather than a service provider, is the recommendation. Booting at 5 +or later still works, with the whole sequence running inline and reported through +`_doing_it_wrong()`. [Configuration][configuration] explains both, and closes with a complete +bootstrap — two sub-plugins, every optional key. ## Docs @@ -65,9 +65,8 @@ two sub-plugins, every optional key. - [Tests][tests] — running the suite, the fixtures and traits it offers, and every scenario it drives the library through. -`docs/` and `tests/` are both `export-ignore`d, so neither is in a vendored copy of this library — -these point at the repository rather than at a path that would be missing beside the installed -source. +`docs/` and `tests/` are both `export-ignore`d, so neither ships in a vendored copy — these links +point at the repository rather than at paths that would be missing beside the installed source. [installing]: https://github.com/stellarwp/plugin-absorber/blob/main/docs/installing.md [configuration]: https://github.com/stellarwp/plugin-absorber/blob/main/docs/configuration.md diff --git a/docs/configuration.md b/docs/configuration.md index 7195a2d..b1f753a 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -9,65 +9,47 @@ Config::set_hook_prefix( 'give' ); // required — keys hooks and optio Config::set_container( give()->container ); // required — everything is resolved from it ``` -The hook prefix accepts letters, numbers, hyphens, and underscores. Anything else throws -`Config_Exception`, as does reading the prefix before it is set. Hook names repeat it verbatim; -option names lowercase it and turn hyphens into underscores, so `Give-Core` hooks -`Give-Core/plugin_absorber/should_load` and stores `give_core_plugin_absorber_notices`. +The hook prefix accepts letters, numbers, hyphens, and underscores; anything else throws +`Config_Exception`, as does reading it before it is set. Hook names repeat it verbatim; option +names lowercase it and fold hyphens to underscores. -Any implementation of StellarWP's `ContainerInterface` will do — the one your plugin already -hands to Telemetry, Uplink or Harbor. It is required: `Config::get_container()` throws -`Config_Exception` when none is set, and `Config::has_container()` is the probe. To replace one -of the library's own pieces, see [Extending](extending.md). +Any implementation of StellarWP's `ContainerInterface` will do. `Config::get_container()` throws +`Config_Exception` when none is set; `Config::has_container()` is the probe. To replace one of the +library's own pieces, see [Extending](extending.md). `Config::set_host_plugin_basename( plugin_basename( __FILE__ ) )` is optional and matters only on -multisite. It lets the library compare your plugin's activation scope against a standalone's, so it -declines to deactivate a network-active standalone when your plugin is not itself network-activated -— a deactivation that would otherwise pull the standalone from the network's other sites, where your -bundled copy never loads. Left unset, that guard stays off and deactivation behaves as it always has. +multisite: it stops the library deactivating a network-active standalone when your plugin is not +itself network-activated, stranding the network's other sites. Left unset, nothing changes. -Both calls belong at `plugins_loaded` priority 0, in the block that owns your container rather -than in a service provider. Priority matters twice, for unrelated reasons: - -- Conflict resolution runs at `plugins_loaded` priority 5 and the load at 6, and WordPress - ignores a callback added at or past the priority it is already dispatching. Boot after that - and the whole sequence runs inline, reported with `_doing_it_wrong()`. -- A host that builds its container lazily may *replace* it at priority 0. Hand this library the - container before that happens and it holds an orphan whose bindings were discarded. - -Only the second reason picks 0 out of 0 through 4; if your container is already built by then, -anywhere below 5 works. Order among the configuration calls does not matter, so long as they all -precede `Absorber::boot()`. - -`Absorber::boot()` is idempotent, so calling it from more than one code path is safe — but the -first call is the one that counts, and the container it saw is the container the library keeps. -Setting a different one afterwards binds nothing into it, and leaves the accessors asking a -container that was never taught about this library. Set the container once, before booting. +Both calls belong at `plugins_loaded` priority 0, in the block that owns your container rather than +in a service provider, since a host that builds its container lazily may replace it at priority 0. +Anything below 5 works for a container already built — conflict resolution runs at +`plugins_loaded` priority 5 and the load at 6, and booting at 5 or later runs the whole sequence +inline, reported with `_doing_it_wrong()`. `Absorber::boot()` is idempotent but keeps the container +its first call saw. ## Registering a sub-plugin | Key | Type | Required | Meaning | |---|---|:--:|---| | `slug` | `string` | ✔ | Unique id — registry key, notice id, activation-tracking key. | -| `bundled_plugin_file` | `string` | ✔ | Absolute path to the **bundled** plugin's main file. This is what gets `require_once`d. | -| `plugin_loaded_constant` | `string` | ✔ | A constant the plugin defines when it loads, **at file scope**. Both copies normally define the *same* name, which is what stands one of them down. **Load guard only** — see [Conflict handling](conflict-handling.md#the-load-guard). | -| `standalone_plugin_basename` | `string` | | The standalone's `dir/file.php` basename, used to detect and deactivate it. Omit when there is no standalone. **Detection only.** | +| `bundled_plugin_file` | `string` | ✔ | Absolute path to the **bundled** plugin's main file — what gets `require_once`d. | +| `plugin_loaded_constant` | `string` | ✔ | A constant the plugin defines **at file scope** when it loads; both copies normally define the *same* name. **Load guard only** — see [Conflict handling](conflict-handling.md#the-load-guard). | +| `standalone_plugin_basename` | `string` | | The standalone's `dir/file.php` basename, used to detect and deactivate it; omit when there is none. **Detection only.** | | `enabled` | `bool\|callable` | | `true` by default. A `callable( Sub_Plugin ): bool` is re-evaluated on every call, not cached. | | `conflict_policy` | `string\|callable` | | `Conflict_Policy::DEACTIVATE` by default. See [Conflict handling](conflict-handling.md#policies). | -| `conflict_notice_message` | `callable` | | Used in all three places a conflict is reported — the merge notice, the still-active notice, and the rewritten activation-error screen. Each falls back to its own generic sentence naming the slug. | +| `conflict_notice_message` | `callable` | | Words all three conflict reports: the merge notice, the still-active notice, and the rewritten activation-error screen. Each falls back to a generic sentence naming the slug. | | `dependency_notice_message` | `callable` | | Shown when `dependency_check` fails. Defaults to a generic, untranslated sentence naming the raw slug. | | `activation_callback` | `callable( Sub_Plugin )` | | Runs **once, ever**, per slug, after a successful load. Make it idempotent. | | `dependency_check` | `callable( Sub_Plugin ): bool` | | Skips the load and queues a notice when it returns false. | -Sub-plugins load in **registration order**, so register a dependency before anything that -extends it at include time, and register each slug exactly once. A config array the library -cannot use throws `Config_Exception` on the spot, in the call you can see in your own stack -trace. A duplicate slug is found later, at the first read of the registry — normally on -`plugins_loaded` — since registrations are buffered until then: it is refused there and reported -through `_doing_it_wrong()`, the first registration under the slug stands, and the second is -discarded. +Sub-plugins load in **registration order**, so register a dependency before anything that extends +it at include time, and register each slug exactly once. An unusable config array throws +`Config_Exception` from `Absorber::register()`; a duplicate slug is caught later, at the first read +of the registry on `plugins_loaded`, where the second registration is discarded and reported through +`_doing_it_wrong()`. -Register unconditionally and put anything you cannot decide up front — a licence, a setting the -site owner can change — in `enabled`, which is re-evaluated on every load. See +Register unconditionally: anything you cannot decide up front belongs in `enabled`. See [Toggle a sub-plugin from a setting](recipes.md#toggle-a-sub-plugin-from-a-setting). ## How a sub-plugin loads @@ -89,20 +71,14 @@ flowchart TD F --> G["activation_callback, once ever"] ``` -Only the dependency gate says anything to the site owner — see [Notices](notices.md). An -unreadable `bundled_plugin_file` is a broken build in your plugin, so it is reported with -`_doing_it_wrong()` instead. - -The guard constant is checked **before** the dependency check, so a plugin the admin can watch -working is never reported as missing its requirements. The -[`should_load` filter](filters.md#the-load-gate) sits last and can only veto: it cannot force a -load past a copy already in memory. +Only the dependency gate says anything to the site owner — see [Notices](notices.md). The guard +constant is deliberately checked before the dependency check, and the +[`should_load` filter](filters.md#the-load-gate) can only veto. ## Activation -A bundled plugin is `require_once`d, not activated, so `register_activation_hook()` never fires -for it. Whatever that hook would have done — create a table, seed options — goes in -`activation_callback` instead: +A bundled plugin is `require_once`d, not activated, so `register_activation_hook()` never fires for +it. Whatever that hook would have done goes in `activation_callback` instead: ```php 'activation_callback' => static function ( Sub_Plugin $sub_plugin ) { @@ -110,34 +86,29 @@ for it. Whatever that hook would have done — create a table, seed options — }, ``` -**The wrapper is load-bearing, not decoration.** Like every callable key, `activation_callback` is -checked with `is_callable()` at registration — which happens before the bundled plugin has been -`require_once`d, so before `Give\Recurring\Install` exists. `[ Install::class, 'create_tables' ]` -fails that check and throws `Config_Exception`, on a class that would have been perfectly loadable by -the time the callback ran. A closure is callable the moment it is written and names the class only -when the load pass calls it, with the plugin's code already in memory. - -It runs once ever per slug, is passed the `Sub_Plugin`, and only after a require that actually -happened — never for a sub-plugin whose load was skipped. The record lives in the -`{option_prefix}_plugin_absorber_activations` option, a network option on multisite, and is -written *after* the callback returns, so a callback that throws is reported with -`_doing_it_wrong()` and retried next request rather than marked done for good. - -**Write it to be idempotent.** "Once, ever" is bookkeeping, not a lock: the record is read, the -callback runs, and the record is written, so two first requests arriving together can both pass -the check. A `dbDelta()` migration survives that; a blind `INSERT` of seed rows does not. - -One record for the network is also one *run* for the network, in whichever site's request -reached the load pass first. A site created on the network **after** that run never gets the -callback at all: the record is already set, and nothing re-reads it per site. Per-site work — a -`$wpdb->prefix` table, a per-site option — is yours to loop over, and a site created later is yours -to catch on `wp_initialize_site`: see +**The wrapper is load-bearing.** `activation_callback` is `is_callable()`-checked at registration, +before the bundled plugin is loaded, so `[ Install::class, 'create_tables' ]` throws +`Config_Exception` on a class that does not exist yet. A closure names it only when the load pass +calls it. + +It runs only after a require that actually happened. The record lives in the +`{option_prefix}_plugin_absorber_activations` option — a network option on multisite — and is +written *after* the callback returns, so one that throws is reported with `_doing_it_wrong()` and +retried next request. + +**Write it to be idempotent.** "Once, ever" is bookkeeping, not a lock: two first requests arriving +together can both run the callback. A `dbDelta()` migration survives that; a blind `INSERT` of seed +rows does not. + +One record for the network is one *run* for the network, in whichever site's request reached the +load pass first; a site created afterwards never gets it. Per-site work is yours to loop over, and +a later site yours to catch on `wp_initialize_site`: see [Do per-site work on multisite](recipes.md#do-per-site-work-on-multisite). ## What changes for the bundled plugin -WordPress includes plugins at global scope; this library includes them from inside a method, so -variables assigned at the top level of the bundled file are function-local, not globals: +This library includes bundled plugins from inside a method, not at global scope, so variables +assigned at the top level of the bundled file are function-local, not globals: ```php // In the bundled plugin's main file. @@ -150,9 +121,9 @@ is unaffected. ## Messages are callables, never strings -Your config array is built at plugin load — before `init`, and before your textdomain, so -calling `__()` there raises WordPress's `_load_textdomain_just_in_time` notice. The two message -keys therefore take something to call, and refuse a string outright: +Your config array is built before `init` and before your textdomain, so `__()` there raises +WordPress's `_load_textdomain_just_in_time` notice. The two message keys therefore take something +to call, and refuse a string outright: ```php 'conflict_notice_message' => static fn() => __( 'Recurring ships with Give now.', 'give' ), @@ -165,29 +136,24 @@ keys therefore take something to call, and refuse a string outright: 'conflict_notice_message' => __( 'Recurring ships with Give now.', 'give' ), ``` -Each callable is passed the `Sub_Plugin` and called on every read; a return that will not cast -to a string is treated as though nothing were configured. - -**A plain function name is text, not a call.** `date`, `flush` and `key` are all real functions -and all plausible values, so wherever a string *is* accepted it is the value itself — which bars -`'Give_Recurring::get_conflict_message'` as much as `'give_recurring_conflict_message'`. +Each callable is passed the `Sub_Plugin` and called on every read; a return that will not cast to a +string is treated as though nothing were configured. Wherever a string *is* accepted it is the +value itself and never a function name to call. -`conflict_policy` is the one key that takes either, since a policy is never text a user reads: +`conflict_policy` takes either, since a policy is never text a user reads: ```php 'conflict_policy' => Conflict_Policy::DEFER, 'conflict_policy' => static fn( Sub_Plugin $sub_plugin ) => give_conflict_policy_for( $sub_plugin ), ``` -`standalone_plugin_basename` takes a string only: it names a file already on disk. -`dependency_check` and `activation_callback` have nothing a string could collide with, so they +`standalone_plugin_basename` takes a string only. `dependency_check` and `activation_callback` accept every callable form, a plain function name included. -Every typed key rejects a shape it cannot use at registration rather than at read time — -including a `[ class, method ]` pair naming a method that does not exist. `enabled` is the -exception: it is read as a boolean if it is not callable, so an array or an object there passes -registration and evaluates as enabled. Give it a `bool` or a `callable`, and nothing else. The -[filters](filters.md) are the other way in, and run last, after the configured value. +Every typed key rejects a shape it cannot use at registration rather than at read time, including a +`[ class, method ]` pair whose method does not exist. `enabled` is the exception: not callable means +it is read as a boolean, so an array or an object there evaluates as enabled — give it a `bool` or +a `callable`. The [filters](filters.md) are the other way in, and run last. ## Complete example diff --git a/docs/conflict-handling.md b/docs/conflict-handling.md index d53f908..015ef9f 100644 --- a/docs/conflict-handling.md +++ b/docs/conflict-handling.md @@ -2,10 +2,9 @@ ## Policies -A policy is only reached for a sub-plugin that is enabled, that names a `standalone_plugin_basename` -whose plugin is active right now, and that the [`should_load` filter](filters.md#the-load-gate) has -not vetoed — a vetoed sub-plugin has no bundled copy to put in the standalone's place. Everything -else is skipped before any policy is read. +A policy is only reached for a sub-plugin that is enabled, names a `standalone_plugin_basename`, +whose standalone is active right now, and that the [`should_load` filter](filters.md#the-load-gate) +has not vetoed — a vetoed sub-plugin has no bundled copy to put in the standalone's place. | Policy | Behavior | |---|---| @@ -13,11 +12,11 @@ else is skipped before any policy is read. | `Conflict_Policy::DEFER` | Leave the standalone active; the load guard then stands the bundled copy down. | | `Conflict_Policy::NOTICE_ONLY` | Leave it active and ask the user to deactivate it. | -A policy decides what happens to the *standalone*, and nothing else. Under `DEFER` the bundled copy -stands down because of [the load guard](#the-load-guard), not because of the policy. +A policy decides what happens to the *standalone* only: under `DEFER` the bundled copy stands down +because of [the load guard](#the-load-guard), not because of the policy. -Set one per sub-plugin with the `conflict_policy` key — a constant, or a `callable( Sub_Plugin ): -string`. The `conflict_policy` [filter](filters.md) runs after that and has the final say: +Set one with the `conflict_policy` key — a constant or a `callable( Sub_Plugin ): string` — and the +`conflict_policy` [filter](filters.md) runs last: ```php // In the config: leave the standalone alone when it is not the code that was absorbed. @@ -31,92 +30,72 @@ add_filter( 'give/plugin_absorber/conflict_policy', static function ( $policy, $ }, 10, 2 ); ``` -**An unrecognised policy is treated as `NOTICE_ONLY`**, never as consent to deactivate — a typo -like `'defered'`, from a persisted option or from the filter, only produces a notice. +**An unrecognised policy is treated as `NOTICE_ONLY`**, never as consent to deactivate: a typo like +`'defered'` only produces a notice. For what the site owner sees under each policy, see [the recipe](recipes.md#choose-a-policy-and-know-what-the-site-owner-sees). ## When resolution runs -At `plugins_loaded` priority 5, one ahead of the load at priority 6: a standalone that survives the -conflict defines the guard constant as it loads, and the load has to see that. Priority 5 is also -the deadline for `Absorber::boot()`, since this is the first step it wires. - -Resolution runs **only on an interactive admin `GET`** — not WP-CLI, cron, ajax, or a form -POST — because resolving can deactivate a plugin and end the request with a redirect, and a 302 -would discard whatever a POST submitted. Waiting costs nothing: the standalone is still there to -detect on the next page view. - -**A `GET` carrying an action is skipped too.** `update.php?action=upgrade-plugin` and -`plugins.php?action=activate` are admin `GET`s that *do* something, and a redirect discards their -work exactly as it would a POST's. Anything naming `action` or `action2` waits for the next plain -page view — deliberately blunt, so a read-only `post.php?action=edit` waits as well. - -Resolution also requires the capability matching what deactivation does: `manage_network_plugins` on -multisite and `activate_plugins` otherwise, since deactivating a standalone is network-wide wherever -a network exists. The check matters because `plugins_loaded` fires well before `auth_redirect()`, so -an unauthenticated GET of an admin URL reaches this code on its way to the login screen. - -These gates apply whatever the policy is — the non-destructive policies only queue a notice, and the -[notice queue](notices.md#who-sees-them) asks for that same capability by name before it shows a -notice or clears one, so nothing a policy queued is consumed by someone the gate would have refused. - -The deactivation itself is silent, and covers both scopes on multisite. Silent because the -standalone's own deactivation hook would otherwise run this early: a routine `flush_rewrite_rules()` -in it would regenerate the rules before `init` declared a single post type, and every custom -permalink on the site would start 404ing. On multisite it can also decline outright — see -[the stranding guard](#the-multisite-stranding-guard). - -**A site that never loads wp-admin never resolves.** Every gate above needs an interactive admin -page view. Any website administered entirely over SFTP, Composer, or WP-CLI — one whose owner never -opens an admin screen in a browser — keeps the standalone active for as long as that holds. There -is still no fatal, because [the load guard](#the-load-guard) runs on every request and stands the -bundled copy down regardless; what waits is the *switchover*. The standalone, frozen at the version -installed, goes on serving in place of the bundled copy the host ships updates for until the first -request that clears every gate above arrives. That is the price of never ending a non-admin request -with a redirect — one that would drop a visitor's POST or cut a WP-CLI run short — and on such a -site the switchover is simply deferred until then. +At `plugins_loaded` priority 5, one ahead of the load at priority 6, so the load sees the guard +constant a surviving standalone defines. Priority 5 is also the deadline for `Absorber::boot()`. + +Resolution runs **only on an interactive admin `GET`** — not WP-CLI, cron, ajax, or a form POST, +since it can end the request with a redirect that would discard what a POST submitted. Nothing is +lost: the standalone is still there on the next page view. + +**A `GET` carrying an action is skipped too.** Anything naming `action` or `action2` waits for the +next plain page view, because a redirect would discard the work behind +`plugins.php?action=activate`. The rule is deliberately blunt, so `post.php?action=edit` waits too. + +Resolution also requires the capability deactivation needs: `manage_network_plugins` on multisite, +`activate_plugins` otherwise. An unauthenticated admin GET reaches this code before +`auth_redirect()` has sent it to the login screen, and resolves nothing. + +These gates apply whatever the policy is, and the [notice queue](notices.md#who-sees-them) asks for +the same capability. + +Deactivation is silent, and covers both scopes on multisite: the standalone's own deactivation hook +must not run this early, where a `flush_rewrite_rules()` in it would 404 every custom permalink. On +multisite it can also decline outright — see [the stranding guard](#the-multisite-stranding-guard). + +**A site that never loads wp-admin never resolves.** One administered only over SFTP, Composer, or +WP-CLI keeps the standalone active indefinitely. There is no fatal — [the load +guard](#the-load-guard) stands the bundled copy down on every request — but the switchover waits for +the first request that clears every gate above. ## The redirect -The standalone's code is already in memory by the time the conflict is resolved — WordPress -included it before `plugins_loaded` — so the redirect is how the request sheds it. The destination -is **the screen being requested**, not the one the user came from: it re-renders without the -standalone, and the admin stays where they asked to be, network or user admin included. `/wp-admin/` -and the admin roots mean the dashboard; `update.php` and `update-core.php` go to `plugins.php` -instead, because reloading either would re-run an update, and so does anything naming no usable -admin screen. There is no redirect loop: the next request has no active standalone, so nothing -resolves. +The standalone's code is already in memory when the conflict is resolved, so the redirect is how +the request sheds it. The destination is **the screen being requested**, not the one the user +came from, network and user admin included. `/wp-admin/` and the admin roots mean the dashboard; +`update.php` and `update-core.php` go to `plugins.php` instead, since reloading either would re-run +an update, and so does a URI naming no usable admin screen. There is no redirect loop: the next +request has no active standalone to resolve. -With several sub-plugins in conflict, all of them are resolved before the one redirect at the end, -and the redirect is skipped entirely once headers have been sent — the request then finishes -rendering instead of dying blank. The [merge notice](notices.md) is queued first either way, so the -explanation survives whether or not the request ends in a redirect. +With several sub-plugins in conflict, all are resolved before the one redirect at the end, and the +redirect is skipped once headers have been sent. The [merge notice](notices.md) is queued first +either way. ## The multisite stranding guard `deactivate_plugins()` runs with no `$network_wide` argument, so a network-active standalone is taken -out of *every* site's plugins. But the bundled copy only loads where the host plugin runs, so on a -network where the host is active on only some sites, deactivating a network-active standalone would -remove it from the sites the host never reached — leaving them with no copy of it at all, bundled or -standalone. +out of *every* site's plugins — including sites the host plugin never runs on, which would be left +with no copy at all. When the host names itself with `Config::set_host_plugin_basename( plugin_basename( __FILE__ ) )`, -the `DEACTIVATE` policy checks for exactly that case — a network-active standalone whose host is not -itself network-activated — and declines: the standalone is left active, [the load -guard](#the-load-guard) stands the bundled copy down network-wide as under `DEFER`, and a [stranding -notice](notices.md) tells a network administrator why and how to finish, by network-activating the -host or removing the standalone from the Network Admin. It recurs until one of those is done. +the `DEACTIVATE` policy detects exactly that case — a network-active standalone whose host is not +network-activated — and declines: the standalone stays active, [the load guard](#the-load-guard) +stands the bundled copy down network-wide as under `DEFER`, and a [stranding notice](notices.md) +explains it, recurring until a network administrator resolves the topology. -Pass `plugin_basename( __FILE__ )`, not `__FILE__`. A basename no installed plugin answers to reads -as a host that is never network-active, so the guard would decline for ever and the notice would -recur with nothing to act on — the library reports that one to the developer instead, through -`_doing_it_wrong()`, and leaves the standalone where it is. +Pass `plugin_basename( __FILE__ )`, not `__FILE__`: a basename no installed plugin answers to +reads as a host that is never network-active, so the guard would decline for ever. That one is +reported to the developer through `_doing_it_wrong()`, and the standalone is left where it is. -The guard is **opt-in and single-site-safe**: with no host basename set it never fires, and off a -network it never fires, so in every other topology — both network-active, both per-site, or a -per-site standalone — deactivation behaves exactly as it always has. +The guard is **opt-in and single-site-safe**: it never fires without a host basename set, and never +off a network, so every other topology deactivates as it always has. ## The load guard @@ -133,37 +112,35 @@ if ( ! defined( 'GIVE_RECURRING_VERSION' ) ) { ``` A standalone that defines it from a bootstrap hooked at `plugins_loaded` or later has not defined it -yet at the moment the guard is read, and the bundled copy would load on top of it. A standalone that -never defines the name at all is never stood down either, and the bundled copy loads alongside it; -[an included recipe](recipes.md#defer-to-a-standalone-that-is-a-new-codebase) is a functional -example of this behavior. +when the guard is read, so the bundled copy loads on top of it. One that never defines the name is +never stood down at all — +[an included recipe](recipes.md#defer-to-a-standalone-that-is-a-new-codebase) is a worked example. ## Reactivating the standalone The guard cannot help on the request that *activates* the standalone: WordPress includes the plugin -being activated **after** the bundled copy has already loaded, so that re-declaration is a real -fatal. Core catches it in its activation sandbox and prints *"Plugin could not be activated because -it triggered a fatal error."* — true, and useless to whoever pressed the button. +being activated **after** the bundled copy has loaded, so that re-declaration is a real fatal. Core +catches it in its activation sandbox and prints *"Plugin could not be activated because it triggered +a fatal error."* So the library filters `wp_admin_notice_markup` and swaps that sentence for the sub-plugin's -`conflict_notice_message`, falling back to a generic one naming the slug. This is what puts the -WordPress floor at 6.4: the filter does not exist before it. +`conflict_notice_message`, falling back to a generic one naming the slug. That filter is what puts +the WordPress floor at 6.4. -It touches nothing else. The markup comes back untouched unless every one of these holds — the -screen is `plugins`, or `plugins-network` in the network admin; the `plugin` query arg names a -standalone this library has registered; and `_error_nonce` verifies. +It touches nothing else: the markup comes back untouched unless the screen is `plugins`, or +`plugins-network` in the network admin; the `plugin` query arg names a registered standalone; and +`_error_nonce` verifies. The replacement runs through `wp_kses_post()`, so a knowledge-base link survives, and a message that -filters down to nothing leaves core's wording in place rather than blanking the notice. A host that -would rather keep core's wording throughout can remove the filter — see [Extending](extending.md). +filters down to nothing leaves core's wording standing. To keep core's wording throughout, remove +the filter — see [Extending](extending.md). ## Out of scope -**Version negotiation.** The library never compares versions. Express it yourself: read the version -and return `Conflict_Policy::DEFER` from the config or the `conflict_policy` [filter](filters.md), -which has the final say — [this recipe](recipes.md#defer-to-a-standalone-that-is-a-new-codebase) is -an example. +**Version negotiation.** The library never compares versions. Read the version yourself and return +`Conflict_Policy::DEFER` from the config or the `conflict_policy` [filter](filters.md), which has the +final say — [this recipe](recipes.md#defer-to-a-standalone-that-is-a-new-codebase) is an example. **Renamed standalone directories.** `standalone_plugin_basename` is the path as installed. A site -that renamed the standalone's directory is not detected, and there is no fallback deriving the path -from the load guard: one key is the guard, the other is the path, and no constant does both jobs. +that renamed the standalone's directory is not detected, and no fallback derives the path from the +load guard: one key is the guard, the other is the path. diff --git a/docs/extending.md b/docs/extending.md index 471aa6c..e3cabc4 100644 --- a/docs/extending.md +++ b/docs/extending.md @@ -1,8 +1,7 @@ # Extending -Everything the library does past registration is a small object resolved from your container, so any -one piece can be swapped without replacing the rest. This is the one doc that names those classes; -nothing else in the docs asks you to know them. +Everything the library does past registration is a small object resolved from your container, so +any one piece can be swapped on its own. This is the one doc that names those classes. ## The seams @@ -25,29 +24,24 @@ $container->singleton( Registrar_Interface::class, My_Registrar::class ); | `Contracts\Activator_Interface` | `Activator` | Runs a sub-plugin's activation callback once, ever. | **Rebind `Plugin\Contracts\Checker_Interface` when your plugin filters `option_active_plugins` or -`site_option_active_sitewide_plugins`** — LearnDash injects and then strips a synthetic path — because -`is_plugin_active()` then does not report what is in the database. +`site_option_active_sitewide_plugins`**: `is_plugin_active()` then does not report what is in the +database. **Rebind `Activator_Interface` to record "once, ever" somewhere else**: your own migration table, or -a per-site option on a large multisite network where one run for the whole network is not what you -want. See [the recipe](recipes.md#do-per-site-work-on-multisite). +a per-site option on a large network. See [the recipe](recipes.md#do-per-site-work-on-multisite). ## What each seam must implement -The table names the job; these are the methods, and every one of them is required — PHP refuses to -load a class that leaves one out, at class-declaration time rather than when the library first calls -it. Two of this library's types appear below: +Every method below is required. Two of this library's types appear in them: ```php use Nexcess\PluginAbsorber\Exceptions\Config_Exception; use Nexcess\PluginAbsorber\Sub_Plugin; ``` -`Sub_Plugin` is one registered configuration, and its accessors — `get_slug()`, -`get_standalone_plugin_basename()`, the message accessors named below — are how an implementation -reads the config keys and runs [their filters](filters.md). A throw from any of these is caught at -the hook boundary and reported through `_doing_it_wrong()`, so it costs that one step rather than the -site; nothing retries it on the same request. +`Sub_Plugin` is one registered configuration; its accessors — `get_slug()` and the message +accessors below — read the config keys and run [their filters](filters.md). A throw from any method +here is caught at the hook boundary, reported through `_doing_it_wrong()`, and never retried. ### `Registry\Contracts\Registrar_Interface` @@ -56,12 +50,9 @@ public function register( Sub_Plugin $sub_plugin ): void; public function all(): array; ``` -`register()` stores one sub-plugin, and a slug may only be registered once — throw a -`Config_Exception` on the second rather than overwriting, or a copy-pasted slug silently replaces the -sub-plugin it collided with. `all()` returns every registered sub-plugin keyed by slug, in -registration order: `array`. The library narrows that array to `Sub_Plugin` -instances before reading it, so anything else in it is dropped rather than fataling — which loses a -sub-plugin quietly, and is worth not doing. +`register()` stores one sub-plugin; throw a `Config_Exception` on a slug already registered rather +than overwriting it. `all()` returns `array`, keyed by slug, in registration +order — anything else in it is dropped quietly. ### `Notices\Contracts\Writer_Interface` @@ -73,24 +64,21 @@ public function queue_dependency_notice( Sub_Plugin $sub_plugin ): void; public function option_name(): string; ``` -- `queue_merge_notice()` — a deactivation has just been performed. Raised exactly once and never - re-queued, and the resolver redirects immediately afterwards, so it has to be kept somewhere that - outlives the request that wrote it. Word it from +- `queue_merge_notice()` — a deactivation has just been performed. Raised once, never re-queued, + and followed by a redirect, so keep it somewhere that outlives the request. Word it from `$sub_plugin->get_conflict_notice_message( $your_default )`, which applies the config key and its - filter over the default you pass. + filter over it. - `queue_conflict_notice()` — `NOTICE_ONLY`: the standalone is still running and the user is being asked to turn it off. Same accessor, a default that asks rather than reports. - `queue_stranding_notice()` — multisite only, and recurring rather than once-ever: a network-active - standalone was left active because deactivating it network-wide would strand the sites the host - plugin never reaches. Its wording is `$sub_plugin->get_stranding_notice_message()`, and it must not - tell the user to deactivate the standalone — see + standalone was left running rather than strand sites. Word it from + `$sub_plugin->get_stranding_notice_message()`, and do not tell the user to deactivate it — see [the stranding guard](conflict-handling.md#the-multisite-stranding-guard). - `queue_dependency_notice()` — `dependency_check` returned false, so the sub-plugin did not load at all. `$sub_plugin->get_dependency_notice_message()`. -- `option_name()` — where *your* implementation keeps the queue. It is on the contract rather than on - the default class because `Absorber::notices()->option_name()` is what a host - [rendering the queue itself](notices.md#rendering-them-yourself) reads: name an option nothing - writes to and that host reads an empty one. +- `option_name()` — where *your* implementation keeps the queue, and what a host + [rendering the queue itself](notices.md#rendering-them-yourself) reads through + `Absorber::notices()->option_name()`. ### `Plugin\Contracts\Deactivator_Interface` @@ -98,12 +86,10 @@ public function option_name(): string; public function deactivate( string $basename ): void; ``` -One method, one argument: a plugin basename like `give-recurring/give-recurring.php`, the only -identifier WordPress itself accepts. Called unattended during `plugins_loaded`, under the -`DEACTIVATE` policy, on behalf of a user who did not ask for it. It reports nothing — whether the -standalone actually went away is asked of the checker afterwards, not of you — so a no-op -implementation is the honest way to say "plugin state is managed outside WordPress here", and an -implementation that means to deactivate but does not leaves two copies of the plugin to load. +One argument: a plugin basename like `give-recurring/give-recurring.php`. Called unattended during +`plugins_loaded`, under the `DEACTIVATE` policy. It reports nothing — the checker is asked +afterwards whether the standalone went away — so a deliberate no-op is fine, while a silent +failure leaves two copies to load. ### `Plugin\Contracts\Checker_Interface` @@ -112,14 +98,11 @@ public function is_active( string $basename ): bool; public function is_network_active( string $basename ): bool; ``` -Two methods, asked for different reasons. `is_active()` answers "this plugin's code is going to run -this request", so both scopes count: a network activation runs it as surely as a site one. -`is_network_active()` is network scope only, and it is the one question the -[stranding guard](conflict-handling.md#the-multisite-stranding-guard) asks — "would deactivating this -reach every site". Return `false` whenever the site is not multisite, exactly as core's -`is_plugin_active_for_network()` does: callers lean on that instead of guarding with `is_multisite()` -themselves, so answering `true` off a network can have the guard decline a deactivation on a site -with nothing to strand. +`is_active()` answers "this plugin's code is going to run this request", so both scopes count. +`is_network_active()` is network scope only, and is what the +[stranding guard](conflict-handling.md#the-multisite-stranding-guard) asks: return `false` off +multisite, as core's `is_plugin_active_for_network()` does, since `true` there has the guard decline +a deactivation with nothing to strand. ```php use Nexcess\PluginAbsorber\Plugin\Contracts\Checker_Interface; @@ -149,13 +132,11 @@ class My_Checker implements Checker_Interface { public function resolve_all(): void; ``` -Called once per request, at `plugins_loaded` priority 5, and only after the -[request and capability gates](conflict-handling.md#when-resolution-runs) pass and a conflict has -been found. It is handed nothing: read the registry from the container your -implementation was built with, and resolve every sub-plugin whose standalone is active — including -the ones behind the first, which is why the default catches per sub-plugin. Ending the request is -allowed, and the default does (`wp_safe_redirect()` then `exit`) — but after the loop, never inside -it, or a site with two active standalones never reaches the second. +Called once per request at `plugins_loaded` priority 5, after the +[request and capability gates](conflict-handling.md#when-resolution-runs) pass and a conflict is +found. It is handed nothing: read the registry from the container it was built with and resolve +every sub-plugin whose standalone is active, catching per sub-plugin. Ending the request is allowed +— the default does `wp_safe_redirect()` then `exit` — but after the loop, never inside it. ### `Contracts\Activator_Interface` @@ -163,43 +144,37 @@ it, or a site with two active standalones never reaches the second. public function maybe_run( Sub_Plugin $sub_plugin ): void; ``` -Called by the load pass after a `require_once` that actually happened, on every request, for every -sub-plugin that loaded. Deciding it has already run for this slug is the whole job: -`$sub_plugin->get_activation_callback()` hands back the configured `callable` or `null`, it is -invoked with the `Sub_Plugin`, and where "already run" is recorded is yours. Record it after the -callback returns rather than before, so a callback that throws is retried on the next request instead -of being marked done half-finished. +Called on every request, for every sub-plugin whose `require_once` actually happened; deciding it +has already run for this slug is the whole job. `$sub_plugin->get_activation_callback()` hands back +the configured `callable` or `null` — invoke it with the `Sub_Plugin` and record "already run" +after the callback returns, so one that throws is retried rather than marked done. ## Class-name bindings must come after boot Everything without an interface is bound by class name — `Notices\Store`, `Notices\Renderer`, `Notices\Presenter`, `Conflict\Detector`, `Conflict\Gatekeeper`, `Conflict\Redirector`, -`Conflict\Rewriter`, `Loader`, `Registry\Reader`, `Boot\Scheduler`. Bind one of those **after** -`Absorber::boot()`: di52 reports `has()` true for any class that exists, bound or not, so boot cannot -tell your binding from the container's own willingness to build the class, and replaces it. +`Conflict\Rewriter`, `Loader`, `Registry\Reader`, `Boot\Scheduler`. Bind those **after** +`Absorber::boot()`: di52 reports `has()` true for any class that exists, bound or not, so boot +cannot tell your binding from an autowirable class, and overwrites it. -Boot also binds `StellarWP\ContainerContract\ContainerInterface` to the container itself, first and -before anything else, so a container that builds unbound classes reflectively can still satisfy the -library classes that take one. That id is an interface rather than a class, so the skip above applies -to it: a container that already answers for it keeps whatever it has, whenever you bound it. +Boot also binds `StellarWP\ContainerContract\ContainerInterface` to the container itself, so library +classes that take one can be built reflectively — an interface id, so your own binding stands. ## What rebinding does not buy you Binding your own `Conflict\Contracts\Resolver_Interface` does not put you in charge of *when* -resolution may run. The request and capability gates — [an interactive admin `GET` carrying no -action, and the capability to deactivate across the -network](conflict-handling.md#when-resolution-runs) — are asked before your resolver is built at all, -so an implementation that never thought about either is still safe. Everything the resolver *does* — -which policy branch, what the notice says, where the user lands — is yours. +resolution runs: the gates — [an interactive admin `GET` carrying no action, +and the capability to deactivate across the +network](conflict-handling.md#when-resolution-runs) — are asked before your resolver is built. What +it *does* is yours. ## The notice queue Four objects, so you can replace the part you have an opinion about: -- `Notices\Writer` decides what a notice says. The one behind an interface, and the seam for a host - already running its own notices library. -- `Notices\Store` keeps the queue. Rebind to store it elsewhere. -- `Notices\Renderer` draws it. Rebind to change the markup and leave the storage alone. +- `Notices\Writer` decides what a notice says — the one behind an interface. +- `Notices\Store` keeps the queue; rebind to store it elsewhere. +- `Notices\Renderer` draws it; rebind for the markup alone. - `Notices\Presenter` decides who may consume it, and does the render-then-clear. Rendering the queue yourself needs none of this — read @@ -220,22 +195,13 @@ remove_action( 'all_admin_notices', [ Absorber::class, 'render_notices' ] ); ## When a binding is wrong `Absorber::registrar()`, `notices()`, `resolver()` and `all()` check what your container hands back -and throw a `Config_Exception` naming the id they asked for and the class that failed it, rather than -letting a `TypeError` blame this library for your typo inside `plugins_loaded`. A binding your -container cannot build at all is reported the same way and not raised at you raw: whatever it threw -is caught and wrapped in a `Config_Exception` that names the id, keeping the original as -`getPrevious()`. `Absorber::boot()` is the one that does not wrap — a container that cannot build the -provider or the scheduler throws its own exception out of your `boot()` call. Past the check, -`Absorber::all()` also drops anything a rebound registrar returns that is not a `Sub_Plugin`, and -`Absorber::registrar()` hands back a registrar with every registration made so far already in it, so -the two answer alike whenever you ask. - -`Absorber::resolver()` hands you the resolver, not the gates in front of it: `resolve_all()` re-checks -neither the request gate nor the capability gate, because the conflict step has already asked both by -the time it runs. Calling it yourself can therefore deactivate a plugin and `exit` on a POST, a cron -run, or a request from someone who may not deactivate anything. Leave it to the hook unless you have -made both checks first. - -Nothing is built at boot beyond the two objects that do the booting: each hook resolves its -collaborator when it fires, so a request that reaches none of them builds none of them, and you may -rebind right up until the hook runs. +and throw a `Config_Exception` naming the id and the class that failed it. A binding the container +cannot build at all is wrapped the same way, keeping the original as `getPrevious()` — except in +`Absorber::boot()`, where a container that cannot build the provider or the scheduler throws its own +exception out of your `boot()` call. Past the check, `Absorber::all()` drops anything a rebound +registrar returns that is not a `Sub_Plugin`, and `Absorber::registrar()` hands back a registrar with +every registration made so far already in it, so the two answer alike whenever you ask. + +`Absorber::resolver()` hands you the resolver, not the gates in front of it: `resolve_all()` +re-checks neither, so calling it yourself can deactivate a plugin and `exit` on a POST, a cron run, +or a request from someone who may not deactivate anything. Make both checks first. diff --git a/docs/filters.md b/docs/filters.md index 8368d2b..57b4182 100644 --- a/docs/filters.md +++ b/docs/filters.md @@ -9,10 +9,10 @@ | `{prefix}/plugin_absorber/dependency_notice_message` | `string $message`, `Sub_Plugin $sub_plugin` | Final say over the dependency notice text. Receives the configured message, or the generic default sentence when nothing is configured. | | `{prefix}/plugin_absorber/stranding_notice_message` | `string $message`, `Sub_Plugin $sub_plugin` | Final say over the multisite stranding notice text. Receives the generic default; this notice has no config key, so the filter is its only override. | -Each runs last, after the configured value and any fallback, and fires when the value is asked for -rather than when the sub-plugin is registered — so it is also the place to call `__()`. A -non-scalar return yields an empty string rather than a fatal cast, and a `conflict_policy` return -that is not one of the three constants is treated as [`NOTICE_ONLY`, never as consent to +Each runs last, after the configured value and any fallback, and fires when the value is read rather +than at registration — so it is also the place to call `__()`. A non-scalar return yields an empty +string rather than a fatal cast, and a `conflict_policy` return that is not one of the three +constants is treated as [`NOTICE_ONLY`, never as consent to deactivate](conflict-handling.md#policies). ## The load gate @@ -29,20 +29,17 @@ add_filter( 'give/plugin_absorber/should_load', function ( $should_load, $sub_pl It is consulted only for a sub-plugin that would otherwise have loaded — after the enabled check, the guard constant, the dependency check and the file check. Returning `true` cannot force a load -past the guard constant; anything other than a truthy return skips the load, which is the safe -direction. +past the guard constant, and anything but a truthy return skips the load. -**Conflict handling reads it too**, at `plugins_loaded` priority 5, one ahead of the load. A -sub-plugin you veto is invisible to it: no standalone is deactivated to make room for a bundled copy -that is not going to load. So the filter may be asked more than once in a request, and it has to -*decide* rather than do — no logging, no counters, no writes. +**Conflict handling reads it too**, at `plugins_loaded` priority 5, one ahead of the load: a +sub-plugin you veto is invisible to it, so no standalone is deactivated to make room for a bundled +copy that is not going to load. The filter is therefore asked more than once in a request, and has +to *decide* rather than do — no logging, no counters, no writes. Whether that pass sees your callback +turns on when you call `add_filter()`, not on the priority you give it: registered after +`plugins_loaded` priority 5, it has missed the pass. Put a toggle both passes always read in the +`enabled` [config key](configuration.md) instead. -Whether the conflict pass sees your callback turns on when you call `add_filter()`, not on the -priority you give the callback: one registered after `plugins_loaded` priority 5 has missed that -pass whatever its own priority. For a sub-plugin with a `standalone_plugin_basename`, put the toggle -in the `enabled` [config key](configuration.md) instead: both passes read it, whenever it is set. - -**Watch the polarity when you wire an existing gate to this one.** `should_load` is true means *do +**Watch the polarity when you wire an existing gate to this one.** `should_load` true means *do load*. A host filter named for the opposite — LearnDash's `learndash_module_{x}_disabled`, where -true means *do not load* — inverts the gate if passed through unnegated, and the failure is silent -in the direction that loads a plugin the site turned off. +true means *do not load* — inverts the gate if passed through unnegated, and fails silently in the +direction that loads a plugin the site turned off. diff --git a/docs/installing.md b/docs/installing.md index b533d0a..39bede0 100644 --- a/docs/installing.md +++ b/docs/installing.md @@ -5,16 +5,15 @@ composer require stellarwp/plugin-absorber ``` Requires PHP 7.4+ and WordPress 6.4+. The WordPress floor comes from the `wp_admin_notice_markup` -filter, which does not exist before 6.4; WordPress is not a Composer dependency, so it is stated -here rather than enforced in `require`. +filter; WordPress is not a Composer dependency, so nothing enforces it at install time. ## Strauss Prefix this library with [Strauss](https://github.com/stellarwp/global-docs/blob/main/docs/strauss-setup.md). Two plugins shipping different versions of it will collide otherwise. -> **Nothing may rewrite a sub-plugin's `plugin_loaded_constant`.** Those are real, shared runtime -> constants: the whole safety mechanism depends on the bundled copy and the standalone defining the -> *same* name. This library only ever reads such a name out of your config, so its own source is -> safe to prefix in full — but if your build also runs the bundled plugin's own files through -> Strauss, keep `extra.strauss.constant_prefix` away from them. +> **Nothing may rewrite a sub-plugin's `plugin_loaded_constant`.** The bundled copy and the +> standalone must define the *same* name, or the guard matches nothing. This library only ever reads +> such a name out of your config, so its own source is safe to prefix in full — but if your build +> also runs the bundled plugin's files through Strauss, keep `extra.strauss.constant_prefix` away +> from them. diff --git a/docs/notices.md b/docs/notices.md index 73ec670..15777f0 100644 --- a/docs/notices.md +++ b/docs/notices.md @@ -2,31 +2,24 @@ The notices this library raises — the standalone was deactivated, the standalone is still active, a network-active standalone was left active to avoid stranding sites, a dependency check failed — are -queued in a single option named -`{option_prefix}_plugin_absorber_notices`, where `{option_prefix}` is the hook prefix lowercased -with hyphens folded to underscores: a hook prefix of `Give-Core` stores -`give_core_plugin_absorber_notices`. On multisite it is a **network** option, so the queue is shared -across every site on the network. It is an option and not a transient because a persistent object -cache keeps transients out of the database entirely, and a `wp_cache_flush()` would destroy a notice -that is raised exactly once and never re-queued. +queued in a single option named `{option_prefix}_plugin_absorber_notices`, where `{option_prefix}` +is the hook prefix lowercased with hyphens folded to underscores: a hook prefix of `Give-Core` +stores `give_core_plugin_absorber_notices`. On multisite it is a **network** option, so the queue is +shared across every site on the network. It is an option and not a transient because a persistent +object cache keeps transients out of the database, where a `wp_cache_flush()` would destroy a notice +raised exactly once and never re-queued. ## Who sees them Rendering prints the queue and then clears it, gated on the capability [conflict resolution](conflict-handling.md#when-resolution-runs) asks for: `manage_network_plugins` on multisite and `activate_plugins` otherwise. Since rendering consumes the queue, a user who cannot -act on a notice must not be shown one — a subscriber loading their profile page would otherwise -silently swallow the only warning an administrator was ever going to get. - -On multisite that means a network administrator rather than the site administrator who installed the -plugin, and the network capability is asked for by name rather than left to core's mapping of -`activate_plugins`, which only widens into it while the network keeps the Plugins menu off for -individual sites. On a network that has turned that menu on, every site administrator holds -`activate_plugins` outright: any one of them opening any admin screen would otherwise print a notice -raised for the network and clear it network-wide, and the queue is one option shared by every site, -so it would be gone for everyone else. That covers the dependency notice too — a site administrator -no longer consumes one, which is no loss, since a queue shared by the whole network was never theirs -alone to consume. +act on a notice must not be shown one. + +On multisite that means a network administrator, not the site administrator who installed the +plugin: the queue is one option shared by every site, so a site administrator with `activate_plugins` +opening any admin screen would otherwise print a notice raised for the network and clear it for +everyone. That covers the dependency notice too. ## One message, two places @@ -37,23 +30,22 @@ done and as the explanation standing in for a fatal-error warning. ## The stranding notice -On multisite only, a network-active standalone whose bundled copy ships in a host plugin that is not -itself network-activated is left active rather than deactivated: turning it off across the network -would remove it from the sites the host is not active on, where nothing loads the bundled copy. This -notice explains that, and — unlike the one-time deactivation notice — it recurs until the topology is -resolved, either by network-activating the host or by removing the standalone from the Network Admin. -Its text is the `stranding_notice_message` [filter](filters.md); there is no config key for it. +On multisite only, a network-active standalone whose host plugin is not itself network-activated is +left active rather than deactivated: turning it off across the network would remove it from the +sites the host is not active on, where nothing loads the bundled copy. This notice explains that, +and — unlike the one-time deactivation notice — recurs until the topology is resolved, either by +network-activating the host or by removing the standalone from the Network Admin. Its text is the +`stranding_notice_message` [filter](filters.md); there is no config key for it. ## Rendering them yourself `Absorber::notices()->option_name()` tells you where the queue is kept, so you can render it yourself without replacing anything. The value is an `array` keyed `slug:type`, where the type is `merge`, `conflict`, `stranding` or `dependency` — `give-recurring:merge`, for example. -The first three render as `notice-warning` and the last as `notice-error`, since a dependency notice -reports a plugin that did not load at all. The messages may contain markup; the built-in rendering passes -them through `wp_kses_post()`, so a link, emphasis or a list survives while scripts and event -handlers are stripped. Paragraphs come from `wpautop()`, so send the message unwrapped and let a -blank line break it — a `

      ` of your own is left as it is rather than nested inside another. +The first three render as `notice-warning` and the last as `notice-error`. The messages may contain +markup; the built-in rendering passes them through `wp_kses_post()`, so a link, emphasis or a list +survives while scripts and event handlers are stripped. Paragraphs come from `wpautop()`, so send +the message unwrapped and let a blank line break it. ```php use Nexcess\PluginAbsorber\Absorber; @@ -61,7 +53,7 @@ use Nexcess\PluginAbsorber\Absorber; add_action( 'admin_init', function () { // Gates the read, not just the delete: `admin_init` fires for every logged-in user, and // draining the queue for one who cannot act on it destroys the only warning an - // administrator was going to get. The same capability the built-in rendering asks for. + // administrator was going to get. if ( ! current_user_can( is_multisite() ? 'manage_network_plugins' : 'activate_plugins' ) ) { return; } diff --git a/docs/recipes.md b/docs/recipes.md index 93a5566..3b88c3c 100644 --- a/docs/recipes.md +++ b/docs/recipes.md @@ -7,8 +7,8 @@ behind them. ## Toggle a sub-plugin from a setting -Register unconditionally and put the condition in `enabled`. It is re-read on every request rather -than resolved at registration, so the settings screen saves an option and does nothing else: +Register unconditionally and put the condition in `enabled`, re-read on every request, so the +settings screen saves an option and does nothing else: ```php Absorber::register( [ @@ -20,32 +20,19 @@ Absorber::register( [ ] ); ``` -`enabled` is the first gate, ahead of the guard constant — -[how a sub-plugin loads](configuration.md#how-a-sub-plugin-loads) has the rest of the chain. Three -things follow from where it sits. +`enabled` is the first gate in [how a sub-plugin loads](configuration.md#how-a-sub-plugin-loads), +and gates conflict resolution too: off leaves the standalone alone as well. -**Switching the toggle off unloads nothing.** The `require_once` on this request already happened; -the next request is the one that skips it. Anything that has to stop immediately is the sub-plugin's -own business to gate. +**Switching the toggle off unloads nothing**: the next request is the one that skips the +`require_once`, so anything that has to stop sooner the sub-plugin gates itself. -**A disabled sub-plugin is invisible to conflict resolution too.** The toggle is checked first there -as well, so an off toggle also stops the standalone being deactivated: off means this library leaves -the plugin alone, standalone included. - -**Keep the callable cheap.** It runs on the conflict pass and again on the load pass, so at least -twice on an admin page view. An option read is fine; a remote licence check belongs behind a value -you have already cached. +**Keep the callable cheap** — it runs on both the conflict pass and the load pass: an option read +is fine, a remote licence check is not. ## Register several add-ons from one manifest -One `Absorber::register()` call per sub-plugin still happens here; the loop only builds each config -array on the way in. The manifest carries what differs between entries, the `+` union adds the keys -they share — so `bundled_plugin_file` is derived from each entry's own slug rather than written out -per entry — and the union never overwrites the left-hand side, so an entry that spells a shared key -out itself keeps its own value. - -The loop is also where load order is decided: sub-plugins load in registration order, so anything -extended at include time has to be registered before its extender. +The manifest carries what differs between entries; the loop supplies the keys they share. Its order +is load order. ```php $sub_plugins = [ @@ -64,17 +51,13 @@ foreach ( $sub_plugins as $slug => $constant ) { } ``` -An entry the library cannot use throws `Config_Exception` out of the `Absorber::register()` call it -is in, so a typo names itself in a stack trace pointing at your loop. A duplicate `slug` is found -later, at the first read of the registry — normally on `plugins_loaded` — because registrations are -buffered until then. It is refused there and reported through `_doing_it_wrong()`: the first -registration under the slug stands, the second is discarded, and every other sub-plugin loads as -normal. +An unusable entry throws `Config_Exception` from its own `Absorber::register()` call, so the stack +trace points at your loop; a duplicate `slug` is caught at the first read, on `plugins_loaded`, where +the second registration is discarded and reported through `_doing_it_wrong()`. ## Choose a policy, and know what the site owner sees -The policy is only reached for a sub-plugin that is enabled, names a `standalone_plugin_basename`, -and whose standalone is active right now: +What each branch leaves the site owner with: ```mermaid flowchart TD @@ -90,19 +73,17 @@ flowchart TD | Policy | The standalone | This request | Afterwards | |---|---|---|---| -| `DEACTIVATE` | turned off, silently, network-wide | its code is still in memory, so the bundled copy stands down; the user is redirected back to the screen they asked for | the bundled copy loads, and a merge notice explains the swap | +| `DEACTIVATE` | turned off silently, network-wide | still in memory, so the bundled copy stands down; the user is redirected to the screen they asked for | the bundled copy loads; a merge notice explains the swap | | `NOTICE_ONLY` | left running | the bundled copy stands down | unchanged until someone acts on the notice | | `DEFER` | left running | the bundled copy stands down | unchanged, and nothing is said | -The redirect under `DEACTIVATE` re-renders the screen with the standalone's code gone. It happens -once per request however many standalones were turned off, and only on an interactive admin `GET` -carrying no `action` — [conflict handling](conflict-handling.md#when-resolution-runs) has the gates. +Only an interactive admin `GET` with no `action` reaches any of this — +[conflict handling](conflict-handling.md#when-resolution-runs) has the rest of the gates. ## Ship the absorption over several releases -Bundling the code and taking over from the standalone do not have to be the same release. Moving the -`conflict_policy` one step per release lets a site be warned before anything of theirs is turned -off: +Bundling the code and taking over from the standalone need not be the same release. Moving the +`conflict_policy` one step per release warns a site before anything of theirs is turned off: ```mermaid flowchart LR @@ -111,16 +92,13 @@ flowchart LR R3["Release 3
      DEACTIVATE
      the remainder are merged,
      and told so"] ``` -Release 1 is the safe one to leave in place for a while: the bundled copy ships dormant on every -site that has the standalone, which is the population you are least sure about. Release 3 is the -only one that touches a site's active plugins. Each step is a one-constant change — or none at all, -if the policy comes from a callable reading a value you can move without shipping: +Only release 3 touches a site's active plugins. Each step is a one-constant change — or none, if +the policy reads a value you can move without shipping: ```php /** - * Read fresh on every conflict pass, so whatever writes this option -- a settings screen, a - * support tool, WP-CLI, a migration -- changes how the next admin page view resolves the - * conflict. Nothing is cached, and no release has to ship for the stage to move. + * Read fresh on every conflict pass, so a settings screen, WP-CLI or a migration can move the + * stage with no release. */ 'conflict_policy' => static fn() => get_option( 'give_absorption_stage', Conflict_Policy::DEFER ), ``` @@ -130,14 +108,11 @@ misspelt option cannot turn a plugin off. ## Defer to a standalone that is a new codebase -This is an edge case, but one that has occurred before: a later version of a standalone that shares -no code with the version you bundled, while still shipping under the same folder and file name. - -LearnDash's ProPanel is a case where this happened. `learndash-propanel/learndash_propanel.php` was -ProPanel 2.x, which LearnDash absorbed into its Reports module. ProPanel 3.0 arrived at that same -path as a new codebase — its own namespace, its own `LDRP_*` constants, none of the 2.x code. The -basename is all the two versions have in common, so `standalone_plugin_basename` cannot tell them -apart. The version installed at that path can, so read it and defer: +An edge case that has happened: `learndash-propanel/learndash_propanel.php` was ProPanel 2.x, which +LearnDash absorbed into its Reports module, and ProPanel 3.0 later arrived at that same path as an +unrelated codebase with its own namespace and its own `LDRP_*` constants. +`standalone_plugin_basename` cannot tell the two apart; the installed version can, so read it and +defer: ```php add_filter( 'learndash/plugin_absorber/conflict_policy', static function ( $policy, $sub_plugin ) { @@ -155,10 +130,7 @@ add_filter( 'learndash/plugin_absorber/conflict_policy', static function ( $poli false ); - /** - * 3.0 and up at this path is the newer codebase, not an older copy of ours, so there is - * nothing to take over from. Anything below it is the version that was absorbed. - */ + // 3.0 and up at this path is the newer codebase, not an older copy of ours. return version_compare( $installed['Version'], '3.0.0-dev', '>=' ) ? Conflict_Policy::DEFER : $policy; @@ -173,16 +145,14 @@ add_filter( 'learndash/plugin_absorber/conflict_policy', static function ( $poli | an older version of the same code | defined by it | stands down | | a new codebase at the same path | never defined | **loads, alongside it** | -ProPanel 2.x defines `LD_PP_PLUGIN_DIR` and 3.x defines only `LDRP_*`, so with 3.x active the -bundled Reports module loads too — correct here, since the two share a file name and nothing else. -Treat it as the exception it is: two copies loading at once is normally the fatal this library -exists to prevent, and it is safe only because these two are not copies of each other. +ProPanel 3.x defines only `LDRP_*`, never 2.x's `LD_PP_PLUGIN_DIR`, so the bundled Reports module +loads alongside it. Safe only because the two are not copies of each other: two copies loading at +once is the fatal this library exists to prevent. ## Do per-site work on multisite -Deactivation is network-wide, the notice queue is a network option, and so is the activation record, -so `activation_callback` runs **once for the network** — in whichever site's request reached the -load pass first. Per-site work loops: +The activation record is a network option, so `activation_callback` runs **once for the network**. +Per-site work loops: ```php 'activation_callback' => static function ( Sub_Plugin $sub_plugin ) { @@ -200,8 +170,7 @@ load pass first. Per-site work loops: }, ``` -That is the right shape for a handful of sites and the wrong one for a large network, where the loop -runs inside `plugins_loaded` on one unlucky request. Replace the once-ever bookkeeping — see -[Extending](extending.md) — and record it per site, so each site pays only for itself. Either way, -write the callback to be idempotent: "once, ever" is bookkeeping rather than a lock, and -[activation](configuration.md#activation) has the retry and concurrency detail. +That loop runs inside `plugins_loaded` on one unlucky request, so on a large network replace the +once-ever bookkeeping and record it per site instead — see [Extending](extending.md). Either way +write the callback to be idempotent; [activation](configuration.md#activation) has the retry and +concurrency detail. From 9896cf4d751a255824f12285d1cc1046d6df0301 Mon Sep 17 00:00:00 2001 From: Nikolay Strikhar Date: Tue, 25 Aug 2026 16:27:07 +0200 Subject: [PATCH 03/10] Correct the claims the trim left behind The should_load comment said an odd return is merely falsy and skips: a non-empty array, an object or a string is truthy and loads. The activator comment blamed the once-ever record on a skipped load, which cannot reach it -- every gate returns first; the real danger is running the callback for code that is not in memory. The detector claimed to only report, while the read it makes drains the buffer and its filters run host code; what it never does is change an activation state. The load guard's boundary is priority 6, not "plugins_loaded or later", and it belongs in the file whose subject is the guard. Extending says which ids may be bound before boot, and points at the section that owns the class-name rule rather than restating it. Naming the hosts that wire at priority 1 keeps Harbor readable as somebody else's API. --- docs/conflict-handling.md | 6 +++--- docs/extending.md | 5 +++-- src/Boot/Scheduler.php | 3 ++- src/Conflict/Detector.php | 4 ++-- src/Loader.php | 9 +++++---- 5 files changed, 15 insertions(+), 12 deletions(-) diff --git a/docs/conflict-handling.md b/docs/conflict-handling.md index 015ef9f..347ef44 100644 --- a/docs/conflict-handling.md +++ b/docs/conflict-handling.md @@ -111,9 +111,9 @@ if ( ! defined( 'GIVE_RECURRING_VERSION' ) ) { } ``` -A standalone that defines it from a bootstrap hooked at `plugins_loaded` or later has not defined it -when the guard is read, so the bundled copy loads on top of it. One that never defines the name is -never stood down at all — +The guard is read on `plugins_loaded` at priority 6, so a standalone defining it from a bootstrap +hooked below that is still seen in time; one hooked at 6 or later is not, and the bundled copy loads +on top of it. One that never defines the name is never stood down at all — [an included recipe](recipes.md#defer-to-a-standalone-that-is-a-new-codebase) is a worked example. ## Reactivating the standalone diff --git a/docs/extending.md b/docs/extending.md index e3cabc4..efdc235 100644 --- a/docs/extending.md +++ b/docs/extending.md @@ -5,8 +5,9 @@ any one piece can be swapped on its own. This is the one doc that names those cl ## The seams -Bind any of these ids before or after `Absorber::boot()` — boot binds the defaults and skips an -*interface* your container already answers for, so your binding wins either way: +Bind any of these interface ids before or after `Absorber::boot()` — boot binds the defaults and +skips an interface your container already answers for, so your binding wins either way; +[class-name ids are different](#class-name-bindings-must-come-after-boot): ```php use Nexcess\PluginAbsorber\Registry\Contracts\Registrar_Interface; diff --git a/src/Boot/Scheduler.php b/src/Boot/Scheduler.php index 4fde07f..f15f0d8 100644 --- a/src/Boot/Scheduler.php +++ b/src/Boot/Scheduler.php @@ -41,7 +41,8 @@ class Scheduler { * * A surviving standalone defines the guard constant as it loads and the load pass has to see * that. Also the number a host is measured against, being the first step in the sequence — 5 - * rather than 1 because hosts already wire Harbor's `set_container()` at priority 1. + * rather than 1 because LearnDash and MemberDash already wire Harbor's `set_container()` at + * priority 1, so a host copying that habit would land on the barrier. * * @since 1.0.0 * diff --git a/src/Conflict/Detector.php b/src/Conflict/Detector.php index d69b2b6..a740ff8 100644 --- a/src/Conflict/Detector.php +++ b/src/Conflict/Detector.php @@ -18,8 +18,8 @@ * Whether a bundled sub-plugin's standalone counterpart is still active. * * Its own class so the conflict step has something cheap to ask before `current_user_can()`, which - * pins the current user for the rest of the request. Everything here only reports. Not `final`: it - * is bound by class name, the seam a host rebinds and a test subclasses. + * pins the current user for the rest of the request. It changes no plugin's activation state. Not + * `final`: it is bound by class name, the seam a host rebinds and a test subclasses. * * @since 1.0.0 */ diff --git a/src/Loader.php b/src/Loader.php index d7808f7..19a7a8e 100644 --- a/src/Loader.php +++ b/src/Loader.php @@ -171,8 +171,8 @@ private function load( Sub_Plugin $sub_plugin ): void { return; } - // No type guard on the return: there is no cast here, so anything odd is merely falsy and - // skips the load, which is the safe direction. + // Truthiness, not a cast: only a falsy return vetoes, so an array or an object here loads + // rather than fataling on the way to a string. $should_load = apply_filters( Config::get_hook_name( 'should_load' ), true, $sub_plugin ); if ( ! $should_load ) { @@ -208,8 +208,9 @@ private function load( Sub_Plugin $sub_plugin ): void { } // Last, and only after a require that happened: register_activation_hook() never fires for - // an included plugin, so this stands in for it with the sub-plugin's code in memory, and a - // skipped load would spend the once-ever record for good. + // an included plugin, so this stands in for it with the sub-plugin's code in memory, and + // creating its tables for code that is not loaded would spend the once-ever record before + // the first real load. $this->activator->maybe_run( $sub_plugin ); // Behind the activation callback, not in front of it. A listener here is host code that will From 889599bdd534df30ddb820b0fc83b52cfdc30d63 Mon Sep 17 00:00:00 2001 From: Nikolay Strikhar Date: Tue, 25 Aug 2026 16:43:05 +0200 Subject: [PATCH 04/10] Stop promising a throw is never retried Nothing about a Sub_Plugin accessor's throw is recorded: it is reported and the pass moves on, and plugins_loaded fires again on the next request. The one place a retry is a decision rather than an accident is the activation callback, which records after the callback returns for exactly that reason. --- docs/extending.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/extending.md b/docs/extending.md index efdc235..35ad82e 100644 --- a/docs/extending.md +++ b/docs/extending.md @@ -41,8 +41,9 @@ use Nexcess\PluginAbsorber\Sub_Plugin; ``` `Sub_Plugin` is one registered configuration; its accessors — `get_slug()` and the message -accessors below — read the config keys and run [their filters](filters.md). A throw from any method -here is caught at the hook boundary, reported through `_doing_it_wrong()`, and never retried. +accessors below — read the config keys and run [their filters](filters.md). A throw from one is +caught at the hook boundary and reported through `_doing_it_wrong()`; nothing is recorded, so the +next request tries again. ### `Registry\Contracts\Registrar_Interface` From ec7c0a6fc4e61e03af9420dde142acbcec76a265 Mon Sep 17 00:00:00 2001 From: Nikolay Strikhar Date: Tue, 25 Aug 2026 16:45:39 +0200 Subject: [PATCH 05/10] Define the actions link the docs list already uses The bullet landed with the lifecycle actions; its reference definition did not, so "Actions" renders as literal brackets between two working links. --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 59db757..36cb667 100644 --- a/README.md +++ b/README.md @@ -73,6 +73,7 @@ point at the repository rather than at paths that would be missing beside the in [recipes]: https://github.com/stellarwp/plugin-absorber/blob/main/docs/recipes.md [conflicts]: https://github.com/stellarwp/plugin-absorber/blob/main/docs/conflict-handling.md [filters]: https://github.com/stellarwp/plugin-absorber/blob/main/docs/filters.md +[actions]: https://github.com/stellarwp/plugin-absorber/blob/main/docs/actions.md [notices]: https://github.com/stellarwp/plugin-absorber/blob/main/docs/notices.md [extending]: https://github.com/stellarwp/plugin-absorber/blob/main/docs/extending.md [tests]: https://github.com/stellarwp/plugin-absorber/blob/main/tests/README.md From b25db734f48d4c520f1e0abb6be529033c863132 Mon Sep 17 00:00:00 2001 From: Eric Defore <7770631+d4mation@users.noreply.github.com> Date: Tue, 25 Aug 2026 11:08:36 -0400 Subject: [PATCH 06/10] Update docs/recipes.md --- docs/recipes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/recipes.md b/docs/recipes.md index 3b88c3c..b593413 100644 --- a/docs/recipes.md +++ b/docs/recipes.md @@ -109,7 +109,7 @@ misspelt option cannot turn a plugin off. ## Defer to a standalone that is a new codebase An edge case that has happened: `learndash-propanel/learndash_propanel.php` was ProPanel 2.x, which -LearnDash absorbed into its Reports module, and ProPanel 3.0 later arrived at that same path as an +LearnDash absorbed into its Reports module, and ProPanel 3.0 arrived at that same path as an unrelated codebase with its own namespace and its own `LDRP_*` constants. `standalone_plugin_basename` cannot tell the two apart; the installed version can, so read it and defer: From 2ff6d8ffc3cbbaa8b3d671405ce0a62dd101d04a Mon Sep 17 00:00:00 2001 From: Eric Defore Date: Tue, 25 Aug 2026 11:21:06 -0400 Subject: [PATCH 07/10] Say the Plugins menu may be turned on, not that it usually is --- src/Traits/Guards_Plugin_Capability.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Traits/Guards_Plugin_Capability.php b/src/Traits/Guards_Plugin_Capability.php index 3fceb29..526833f 100644 --- a/src/Traits/Guards_Plugin_Capability.php +++ b/src/Traits/Guards_Plugin_Capability.php @@ -16,7 +16,7 @@ * * Network-scoped wherever a network exists, because that is how far the act reaches. * `activate_plugins` cannot be relied on to imply the network capability: core widens it only while - * the `menu_items` site option keeps the Plugins menu off, and that menu is usually on. + * the `menu_items` site option keeps the Plugins menu off, and a network may turn that menu on. * * @since 1.0.0 */ From 3f6f1eb7d6b35dd749e2f2c6a5a703fd995056b7 Mon Sep 17 00:00:00 2001 From: Eric Defore Date: Tue, 25 Aug 2026 11:21:09 -0400 Subject: [PATCH 08/10] Say what the guard sees at the load pass's own priority --- docs/conflict-handling.md | 5 +++-- src/Sub_Plugin.php | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/conflict-handling.md b/docs/conflict-handling.md index 347ef44..ff9fac8 100644 --- a/docs/conflict-handling.md +++ b/docs/conflict-handling.md @@ -112,8 +112,9 @@ if ( ! defined( 'GIVE_RECURRING_VERSION' ) ) { ``` The guard is read on `plugins_loaded` at priority 6, so a standalone defining it from a bootstrap -hooked below that is still seen in time; one hooked at 6 or later is not, and the bundled copy loads -on top of it. One that never defines the name is never stood down at all — +hooked below that is still seen in time and one hooked past it is not, leaving the bundled copy to +load on top. At 6 itself it comes down to which of the two hooked first. One that never defines the +name is never stood down at all — [an included recipe](recipes.md#defer-to-a-standalone-that-is-a-new-codebase) is a worked example. ## Reactivating the standalone diff --git a/src/Sub_Plugin.php b/src/Sub_Plugin.php index 32aad71..bea7fe5 100644 --- a/src/Sub_Plugin.php +++ b/src/Sub_Plugin.php @@ -272,8 +272,8 @@ public function is_enabled(): bool { /** * True when the plugin's code is already present, from either copy. The fatal guard. * - * Only sound when the constant is defined at file scope: a standalone defining it from a hook at - * plugins_loaded or later has not defined it yet when this is asked. + * Only sound when the constant is defined at file scope: a standalone defining it from a hook + * past the load pass has not defined it yet when this is asked. * * @since 1.0.0 * From e6a7a5b11db7a2586f5a80cb14c760955d51e597 Mon Sep 17 00:00:00 2001 From: Eric Defore Date: Tue, 25 Aug 2026 11:21:13 -0400 Subject: [PATCH 09/10] Name the shapes should_load really loads on --- src/Loader.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Loader.php b/src/Loader.php index 19a7a8e..da9899d 100644 --- a/src/Loader.php +++ b/src/Loader.php @@ -171,8 +171,8 @@ private function load( Sub_Plugin $sub_plugin ): void { return; } - // Truthiness, not a cast: only a falsy return vetoes, so an array or an object here loads - // rather than fataling on the way to a string. + // Truthiness, not a cast: only a falsy return vetoes, so an object or a non-empty array here + // loads rather than fataling on the way to a string. $should_load = apply_filters( Config::get_hook_name( 'should_load' ), true, $sub_plugin ); if ( ! $should_load ) { From 6a3a61e30f0ff90e73f06704c88b056c8bb6b119 Mon Sep 17 00:00:00 2001 From: Eric Defore Date: Tue, 25 Aug 2026 11:21:13 -0400 Subject: [PATCH 10/10] Promise the step runs again, not that the work is still there --- docs/extending.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/extending.md b/docs/extending.md index 35ad82e..0f7a982 100644 --- a/docs/extending.md +++ b/docs/extending.md @@ -42,8 +42,8 @@ use Nexcess\PluginAbsorber\Sub_Plugin; `Sub_Plugin` is one registered configuration; its accessors — `get_slug()` and the message accessors below — read the config keys and run [their filters](filters.md). A throw from one is -caught at the hook boundary and reported through `_doing_it_wrong()`; nothing is recorded, so the -next request tries again. +caught at the hook boundary and reported through `_doing_it_wrong()`: it costs that step, not the +site, and the next request runs the step again. ### `Registry\Contracts\Registrar_Interface`