From 6629a58ff76ff662603be75643c3a9d9987d1218 Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Thu, 8 Dec 2022 14:36:06 +0530 Subject: [PATCH 1/2] Handle conversion operation as operands of IThrowOperation Fixes #6284 --- .../RethrowToPreserveStackDetails.cs | 2 +- ...lationTokenThrowIfCancellationRequested.cs | 4 +-- .../Runtime/UseExceptionThrowHelpers.cs | 2 +- .../RethrowToPreserveStackDetailsTests.cs | 15 ++++++--- ...nTokenThrowIfCancellationRequestedTests.cs | 31 +++++++++++++++---- .../Extensions/IOperationExtensions.cs | 7 +++-- 6 files changed, 44 insertions(+), 17 deletions(-) diff --git a/src/NetAnalyzers/Core/Microsoft.CodeQuality.Analyzers/QualityGuidelines/RethrowToPreserveStackDetails.cs b/src/NetAnalyzers/Core/Microsoft.CodeQuality.Analyzers/QualityGuidelines/RethrowToPreserveStackDetails.cs index 190ed881f3..823325fe07 100644 --- 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 index e5f1e1ab20..efcafe46b4 100644 --- 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 index 0ff65a9265..cad24dfbb2 100644 --- 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 index 9c2d8ad243..d175db82f8 100644 --- 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 index 447f36e67f..e99340182e 100644 --- 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 LanguageVersionsToTest_CS + { + get + { + yield return CodeAnalysis.CSharp.LanguageVersion.CSharp7.ToString(); + yield return CodeAnalysis.CSharp.LanguageVersion.CSharp10.ToString(); + } + } + private static IEnumerable OperationCanceledExceptionCtors { get @@ -46,13 +55,13 @@ static IEnumerable 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( @@ -60,13 +69,15 @@ public Task SimpleAffirmativeCheck_ReportedAndFixed_CSAsync(string operationCanc @"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 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 CartesianProduct(IEnumerable left, return left.SelectMany(x => right.Select(y => new[] { x, y })); } + private static IEnumerable CartesianProduct(IEnumerable first, IEnumerable second, IEnumerable 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 index 8eb6c2dfe8..f1fd055342 100644 --- a/src/Utilities/Compiler/Extensions/IOperationExtensions.cs +++ b/src/Utilities/Compiler/Extensions/IOperationExtensions.cs @@ -741,7 +741,7 @@ public static IOperation WalkDownConversion(this IOperation operation, Func operation.GetThrownException()?.Type; + /// /// Determines if the one of the invocation's arguments' values is an argument of the specified type, and if so, find /// the first one. From 3b84a3dc04789768d8acc3f596d87f9461086ca3 Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Thu, 8 Dec 2022 16:19:58 +0530 Subject: [PATCH 2/2] Address feedback --- src/Utilities/Compiler/Extensions/IOperationExtensions.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Utilities/Compiler/Extensions/IOperationExtensions.cs b/src/Utilities/Compiler/Extensions/IOperationExtensions.cs index f1fd055342..a41577ab41 100644 --- a/src/Utilities/Compiler/Extensions/IOperationExtensions.cs +++ b/src/Utilities/Compiler/Extensions/IOperationExtensions.cs @@ -746,8 +746,9 @@ public static IOperation WalkDownConversion(this IOperation operation, Func