Introduce Functions\stubWpUrlFunctions() helper

[gmazzap]

It makes use of a new UrlsHelper class.

Tests and documentation added.

See #125

[jrfnl]

I've not completely reviewed this PR yet as I believe this remark needs to be addressed/hashed out first:

It looks like most of these don't take passed parameters to the original function into account, while WP does support passing parameters.

Stubbing these in the framework without taking the passes parameters into account will probably be confusing and lead to support overhead or to people just not using these stubs as they don't do what they expect.

As addressing that will require some rework in the PR (if it will be addressed), I figured my time would be better spend reviewing after the discussion has been had about the above.

[jrfnl]

All @param declarations in this file are missing the type information, which makes them kind of useless.

[gmazzap]

All @param are mixed anyway. The type is checked inside the function via is_string and similar. So you can pass whatever. Mixed is implicit so to me no explicit type would be fine.

If we would add Psalm or similar, as soon I would declare @param string and then do is_string() checks inside the function Psalm would rightfully complain for "redundant condition" and "doc bloc type contradiction" (see an example).

I prefer to keep the check at runtime so all the parameters are mixed, and IMO there's no point in typing "mixed" all over the place.
Still, I like to keep the doc block for "visual help". I personally find it hard to digest a wall of code without some "calming" doc block in between functions.

Anyway, I added mixed pretty much everywhere, unless a private method where the type of the parameters is ensured.

[jrfnl]

It looks like most of these don't take passed parameters to the original function into account, while WP does support passing parameters.

Stubbing these in the framework without taking the passes parameters into account will probably be confusing and lead to support overhead or to people just not using these stubs as they don't do what they expect.

[gmazzap]

@jrfnl I think you need to look at it again :)

$helper->stubUrlCallback() and $helper->stubUrlForSiteCallback() return callbacks that take respectively 2 ($path, $schema) and 3 parameters ($blog_id, $path, $schema), that are the same core functions take.

There are 2 exceptions: content_url() and wp_login_url().

wp_login_url() takes still 2 arguments, but different from the others, and that is why it is stubbed differently.

content_url() takes 1 parameter: $path. That is why stubUrlCallback() accepts a 3rd parameter $use_schema_arg that defaults to true, but it is passed as false for content_url().

To be noted:

• the $blog_id parameter for functions that accepts it is just ignored in stubbed functions because the base URL is taken from what is passed to the stubWpUrlFunctions() API function.
• When $schema param is something like 'admin' or 'login' it is taken into account only if HTTPs is not forced (passing true as stubWpUrlFunctions()'s 2nd parameter) and force_ssl_admin() is available (likely because stubbed separately).
• rest_url() and get_rest_url() use 'rest' as scheme. That is not relevant for the generation of the URL (in WP just like in the stubbed version), it is used in WP only as the 3rd parameter, $orig_scheme, passed to the 'set_url_scheme' hook, something that I think is pretty safe to ignore for the stubbed version of the function.

TL;DR: all the stubbed functions accept the same parameters and work very similarly to the WP counterparts. For some edge cases, it might be needed to stub is_ssl() and/or force_ssl_admin() separately.

You can even see the parameters used with success in tests: https://github.com/Brain-WP/BrainMonkey/blob/feature/stub-wp-urls/tests/cases/unit/Api/FunctionsTest.php#L439-L455

[codecov-commenter]

Codecov Report

Patch coverage: 97.91% and project coverage change: -0.02 ⚠️

Comparison is base (d9ff201) 92.25% compared to head (dfffa27) 92.23%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #129      +/-   ##
============================================
- Coverage     92.25%   92.23%   -0.02%     
- Complexity      312      342      +30     
============================================
  Files            28       29       +1     
  Lines           710      876     +166     
============================================
+ Hits            655      808     +153     
- Misses           55       68      +13     

Impacted Files	Coverage Δ
src/Expectation/UrlsHelper.php	97.91% <97.91%> (ø)

... and 20 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[gmazzap]

I've not completely reviewed this PR yet as I believe this remark needs to be addressed/hashed out first:

It looks like most of these don't take passed parameters to the original function into account, while WP does support passing parameters.
Stubbing these in the framework without taking the passes parameters into account will probably be confusing and lead to support overhead or to people just not using these stubs as they don't do what they expect.

As addressing that will require some rework in the PR (if it will be addressed), I figured my time would be better spend reviewing after the discussion has been had about the above.

Commented all your points.

Your concerns about parameters, as better explained below, aren't correct because parameters are accepted in the same way WP does. Besides my reasoning in other comments, there're tests to prove it :)

So, please @jrfnl, review it again when/if you have time.