From 7947651abd1cc4308f8a03976de84c89ef9be7ab Mon Sep 17 00:00:00 2001 From: james_hargreaves Date: Thu, 13 Apr 2023 22:37:52 +0100 Subject: [PATCH 1/3] with fix and tests --- .../CSharp/SyntaxLogicalInverter.cs | 17 +++ .../RCS1208ReduceIfNestingTests.cs | 120 ++++++++++++++++++ 2 files changed, 137 insertions(+) diff --git a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs index 2b1609e169..bbf7559c92 100644 --- a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs +++ b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs @@ -208,6 +208,23 @@ private ExpressionSyntax LogicallyInvertImpl( { return DefaultInvert(expression); } + case SyntaxKind.CoalesceExpression: + { + var binaryExpression = (BinaryExpressionSyntax)expression; + if (binaryExpression.Right.Kind() == SyntaxKind.FalseLiteralExpression) + { + // !(x ?? false) === (x != true) + return BinaryExpression(SyntaxKind.NotEqualsExpression, binaryExpression.Left, LiteralExpression(SyntaxKind.TrueLiteralExpression)); + } + + if (binaryExpression.Right.Kind() == SyntaxKind.TrueLiteralExpression) + { + // !(x ?? true) === (x == false) + return BinaryExpression(SyntaxKind.EqualsExpression, binaryExpression.Left, LiteralExpression(SyntaxKind.FalseLiteralExpression)); + } + + return DefaultInvert(expression); + } } Debug.Fail($"Logical inversion of unknown kind '{expression.Kind()}'"); diff --git a/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs b/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs index c8a3df99c2..9a91c5a142 100644 --- a/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs @@ -50,6 +50,126 @@ void M2() "); } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] + public async Task Test_InvertingCoalesceToFalse() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + void M(bool? p) + { + [|if|] (p??false) + { + M2(); + } + } + + void M2() + { + } +} +", @" +class C +{ + void M(bool? p) + { + if (p != true) + { + return; + } + + M2(); + } + + void M2() + { + } +} +"); + } + + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] + public async Task Test_InvertingCoalesceToTrue() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + void M(bool? p) + { + [|if|] (p??true) + { + M2(); + } + } + + void M2() + { + } +} +", @" +class C +{ + void M(bool? p) + { + if (p == false) + { + return; + } + + M2(); + } + + void M2() + { + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] + public async Task Test_InvertingCoalesceToUnknown() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + bool b { get; set; } + + void M(bool? p) + { + [|if|] (p??b) + { + M2(); + } + } + + void M2() + { + } +} +", @" +class C +{ + bool b { get; set; } + + void M(bool? p) + { + if (!(p ?? b)) + { + return; + } + + M2(); + } + + void M2() + { + } +} +"); + } + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] public async Task TestNoDiagnostic_OverlappingLocalVariables() { From 1233a422b48f5a8163007660058ab659e7ed308f Mon Sep 17 00:00:00 2001 From: james_hargreaves Date: Thu, 13 Apr 2023 22:51:20 +0100 Subject: [PATCH 2/3] With ChangeLog entry --- ChangeLog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/ChangeLog.md b/ChangeLog.md index ed6c4c1a51..406a4370cd 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -31,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [CLI] Analyze command does not create the XML output file and returns incorrect exit code when only compiler diagnostics are reported ([#1056](https://github.com/JosefPihrt/Roslynator/pull/1056) by @PeterKaszab). - [CLI] Fix exit code when multiple projects are processed ([#1061](https://github.com/JosefPihrt/Roslynator/pull/1061) by @PeterKaszab). - Fix code fix for CS0164 ([#1031](https://github.com/JosefPihrt/Roslynator/pull/1031)). +- Improve support for coalesce expressions in code fixes that require computing the logical inversion of an expression, such as [RCS1208](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1208.md) ([#1069](https://github.com/JosefPihrt/Roslynator/pull/1069)). ## [4.2.0] - 2022-11-27 From 61086a2367d3f3ecf811f442ae84d0d56bd25959 Mon Sep 17 00:00:00 2001 From: james_hargreaves Date: Sat, 15 Apr 2023 15:19:42 +0100 Subject: [PATCH 3/3] CR comments --- src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs index bbf7559c92..d42090810f 100644 --- a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs +++ b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs @@ -211,16 +211,16 @@ private ExpressionSyntax LogicallyInvertImpl( case SyntaxKind.CoalesceExpression: { var binaryExpression = (BinaryExpressionSyntax)expression; - if (binaryExpression.Right.Kind() == SyntaxKind.FalseLiteralExpression) + if (binaryExpression.Right.IsKind(SyntaxKind.FalseLiteralExpression)) { // !(x ?? false) === (x != true) - return BinaryExpression(SyntaxKind.NotEqualsExpression, binaryExpression.Left, LiteralExpression(SyntaxKind.TrueLiteralExpression)); + return NotEqualsExpression(binaryExpression.Left, TrueLiteralExpression()); } - if (binaryExpression.Right.Kind() == SyntaxKind.TrueLiteralExpression) + if (binaryExpression.Right.IsKind(SyntaxKind.TrueLiteralExpression)) { // !(x ?? true) === (x == false) - return BinaryExpression(SyntaxKind.EqualsExpression, binaryExpression.Left, LiteralExpression(SyntaxKind.FalseLiteralExpression)); + return EqualsExpression(binaryExpression.Left, FalseLiteralExpression()); } return DefaultInvert(expression);