Uh oh!
There was an error while loading. Please reload this page.
Align the prep-diff tool with the exporter's normalization rules - #279
Open
sirreal wants to merge 2 commits into
Open
Align the prep-diff tool with the exporter's normalization rules#279sirreal wants to merge 2 commits into
sirreal wants to merge 2 commits into
Conversation
Extracted from the original "Preserve expression output" commit so the diff tool's changes can land separately from the exporter's. The diff normalization must follow the same rules the exporter uses when it strips PHP-Parser's synthetic global namespace prefixes, otherwise the normalized output no longer matches what the exporter produces.
`tests/prep-diff-test.php` was a standalone script that nothing executed: `phpunit.xml.dist` only loads `tests/phpunit/tests/`, and both `composer test` and CI run PHPUnit with that configuration. Move it under `tests/phpunit/tests/` as a test case so that it runs with the rest of the suite. `prep-diff.php` normalized printed expressions with its own prose-oriented regular expression, which also matched global names appearing inside string literals. An argument default of `'see \Foo bar'` was rewritten to `'see Foo bar'`, which is exactly the mangled output the script is meant to expose, so a regression in expression printing diffed clean. Reuse the exporter's `WP_Parser\strip_global_namespace_prefix()` instead, which only strips a prefix at the start of a value. Inline documentation references are unaffected: they never contain whitespace, so both rules produce the same result for them. Diffs of output generated before and after this branch now show the string-literal fix instead of hiding it, which is intended.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Stacked on #262 (base branch:
fix-254). Extracts the diff-tool work from that PR so it can be reviewed separately from the export-semantics changes.Two commits:
tests/prep-diff-test.phptotests/phpunit/tests/prep-diff.phpas a PHPUnit class socomposer testand CI actually execute it (previously no runner invoked it), and replaces the tool's hand-rolled prose-style regex with the exporter's own\WP_Parser\strip_global_namespace_prefix(). The old regex matched inside string literals, so it normalized'see \Foo bar'to'see Foo bar'on both sides of a diff — masking exactly the string-corruption class Preserve documentation and hook string semantics #262 fixes. With the shared helper, a regression re-introducing that corruption shows up in the diff instead of vanishing.Suite on this branch: 173 tests green (the 9 prep-diff tests on top of #262's 164).
prep-diff.phpstill runs standalone (php prep-diff.php < export.json).🤖 Generated with Claude Code