From 76a88d220fa6b0327229eb5022f1ac6b38485676 Mon Sep 17 00:00:00 2001 From: sdelarosbil Date: Wed, 14 Feb 2024 11:00:58 -0500 Subject: [PATCH 1/2] UseConditionExpression: Add support for checked expression in if and return statement --- ...ionalExpressionForReturnCodeFixProvider.cs | 16 ++ .../UseConditionalExpressionForReturnTests.cs | 140 ++++++++++++++++++ ...UseConditionalExpressionCodeFixProvider.cs | 17 ++- 3 files changed, 170 insertions(+), 3 deletions(-) diff --git a/src/Analyzers/CSharp/CodeFixes/UseConditionalExpression/CSharpUseConditionalExpressionForReturnCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseConditionalExpression/CSharpUseConditionalExpressionForReturnCodeFixProvider.cs index 8e4389c0577ed..f00b94fe540ab 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseConditionalExpression/CSharpUseConditionalExpressionForReturnCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseConditionalExpression/CSharpUseConditionalExpressionForReturnCodeFixProvider.cs @@ -45,6 +45,22 @@ protected override StatementSyntax WrapWithBlockIfAppropriate( return statement; } + protected override SyntaxNode WrapIfStatementIfNecessary(IConditionalOperation operation) + { + if (operation.Syntax is IfStatementSyntax { Condition: CheckedExpressionSyntax exp }) + return exp; + + return base.WrapIfStatementIfNecessary(operation); + } + + protected override ExpressionSyntax WrapReturnExpressionIfNecessary(ExpressionSyntax returnExpression, IOperation returnOperation) + { + if (returnOperation.Syntax is ReturnStatementSyntax { Expression: CheckedExpressionSyntax exp }) + return exp; + + return base.WrapReturnExpressionIfNecessary(returnExpression, returnOperation); + } + protected override ExpressionSyntax ConvertToExpression(IThrowOperation throwOperation) => CSharpUseConditionalExpressionHelpers.ConvertToExpression(throwOperation); diff --git a/src/Analyzers/CSharp/Tests/UseConditionalExpression/UseConditionalExpressionForReturnTests.cs b/src/Analyzers/CSharp/Tests/UseConditionalExpression/UseConditionalExpressionForReturnTests.cs index 1165bc343c5d5..0b7a6f3a1fc48 100644 --- a/src/Analyzers/CSharp/Tests/UseConditionalExpression/UseConditionalExpressionForReturnTests.cs +++ b/src/Analyzers/CSharp/Tests/UseConditionalExpression/UseConditionalExpressionForReturnTests.cs @@ -502,6 +502,146 @@ int M() """); } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/70748")] + public async Task TestMissingWithCheckedInIf() + { + await TestInRegularAndScriptAsync( + """ + class C + { + int M() + { + int x = 0; + int y = 0; + [||]if (checked(x == y)) + { + return 0; + } + else + { + return 1; + } + } + } + """, + """ + class C + { + int M() + { + int x = 0; + int y = 0; + return checked(x == y) ? 0 : 1; + } + } + """, parseOptions: CSharp8); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/70748")] + public async Task TestMissingWithUncheckedInIf() + { + await TestInRegularAndScriptAsync( + """ + class C + { + int M() + { + int x = 0; + int y = 0; + [||]if (unchecked(x == y)) + { + return 0; + } + else + { + return 1; + } + } + } + """, + """ + class C + { + int M() + { + int x = 0; + int y = 0; + return unchecked(x == y) ? 0 : 1; + } + } + """, parseOptions: CSharp8); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/70748")] + public async Task TestMissingWithCheckedInTrueStatement() + { + await TestInRegularAndScriptAsync( + """ + class C + { + int M() + { + int x = 0; + int y = 0; + [||]if (x == y) + { + return checked(x - y); + } + else + { + return 1; + } + } + } + """, + """ + class C + { + int M() + { + int x = 0; + int y = 0; + return x == y ? checked(x - y) : 1; + } + } + """, parseOptions: CSharp8); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/70748")] + public async Task TestMissingWithUncheckedInTrueStatement() + { + await TestInRegularAndScriptAsync( + """ + class C + { + int M() + { + int x = 0; + int y = 0; + [||]if (x == y) + { + return unchecked(x - y); + } + else + { + return 1; + } + } + } + """, + """ + class C + { + int M() + { + int x = 0; + int y = 0; + return x == y ? unchecked(x - y) : 1; + } + } + """, parseOptions: CSharp8); + } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/70750")] public async Task TestMissingWithUnchecked() { diff --git a/src/Analyzers/Core/CodeFixes/UseConditionalExpression/AbstractUseConditionalExpressionCodeFixProvider.cs b/src/Analyzers/Core/CodeFixes/UseConditionalExpression/AbstractUseConditionalExpressionCodeFixProvider.cs index 3587747ae6e4d..104a04b631717 100644 --- a/src/Analyzers/Core/CodeFixes/UseConditionalExpression/AbstractUseConditionalExpressionCodeFixProvider.cs +++ b/src/Analyzers/Core/CodeFixes/UseConditionalExpression/AbstractUseConditionalExpressionCodeFixProvider.cs @@ -104,7 +104,7 @@ protected async Task CreateConditionalExpressionAsync( var generatorInternal = document.GetRequiredLanguageService(); var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); - var condition = ifOperation.Condition.Syntax; + var condition = WrapIfStatementIfNecessary(ifOperation); if (CanSimplify(trueValue, falseValue, isRef, out var negate)) { return negate @@ -112,10 +112,15 @@ protected async Task CreateConditionalExpressionAsync( : (TExpressionSyntax)condition.WithoutTrivia(); } + var trueExpression = MakeRef(generatorInternal, isRef, CastValueIfNecessary(generator, trueStatement, trueValue)); + var falseExpression = MakeRef(generatorInternal, isRef, CastValueIfNecessary(generator, falseStatement, falseValue)); + trueExpression = WrapReturnExpressionIfNecessary(trueExpression, trueStatement); + falseExpression = WrapReturnExpressionIfNecessary(falseExpression, falseStatement); + var conditionalExpression = (TConditionalExpressionSyntax)generator.ConditionalExpression( condition.WithoutTrivia(), - MakeRef(generatorInternal, isRef, CastValueIfNecessary(generator, trueStatement, trueValue)), - MakeRef(generatorInternal, isRef, CastValueIfNecessary(generator, falseStatement, falseValue))); + trueExpression, + falseExpression); conditionalExpression = conditionalExpression.WithAdditionalAnnotations(Simplifier.Annotation); var makeMultiLine = await MakeMultiLineAsync( @@ -130,6 +135,12 @@ protected async Task CreateConditionalExpressionAsync( return MakeRef(generatorInternal, isRef, conditionalExpression); } + protected virtual SyntaxNode WrapIfStatementIfNecessary(IConditionalOperation operation) + => operation.Condition.Syntax; + + protected virtual TExpressionSyntax WrapReturnExpressionIfNecessary(TExpressionSyntax returnExpression, IOperation returnOperation) + => returnExpression; + private static TExpressionSyntax MakeRef(SyntaxGeneratorInternal generator, bool isRef, TExpressionSyntax syntaxNode) => isRef ? (TExpressionSyntax)generator.RefExpression(syntaxNode) : syntaxNode; From f86f2f62c539d7dd806ad9953146dc35cd9792a2 Mon Sep 17 00:00:00 2001 From: sdelarosbil Date: Wed, 14 Feb 2024 12:00:43 -0500 Subject: [PATCH 2/2] Add missing tests --- .../UseConditionalExpressionForReturnTests.cs | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/src/Analyzers/CSharp/Tests/UseConditionalExpression/UseConditionalExpressionForReturnTests.cs b/src/Analyzers/CSharp/Tests/UseConditionalExpression/UseConditionalExpressionForReturnTests.cs index 0b7a6f3a1fc48..e4403ee95da29 100644 --- a/src/Analyzers/CSharp/Tests/UseConditionalExpression/UseConditionalExpressionForReturnTests.cs +++ b/src/Analyzers/CSharp/Tests/UseConditionalExpression/UseConditionalExpressionForReturnTests.cs @@ -642,6 +642,76 @@ int M() """, parseOptions: CSharp8); } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/70748")] + public async Task TestMissingWithCheckedInFalseStatement() + { + await TestInRegularAndScriptAsync( + """ + class C + { + int M() + { + int x = 0; + int y = 0; + [||]if (x == y) + { + return 1; + } + else + { + return checked(x - y); + } + } + } + """, + """ + class C + { + int M() + { + int x = 0; + int y = 0; + return x == y ? 1 : checked(x - y); + } + } + """, parseOptions: CSharp8); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/70748")] + public async Task TestMissingWithUncheckedInFalseStatement() + { + await TestInRegularAndScriptAsync( + """ + class C + { + int M() + { + int x = 0; + int y = 0; + [||]if (x == y) + { + return 1; + } + else + { + return unchecked(x - y); + } + } + } + """, + """ + class C + { + int M() + { + int x = 0; + int y = 0; + return x == y ? 1 : unchecked(x - y); + } + } + """, parseOptions: CSharp8); + } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/70750")] public async Task TestMissingWithUnchecked() {