Repository navigation
Fix the scoping pipeline and pin the rules consumers rely on - #61
Merged
Merged
Conversation
The locks trailed the latest releases of the PHP and Node tooling. Composer moves every package to its newest release within range; symfony/filesystem and symfony/finder stay on 7.4, which humbug/php-scoper caps. npm refreshes every package but TypeScript, which stays on 6.0.3 until typescript-eslint supports 7; the peerDependency range is unchanged, so the lock keeps 6.0.3 because it satisfies it. Lighthouse, under the unused @wordpress/e2e-test-utils-playwright devDependency, now brings Sentry's CLI and bundler plugins. Assisted-by: Claude Code:claude-opus-5-5
The scope run lets some broken output through, misreports some failures, and rejects some valid projects. Its declared-symbol check misses the code in a nowdoc that opens with a blank line, the class of a 'Class::MEMBER' string, a class satisfied only by a function or a namespace of its name, and a constant written in another case. It also lets a scoped package keep a class outside the prefix. It crashes on a nowdoc template that does not parse, and on an unparsable file without naming it. A patcher failure deletes the previous build. A text domain quoted inside another string turns into invalid PHP. A package that shares the project's text domain fails the run. The namespace-string patcher prefixes a host class the stubs exclude. A stubs file's uppercase DEFINE() and a class_alias() named by Foo::class are ignored. A dependency the root replaces or provides fails resolution. These rows fail until the fixes land. ScopedPackagesTest also pins the replace, provide, composer-plugin and metapackage branches, which pass today. Both tests now record installed.php beside installed.json, as Composer does, through one Support trait. Port audit A146-A154 and A182 (X rulings 184, 191), A493 (X ruling 240). Assisted-by: Claude Code:claude-opus-5-5
ScopedPackages read the packages to scope and their replace and provide links from installed.json, which never lists the root package. A project that replaces a polyfill at its root, a common idiom on a PHP floor the polyfill no longer serves, could therefore not scope any library that requires it: the run failed with "requires ..., which is not installed", and the README's ! drop could not help, because the package is not installed. Composer records in installed.php every name a package or the root replaces or provides. A requirement no installed package fills now counts as met when it is recorded there, which also covers the provide map installed.json fed. Port audit A146 (X ruling 184). Assisted-by: Claude Code:claude-opus-5-5
php-scoper empties its output directory, and deletes it again when a patcher throws, such as the text-domain check or a recipe whose search string is gone. vendor-prefixed/ lost its autoloader with it, so the project's autoloader fataled in PHPUnit, PHPStan and the development site until a run succeeded. The run now scopes into vendor-prefixed.tmp/ and replaces the previous build only once php-scoper succeeds. The move also absorbs the vendor directory php-scoper drops for a single-vendor closure. Port audit A149 (X ruling 184). Assisted-by: Claude Code:claude-opus-5-5
The declared-symbol check kept every declaration in one lowercase map and accepted a reference that matched any declaration or namespace. A function satisfied a missing class, a namespace satisfied new, and a constant written in another case passed, although PHP compares constant names by case. A Class::MEMBER string was never checked, though php-scoper prefixes its class. A nowdoc that opens with a blank line went unread although php-scoper scopes it, and a nowdoc template that does not parse, which php-scoper skips, crashed the check, as did an unparsable file, without naming it. The collector now keys declarations by kind, a constant by its exact name below a lowercase namespace, and gives each reference the kinds of symbol it can name where it stands: a function call a function, a constant fetch a constant, an import a class or a namespace, a group import's prefix a namespace, a string any of them, a Class::MEMBER string its class, and any other name a class. It reads a nowdoc as php-scoper does, after trimming it, and skips one that does not parse, and the run names a scoped file it cannot parse. define() and class_alias() recognition has one home on StubSymbolCollector, so a stubs file's uppercase DEFINE() and an alias named by Foo::class stay global too, and one SYMBOL_NAME_REGEX serves both visitors. Port audit A148, A151, A154 (X ruling 184) and A182 (X ruling 191). Assisted-by: Claude Code:claude-opus-5-5
php-scoper leaves a declaration global when its namespace is excluded, and CollectScopingStubs excludes the namespace of every host symbol. A scoped package that bundles its own copy of a library WordPress ships, such as PHPMailer or php-ai-client, therefore kept that copy unprefixed and registered it in the class map with no warning, so at runtime the host's copy and the plugin's replaced each other. The check looked only at prefixed references. verify() now fails when the scoped output declares a class, interface, trait or enum outside the prefix, naming its namespace, but for the polyfill of a PHP class, which php-scoper aliases to its global name. No consumer's scoped output in the workspace declares one. Port audit A147 (X ruling 184). Assisted-by: Claude Code:claude-opus-5-5
The text-domain patcher replaced every quoted occurrence of a package's domain across the file, so a domain quoted inside a string of the other quote style turned into invalid PHP, which then failed the declared-symbol parse instead of the residual-domain check meant to report it. A package whose domain equals the project's always failed, since the patcher replaced the domain with itself and then found it still there. The patcher now rewrites only the string-literal tokens that are exactly the domain, keeps the residual check for every other occurrence, and leaves a domain the project shares alone. Port audit A150, A152 (X ruling 184). Assisted-by: Claude Code:claude-opus-5-5
The namespace-string patcher prefixed every string that starts with a scoped package's PSR-4 root, even one naming a host class php-scoper keeps global, as when a library and its host share a namespace root. Declaring the host's stubs then still failed the run on an undeclared prefixed name. The patcher now leaves a string alone when the name it holds, or that name's namespace, matches a namespace the run excludes, which covers every namespaced symbol the stubs and the dropped packages declare. The run passes php-scoper the same merged list. Token reconstruction reads the texts with array_column(). The A153 row expected the string printed with doubled backslashes, where php-scoper prints single ones. Port audit A153 (X ruling 184), A192 (X ruling 192). Assisted-by: Claude Code:claude-opus-5-5
The scoped autoloader orders the packages' files entries by each package's direct require, and ScopedPackages gives a metapackage no directory, so a package that reaches a dependency only through a metapackage includes its files before that dependency's. No consumer depends that way, so the README's scoping section names the limitation, and flattening metapackage requires waits for a consumer that needs it. Port audit A728 (X rulings 284, 303). Assisted-by: Claude Code:claude-opus-5-5
The scoping pipeline decoded Composer's JSON files four ways, read a manifest's extra entries three ways, and checked the scoping prefix twice, in ScopePhpDependencies, ScopedPackages, CollectScopingStubs and scoper-base.inc.php. The copies drifted: scoper-base's closure turned an unreadable manifest into a JsonException without a path. One internal final class, Manifest, now reads a JSON file, returns a manifest's extra entries and returns the checked scoping prefix, and every caller uses it, so scoper-base drops its $manifest and $extra closures and ScopePhpDependencies its extra() and manifest() helpers. Port audit A181 (X rulings 191, 302). Assisted-by: Claude Code:claude-opus-5-5
Seven protected helpers on final classes had one call site each, against the rule that inlines such a helper unless it is a match target, a hook callback, an override or an assertion: ScopePhpDependencies' verify(), autoload() and excluded(), ScopedPackages' installed(), CollectScopingStubs' declared_stubs() and ScopedSymbolCollector's string(). Each body now stands at its call site. scope() runs its steps in order, checking the project, running php-scoper, swapping the build in, writing the autoloader and verifying the scoped code, and nests no deeper than autoload() did on its own. Its inlined autoloader names $package_manifests and $package_name, and its check $scoped_file, so no name changes meaning within the method. phpstan-strict-rules also reports the autoload loops' reused $path, $class and $file, which take $autoload_path, $file_entry, $class_name and $class_file. Each docblock's why stays as a comment over its block. ScopedPackages reads installed.json and the replace links in one loop. Port audit A185 (X rulings 191, 301). Assisted-by: Claude Code:claude-opus-5-5
Consumers call only ScopePhpDependencies::postAutoloadDump() and scoper-base.inc.php, yet ScopedPackages, CollectScopingStubs, scope() and the PHPStan profile's GlobalTagVisitor were public with no @internal tag, the static-only classes could be constructed, and the visitors' collections were writable from outside. The visitors sat in a namespace named after their visibility, and CollectScopingStubs was named for one of its five inputs. ScopedPackages, GlobalSymbols, scope() and GlobalTagVisitor carry @internal, the three static-only classes declare a protected constructor, and the collections are protected(set), with GlobalTagVisitor's promoted properties protected readonly. Composer\Internal\ becomes Composer\Visitors\, and CollectScopingStubs becomes GlobalSymbols, which the issue templates and AGENTS.md name. Port audit A183, A184, A186, A187 (X ruling 191). Assisted-by: Claude Code:claude-opus-5-5
Configs wrote the same things two ways. Five null checks compared with null where the rest use \is_null(), and one return kept a blank line after a one-line statement. The self-lint rulesets carried two exclusions the tests no longer need, a function-name exclusion for one Composer callback, comments whose reasons were wrong, em dashes and a <description> that repeats the opening comment. The shared base disabled two WordPress-Core sniffs as severity-0 blocks beside the excludes that serve the same purpose, with its purpose comments inside the rules where v1 and a8csp-configs put them above. The scope run's ten-argument command was one 300-character line. The null checks use \is_null() and the blank line goes. The tests ruleset drops ValidVariableName and NoSilencedErrors, postAutoloadDump() takes a trailing phpcs:ignore with its reason, and the comments name what each exclusion covers. The base excludes PSR12's IncorrectOrder and Universal's DisallowShortTernary inside its WordPress rule, keeps WrongNumber's severity 0 because WordPress-Docs adds that sniff first, and puts each purpose comment above its rule. The command takes one argument per line. Port audit A188, A189, A190, A191 (X ruling 192). Assisted-by: Claude Code:claude-opus-5-5
Configs' test classes opened with a blank line, ScopePhpDependenciesTest named no #[CoversClass] although it covers one class, three tests removed their temporary directories three ways, the workflow-step trait carried no Trait suffix, and the autoload row passed a 450-character script to php -r through ten sprintf placeholders while the dompdf row writes a readable nowdoc. The opening blank lines go, ScopePhpDependenciesTest covers ScopePhpDependencies, the temporary directories go through Filesystem::remove(), RunsWorkflowSteps becomes RunsWorkflowStepsTrait, and the autoload probe is a nowdoc file. tests/README and AGENTS.md name the renamed trait, ScopedPackagesTest and RecordsInstalledPackagesTrait. Port audit A495, A496, A498 (X ruling 241) and A191's probe (X ruling 192). Assisted-by: Claude Code:claude-opus-5-5
The slash hook-name delimiter, the generic-type exclusion and the generated-file patterns had no test row, so removing one left configs' suite green and broke only each consumer's next lint. Nothing checked the set of files a profile scans as a whole, or that the tests profile scans a tests/ directory at all. PhpcsProfilesTest now scans a project laid out as a8csp-configs' exclusions smoke is, with configs' tooling, generated and case-twin paths, and asserts the scanned set; checks that the tests profile reports a tests/ file the production profile skips; and passes a slash-separated hook name and a class-string @PARAM over a string hint. Each row fails when its rule leaves the base ruleset. Port audit A162 (X ruling 186), A712 (X ruling 279). Assisted-by: Claude Code:claude-opus-5-5
Several of the release's assertion steps ran under no test. Nothing exercised the input checks: the version-tag pattern, publish from a branch, a .. segment, an absolute path or a slug out of pattern. The secret arms of the forbidden-entry check and the certificate arm that keeps bundled .pem files were untested, the override test looked for one substring of the JSON it writes, and the development-port fallback had no row. The provenance stub printed canned paths without reading its arguments, so the step's head_sha, status, event and conclusion filters could all be deleted with the suite green. ReleaseWorkflowTest now runs the input checks, the secret, node_modules and certificate arms, the override's whole JSON and the port fallback. The fake gh records its arguments and, as the runs API does, answers only the runs of the head_sha it is asked for, piping a canned page through the step's own --jq; rows for a pull_request run, a failed run and another commit's run fail, and each filter's removal fails a row. The harness gains write_archive(), run_step() returns the GitHub env beside the outputs and sets GITHUB_PATH, and write_executable() takes a path below the work directory, so the wp-env stubs use it. Port audit A164 (X ruling 186), A494 (X ruling 240), A497 (X ruling 241). Assisted-by: Claude Code:claude-opus-5-5
The lint:config smoke compiled the tsconfig probe with --noEmit, which masks the base's own noEmit, and checked no other delta; it never looked for a @wordpress/stylelint-config rule in the printed config, a Playwright rule on a tests/e2e file, or the build/ and vendor/ ignores. A regression in any of them passed the smoke. The smoke compiles the probe without the flag and checks it writes no JavaScript, reads tsc --showConfig and checks each of the base's deltas, checks that the printed Stylelint config carries font-weight-notation, which @wordpress/stylelint-config sets, and that tests/e2e/probe.js gets the playwright/ rules while build/x.js and vendor/x.js are ignored. Port audit A492 (X ruling 240). Assisted-by: Claude Code:claude-opus-5-5
Infection mutated php/phpstan, which PHPUnit's coverage source omits and only a PHPStan subprocess runs, so no PHPUnit process covers its code and every mutant there counted as uncovered, although PhpstanConfigTest's @global row pins the visitor. Infection's source now matches PHPUnit's, and tests/README says why php/phpstan stays out. Port audit A163 (X ruling 186). Assisted-by: Claude Code:claude-opus-5-5
reusable-phpunit.yml starts wp-env for a consumer's Integration lane, a branch no job in this repository runs, and neither AGENTS.md nor tests/README said so. Porting a8csp-configs' wp-env consumer fixture and job would cover it at the cost of a second plugin-shaped fixture and a Docker job in configs' CI, while every consumer's Integration lane already runs the branch and a pin-bump pull request catches a break. AGENTS.md Limitations and tests/README's "Not covered here" now name it. Port audit A713 (X ruling 279). Assisted-by: Claude Code:claude-opus-5-5
The namespace-string patcher left a string alone when its name or that name's parent matched an excluded namespace, so it could skip a symbol in a host namespace. A string that ends in a separator names a namespace, though, and php-scoper's exclusions match a namespace exactly: with Acme kept global, php-scoper still prefixed the classes of a scoped Acme\Commerce, while the patcher left 'Acme\Commerce\' unprefixed, so class names built from it resolved to no class. The patcher checks the parent only for a name the string does not follow with a separator. A row scopes a package whose root sits below a namespace a stubs file declares, and fails without the change. Review of the branch; follows up A153 (X ruling 184). Assisted-by: Claude Code:claude-opus-5-5
The text-domain patcher rewrote only string-literal tokens and failed on the domain anywhere else in the code, so a scoped package whose nowdoc holds PHP code with a translation call, such as <?php return __( 'Other', 'acme-commerce' );, now failed the run, where the replacement before it rewrote that literal. php-scoper scopes the code of such a nowdoc like a file, so its literals belong to the package as much as the file's own. The patcher tokenizes the body of a nowdoc that opens with a PHP tag and applies the same rule to it, literal by literal; the domain anywhere else in it still fails the run. A row puts a translation call in a nowdoc and fails without the change. Review of the branch; follows up A150 and A152 (X ruling 184). Assisted-by: Claude Code:claude-opus-5-5
The collector parsed a nowdoc's code without its leading whitespace, as
php-scoper's own detection reads it, because php-scoper puts the prefix
namespace after that whitespace and the result parses only trimmed.
php-scoper scopes the code as written, though, and code with a namespace
or a declare of its own does not parse behind leading whitespace, so
php-scoper leaves it unscoped. The collector still read such a template,
so a nowdoc holding "\n<?php namespace Generated; class Example {}"
failed the run on a class declared outside the prefix.
Behind leading whitespace, the collector reads a nowdoc's code only when
it opens with the prefix namespace php-scoper adds. A row scopes that
template and fails without the change, and the row for a scoped nowdoc
after a blank line still fails on its undeclared name.
Review of the branch; follows up A147 (X ruling 184).
Assisted-by: Claude Code:claude-opus-5-5
php-scoper removes its output directory when it throws, but a fatal error such as an exhausted memory limit, or a signal, ends it without that cleanup, which left a partial vendor-prefixed.tmp/ in the project. No PHPCS, PHPStan or .gitignore pattern covers that directory, so the next lint scanned the scoped files and a broad git add could commit them. The README also promised that a failed run keeps the previous build, while the checks that run after the swap leave the new build in place. The run removes vendor-prefixed.tmp/ whenever php-scoper exits with an error, and the README says a run php-scoper fails keeps the previous build. A row ends php-scoper from a patcher with exit( 1 ) and fails without the removal. Review of the branch; follows up A149 (X ruling 184). Assisted-by: Claude Code:claude-opus-5-5
The tsconfig probe compiles the fixture without --noEmit and fails when probe.js appears, but it left the file in place, so once the base kept noEmit again the smoke still failed on the stale file until someone deleted it by hand. The probe deletes probe.js before it reports the emit. Review of the branch; follows up A492 (X ruling 240). Assisted-by: Claude Code:claude-opus-5-5
StubSymbolCollector::declared_by_call()'s docblock named its argument forms through a placeholder class, Foo::class, which the docblock voice keeps out; the comment above the config load in scope() carried a second sentence explaining the closure idiom, which the inline-comment rule limits to one; and a test helper kept a blank line between a one-line statement and its return. The docblock reads "a string literal or a ::class constant", the comment keeps its one sentence, and the blank line goes. Review of the branch. Assisted-by: Claude Code:claude-opus-5-5
ahegyes
changed the title
Moving the base ruleset's purpose comments above their rule blocks made each one read as the purpose of the whole block. Above Squiz.Commenting.FunctionComment, the @inheritdoc comment seemed to cover the IncorrectTypeHint exclude beside it as well, and the hook-name, file-comment and @throws comments read as describing the sniff rather than the one property, pattern or severity they explain. Each comment sits above the element it explains, as the WordPress-Docs pattern comment does, and a comment above a rule describes the whole reference. The WrongNumber comment keeps the reason it stays a severity-0 block. Review of the branch; follows up A190 (X ruling 192). Assisted-by: Claude Code:claude-opus-5-5
php-scoper can expose a scoped class under its global name through a class_alias() line it appends, but the namespace-string patcher prefixes that line's global name too, since it starts with a scoped package's own namespace, so the alias names the prefixed class itself and exposes nothing. The README named no expose option either way, and exposure undoes the per-plugin copy that prefixing keeps. The README's "Dependency scoping" section says the run supports none of the expose-* options, beside the metapackage limitation. X ruling 304. Assisted-by: Claude Code:claude-opus-5-5
Configs' PHP ran its steps together: in php/, 44 of the 104 places where a block closes before another statement at the same depth had no blank line, against 30 of 594 in the framework's packages, so a loop, the guard after it and the next step read as one block. scope() also opened its command's comment directly under the command array, the recipe loop ran its exclude-namespaces merge into the replacement patchers, and two values were computed among setup lines their only reader sits far below. A blank line follows each closing brace before the next statement, as in the framework, but for a case's break or return and the PHP code inside a test's nowdoc; scope()'s console comment and the recipe's patcher loop open groups of their own; $relative moves beside the files entries that read it, and the scoped package's $autoload beside the namespace loop that reads it. Only whitespace and those two assignments' positions change, which a token comparison against the previous commit confirms. Review of the branch. Assisted-by: Claude Code:claude-opus-5-5
GlobalSymbols parsed each declared stubs file and each dropped package's
PHP file without catching php-parser's error, so a file that did not
parse failed the run with only "Syntax error, unexpected '{' on line 3"
in php-scoper's output, while a dropped package can hold hundreds of
files. A scoped file that does not parse is already named.
The parse throws "Could not parse '<file>', whose symbols the run keeps
global: …" with php-parser's message, and the README lists the failure.
A row declares a stubs file with a syntax error and fails without the
change.
X ruling 306.
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 scope run let broken output through, misreported failures and rejected valid projects; this fixes each case, gives the pipeline's internals one home per concern, and adds tests for the PHPCS profile rules, release checks and Node baselines that consumers depend on.
Type of change
.github/workflows/reusable-*.yml)php/quality-assurance/)Checklist
tests/Unit/)composer testpasses locallydocs/workflows.mdis updated to reflect the new contractdev-trunkconsumers immediately)Breaking changes for downstream consumers
The scope run is stricter. It fails when a scoped package declares a class, interface, trait or enum outside the prefix, but for a polyfill of a PHP class. It fails when the scoped code references a prefixed name that no scoped package declares as the kind of symbol the reference needs, and on a text domain left anywhere other than its own string literal. The plugin template, Internal Comments, Linked Orders and Locked Payment Methods scope with this branch to output byte-identical to their current builds.
A run scopes into
vendor-prefixed.tmp/and replacesvendor-prefixed/only once php-scoper succeeds; a run php-scoper fails keeps the previous build.The internal classes move and carry
@internal:CollectScopingStubsbecomesGlobalSymbols,Composer\Internal\becomesComposer\Visitors\, andScopePhpDependencies::scope()is internal. Consumers call onlyScopePhpDependencies::postAutoloadDump()andscoper-base.inc.php, which keep their signatures. The shared PHPCS base's two disabled sniffs move to<exclude>lines with the same effect.