SonarCloud dealing with WordPress hooks

Clean Code Report False-positive / False-negative...
1
  • ALM used: GitHub
  • PHP (WordPress)
  • Remove the unused function parameter "$var".sonarlint(php:S1172)
  • Steps to reproduce:
  • Create a callback for any WP Hook that has more than one parameter and use only the second or further parameter in your callback.
  • Potential workaround
  • Ignore SonarCloud on that line

Assume you have a hook in WP that you write a callback for. For example:
add_action( 'profile_update', 'do_things_when_profile_updates', 10, 3 )
This hook offers three parameters: int $user_id, WP_User $old_user_data, array $userdata

Thus your callback may look like this:

function do_things_when_profile_updates( $user_id, $old_user_data, $user_data ) {

    $old_user_email = $old_user_data->data->user_email;
    $new_user_email = $user_data['user_email'];
    //do further things
}

You will not always use all three parameter. Of course, you can omit any subsequent parameter, however you cannot omit preceding parameters, AFAIK.

What is the suggested way to deal with this?

4

Hello @smileBeda ,

thanks for the report!
You’re right; it does not make sense to raise it here.

We will investigate how we can improve the rule, to not exhibit this behavior.

Best,
Jonas

1 Like
7

Hello @smileBeda,

after looking into it in more detail, I, unfortunately, need to inform you that with the current capabilities of the sonar-php analyzer, we’re not able to check automatically whether a method is registered as a callback hook in WordPress.

To mitigate the false positive, there are a couple of things you can do:

  • Disable the rule ( Unused function parameters should be removed; php:S1172) for your WordPress project in your QualityProfile
  • Accept the findings in SonarCloud
  • Add // NOSONAR at the end of the function to not raise any issues on this line. The result would may look like this:
function do_things_when_profile_updates( $user_id, $old_user_data, $user_data ) { // NOSONAR

    $old_user_email = $old_user_data->data->user_email;
    $new_user_email = $user_data['user_email'];
    //do further things
}

We apologize for any inconvenience this may cause.
Please feel free to share any other suggestions or
requirements you might have.

Best,
Jonas

12

Thanks, I will go with NOSONAR I guess for now.

Is there a chance this can be implemented, or is it impossible at all?
Asking because just considering adding it as a feature request, or not.

14

Hi there, sorry for taking so long.
To be honest, the request is fair but the amount of work that goes behind it is not small, so I am not sure it would come in any reasonable timeframe.
On a side note, there are better suppression mechanisms than NOSONAR, which is kind of the nuclear option as it silences any kind of issue on the line.

Denis

2 Likes