Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle conversion operation as operands of IThrowOperation #6304

Merged
merged 2 commits into from
Dec 8, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -162,8 +166,9 @@ void ThrowException()
{
throw new ArithmeticException();
}
}
");
}",
LanguageVersion = languageVersion,
}.RunAsync();

await VerifyVB.VerifyAnalyzerAsync(@"
Imports System
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -149,7 +160,7 @@ static IEnumerable<string> ConditionalFormatStrings()
{2}";
}

return CartesianProduct(OperationCanceledExceptionCtors, ConditionalFormatStrings());
return CartesianProduct(OperationCanceledExceptionCtors, ConditionalFormatStrings(), LanguageVersionsToTest_CS);
}
}

Expand Down Expand Up @@ -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() { }";
Expand All @@ -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();
}
Expand Down Expand Up @@ -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
}
Expand Down
7 changes: 5 additions & 2 deletions src/Utilities/Compiler/Extensions/IOperationExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,7 @@ 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)
mavasani marked this conversation as resolved.
Show resolved Hide resolved
{
var thrownObject = operation.Exception;

Expand All @@ -752,9 +752,12 @@ public static IOperation WalkDownConversion(this IOperation operation, Func<ICon
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.
Expand Down