Repository navigation
Turn bootstrap into classes, drop shared functions, state the contract - #28
Merged
Merged
Conversation
The structure-audit phase opens on the latest of every package. Composer had nothing to move: every direct dependency is at its latest release and the lock is unchanged, with configs still at 6302f1f, which is configs' trunk, so the reusable-workflow pins stay. npm moved two transitive development packages up: @types/node 26.6.4 to 26.6.5 and strnum 2.4.2 to 2.5.1. No code change was needed. Assisted-by: Claude Code:claude-opus-5-5
The bootstrap package exposed every helper as a namespaced function. @internal tags marked them, but PHP cannot hide a function, and phpstan-no-private misses calls made from class methods, so any helper a plugin happened to call became API the package had to keep. Each by-path file now holds a final class with one public static entry point: Requirements::check(), Updater::register() and Uninstaller::run(). Every helper is a protected static method, the four single-call ones included, and each class declares a protected constructor, so nothing else is reachable. The hooks stay static closures, because a named callback on a static-only class would have to be public. The paths, load.php and PHP 7.4 parseability are unchanged. The Integration tests call the classes, and GitHubUpdaterTest is renamed UpdaterTest after the class it covers. Assisted-by: Claude Code:claude-opus-5-5
NoticeRecords holds only static methods, yet anyone could construct it, which made an instance a reachable part of its surface. The class declares an empty protected constructor, so it cannot be instantiated from outside, as every static-only framework class does. Assisted-by: Claude Code:claude-opus-5-5
Core read its stored integers through Shared\parse_int(), a one-line wrapper around filter_var() that PHPStan already types as int|null. It added no typing, and it put an extension dependency and a function that every package could reach into shared. The six reads in MigrationRunner and the two scheduler backends call filter_var() with FILTER_VALIDATE_INT and FILTER_NULL_ON_FAILURE directly; FILTER_NULL_ON_FAILURE keeps an array from reading as false, which the filter's default option returns. Core declares ext-filter, which PackageManifestsTest pins, and an Integration case pins that an array stored as the revision reads as 0 and stops the run with an error. Assisted-by: Claude Code:claude-opus-5-5
ErrorLogLogger and MigrationRunner's logger fallback interpolated through
Shared\interpolate_log_message(), so a log-formatting detail was API
every package could reach, and the fallback reran formatting code inside
the catch that handles a broken logger.
ErrorLogLogger carries a protected interpolate() with the same rules:
only placeholders the message uses are cast, and a value that throws
while casting leaves its placeholder. Its cases moved from the shared
function's Unit test to a data provider, ProvidesLogMessagesTrait, run
through ErrorLogLogger::log(), so another logger can run the same cases.
The runner's fallback writes the raw message, the context as JSON
without the exception, the exception and the logger's failure; it never
delegates to an ErrorLogLogger, whose qm/{level} action would run again
inside the catch.
Assisted-by: Claude Code:claude-opus-5-5
Shared's functions.php held three unrelated helpers, insert_after(), interpolate_log_message() and parse_int(), loaded on every request through a Composer files entry. Each was reachable API with no second caller that needed it: core reads integers with filter_var(), each logger interpolates its own placeholders, and every v1 insert_after() site has a native rewrite. The file, its files entry and its Unit test are deleted, and shared no longer requires ext-filter, which PackageManifestsTest pins. The package's description drops "helper functions". Assisted-by: Claude Code:claude-opus-5-5
AbstractValueObject is meant to be extended, so its protected are_equal() was surface every subclass could call and the package would have to keep. No subclass calls it, and it is private now; the closure inside it still recurses through self::. Notice::ID_PATTERN is public on purpose, since consumers build notice ids that must match it, and its docblock now says so. Assisted-by: Claude Code:claude-opus-5-5
A package installed where the web server lists directories showed its file tree, since no package directory had an index file, and nothing would notice a directory added without one. Every directory under packages/ carries the stub `<?php declare( strict_types=1 ); // Silence is golden.`, which PHPCS accepts, and PackageManifestsTest fails on a package directory that lacks one. Assisted-by: Claude Code:claude-opus-5-5
AGENTS.md still mapped the bootstrap package as free functions, shared as owning functions.php and ext-filter, and settings and woocommerce as shipping functions.php files. It also let any primitive with one call site into shared, and it never said what the 2.x contract covers. The file map names the bootstrap classes and their entry points, core's ext-filter, the settings and woocommerce function families as final classes, and WooCommerceUninstaller::get_handlers(). Convention 4 states the shared admission rule, convention 6 puts shared logic in static methods of final classes, and convention 10 says each by-path file holds a final class. A Contract section states what the contract covers, the README-named internal namespaces and SemVer from the first tag. The Style section adds the index.php stub, private internals of non-final classes, the no-free-functions rule with its protected constructors and static closures, and the @Package tag group. Assisted-by: Claude Code:claude-opus-5-5
Convention 10 said every by-path file holds a final class, but load.php only requires its siblings and the index.php stub holds no code, so the rule read as broken by two files it covers. It now says the by-path files define code only as final classes with public static entry points, which every one of them meets. Assisted-by: Claude Code:claude-opus-5-5
ahegyes
changed the title
The bootstrap classes lived in kebab-case files named after their former functions, so requirements.php held Requirements, against the one class per ClassName.php rule. Their file docblocks repeated the class docblocks. And the package root mixed the two paths consumers require with two files only load.php loads. The classes live in src/Requirements.php, src/Updater.php and src/Uninstaller.php, with no file docblock, like every other class file. The root holds only the two public paths: load.php, which loads Requirements and Updater, and uninstall.php, which is now a loader for Uninstaller. src/ is loaded by path, not autoloaded, because Composer's autoloader requires every package's files entries as it loads, and PHP-DI's functions.php fails to parse on the PHP 7.4 the requirements gate has to run on. PackageManifestsTest fails if bootstrap's manifest gains an autoload key. AGENTS.md maps the new layout and says that only the loader paths are contract. Assisted-by: Claude Code:claude-opus-5-5
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
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.
Summary
The framework exposed more API than it meant to. The bootstrap package's helpers were namespaced functions that PHP cannot hide, shared loaded three unrelated helper functions on every request through a Composer
filesentry, and AGENTS.md never said what the 2.x contract covers.Requirements::check(),Updater::register()andUninstaller::run()replace the bootstrap functions. Each class is final, its helpers are protected static, and it declares a protected constructor. The classes live insrc/, loaded by path and never autoloaded; consumers still require onlyload.phpanduninstall.php, and PHP 7.4 parsing is unchanged.functions.phpand itsfilesentry are gone:filter_var( …, FILTER_VALIDATE_INT, FILTER_NULL_ON_FAILURE )and requiresext-filter;ErrorLogLoggerinterpolates placeholders with its own protected method;MigrationRunner's logger fallback writes the raw message, the context as JSON, the exception and the logger's failure.NoticeRecordsdeclares a protected constructor, andAbstractValueObject::are_equal()is private.index.phpstub.Affected packages
wp-framework-bootstrapwp-framework-sharedwp-framework-corewp-framework-settings(index.phpstub only)wp-framework-woocommerce(index.phpstub only)Checklist
GitHubUpdaterTestis nowUpdaterTest.ProvidesLogMessagesTraitruns the interpolation cases throughErrorLogLogger::log(); new cases cover the raw logger fallback and an array stored as the migration revision.PackageManifestsTestpinsext-filteron core, anindex.phpin every package directory, and noautoloadkey for bootstrap.composer quality-checkpasses locally (Unit 200, Integration 494), pluscomposer test:integration:multisite(26) and PHP 7.4php -loverpackages/bootstrapREADME.md(the package READMEs stay pointer-form; AGENTS.md carries the file map and the Contract section)Breaking changes / migration notes
No version is tagged yet, so nothing released breaks. Code built on trunk changes:
Requirements::check( __FILE__ )andUpdater::register( __FILE__ )instead ofcheck_requirements()andregister_github_updater();Uninstaller::run()instead ofuninstall(), with the same arguments;Shared\insert_after(),Shared\interpolate_log_message()orShared\parse_int()move to native PHP, for examplefilter_var( …, FILTER_VALIDATE_INT, FILTER_NULL_ON_FAILURE )forparse_int().Every reachable symbol is contract the framework has to keep. The helpers are made unreachable, and shared drops the functions that no second caller needed.