Rename usage of `has_permission()` to account for a `WP_Error`

[johnbillion]

The following pages document how to use return value of the has_permission() method:

• https://github.com/WordPress/abilities-api/blob/v0.1.1/docs/2.getting-started.md
• https://github.com/WordPress/abilities-api/blob/v0.1.1/docs/4.using-abilities.md

This method can return a WP_Error or a boolean, so a check like if ( $ability->has_permission( $input ) ) does not do what is expected. A WP_Error instance will evaluate to true and cause this logical check to unexpectedly pass.

The WP_Ability::execute() method correctly handles a WP_Error being returned by the permissions check.

There are a few options.

• Adjust has_permission() to only return a boolean, so it behaves more like a traditional capability check function such as current_user_can().
  • Can be combined with retaining the existing method under a different name.
  • Can be combined with throwing an exception. Not very WordPress-y.
• Retain everything as-is but update all the developer documentation to include checks for is_wp_error(). I think this is risky, it's too easy for a developer to implement a simple boolean check and therefore incorrectly handle a WP_Error.

[Ref34t]

@gziolo can I jump on this bug?

[gziolo]

@Ref34t, sure thing 🚀

Feel free to comment on subsequent issues and start working on them immediately and someone will update the issue to reflect status change 👍🏻

[Ref34t]

@gziolo Roger that 😎

[justlevine]

There are a few options.

@johnbillion @Ref34t after reviewing #76 , I believe there's a third option:

3. Rename the function from ::has_permissions() to ::check_permissions() (or similar), but keep the rest of the method signature.

By avoiding the 'has_' prefix, we should be able to avoid the confusion of an expected bool and the problematic shorthand, while maintaining consistency with the other places in WP Core that use bool|WP_Error or the even more common true|WP_Error.

(PS we were doing throw and _doing_it_wrong() before we switched to using WP_Error in order to do things "the WordPress way" )

[gziolo]

Retain everything as-is but update all the developer documentation to include checks for is_wp_error(). I think this is risky, it's too easy for a developer to implement a simple boolean check and therefore incorrectly handle a WP_Error.

That would be my preferred approach to extend the docs.

Alternatively, I'm also open to renaming the method to check_permission(), but at this point, we would have to add a temporary deprecation for has_permission() to give some time for devs to update their usage.