Preserve backslashes in imported tag, argument, alias, and class meta - #287
Draft
sirreal wants to merge 2 commits into
Draft
Preserve backslashes in imported tag, argument, alias, and class meta#287sirreal wants to merge 2 commits into
sirreal wants to merge 2 commits into
Conversation
WordPress metadata APIs unslash their input, so backslashes in tag, argument, alias, extends, implements, and properties meta are silently destroyed on import. Pin the expected values before fixing the importer.
update_post_meta() unslashes its input, which silently destroyed backslashes in namespaced values: \Foo became Foo, \Foo\Bar became FooBar, and Vendor\Foo became VendorFoo. Compensate with map_deep( $value, 'wp_slash' ), matching the treatment already applied to snippets and Blueprints. _wp_parser_namespace is left alone; it is already compensated with addslashes() and slashing it again would double the separators.
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.
Merging this is blocked on theme-side fixes in
wporg-developer. Today theimporter's unslash bug accidentally strips backslashes out of tag/arg/class
meta, and that accidental stripping is what currently masks several
backslash-intolerant spots in the theme. Fixing the importer alone would start
delivering backslashes to code that mishandles them, turning a silent data
corruption into a visible regression.
The theme items that must land first or simultaneously (items B1/B2/B4 in the
internal follow-ups handoff):
inc/template-tags.phpcallsDevHub_Formatting::link_internal_element()directly for the deprecation notice and drops the whole "Use %s instead."
sentence when nothing links; a backslashed
@see \Some_Classeither vanishesor emits a wrong-post-type URL.
get_used_by()compares_wp-parser_extendsmeta_valueagainstpost_namewith raw SQL equality, so any\in the storedextendsbreaks aparent class's "Used by" list. This is the hard constraint:
extendsmuststay stripped until B2 is fixed.
link_internal_element()ininc/formatting.phpuses an anchored regex plus an exact-match exceptionlist, so
\WP_Query/\wpdbdo not link there.The bug
update_post_meta()runs its value throughwp_unslash()→stripslashes_deep(), so backslashes are eaten on write. The importer alreadycompensates for two things and nothing else:
_wp_parser_namespaceviaaddslashes()_wp-parser_code_snippets/_wp-parser_setup_blueprintsviamap_deep( $value, 'wp_slash' )(added with Export interactive PHP DocBlock snippets #258, with an explanatory commentand a pinning test)
Everything else is corrupted live, today, for any namespaced value:
\FooFoo\Foo\BarFooBarVendor\FooVendorFooSo a
@param \Foo\Bar $xis stored asFooBar.The fix
Apply the same
map_deep( $value, 'wp_slash' )treatment, with a comment in thesame style, to the remaining fields:
_wp-parser_extendsImporter::import_class()_wp-parser_implementsImporter::import_class()_wp-parser_propertiesImporter::import_class()_wp-parser_argsImporter::import_item()_wp_parser_aliasesImporter::import_item()_wp-parser_tagsImporter::import_item()_wp_parser_namespaceis deliberately untouched — it is already compensatedwith
addslashes(), and slashing it again would double the separators. A newassertion pins that it still round-trips as
Vendor\Docs.This is importer-side only; the exporter is not touched, so a corpus diff should
show 0 hunks.
TDD
Add failing tests for backslash loss in imported meta— REDSlash imported tag, argument, alias, and class meta— GREENThe tests follow the existing pinning-test pattern
(
File_Import_Test::test_function_snippet_metadata_preserves_backslashes) andcover all three shapes — single leading backslash (
\Foo), interior separators(
\Foo\Bar), and no-leading-slash namespaced (Vendor\Foo) — across tags(
refersandparamtypes), argument types and defaults, aliases,extends,implements, and property types.RED (before the fix), showing exactly the corruption described above:
GREEN (after the fix), full suite:
Open question — the second strip at display time
lib/class-plugin.php:238runsstripslashes_deepagain at display time, asthe last filter in the
sanitize_argument()chain used bymake_args_safe().Whether that second strip should survive is undecided, and this PR deliberately
does not change it. Worth settling together with the theme work: with the
importer fixed, that filter is now the only remaining place that silently eats
backslashes out of argument data on the way to the template.
Deviation from the reference implementation
An earlier local draft of this change used bare
wp_slash()rather thanmap_deep( $value, 'wp_slash' ), and also rewrote_wp_parser_namespacefromaddslashes()towp_slash(). This PR usesmap_deep()for consistency withthe adjacent snippet/Blueprint code (it also reaches object properties, not just
array members, should any of these values ever carry decoded JSON objects), and
leaves
_wp_parser_namespacealone. That earlier draft was also mixed withunrelated type-rendering work; this PR contains only the meta-slashing concern.
Not included
The
--env-cwdhardcoding inpackage.jsonbreaksnpm run test:phpunitfromany worktree not literally named
phpdoc-parser. That is fixed separately in#276 and is deliberately not part of this branch.
🤖 Generated with Claude Code