csharpsquid:S4457 reporting incorrectly?

Clean Code Report False-positive / False-negative...
1

Hello,

I sometimes get an error for this rule:

  • What language is this for?
    C#
  • Which rule?
    csharpsquid:S4457
  • Why do you believe it’s a false-positive/false-negative?
    I have split up the method, see example.
  • Are you using
    • SonarQube - 9.9.0.65466
  • How can we reproduce the problem? Give us a self-contained snippet of code (formatted text, no screenshots)
public async Task RemoveIdentityAsync(string identityId)
{
    if (string.IsNullOrEmpty(identityId))
    {
        throw new ArgumentNullException(nameof(identityId));
    }
    await RemoveIdentityInternalAsync(identityId);
}

image
image872×254 8.72 KB

3

Ah, it turns out that it’s not a bug and it’s written down correctly, but i just didn’t spot what i was doing wrong. This makes the rule happy:

public Task RemoveIdentityAsync(string identityId)
{
    if (string.IsNullOrEmpty(identityId))
    {
        throw new ArgumentNullException(nameof(identityId));
    }
    return RemoveIdentityInternalAsync(identityId);
}
1 Like
7

Hello @Jeroen_Pot,

Thanks for letting us know that you figured out what was the issue.

Is there something in the rule description or issue message that caused confusion in the first place, so that we can improve it and make it clearer?

Thanks a lot!

9

I"ve spend more time on reading the rule and i just have to confess i didn’t read it properly, and was mostly making assumptions. My focus was on the splitting part and overlooked the clearly stated part

Therefore it is recommended to split the method into two: an outer method handling the parameter checks (without being async/await ) and an inner method to handle the iterator block with the async/await pattern.

I don’t see how this could be described better. Perhaps it can be higlighted in bold for people like me :wink:

2 Likes