Roslyn-analyzers: CA2200: Rethrow to preserve stack details - should not warn when throwing different exception

[buyaa-n]

Analyzer

Diagnostic ID: CA2200: Rethrow to preserve stack details

Analyzer source

SDK: Built-in CA analyzers in .NET 5 SDK or later

Version: SDK 5.0.100-preview.8

Describe the bug

The caught exception ex is only thrown when it is mapped into a different exception by the MapSendException(...) method. The analyzer should not warn in this case, further I would assume CA2200 should not warn when the catch block is throwing a different exception (mappedEx) than the catched exception (ex)

    try
    {
          ... 
    }
    catch (Exception ex) when (MapSendException(ex, cancellationToken, out Exception mappedEx))
    {
        throw mappedEx;
    }

The exact warning found here, related to https://github.com/dotnet/runtime/pull/42957#discussion_r498532030

Expected behavior

Should not warn in above case

Actual behavior

Warning

[Youssef1313]

A similar case is being tested:

https://github.com/dotnet/roslyn-analyzers/blob/76374e785b11c2fab3515cf35f89f5f2c4c0eeeb/src/NetAnalyzers/UnitTests/Microsoft.CodeQuality.Analyzers/QualityGuidelines/RethrowToPreserveStackDetailsTests.cs#L56

@Evangelink Was this fixed in your re-write of the analyzer? or it should have been working from before the rewrite?

[mavasani]

@buyaa-n Can you test with the latest package from https://dev.azure.com/dnceng/public/_packaging?_a=package&feed=dotnet6&package=Microsoft.CodeAnalysis.NetAnalyzers&protocolType=NuGet&version=6.0.0-preview1.20501.5&view=overview? I suspect @Evangelink's recent fix in master would have fixed this. We should consider backporting the change to release/dotnet5-rc2 branch if that is the case.

[buyaa-n]

A similar case is being tested:
@Evangelink Was this fixed in your re-write of the analyzer?

Yep, it is the same case, looks already fixed

@buyaa-n Can you test with the latest package from https://dev.azure.com/dnceng/public/_packaging?_a=package&feed=dotnet6&package=Microsoft.CodeAnalysis.NetAnalyzers&protocolType=NuGet&version=6.0.0-preview1.20501.5&view=overview? I suspect @Evangelink's recent fix in master would have fixed this. We should consider backporting the change to release/dotnet5-rc2 branch if that is the case.

Yep i used RC2 branch, not master, agree we should port it to RC2

[buyaa-n]

@buyaa-n Can you test with the latest package from https://dev.azure.com/dnceng/public/_packaging?_a=package&feed=dotnet6&package=Microsoft.CodeAnalysis.NetAnalyzers&protocolType=NuGet&version=6.0.0-preview1.20501.5&view=overview

Hm, i still see the warning with that version

[Youssef1313]

@buyaa-n Interesting! Let me add a test and debug it.

[Evangelink]

I didn't consider this case, honestly that's the first time I am seeing this pattern :)

@Youssef1313 has already filed a PR