From 196c2341d2f5889248a14c08fc58a2bf90d29443 Mon Sep 17 00:00:00 2001 From: james_hargreaves Date: Tue, 2 May 2023 23:43:50 +0100 Subject: [PATCH 1/4] add constant sting and number; reduce unneccessary parens; tests --- .../CSharp/SyntaxLogicalInverter.cs | 27 ++++--- .../SyntaxLogicalInverterTests.cs | 72 +++++++++++++++++++ 2 files changed, 90 insertions(+), 9 deletions(-) create mode 100644 src/Tests/CSharp.Workspaces.Tests/SyntaxLogicalInverterTests.cs diff --git a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs index 3c3a6914d9..7a4d40d7b9 100644 --- a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs +++ b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs @@ -67,7 +67,7 @@ public ExpressionSyntax LogicallyInvert( return newExpression.WithTriviaFrom(expression); } - private ParenthesizedExpressionSyntax LogicallyInvertAndParenthesize( + private ExpressionSyntax LogicallyInvertAndParenthesize( ExpressionSyntax expression, SemanticModel semanticModel, CancellationToken cancellationToken) @@ -75,7 +75,10 @@ private ParenthesizedExpressionSyntax LogicallyInvertAndParenthesize( if (expression is null) return null; - return LogicallyInvertImpl(expression, semanticModel, cancellationToken).Parenthesize(); + var inverted = LogicallyInvertImpl(expression, semanticModel, cancellationToken); + if (inverted.IsKind(SyntaxKind.LogicalNotExpression, SyntaxKind.ParenthesizedExpression)) + return inverted; + return inverted.Parenthesize(); } private ExpressionSyntax LogicallyInvertImpl( @@ -88,18 +91,21 @@ private ExpressionSyntax LogicallyInvertImpl( switch (expression.Kind()) { + case SyntaxKind.IdentifierName: case SyntaxKind.SimpleMemberAccessExpression: case SyntaxKind.InvocationExpression: case SyntaxKind.ElementAccessExpression: + case SyntaxKind.CheckedExpression: + case SyntaxKind.UncheckedExpression: + case SyntaxKind.DefaultExpression: + { + return DefaultInvert(expression, false); + } case SyntaxKind.PostIncrementExpression: case SyntaxKind.PostDecrementExpression: case SyntaxKind.ObjectCreationExpression: case SyntaxKind.AnonymousObjectCreationExpression: case SyntaxKind.TypeOfExpression: - case SyntaxKind.DefaultExpression: - case SyntaxKind.CheckedExpression: - case SyntaxKind.UncheckedExpression: - case SyntaxKind.IdentifierName: { return DefaultInvert(expression); } @@ -500,7 +506,7 @@ private ExpressionSyntax InvertIsPattern(IsPatternExpressionSyntax isPattern) return isPattern.WithPattern(newConstantPattern); } - else if (constantExpression.IsKind(SyntaxKind.NullLiteralExpression)) + else if (constantExpression.IsKind(SyntaxKind.NullLiteralExpression,SyntaxKind.NumericLiteralExpression,SyntaxKind.StringLiteralExpression)) { UnaryPatternSyntax notPattern = NotPattern(constantPattern.WithoutTrivia()).WithTriviaFrom(constantPattern); @@ -515,14 +521,17 @@ private ExpressionSyntax InvertIsPattern(IsPatternExpressionSyntax isPattern) return DefaultInvert(isPattern); } - private static PrefixUnaryExpressionSyntax DefaultInvert(ExpressionSyntax expression) + private static PrefixUnaryExpressionSyntax DefaultInvert(ExpressionSyntax expression, bool needsParenthesize=true) { SyntaxDebug.Assert(expression.Kind() != SyntaxKind.ParenthesizedExpression, expression); SyntaxTriviaList leadingTrivia = expression.GetLeadingTrivia(); + expression = expression.WithoutLeadingTrivia(); + if(needsParenthesize) + expression = expression.Parenthesize(); return LogicalNotExpression( - expression.WithoutLeadingTrivia().Parenthesize(), + expression, Token(leadingTrivia, SyntaxKind.ExclamationToken, SyntaxTriviaList.Empty)); } } diff --git a/src/Tests/CSharp.Workspaces.Tests/SyntaxLogicalInverterTests.cs b/src/Tests/CSharp.Workspaces.Tests/SyntaxLogicalInverterTests.cs new file mode 100644 index 0000000000..c041e2e5a5 --- /dev/null +++ b/src/Tests/CSharp.Workspaces.Tests/SyntaxLogicalInverterTests.cs @@ -0,0 +1,72 @@ +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Text; +using Xunit; + +namespace Roslynator.CSharp.Workspaces.Tests; + +public class SyntaxLogicallyInvertTests +{ + private SyntaxLogicalInverter _inverter; + + public SyntaxLogicallyInvertTests() + { + _inverter = SyntaxLogicalInverter.Default; + } + + [Theory] + [InlineData(@"x", @"!x")] + [InlineData(@"!x", @"x")] + [InlineData(@"x is ""abc""", @"x is not ""abc""")] + [InlineData(@"x is 1", @"x is not 1")] + [InlineData(@"x is null", @"x is not null")] + [InlineData(@"x is true", @"x is false")] + [InlineData(@"true", @"false")] + [InlineData(@"false", @"true")] + [InlineData(@"x >= 3", @"x < 3")] + [InlineData(@"x > 3", @"x <= 3")] + [InlineData(@"x <= 3", @"x > 3")] + [InlineData(@"x < 3", @"x >= 3")] + [InlineData(@"x == y", @"x != y")] + [InlineData(@"x != y", @"x == y")] + [InlineData(@"(bool)x || (bool)y", @"!((bool)x) && !((bool)y)")] + [InlineData(@"(bool)x && (bool)y", @"!((bool)x) || !((bool)y)")] + [InlineData(@"x ?? true", @"x == false")] + [InlineData(@"x ?? false", @"x != true")] + [InlineData(@"(bool)x ? y : z", @"(bool)x ? !y : !z")] + [InlineData(@"x[0]", @"!x[0]")] + [InlineData(@"default(bool)", @"!default(bool)")] + [InlineData(@"checked(x + y)", @"!checked(x + y)")] + [InlineData(@"unchecked(x + y)", @"!unchecked(x + y)")] + [InlineData(@"(bool)x", @"!((bool)x)")] + [InlineData(@"x & y", @"!x | !y")] + [InlineData(@"x ^ y", @"!(x ^ y)")] + [InlineData(@"x | y", @"!x & !y")] + [InlineData(@"x = y", @"!(x = y)")] + [InlineData(@"await x", @"!(await x)")] + [InlineData(@"x ?? y", @"!(x ?? y)")] + [InlineData(@"x.a", @"!x.a")] + [InlineData(@"x.a()", @"!x.a()")] + public async Task LogicallyInvert(string source, string expected) + { + var sourceCode = $"class C {{ void M(dynamic x, dynamic y, dynamic z){{ if({source})return;}} }}"; + var workspace = new AdhocWorkspace(); + var projectId = ProjectId.CreateNewId(); + var versionStamp = VersionStamp.Create(); + var projectInfo = ProjectInfo.Create(projectId, versionStamp, "TestProject", "TestProject", LanguageNames.CSharp); + var newProject = workspace.AddProject(projectInfo); + var newDocument = workspace.AddDocument(newProject.Id, "TestDocument.cs", SourceText.From(sourceCode)); + var syntaxTree = await newDocument.GetSyntaxTreeAsync(); + var compilation = await newDocument.Project.GetCompilationAsync(); + var semanticModel = compilation.GetSemanticModel(syntaxTree); + + var expression = syntaxTree.GetRoot().DescendantNodes().OfType().Single().Condition; + + var result = _inverter.LogicallyInvert(expression, semanticModel, CancellationToken.None); + Assert.Equal(expected, result.NormalizeWhitespace().ToFullString()); + } + +} From de7bf7d2337702c861f9faa35fa587ebf7e1d7af Mon Sep 17 00:00:00 2001 From: james_hargreaves Date: Wed, 3 May 2023 00:05:49 +0100 Subject: [PATCH 2/4] updated change log --- ChangeLog.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ChangeLog.md b/ChangeLog.md index 66f4b98e3f..2fc70123ad 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Improve inversion of logical expressions to handling additional cases ([#1086](https://github.com/josefpihrt/roslynator/pull/1086)). + ## [4.3.0] - 2023-04-24 ### Changed From 5fe4093093f834e27399c5f9bf1d2810c28c50f9 Mon Sep 17 00:00:00 2001 From: James Hargreaves Date: Tue, 23 May 2023 20:17:10 +0100 Subject: [PATCH 3/4] Update src/Tests/CSharp.Workspaces.Tests/SyntaxLogicalInverterTests.cs Co-authored-by: Josef Pihrt --- .../CSharp.Workspaces.Tests/SyntaxLogicalInverterTests.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Tests/CSharp.Workspaces.Tests/SyntaxLogicalInverterTests.cs b/src/Tests/CSharp.Workspaces.Tests/SyntaxLogicalInverterTests.cs index c041e2e5a5..797031a56a 100644 --- a/src/Tests/CSharp.Workspaces.Tests/SyntaxLogicalInverterTests.cs +++ b/src/Tests/CSharp.Workspaces.Tests/SyntaxLogicalInverterTests.cs @@ -54,10 +54,7 @@ public async Task LogicallyInvert(string source, string expected) { var sourceCode = $"class C {{ void M(dynamic x, dynamic y, dynamic z){{ if({source})return;}} }}"; var workspace = new AdhocWorkspace(); - var projectId = ProjectId.CreateNewId(); - var versionStamp = VersionStamp.Create(); - var projectInfo = ProjectInfo.Create(projectId, versionStamp, "TestProject", "TestProject", LanguageNames.CSharp); - var newProject = workspace.AddProject(projectInfo); + var newProject = workspace.AddProject("TestProject", LanguageNames.CSharp); var newDocument = workspace.AddDocument(newProject.Id, "TestDocument.cs", SourceText.From(sourceCode)); var syntaxTree = await newDocument.GetSyntaxTreeAsync(); var compilation = await newDocument.Project.GetCompilationAsync(); From ab402917442184bd674afcc681f91ca51f846b0b Mon Sep 17 00:00:00 2001 From: james_hargreaves Date: Tue, 23 May 2023 20:22:56 +0100 Subject: [PATCH 4/4] CR comments --- .../CSharp/SyntaxLogicalInverter.cs | 17 ++++++++--------- .../SyntaxLogicalInverterTests.cs | 1 + 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs index 7a4d40d7b9..515508274e 100644 --- a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs +++ b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs @@ -76,9 +76,7 @@ private ExpressionSyntax LogicallyInvertAndParenthesize( return null; var inverted = LogicallyInvertImpl(expression, semanticModel, cancellationToken); - if (inverted.IsKind(SyntaxKind.LogicalNotExpression, SyntaxKind.ParenthesizedExpression)) - return inverted; - return inverted.Parenthesize(); + return inverted.IsKind(SyntaxKind.LogicalNotExpression, SyntaxKind.ParenthesizedExpression) ? inverted : inverted.Parenthesize(); } private ExpressionSyntax LogicallyInvertImpl( @@ -98,9 +96,10 @@ private ExpressionSyntax LogicallyInvertImpl( case SyntaxKind.CheckedExpression: case SyntaxKind.UncheckedExpression: case SyntaxKind.DefaultExpression: - { - return DefaultInvert(expression, false); - } + case SyntaxKind.ConditionalAccessExpression: + { + return DefaultInvert(expression, false); + } case SyntaxKind.PostIncrementExpression: case SyntaxKind.PostDecrementExpression: case SyntaxKind.ObjectCreationExpression: @@ -506,7 +505,7 @@ private ExpressionSyntax InvertIsPattern(IsPatternExpressionSyntax isPattern) return isPattern.WithPattern(newConstantPattern); } - else if (constantExpression.IsKind(SyntaxKind.NullLiteralExpression,SyntaxKind.NumericLiteralExpression,SyntaxKind.StringLiteralExpression)) + else if (constantExpression.IsKind(SyntaxKind.NullLiteralExpression, SyntaxKind.NumericLiteralExpression, SyntaxKind.StringLiteralExpression)) { UnaryPatternSyntax notPattern = NotPattern(constantPattern.WithoutTrivia()).WithTriviaFrom(constantPattern); @@ -521,13 +520,13 @@ private ExpressionSyntax InvertIsPattern(IsPatternExpressionSyntax isPattern) return DefaultInvert(isPattern); } - private static PrefixUnaryExpressionSyntax DefaultInvert(ExpressionSyntax expression, bool needsParenthesize=true) + private static PrefixUnaryExpressionSyntax DefaultInvert(ExpressionSyntax expression, bool needsParenthesize = true) { SyntaxDebug.Assert(expression.Kind() != SyntaxKind.ParenthesizedExpression, expression); SyntaxTriviaList leadingTrivia = expression.GetLeadingTrivia(); expression = expression.WithoutLeadingTrivia(); - if(needsParenthesize) + if (needsParenthesize) expression = expression.Parenthesize(); return LogicalNotExpression( diff --git a/src/Tests/CSharp.Workspaces.Tests/SyntaxLogicalInverterTests.cs b/src/Tests/CSharp.Workspaces.Tests/SyntaxLogicalInverterTests.cs index 797031a56a..90640c8612 100644 --- a/src/Tests/CSharp.Workspaces.Tests/SyntaxLogicalInverterTests.cs +++ b/src/Tests/CSharp.Workspaces.Tests/SyntaxLogicalInverterTests.cs @@ -50,6 +50,7 @@ public SyntaxLogicallyInvertTests() [InlineData(@"x ?? y", @"!(x ?? y)")] [InlineData(@"x.a", @"!x.a")] [InlineData(@"x.a()", @"!x.a()")] + [InlineData(@"x?.a", @"!x?.a")] public async Task LogicallyInvert(string source, string expected) { var sourceCode = $"class C {{ void M(dynamic x, dynamic y, dynamic z){{ if({source})return;}} }}";