S3168 feature suggestion: Async void event delegate with "On" prefix should raise if it's not an event callback

[ryanachten]

Description

We've found that the current rule for S3168 doesn't reliably report issues for async/void usage. This appears to be due to excluding method names that start with "On" on the assumption that these refer to event handler delegates as per Microsoft guidance.

The issue is, there are many scenarios where "On" is used as a prefix in methods unrelated to event delegates, and in these scenarios the SonarQube rule fails to report such code issues. To this end, I have the following questions:

• Is there a way we can improve the current diagnostic analyzer to reliably check for event delegates without excluding all methods prefixed with "On"?
• If not, would it be acceptable to expect users to suppress this issue for false negatives in cases such as event delegates? Instead of reporting false positives
• Failing all of this, what would be the recommended workaround for those who want to ensure async/void issues are flagged for all methods?

Repro steps

Create an async void method with an "On" prefix. See Actual behavior below for an example.

Expected behavior

Given a method unrelated to an event handler delegate, an S3168 rule violation should be raised:

public async void OnResultExecuted(ResultExecutedContext context) // S3168 raised
{
    // ...
}

Actual behavior

Given a method unrelated to an event handler delegate, an S3168 rule violation is not raised:

public async void OnResultExecuted(ResultExecutedContext context) // No S3168 raised
{
    // ...
}

Known workarounds

• Obviously renaming the method to avoid the use of "On" is an option to avoid this issue. However, this isn't always possible when implementing interfaces outside of our domain.

Related information

• Visual Studio 2022 17.10.5
• .NET 8
• SonarScanner 9.0
• Windows 10

[gregory-paidis-sonarsource]

Hey there,

I am afraid this is not possible. The problem is that it is virtually impossible to separate non-event handling OnXXX methods to event-handler ones.

To do that, we would need to traverse the call graph with our Symbolic Execution engine which would really affect performance. For now we only go method by method. We made the choice to exclude OnXXX methods, as they are usually event handler callbacks, and False Negatives are better (less noisy/annoying) than False Positives.