Merge pull request #6304 from mavasani/FixThrowConversion

mavasani · web-flow · commit 1eb2caf97002 · 2022-12-08T18:07:09.000+05:30
Handle conversion operation as operands of IThrowOperation
diff --git a/src/NetAnalyzers/Core/Microsoft.CodeQuality.Analyzers/QualityGuidelines/RethrowToPreserveStackDetails.cs b/src/NetAnalyzers/Core/Microsoft.CodeQuality.Analyzers/QualityGuidelines/RethrowToPreserveStackDetails.cs
@@ -40,7 +40,7 @@ public override void Initialize(AnalysisContext context)
             {
                 var throwOperation = (IThrowOperation)context.Operation;
 
-                if (throwOperation.Exception is not ILocalReferenceOperation localReference)
+                if (throwOperation.GetThrownException() is not ILocalReferenceOperation localReference)
                 {
                     return;
                 }
diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/UseCancellationTokenThrowIfCancellationRequested.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/UseCancellationTokenThrowIfCancellationRequested.cs
@@ -139,7 +139,7 @@ public bool IsSimpleAffirmativeCheck(IConditionalOperation conditional, [NotNull
                 if (conditional.Condition is IPropertyReferenceOperation propertyReference &&
                     SymbolEqualityComparer.Default.Equals(propertyReference.Property, IsCancellationRequestedProperty) &&
                     whenTrueUnwrapped is IThrowOperation @throw &&
-                    @throw.Exception is IObjectCreationOperation objectCreation &&
+                    @throw.GetThrownException() is IObjectCreationOperation objectCreation &&
                     IsDefaultOrTokenOperationCanceledExceptionCtor(objectCreation.Constructor))
                 {
                     isCancellationRequestedPropertyReference = propertyReference;
@@ -171,7 +171,7 @@ public bool IsNegatedCheckWithThrowingElseClause(IConditionalOperation condition
                     unary.Operand is IPropertyReferenceOperation propertyReference &&
                     SymbolEqualityComparer.Default.Equals(propertyReference.Property, IsCancellationRequestedProperty) &&
                     whenFalseUnwrapped is IThrowOperation @throw &&
-                    @throw.Exception is IObjectCreationOperation objectCreation &&
+                    @throw.GetThrownException() is IObjectCreationOperation objectCreation &&
                     IsDefaultOrTokenOperationCanceledExceptionCtor(objectCreation.Constructor))
                 {
                     isCancellationRequestedPropertyReference = propertyReference;
diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/UseExceptionThrowHelpers.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/UseExceptionThrowHelpers.cs
@@ -146,7 +146,7 @@ aooreThrowIfGreaterThan is not null || aooreThrowIfGreaterThanOrEqual is not nul
                     // any exceptions where a meaningful message may have been provided.  This is an attempt to reduce
                     // false positives, at the expense of potentially more false negatives in cases where a non-valuable
                     // error message was used.
-                    if (throwOperation.Exception.WalkDownConversion() is not IObjectCreationOperation objectCreationOperation ||
+                    if (throwOperation.GetThrownException() is not IObjectCreationOperation objectCreationOperation ||
                         HasPossiblyMeaningfulAdditionalArguments(objectCreationOperation))
                     {
                         return;
diff --git a/src/NetAnalyzers/UnitTests/Microsoft.CodeQuality.Analyzers/QualityGuidelines/RethrowToPreserveStackDetailsTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.CodeQuality.Analyzers/QualityGuidelines/RethrowToPreserveStackDetailsTests.cs
@@ -138,10 +138,14 @@ public void M()
 ");
         }
 
-        [Fact]
-        public async Task CA2200_DiagnosticForThrowCaughtExceptionAsync()
+        [Theory]
+        [InlineData(CodeAnalysis.CSharp.LanguageVersion.CSharp7)]
+        [InlineData(CodeAnalysis.CSharp.LanguageVersion.CSharp10)]
+        public async Task CA2200_DiagnosticForThrowCaughtExceptionAsync(Microsoft.CodeAnalysis.CSharp.LanguageVersion languageVersion)
         {
-            await VerifyCS.VerifyAnalyzerAsync(@"
+            await new VerifyCS.Test
+            {
+                TestCode = @"
 using System;
 
 class Program
@@ -162,8 +166,9 @@ void ThrowException()
     {
         throw new ArithmeticException();
     }
-}
-");
+}",
+                LanguageVersion = languageVersion,
+            }.RunAsync();
 
             await VerifyVB.VerifyAnalyzerAsync(@"
 Imports System
diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/UseCancellationTokenThrowIfCancellationRequestedTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/UseCancellationTokenThrowIfCancellationRequestedTests.cs
@@ -19,6 +19,15 @@ namespace Microsoft.NetCore.Analyzers.Runtime.UnitTests
 {
     public class UseCancellationTokenThrowIfCancellationRequestedTests
     {
+        private static IEnumerable<string> LanguageVersionsToTest_CS
+        {
+            get
+            {
+                yield return CodeAnalysis.CSharp.LanguageVersion.CSharp7.ToString();
+                yield return CodeAnalysis.CSharp.LanguageVersion.CSharp10.ToString();
+            }
+        }
+
         private static IEnumerable<string> OperationCanceledExceptionCtors
         {
             get
@@ -46,27 +55,29 @@ static IEnumerable<string> ConditionalFormatStrings()
 }}";
                 }
 
-                return CartesianProduct(OperationCanceledExceptionCtors, ConditionalFormatStrings());
+                return CartesianProduct(OperationCanceledExceptionCtors, ConditionalFormatStrings(), LanguageVersionsToTest_CS);
             }
         }
 
         [Theory]
         [MemberData(nameof(Data_SimpleAffirmativeCheck_ReportedAndFixed_CS))]
-        public Task SimpleAffirmativeCheck_ReportedAndFixed_CSAsync(string operationCanceledExceptionCtor, string simpleConditionalFormatString)
+        public Task SimpleAffirmativeCheck_ReportedAndFixed_CSAsync(string operationCanceledExceptionCtor, string simpleConditionalFormatString, string languageVersion)
         {
             string testStatements = Markup(
                 FormatInvariant(
                     simpleConditionalFormatString,
                     @"token.IsCancellationRequested",
                     $@"throw new {operationCanceledExceptionCtor};"), 0);
             string fixedStatements = @"token.ThrowIfCancellationRequested();";
+            var parsedVersion = (CodeAnalysis.CSharp.LanguageVersion)Enum.Parse(typeof(CodeAnalysis.CSharp.LanguageVersion), languageVersion);
 
             var test = new VerifyCS.Test
             {
                 TestCode = CS.CreateBlock(testStatements),
                 FixedCode = CS.CreateBlock(fixedStatements),
                 ExpectedDiagnostics = { CS.DiagnosticAt(0) },
-                ReferenceAssemblies = ReferenceAssemblies.Net.Net50
+                ReferenceAssemblies = ReferenceAssemblies.Net.Net50,
+                LanguageVersion = parsedVersion,
             };
             return test.RunAsync();
         }
@@ -149,7 +160,7 @@ static IEnumerable<string> ConditionalFormatStrings()
     {2}";
                 }
 
-                return CartesianProduct(OperationCanceledExceptionCtors, ConditionalFormatStrings());
+                return CartesianProduct(OperationCanceledExceptionCtors, ConditionalFormatStrings(), LanguageVersionsToTest_CS);
             }
         }
 
@@ -290,8 +301,10 @@ public void M()
 
         [Theory]
         [MemberData(nameof(Data_NegatedCheckWithElse_ReportedAndFixed_CS))]
-        public Task NegatedCheckWithElse_ReportedAndFixed_CSAsync(string operationCanceledExceptionCtor, string conditionalFormatString)
+        public Task NegatedCheckWithElse_ReportedAndFixed_CSAsync(string operationCanceledExceptionCtor, string conditionalFormatString, string languageVersion)
         {
+            var parsedVersion = (CodeAnalysis.CSharp.LanguageVersion)Enum.Parse(typeof(CodeAnalysis.CSharp.LanguageVersion), languageVersion);
+
             const string members = @"
 private CancellationToken token;
 private void DoSomething() { }";
@@ -311,7 +324,8 @@ public Task NegatedCheckWithElse_ReportedAndFixed_CSAsync(string operationCancel
                 TestCode = CS.CreateBlock(testStatements, members),
                 FixedCode = CS.CreateBlock(fixedStatements, members),
                 ExpectedDiagnostics = { CS.DiagnosticAt(0) },
-                ReferenceAssemblies = ReferenceAssemblies.Net.Net50
+                ReferenceAssemblies = ReferenceAssemblies.Net.Net50,
+                LanguageVersion = parsedVersion,
             };
             return test.RunAsync();
         }
@@ -676,6 +690,11 @@ private static IEnumerable<object[]> CartesianProduct(IEnumerable<object> left,
             return left.SelectMany(x => right.Select(y => new[] { x, y }));
         }
 
+        private static IEnumerable<object[]> CartesianProduct(IEnumerable<object> first, IEnumerable<object> second, IEnumerable<object> third)
+        {
+            return first.SelectMany(x => second.SelectMany(y => third.Select(z => new[] { x, y, z })));
+        }
+
         private static string FormatInvariant(string format, params object[] args) => string.Format(System.Globalization.CultureInfo.InvariantCulture, format, args);
         #endregion
     }
diff --git a/src/Utilities/Compiler/Extensions/IOperationExtensions.cs b/src/Utilities/Compiler/Extensions/IOperationExtensions.cs
@@ -741,20 +741,24 @@ public static IOperation WalkDownConversion(this IOperation operation, Func<ICon
             return operation;
         }
 
-        public static ITypeSymbol? GetThrownExceptionType(this IThrowOperation operation)
+        public static IOperation? GetThrownException(this IThrowOperation operation)
         {
             var thrownObject = operation.Exception;
 
             // Starting C# 8.0, C# compiler wraps the thrown operation within an implicit conversion to System.Exception type.
+            // We also want to walk down explicit conversions such as "throw (Exception)new ArgumentNullException())".
             if (thrownObject is IConversionOperation conversion &&
-                conversion.IsImplicit)
+                conversion.Conversion.Exists)
             {
                 thrownObject = conversion.Operand;
             }
 
-            return thrownObject?.Type;
+            return thrownObject;
         }
 
+        public static ITypeSymbol? GetThrownExceptionType(this IThrowOperation operation)
+            => operation.GetThrownException()?.Type;
+
         /// <summary>
         /// Determines if the one of the invocation's arguments' values is an argument of the specified type, and if so, find
         /// the first one.

Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@ public override void Initialize(AnalysisContext context)`
`40`	`40`	`{`
`41`	`41`	`var throwOperation = (IThrowOperation)context.Operation;`
`42`	`42`
`43`		`- if (throwOperation.Exception is not ILocalReferenceOperation localReference)`
	`43`	`+ if (throwOperation.GetThrownException() is not ILocalReferenceOperation localReference)`
`44`	`44`	`{`
`45`	`45`	`return;`
`46`	`46`	`}`
Original file line number	Diff line number	Diff line change
`@@ -146,7 +146,7 @@ aooreThrowIfGreaterThan is not null \|\| aooreThrowIfGreaterThanOrEqual is not nul`
`146`	`146`	`// any exceptions where a meaningful message may have been provided. This is an attempt to reduce`
`147`	`147`	`// false positives, at the expense of potentially more false negatives in cases where a non-valuable`
`148`	`148`	`// error message was used.`
`149`		`- if (throwOperation.Exception.WalkDownConversion() is not IObjectCreationOperation objectCreationOperation \|\|`
	`149`	`+ if (throwOperation.GetThrownException() is not IObjectCreationOperation objectCreationOperation \|\|`
`150`	`150`	`HasPossiblyMeaningfulAdditionalArguments(objectCreationOperation))`
`151`	`151`	`{`
`152`	`152`	`return;`
Original file line number	Diff line number	Diff line change
`@@ -19,6 +19,15 @@ namespace Microsoft.NetCore.Analyzers.Runtime.UnitTests`
`19`	`19`	`{`
`20`	`20`	`public class UseCancellationTokenThrowIfCancellationRequestedTests`
`21`	`21`	`{`
	`22`	`+ private static IEnumerable<string> LanguageVersionsToTest_CS`
	`23`	`+ {`
	`24`	`+ get`
	`25`	`+ {`
	`26`	`+ yield return CodeAnalysis.CSharp.LanguageVersion.CSharp7.ToString();`
	`27`	`+ yield return CodeAnalysis.CSharp.LanguageVersion.CSharp10.ToString();`
	`28`	`+ }`
	`29`	`+ }`
	`30`	`+`
`22`	`31`	`private static IEnumerable<string> OperationCanceledExceptionCtors`
`23`	`32`	`{`
`24`	`33`	`get`
`@@ -46,27 +55,29 @@ static IEnumerable<string> ConditionalFormatStrings()`
`46`	`55`	`}}";`
`47`	`56`	`}`
`48`	`57`
`49`		`- return CartesianProduct(OperationCanceledExceptionCtors, ConditionalFormatStrings());`
	`58`	`+ return CartesianProduct(OperationCanceledExceptionCtors, ConditionalFormatStrings(), LanguageVersionsToTest_CS);`
`50`	`59`	`}`
`51`	`60`	`}`
`52`	`61`
`53`	`62`	`[Theory]`
`54`	`63`	`[MemberData(nameof(Data_SimpleAffirmativeCheck_ReportedAndFixed_CS))]`
`55`		`- public Task SimpleAffirmativeCheck_ReportedAndFixed_CSAsync(string operationCanceledExceptionCtor, string simpleConditionalFormatString)`
	`64`	`+ public Task SimpleAffirmativeCheck_ReportedAndFixed_CSAsync(string operationCanceledExceptionCtor, string simpleConditionalFormatString, string languageVersion)`
`56`	`65`	`{`
`57`	`66`	`string testStatements = Markup(`
`58`	`67`	`FormatInvariant(`
`59`	`68`	`simpleConditionalFormatString,`
`60`	`69`	`@"token.IsCancellationRequested",`
`61`	`70`	`$@"throw new {operationCanceledExceptionCtor};"), 0);`
`62`	`71`	`string fixedStatements = @"token.ThrowIfCancellationRequested();";`
	`72`	`+ var parsedVersion = (CodeAnalysis.CSharp.LanguageVersion)Enum.Parse(typeof(CodeAnalysis.CSharp.LanguageVersion), languageVersion);`
`63`	`73`
`64`	`74`	`var test = new VerifyCS.Test`
`65`	`75`	`{`
`66`	`76`	`TestCode = CS.CreateBlock(testStatements),`
`67`	`77`	`FixedCode = CS.CreateBlock(fixedStatements),`
`68`	`78`	`ExpectedDiagnostics = { CS.DiagnosticAt(0) },`
`69`		`- ReferenceAssemblies = ReferenceAssemblies.Net.Net50`
	`79`	`+ ReferenceAssemblies = ReferenceAssemblies.Net.Net50,`
	`80`	`+ LanguageVersion = parsedVersion,`
`70`	`81`	`};`
`71`	`82`	`return test.RunAsync();`
`72`	`83`	`}`
`@@ -149,7 +160,7 @@ static IEnumerable<string> ConditionalFormatStrings()`
`149`	`160`	`{2}";`
`150`	`161`	`}`
`151`	`162`
`152`		`- return CartesianProduct(OperationCanceledExceptionCtors, ConditionalFormatStrings());`
	`163`	`+ return CartesianProduct(OperationCanceledExceptionCtors, ConditionalFormatStrings(), LanguageVersionsToTest_CS);`
`153`	`164`	`}`
`154`	`165`	`}`
`155`	`166`
`@@ -290,8 +301,10 @@ public void M()`
`290`	`301`
`291`	`302`	`[Theory]`
`292`	`303`	`[MemberData(nameof(Data_NegatedCheckWithElse_ReportedAndFixed_CS))]`
`293`		`- public Task NegatedCheckWithElse_ReportedAndFixed_CSAsync(string operationCanceledExceptionCtor, string conditionalFormatString)`
	`304`	`+ public Task NegatedCheckWithElse_ReportedAndFixed_CSAsync(string operationCanceledExceptionCtor, string conditionalFormatString, string languageVersion)`
`294`	`305`	`{`
	`306`	`+ var parsedVersion = (CodeAnalysis.CSharp.LanguageVersion)Enum.Parse(typeof(CodeAnalysis.CSharp.LanguageVersion), languageVersion);`
	`307`	`+`
`295`	`308`	`const string members = @"`
`296`	`309`	`private CancellationToken token;`
`297`	`310`	`private void DoSomething() { }";`
`@@ -311,7 +324,8 @@ public Task NegatedCheckWithElse_ReportedAndFixed_CSAsync(string operationCancel`
`311`	`324`	`TestCode = CS.CreateBlock(testStatements, members),`
`312`	`325`	`FixedCode = CS.CreateBlock(fixedStatements, members),`
`313`	`326`	`ExpectedDiagnostics = { CS.DiagnosticAt(0) },`
`314`		`- ReferenceAssemblies = ReferenceAssemblies.Net.Net50`
	`327`	`+ ReferenceAssemblies = ReferenceAssemblies.Net.Net50,`
	`328`	`+ LanguageVersion = parsedVersion,`
`315`	`329`	`};`
`316`	`330`	`return test.RunAsync();`
`317`	`331`	`}`
`@@ -676,6 +690,11 @@ private static IEnumerable<object[]> CartesianProduct(IEnumerable<object> left,`
`676`	`690`	`return left.SelectMany(x => right.Select(y => new[] { x, y }));`
`677`	`691`	`}`
`678`	`692`
	`693`	`+ private static IEnumerable<object[]> CartesianProduct(IEnumerable<object> first, IEnumerable<object> second, IEnumerable<object> third)`
	`694`	`+ {`
	`695`	`+ return first.SelectMany(x => second.SelectMany(y => third.Select(z => new[] { x, y, z })));`
	`696`	`+ }`
	`697`	`+`
`679`	`698`	`private static string FormatInvariant(string format, params object[] args) => string.Format(System.Globalization.CultureInfo.InvariantCulture, format, args);`
`680`	`699`	`#endregion`
`681`	`700`	`}`
Original file line number	Diff line number	Diff line change
`@@ -741,20 +741,24 @@ public static IOperation WalkDownConversion(this IOperation operation, Func<ICon`
`741`	`741`	`return operation;`
`742`	`742`	`}`
`743`	`743`
`744`		`- public static ITypeSymbol? GetThrownExceptionType(this IThrowOperation operation)`
	`744`	`+ public static IOperation? GetThrownException(this IThrowOperation operation)`
`745`	`745`	`{`
`746`	`746`	`var thrownObject = operation.Exception;`
`747`	`747`
`748`	`748`	`// Starting C# 8.0, C# compiler wraps the thrown operation within an implicit conversion to System.Exception type.`
	`749`	`+ // We also want to walk down explicit conversions such as "throw (Exception)new ArgumentNullException())".`
`749`	`750`	`if (thrownObject is IConversionOperation conversion &&`
`750`		`- conversion.IsImplicit)`
	`751`	`+ conversion.Conversion.Exists)`
`751`	`752`	`{`
`752`	`753`	`thrownObject = conversion.Operand;`
`753`	`754`	`}`
`754`	`755`
`755`		`- return thrownObject?.Type;`
	`756`	`+ return thrownObject;`
`756`	`757`	`}`
`757`	`758`
	`759`	`+ public static ITypeSymbol? GetThrownExceptionType(this IThrowOperation operation)`
	`760`	`+ => operation.GetThrownException()?.Type;`
	`761`	`+`
`758`	`762`	`/// <summary>`
`759`	`763`	`/// Determines if the one of the invocation's arguments' values is an argument of the specified type, and if so, find`
`760`	`764`	`/// the first one.`