remove_rewrite_tag()

[swissspidy]

As suggested in #14761 and #35227, a remove_rewrite_tag() helper function would be useful to remove registered rewrite tags.

Figured a separate ticket for this function is better to work on it.

[boonebgorges]

In your tests, you're using array_pop() to test the presence of a value in $wp_rewrite properties. This is dangerous: array_pop() modifies the original array, which might break later tests. Can you use assertContains() and assertNotContains() instead? If it's important to check that it's the last item in the array, maybe you could use end()?

[swissspidy]

Replying to boonebgorges:

In your tests, you're using array_pop() to test the presence of a value in $wp_rewrite properties. This is dangerous: array_pop() modifies the original array, which might break later tests. Can you use assertContains() and assertNotContains() instead? If it's important to check that it's the last item in the array, maybe you could use end()?

Ah, thanks for pointing this out! For WP_Rewrite::$rewritereplace it is indeed important.

For example, when you register custom post types, '(.+)' gets added to WP_Rewrite::$rewritereplace multiple times. We only want to remove this once from the array, not all occurrences.

The positions of tags, regexes and query strings inside the arrays need to be in sync because we remove them that way.

Rewrite tags are unique because you can't add one twice (it gets updated instead).

I now switched to using end() and assertContains() where needed and added some more tests.

[boonebgorges]

Replying to swissspidy:

Ah, thanks for pointing this out! For WP_Rewrite::$rewritereplace it is indeed important.

For example, when you register custom post types, '(.+)' gets added to WP_Rewrite::$rewritereplace multiple times. We only want to remove this once from the array, not all occurrences.

The positions of tags, regexes and query strings inside the arrays need to be in sync because we remove them that way.

Rewrite tags are unique because you can't add one twice (it gets updated instead).

I now switched to using end() and assertContains() where needed and added some more tests.

Oh, interesting. Another strategy would be to stash a copy of the array before running remove_rewrite_tag(), and then compare it to the array after running remove_rewrite_tag(). But what you have here looks like it should work :)

[swissspidy]

35236.3.diff​ adds setUp() / tearDown() methods to properly reset the arrays. Also, using array comparison as suggested by @boonebgorges where it makes sense.

[boonebgorges]

Looks good to me.

[swissspidy]

In 36217:

Rewrite: Add a remove_rewrite_tag() helper function.

It can be used to properly remove registered rewrite tags. Adds unit tests.

Fixes #35236.

[swissspidy]

In 36218:

Tests: Fix unit tests after [36217].

See #35236.