07: Sub_Plugin - #7
Open
nikolaystrikhar wants to merge 15 commits into
Open
Conversation
This was referenced Aug 3, 2026
Open
- is_callable() was the type discriminator on every string-or-callable key, and it is true for any string naming an existing function. conflict_policy is designed to be readable from an option, so a stored 'date' or 'flush' was invoked instead of used: a TypeError at plugins_loaded on PHP 8, a silent empty policy on 7.4. Strings and bools are values now. - Type-check required keys. An array passed the truthiness check and cast to "Array", which every sub-plugin making that mistake would have shared as its registry key, activation key, and notice id. - Reject an uncallable dependency_check or activation_callback at registration. Read-time is_callable() made "not configured" and "configured but uncallable" indistinguishable, so a dependency_check pointing at a private method reported dependencies met and let the load run into the fatal it guards. - Drop the redundant network check: WordPress's is_plugin_active() already ORs it in. The test that pinned it described a state WordPress cannot produce. - Guard the plugin.php require on is_plugin_active_for_network, since is_plugin_active is a function third parties shim. - Give get_conflict_notice_message() a default, guard the filter result with is_scalar(), and pass the Sub_Plugin to every configured callable.
nikolaystrikhar
force-pushed
the
06-conflict-policy
branch
from
August 6, 2026 14:22
127f04c to
686266b
Compare
nikolaystrikhar
force-pushed
the
07-sub-plugin
branch
from
August 6, 2026 14:22
b10ecc7 to
8c84385
Compare
…ugin Sub_Plugin named DEACTIVATE as its own fallback, which put "which policy applies when none is configured" in the object that only holds one sub-plugin's config. Conflict_Policy::default() states it now. The value is unchanged. The two fallbacks stay different, and now say why: unconfigured means the sub-plugin accepted the default, whereas an unrecognised policy is a value nobody chose, and reading a typo as consent to deactivate is the outcome worth refusing. Plugin_State_Interface becomes the library's only route to WordPress's plugin functions. Sub_Plugin was a config value object that also queried global plugin state and required wp-admin/includes/plugin.php -- a second reason to change, and the reason these tests had to stub WordPress functions to exercise plain config reads. The gateway owns the reads and the deactivation, so the include exists once, guarded on deactivate_plugins: a function the library actually calls, and still not is_plugin_active, whose third-party shims would short-circuit the require. is_standalone_plugin_active() is deleted rather than delegated -- forwarding would have bought Sub_Plugin a collaborator to answer a question that was never about its configuration. It keeps the two accessors that name the plugin to ask about, and the consumer pairs them with the gateway. That wiring needs Loader and Conflict\Resolver, neither of which exists yet, so it lands with Task 12. is_standalone_plugin_network_active() is deleted outright: no production caller ever appeared, and its stated reason for existing was disproved -- core's null $network_wide already covers both scopes. Sub_Plugin now makes no global WordPress call beyond the defined() that is intrinsic to it.
d4mation
reviewed
Aug 10, 2026
| * @return mixed | ||
| */ | ||
| private function resolve_callable( $value ) { | ||
| if ( is_string( $value ) || is_bool( $value ) || ! is_callable( $value ) ) { |
There was a problem hiding this comment.
Why do we need to check is_string() || is_bool() here? Is it to cover the test_a_message_string_is_never_invoked_as_a_function() and test_a_policy_string_is_never_invoked_as_a_function() cases?
I wonder if this could cause us possible issues where we do want a globally-scoped function name to be provided here. Something like learndash_is_specific_module_loaded_message with no params.
Very edge case, but if we want to explicitly not support String Callables, we will want to make that very clear in the documentation.
Contributor
Author
There was a problem hiding this comment.
Refactored.
There was a problem hiding this comment.
Ah, ok, I think I may have accidentally mislead you here.
We need to support Callables and/or Closures. This is because otherwise we'll have cases where the annoying _doing_it_wrong() warning can show up due to this code likely running before init otherwise if a translatable string is provided to things like conflict_notice_message. However, these Callables may not be able to be allowed to be basic Strings based on the edge case you were trying to solve for.
So something like give_get_specific_module_conflict_message would be disallowed, but something like [ Give_Specific_Module::class, 'get_conflict_message' ], a basic static fn () => __( 'Some message', 'text-domain' ), or maybe a container callback or something would be allowed because we then can ensure we aren't weirdly calling date() or similar when it isn't expected.
There's a PR here that explains the issue possibly a bit better.
The six lines that build a well-formed Sub_Plugin were about to be copied into a fourth test class, so they move into a WithSubPlugins trait with the $overrides signature the reviewer asked for: a test states the one key it is about and reads as being about that key. The trait derives bundled_plugin_file and plugin_loaded_constant from the slug, so fixtures for two sub-plugins cannot collide on a path or a guard constant, and a registry test can ask for a second sub-plugin by naming its slug alone. The derived constant ends in _VERSION_FIXTURE, replacing the per-class _TEST suffix. define() lasts for the whole PHP process, so the suffix was never about the class -- it was about nothing ever defining the fixture default, which one shared name states better than three per-class ones. Tests that need the constant defined name their own, as they already did. Overrides merge last, so an unusable value still reaches the constructor and the tests for rejected config are unaffected.
Task 7 now writes tests/_support/Traits/WithSubPlugins.php, and Tasks 8, 10 and 13 use it instead of each declaring the same private helper with its own guard constant. The extraction lands at the bottom of the stack so the later PRs consume it rather than one of them reaching back to edit a test file its own base branch is still revising. Task 8's re-registration test says which case it is about: a host that registers everything at load and then re-registers one slug once a licence check or a saved setting resolves. Moving that slug to the end would put an add-on ahead of the class it extends. Task 15's WithBundledPlugins stays a separate trait, and the two now say which is which: one builds config objects in memory, the other writes throwaway plugin files to disk.
Conflicts, and the two silent breakages the merge did not flag: Conflict_Policy: main made all() private and moved it below is_valid(). Kept that, with default() added above as a public method. The test for all() goes with it -- main's providers already draw the set from the constants, which is the same guarantee without reaching through the visibility. Config::reset() no longer exists: main replaced it with the reflection based Tests\Support\Config_State so the library does not ship a reset method for the sake of its own suite. SubPluginTest resets through that instead. This merged cleanly and would have fatalled on the next run. PHPStan now analyses tests/, so WithSubPlugins is covered by level 8. It passes as written.
…regarding network and blog branch handling. Improved readability by restructuring sentences for better flow.
…class. Improved documentation for configuration parameters and refined test case handling in ConflictPolicyTest to ensure accurate type validation.
…idation for string-only keys. Update README for clarity on configuration parameters and their expected types. Enhance unit tests to ensure proper rejection of invalid configurations and improve test structure with generator data providers.
- Introduced CLAUDE.md to document the Plugin Absorber library, outlining its structure, commands, and conventions. - Added configuration, conflict handling, and filters documentation to enhance user guidance. - Updated .gitattributes to export-ignore CLAUDE.md and new documentation files during packaging.
- Updated `configuration.md` to clarify the handling of function names as string values and provided examples for using filters. - Added a new test case in `SubPluginTest` to validate the behavior of host helper functions, ensuring they return the expected raw names instead of invoking the functions.
# Conflicts: # CLAUDE.md
- Introduced `get_hook_name` method in the Config class to build hook names with a specified prefix. - Updated `get_conflict_policy`, `get_conflict_notice_message`, and `get_dependency_notice_message` methods in Sub_Plugin to utilize the new hook name generation and apply filters for dynamic message handling. - Enhanced unit tests in ConfigTest to validate hook name generation and ensure exceptions are thrown when no prefix is set.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sub_Pluginholds one registered sub-plugin, its configuration, and every per-sub-plugin decision. Each predicate is a plain method taking no hooks, so later collaborators stay thin and each decision is testable directly.Stacked on #6.