Sonar-dotnet: Fix S1854 FP: `try` block not connected to `catch` when exception is thrown inside

Created on 15 Aug 2017  路  4Comments  路  Source: SonarSource/sonar-dotnet

Description

Variables that are only used in exception filters incorrectly trigger S1854

Repro steps

bool shouldCatch = false;
try
{
    shouldCatch = true; //S1854 Remove this useless assignment to local variable 'shouldCatch'
    throw new InvalidOperationException("bar");
}
catch (Exception) when(shouldCatch)
{
    Console.WriteLine("Error");
}

Expected behavior

S1854 should not trigger because the shouldCatch assignment is not useless. Indeed, without it the exception would pass through.

Actual behavior

S1854 is triggered.

Related information

  • SonarC# Version 6.3.0.2862
  • Visual Studio Version 2017 (15.2)
C# False Positive Improvement

Most helpful comment

The issue is still there. Found a fix ?
My case :

public static void RetryOnException(int retries, TimeSpan delay, Action operation)
{
    var attempts = 0;
    do
    {
        try
        {
            attempts++; //Remove this useless assignment to local variable 'attempts'.
            operation();
            break;
        }
        catch (Exception ex)
        {
            if (attempts > retries)
                throw;
            Logger.LogError($"Exception caught on attempt {attempts} - will retry after delay {delay}", ex);
            Task.Delay(delay).Wait();
        }
    } while (true);
}

All 4 comments

Hi @ohadschn.
Thanks for the feedback. This is going to be tricky to fix but we will see what we can do.

The issue is still there. Found a fix ?
My case :

public static void RetryOnException(int retries, TimeSpan delay, Action operation)
{
    var attempts = 0;
    do
    {
        try
        {
            attempts++; //Remove this useless assignment to local variable 'attempts'.
            operation();
            break;
        }
        catch (Exception ex)
        {
            if (attempts > retries)
                throw;
            Logger.LogError($"Exception caught on attempt {attempts} - will retry after delay {delay}", ex);
            Task.Delay(delay).Wait();
        }
    } while (true);
}

This problem with the shape of the CFG may have some connection with #2393

Moving @killergege 's example to #2393 because it's a separate root cause (try-catch inside a loop vs try-catch with a throw inside the try, as in this original ticket problem), and we need to keep concerns separate between tickets.

Was this page helpful?
0 / 5 - 0 ratings