WP/AlternativeFunctions: add XML documentation

[rodrigoprimo]

Description

This PR adds XML documentation for the WordPress.WP.AlternativeFunctions sniff.

The documentation is based on the work started by @pamprn09 in #2496 and @bhubbard in #2588. I squashed the original commits and, in a separate commit, made subsequent changes based on code review suggestions.

I suggest squashing those two commits before merging. I'm opening the PR without doing that to make it easier to tell my changes apart from the original changes.

While reviewing #2588, I mentioned the following:

I wonder whether the documentation should contain a single generic <standard>/<code_comparison> block that mentions that some PHP functions shouldn't be used and WP functions should be used instead, or if it should contain one <standard>/<code_comparison> per function group. I can see examples of both approaches in the codebase: WordPress.WP.DeprecatedFunctions uses the generic and short approach, while WordPress.DateTime.RestrictedFunctions uses the function group approach (but the sniff contains just two function groups, while WordPress.WP.AlternativeFunctions contains much more than that).

In this PR, I opted for the first approach and created a single <standard>/<code_comparison> block. I include a few examples using the main "categories" of functions flagged by the sniff, without including every single group, to avoid having too many examples. Let me know if you prefer one <standard>/<code_comparison> per function group or something else.

Suggested changelog entry

N/A

Related issues/external references

Related to: #1722
Supersedes: #2496 and #2588
Closes #2496
Closes #2588

[jrfnl]

In this PR, I opted for the first approach and created a single /<code_comparison> block. I include a few examples using the main "categories" of functions flagged by the sniff, without including every single group, to avoid having too many examples. Let me know if you prefer one /<code_comparison> per function group or something else.

I agree with the single <standard> block approach, but can imagine the code comparisons may be clearer if they were split into multiple <code_comparison> blocks. @dingo-d @GaryJones What do you think ?

[dingo-d]

In this PR, I opted for the first approach and created a single /<code_comparison> block. I include a few examples using the main "categories" of functions flagged by the sniff, without including every single group, to avoid having too many examples. Let me know if you prefer one /<code_comparison> per function group or something else.

I agree with the single <standard> block approach, but can imagine the code comparisons may be clearer if they were split into multiple <code_comparison> blocks. @dingo-d @GaryJones What do you think ?

Sounds good to me. I think this will help with readability.

[rodrigoprimo]

@jrfnl, do you mean one <code_comparison> per function group defined in the sniff, or per the broader categories I used in the examples (HTTP, JSON, URL parsing, etc)?

[jrfnl]

@jrfnl, do you mean one <code_comparison> per function group defined in the sniff, or per the broader categories I used in the examples (HTTP, JSON, URL parsing, etc)?

I'm leaving that up to you. Won't know what I'd prefer until I see it ;-)

[rodrigoprimo]

After playing with the options we discussed, I ended up preferring a single <code_comparison> with one representative example, similar to DeprecatedFunctionsStandard.xml.

I prefer this approach because I don't want to include every single function the sniff checks for to avoid the maintenance burden of updating the list whenever a function is added or removed. And a partial list left me wondering what value multiple examples add to the documentation, while also raising the question of why include those functions and not the others.

What do you think of this approach?

[dingo-d]

Looks good to me 👍🏼

[jrfnl]

@rodrigoprimo Looking at the doc as it is now, I think this doc doesn't add any value in its current state.

I believe we should explain why people should follow this rule, not just blindly repeat the rule.

In nearly all cases, the WP native versions of these functions were created to overcome

• differences in PHP cross-version behaviour, or
• differences in cross-OS behaviour, or
• differences in server configurations etc.

I believe it is probably helpful to mention something along those lines in the <standard> description.

We may actually want to go back to some other recently merged docs to double-check if we should improve the clarity of the information in those too.

[rodrigoprimo]

I believe we should explain why people should follow this rule, not just blindly repeat the rule.

I pushed a new commit, adding a second paragraph to the standard description explaining why these functions should be used. Let me know what you think.

We may actually want to go back to some other recently merged docs to double-check if we should improve the clarity of the information in those too.

That is a good point. I will review the recently merged docs PRs and create follow-up PRs adding the why to the standard description wherever applicable. At first glance, here are the PRs I plan to address: #2676, #2679, #2680, and #2689.

[jrfnl]

Thanks for making that update @rodrigoprimo, the description looks much better now.

When looking at the docs again though, I realized that the one example which is left now, while correct, is also one which will only conditionally be flagged (when we suspect an external URL is being requested, not an internal file).

This sniff flags various other functions unconditionally. I imagine, it might lessen confusion if the code sample referenced one of those instead.
Think: wp_rand() vs mt_rand() or wp_parse_url() vs parse_url() (yes, I know there is a condition for that one too, but condition is dated, so I hope that it will rarely come into play nowadays).

[rodrigoprimo]

Ok, I replaced the file_get_contents()/wp_remote_get() example with mt_rand()/wp_rand() as suggested.

[jrfnl]

Thanks for fixing that up @rodrigoprimo ! All good from me.

[jrfnl]

@dingo-d As quite a lot has changed since you reviewed, I've re-requested your review. Hope you don't mind.

[jrfnl]

For whomever does the second review: please squashmerge.