csharpsquid:S2583 FP - Function with Action Parameter

  • What language is this for?
  • Csharp
  • Which rule?
  • S2583
  • Using -

    • SonarQube - Enterprise Edition v10.5.1 (90531)[ACTIVE]
  • How can we reproduce the problem? Give us a self-contained snippet of code

Below is an example code snippet to reproduce the issue.
Code –

private Vendor ValidateVendor(string vendorId) {     
Vendor vendor = default;     
ExecuteWithExtraExceptionInformation("While retrieving vendor record", () =>          vendor = VendorRepository.Lookup(new VendorByIdQuery(vendorId)).SingleOrDefault());
   return vendor == default   
  ? throw new ApplicationException($"Unable to find the vendor with ID {vendorId}")  :   vendor.VendorStatus == VendorStatus.Inactive  ? throw new   ApplicationException($"Cannot enter transactions for the inactive vendor with ID   {vendorId}.")  : vendor;
 }

Explanation - Here, the variable vendor is of a type Vendor, which is a class (a reference type). In C#, all reference types are passed by value by default, which means that the reference (the pointer to the object) itself is passed by value. However, the method or delegate can still modify the object that the reference points to.

In above code, the vendor variable is used within a lambda expression, which is passed as an action to the ExecuteWithExtraExceptionInformation method. The lambda expression captures the vendor variable, allowing it to be modified within the expression.

  • Why do you believe it’s a false-positive/false-negative? →

However, at ‘return vendor == default’ SonarQube erroneously shows the error -
“Change this condition so that it does not always evaluate to ‘True’. Some code paths are unreachable.” This is a False Positive since the vendor variable gets successfully modified in the line before and hence the condition does NOT always evaluate to True.

Hey there!

I confirm that this is an FP.
The main reason is that we do not go cross-procedure for Symbolic Execution rules, meaning that ValidateVendor is not knowledgable about what ExecuteWithExtraExceptionInformation does.

This is for performance reasons, but there will be some attempts in the near future to make our symbolic execution engine more precise.

For now, feel free to mark this as an FP on SonarQube, and if your code has a lot of actions-as-parameters, you might want to disable the rule from your Quality Profile.

Thanks for raising this!

Hi Gregory, thanks for your reply. Will follow your advice.

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.