From e0ac997a8a3fd5d61eaf2027d82a3bfd52cef1de Mon Sep 17 00:00:00 2001 From: sdelarosbil Date: Fri, 8 Mar 2024 13:49:32 -0500 Subject: [PATCH 1/8] Qualify the type when negating a pattern to a not pattern --- .../CSharpUseNotPatternTests.cs | 43 +++++++++++++++++++ .../SyntaxGeneratorExtensions_Negate.cs | 16 +++++++ 2 files changed, 59 insertions(+) diff --git a/src/Analyzers/CSharp/Tests/UsePatternMatching/CSharpUseNotPatternTests.cs b/src/Analyzers/CSharp/Tests/UsePatternMatching/CSharpUseNotPatternTests.cs index 5a97b2bf607b8..670fadbda1249 100644 --- a/src/Analyzers/CSharp/Tests/UsePatternMatching/CSharpUseNotPatternTests.cs +++ b/src/Analyzers/CSharp/Tests/UsePatternMatching/CSharpUseNotPatternTests.cs @@ -174,6 +174,49 @@ void M(object x) }.RunAsync(); } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/72370")] + public async Task UseNotPattern2() + { + await new VerifyCS.Test + { + TestCode = """ + public class Program + { + public class C + { + } + public static void Main() + { + C C = new(); + object O = C; + + if (!(O [|is|] C)) + { + } + } + } + """, + FixedCode = """ + public class Program + { + public class C + { + } + public static void Main() + { + C C = new(); + object O = C; + + if (O is not Program.C) + { + } + } + } + """, + LanguageVersion = LanguageVersion.CSharp9, + }.RunAsync(); + } + [Fact] public async Task UnavailableInCSharp8() { diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs index 5e19a079b55e8..1bd708759e8d8 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs @@ -158,6 +158,22 @@ private static SyntaxNode GetNegationOfBinaryExpression( return generatorInternal.IsPatternExpression(leftOperand, operatorToken, generatorInternal.ConstantPattern(generator.NullLiteralExpression())); } + // With a not pattern, it is possible to have an ambiguity between a type name or a local identifier. + // For example, if a class is named C and we have: + // C C = new(); + // object O = C; + // if (O is not C) + // ... + // Then there is an ambiguity between the type C and the local named C. + // To address this, we use an expression with the fully qualified type name which the simplifier + // will shorten to the shortest possible without ambiguity + if (operation is IIsTypeOperation { TypeOperand.SpecialType: SpecialType.None } isTypeOperation && + syntaxFacts.SupportsNotPattern(semanticModel.SyntaxTree.Options)) + { + var typeNode = generatorInternal.Type(isTypeOperation.TypeOperand, false); + return generatorInternal.IsPatternExpression(leftOperand, operatorToken, generatorInternal.NotPattern(generatorInternal.ConstantPattern(typeNode))); + } + // `is y` -> `is not y` if (syntaxFacts.SupportsNotPattern(semanticModel.SyntaxTree.Options)) return generatorInternal.IsPatternExpression(leftOperand, operatorToken, generatorInternal.NotPattern(generatorInternal.TypePattern(rightOperand))); From 11c1f9dce90bdbe1aa3a8043bb03138d93a760b7 Mon Sep 17 00:00:00 2001 From: sdelarosbil Date: Fri, 8 Mar 2024 14:56:45 -0500 Subject: [PATCH 2/8] Specify the arg name --- .../Core/Extensions/SyntaxGeneratorExtensions_Negate.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs index 1bd708759e8d8..01b69fa27c64d 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs @@ -170,7 +170,7 @@ private static SyntaxNode GetNegationOfBinaryExpression( if (operation is IIsTypeOperation { TypeOperand.SpecialType: SpecialType.None } isTypeOperation && syntaxFacts.SupportsNotPattern(semanticModel.SyntaxTree.Options)) { - var typeNode = generatorInternal.Type(isTypeOperation.TypeOperand, false); + var typeNode = generatorInternal.Type(isTypeOperation.TypeOperand, typeContext: false); return generatorInternal.IsPatternExpression(leftOperand, operatorToken, generatorInternal.NotPattern(generatorInternal.ConstantPattern(typeNode))); } From 9e2a7f0cf391c767bb34d94e50c01290edf4d6a9 Mon Sep 17 00:00:00 2001 From: sdelarosbil Date: Tue, 19 Mar 2024 13:55:19 -0400 Subject: [PATCH 3/8] Use SyntaxFacts to check if the type operation is predefined --- .../Core/Extensions/SyntaxGeneratorExtensions_Negate.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs index 01b69fa27c64d..2a32997df7af5 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs @@ -167,7 +167,8 @@ private static SyntaxNode GetNegationOfBinaryExpression( // Then there is an ambiguity between the type C and the local named C. // To address this, we use an expression with the fully qualified type name which the simplifier // will shorten to the shortest possible without ambiguity - if (operation is IIsTypeOperation { TypeOperand.SpecialType: SpecialType.None } isTypeOperation && + if (!syntaxFacts.IsPredefinedType(rightOperand) && + operation is IIsTypeOperation isTypeOperation && syntaxFacts.SupportsNotPattern(semanticModel.SyntaxTree.Options)) { var typeNode = generatorInternal.Type(isTypeOperation.TypeOperand, typeContext: false); From 212e46138410c0230e5798f173fadc9ba74de984 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 19 Mar 2024 12:31:20 -0700 Subject: [PATCH 4/8] Update and add tests --- .../CSharpUseNotPatternTests.cs | 99 +++++++++++++++ .../Services/SyntaxFacts/CSharpSyntaxFacts.cs | 6 + .../Core/Services/SyntaxFacts/ISyntaxFacts.cs | 2 + .../SyntaxFacts/VisualBasicSyntaxFacts.vb | 8 ++ .../SyntaxGeneratorExtensions_Negate.cs | 120 +++++++++--------- 5 files changed, 176 insertions(+), 59 deletions(-) diff --git a/src/Analyzers/CSharp/Tests/UsePatternMatching/CSharpUseNotPatternTests.cs b/src/Analyzers/CSharp/Tests/UsePatternMatching/CSharpUseNotPatternTests.cs index 670fadbda1249..6cfbe14ab50e7 100644 --- a/src/Analyzers/CSharp/Tests/UsePatternMatching/CSharpUseNotPatternTests.cs +++ b/src/Analyzers/CSharp/Tests/UsePatternMatching/CSharpUseNotPatternTests.cs @@ -50,6 +50,37 @@ void M(object x) }.RunAsync(); } + [Fact] + public async Task BinaryIsExpression2() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + void M(object x) + { + if (!(x [|is|] string /*trailing*/)) + { + } + } + } + """, + FixedCode = """ + class C + { + void M(object x) + { + if (x is not string /*trailing*/) + { + } + } + } + """, + LanguageVersion = LanguageVersion.CSharp9, + }.RunAsync(); + } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/50690")] public async Task ConstantPattern() { @@ -217,6 +248,43 @@ public static void Main() }.RunAsync(); } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/72370")] + public async Task UseNotPattern3() + { + await new VerifyCS.Test + { + TestCode = """ + public class Program + { + public class C + { + } + public static void Main(object O) + { + if (!(O [|is|] C)) + { + } + } + } + """, + FixedCode = """ + public class Program + { + public class C + { + } + public static void Main(object O) + { + if (O is not C) + { + } + } + } + """, + LanguageVersion = LanguageVersion.CSharp9, + }.RunAsync(); + } + [Fact] public async Task UnavailableInCSharp8() { @@ -268,6 +336,37 @@ void M(object x) }.RunAsync(); } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/50690")] + public async Task BinaryIsObject2() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + void M(object x) + { + if (!(x [|is|] object /*trailing*/)) + { + } + } + } + """, + FixedCode = """ + class C + { + void M(object x) + { + if (x is null /*trailing*/) + { + } + } + } + """, + LanguageVersion = LanguageVersion.CSharp9, + }.RunAsync(); + } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/68784")] public async Task NotInExpressionTree() { diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs index 05f48a4c31a89..c755f94409b07 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs @@ -1552,6 +1552,12 @@ public bool IsMethodDeclaration([NotNullWhen(true)] SyntaxNode? node) public bool IsSimpleName([NotNullWhen(true)] SyntaxNode? node) => node is SimpleNameSyntax; + public bool IsAnyName([NotNullWhen(true)] SyntaxNode? node) + => node is NameSyntax; + + public bool IsAnyType([NotNullWhen(true)] SyntaxNode? node) + => node is TypeSyntax; + public bool IsNamedMemberInitializer([NotNullWhen(true)] SyntaxNode? node) => node is AssignmentExpressionSyntax(SyntaxKind.SimpleAssignmentExpression) { Left: IdentifierNameSyntax }; diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs index 9527a41fdad19..d894dd0069a2a 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs @@ -499,6 +499,8 @@ void GetPartsOfTupleExpression(SyntaxNode node, bool IsMemberAccessExpression([NotNullWhen(true)] SyntaxNode? node); bool IsMethodDeclaration([NotNullWhen(true)] SyntaxNode? node); bool IsSimpleName([NotNullWhen(true)] SyntaxNode? node); + bool IsAnyName([NotNullWhen(true)] SyntaxNode? node); + bool IsAnyType([NotNullWhen(true)] SyntaxNode? node); bool IsNamedMemberInitializer([NotNullWhen(true)] SyntaxNode? node); bool IsElementAccessInitializer([NotNullWhen(true)] SyntaxNode? node); diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb index 0dd63172c64a2..d0a7eaa806e90 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb @@ -1756,6 +1756,14 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService Return TypeOf node Is SimpleNameSyntax End Function + Public Function IsAnyName(node As SyntaxNode) As Boolean Implements ISyntaxFacts.IsAnyName + Return TypeOf node Is NameSyntax + End Function + + Public Function IsAnyType(node As SyntaxNode) As Boolean Implements ISyntaxFacts.IsAnyType + Return TypeOf node Is TypeSyntax + End Function + Public Function IsNamedMemberInitializer(node As SyntaxNode) As Boolean Implements ISyntaxFacts.IsNamedMemberInitializer Return TypeOf node Is NamedFieldInitializerSyntax End Function diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs index 2a32997df7af5..c5d9f2d7809f2 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics; using System.Threading; using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.LanguageService; @@ -147,75 +148,76 @@ private static SyntaxNode GetNegationOfBinaryExpression( syntaxFacts.GetPartsOfBinaryExpression(expressionNode, out var leftOperand, out var operatorToken, out var rightOperand); var operation = semanticModel.GetOperation(expressionNode, cancellationToken); - if (operation is not IBinaryOperation binaryOperation) + if (operation is IBinaryOperation binaryOperation) { - if (syntaxFacts.IsIsTypeExpression(expressionNode)) + if (!s_negatedBinaryMap.TryGetValue(binaryOperation.OperatorKind, out var negatedKind)) + return generator.LogicalNotExpression(expressionNode); + + // Lifted relational operators return false if either operand is null. + // Inverting the operator fails to invert the behavior when an operand is null. + if (binaryOperation.IsLifted + && binaryOperation.OperatorKind is BinaryOperatorKind.LessThan or + BinaryOperatorKind.LessThanOrEqual or + BinaryOperatorKind.GreaterThan or + BinaryOperatorKind.GreaterThanOrEqual) { - // `is object` -> `is null` - if (syntaxFacts.IsPredefinedType(rightOperand, PredefinedType.Object) && - generatorInternal.SupportsPatterns(semanticModel.SyntaxTree.Options)) - { - return generatorInternal.IsPatternExpression(leftOperand, operatorToken, generatorInternal.ConstantPattern(generator.NullLiteralExpression())); - } - - // With a not pattern, it is possible to have an ambiguity between a type name or a local identifier. - // For example, if a class is named C and we have: - // C C = new(); - // object O = C; - // if (O is not C) - // ... - // Then there is an ambiguity between the type C and the local named C. - // To address this, we use an expression with the fully qualified type name which the simplifier - // will shorten to the shortest possible without ambiguity - if (!syntaxFacts.IsPredefinedType(rightOperand) && - operation is IIsTypeOperation isTypeOperation && - syntaxFacts.SupportsNotPattern(semanticModel.SyntaxTree.Options)) - { - var typeNode = generatorInternal.Type(isTypeOperation.TypeOperand, typeContext: false); - return generatorInternal.IsPatternExpression(leftOperand, operatorToken, generatorInternal.NotPattern(generatorInternal.ConstantPattern(typeNode))); - } - - // `is y` -> `is not y` - if (syntaxFacts.SupportsNotPattern(semanticModel.SyntaxTree.Options)) - return generatorInternal.IsPatternExpression(leftOperand, operatorToken, generatorInternal.NotPattern(generatorInternal.TypePattern(rightOperand))); + return generator.LogicalNotExpression(expressionNode); } - // Apply the logical not operator if it is not a binary operation. - return generator.LogicalNotExpression(expressionNode); - } + if (binaryOperation.OperatorKind is BinaryOperatorKind.Or or + BinaryOperatorKind.And or + BinaryOperatorKind.ConditionalAnd or + BinaryOperatorKind.ConditionalOr) + { + leftOperand = generator.Negate(generatorInternal, leftOperand, semanticModel, cancellationToken); + rightOperand = generator.Negate(generatorInternal, rightOperand, semanticModel, cancellationToken); + } - if (!s_negatedBinaryMap.TryGetValue(binaryOperation.OperatorKind, out var negatedKind)) - return generator.LogicalNotExpression(expressionNode); + var newBinaryExpressionSyntax = negatedKind is BinaryOperatorKind.Equals or BinaryOperatorKind.NotEquals + ? generatorInternal.NegateEquality(generator, expressionNode, leftOperand, negatedKind, rightOperand) + : NegateRelational(generator, binaryOperation, leftOperand, negatedKind, rightOperand); + newBinaryExpressionSyntax = newBinaryExpressionSyntax.WithTriviaFrom(expressionNode); - // Lifted relational operators return false if either operand is null. - // Inverting the operator fails to invert the behavior when an operand is null. - if (binaryOperation.IsLifted - && binaryOperation.OperatorKind is BinaryOperatorKind.LessThan or - BinaryOperatorKind.LessThanOrEqual or - BinaryOperatorKind.GreaterThan or - BinaryOperatorKind.GreaterThanOrEqual) - { - return generator.LogicalNotExpression(expressionNode); + var newToken = syntaxFacts.GetOperatorTokenOfBinaryExpression(newBinaryExpressionSyntax); + return newBinaryExpressionSyntax.ReplaceToken( + newToken, + newToken.WithTriviaFrom(operatorToken)); } - - if (binaryOperation.OperatorKind is BinaryOperatorKind.Or or - BinaryOperatorKind.And or - BinaryOperatorKind.ConditionalAnd or - BinaryOperatorKind.ConditionalOr) + else if (operation is IIsTypeOperation isTypeOperation && generatorInternal.SupportsPatterns(semanticModel.SyntaxTree.Options)) { - leftOperand = generator.Negate(generatorInternal, leftOperand, semanticModel, cancellationToken); - rightOperand = generator.Negate(generatorInternal, rightOperand, semanticModel, cancellationToken); - } + // `is object` -> `is null` + if (isTypeOperation.TypeOperand.SpecialType is SpecialType.System_Object) + return generatorInternal.IsPatternExpression(leftOperand, operatorToken, generatorInternal.ConstantPattern(generator.NullLiteralExpression().WithTriviaFrom(rightOperand))); + + // `is y` -> `is not y` + if (syntaxFacts.SupportsNotPattern(semanticModel.SyntaxTree.Options)) + { + SyntaxNode innerPattern; + if (syntaxFacts.IsAnyName(rightOperand)) + { + // For named types, we need to convert to a constant expression (where the named type becomes a member + // access expression). For other types (predefined, arrays, etc) we can keep this as a type pattern. + // For example: `x is int` can just become `x is not int` where that's a type pattern. But `x is Y` + // will need to become `x is not Y` where that's a constant pattern instead. + var typeExpression = generatorInternal.Type(isTypeOperation.TypeOperand, typeContext: false); + innerPattern = syntaxFacts.IsAnyType(typeExpression) && !syntaxFacts.IsAnyName(typeExpression) + ? generatorInternal.TypePattern(typeExpression) + : generatorInternal.ConstantPattern(typeExpression); + } + else + { + // original form was already not a name (like a predefined type, or array type, etc.). Can just + // use as is as a type pattern. + Debug.Assert(syntaxFacts.IsAnyType(rightOperand)); + innerPattern = generatorInternal.TypePattern(rightOperand); + } - var newBinaryExpressionSyntax = negatedKind is BinaryOperatorKind.Equals or BinaryOperatorKind.NotEquals - ? generatorInternal.NegateEquality(generator, expressionNode, leftOperand, negatedKind, rightOperand) - : NegateRelational(generator, binaryOperation, leftOperand, negatedKind, rightOperand); - newBinaryExpressionSyntax = newBinaryExpressionSyntax.WithTriviaFrom(expressionNode); + return generatorInternal.IsPatternExpression(leftOperand, operatorToken, generatorInternal.NotPattern(innerPattern.WithTriviaFrom(rightOperand))); + } + } - var newToken = syntaxFacts.GetOperatorTokenOfBinaryExpression(newBinaryExpressionSyntax); - return newBinaryExpressionSyntax.ReplaceToken( - newToken, - newToken.WithTriviaFrom(operatorToken)); + // Apply the logical not operator if it is not a binary operation and also doesn't support patterns. + return generator.LogicalNotExpression(expressionNode); } private static SyntaxNode GetNegationOfBinaryPattern( From be90730f6b72bbef19126caacf649694f459728c Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 19 Mar 2024 12:45:40 -0700 Subject: [PATCH 5/8] Add test --- .../CSharpUseNotPatternTests.cs | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/Analyzers/CSharp/Tests/UsePatternMatching/CSharpUseNotPatternTests.cs b/src/Analyzers/CSharp/Tests/UsePatternMatching/CSharpUseNotPatternTests.cs index 6cfbe14ab50e7..97c6ac0dd572c 100644 --- a/src/Analyzers/CSharp/Tests/UsePatternMatching/CSharpUseNotPatternTests.cs +++ b/src/Analyzers/CSharp/Tests/UsePatternMatching/CSharpUseNotPatternTests.cs @@ -367,6 +367,41 @@ void M(object x) }.RunAsync(); } + [Fact] + public async Task BinaryIsObject3() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + void M(object x) + { + if (!(x [|is|] Object)) + { + } + } + } + """, + FixedCode = """ + using System; + + class C + { + void M(object x) + { + if (x is null) + { + } + } + } + """, + LanguageVersion = LanguageVersion.CSharp9, + }.RunAsync(); + } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/68784")] public async Task NotInExpressionTree() { From f6667af2a4294199d3e0004547e9ad7bc4756fb4 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 19 Mar 2024 14:28:53 -0700 Subject: [PATCH 6/8] tweak --- .../SyntaxGeneratorExtensions_Negate.cs | 60 +++++++++++-------- 1 file changed, 34 insertions(+), 26 deletions(-) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs index c5d9f2d7809f2..dd14a12a1bc21 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs @@ -183,36 +183,44 @@ BinaryOperatorKind.ConditionalAnd or newToken, newToken.WithTriviaFrom(operatorToken)); } - else if (operation is IIsTypeOperation isTypeOperation && generatorInternal.SupportsPatterns(semanticModel.SyntaxTree.Options)) + else if (syntaxFacts.IsIsTypeExpression(expressionNode) && generatorInternal.SupportsPatterns(semanticModel.SyntaxTree.Options)) { - // `is object` -> `is null` - if (isTypeOperation.TypeOperand.SpecialType is SpecialType.System_Object) - return generatorInternal.IsPatternExpression(leftOperand, operatorToken, generatorInternal.ConstantPattern(generator.NullLiteralExpression().WithTriviaFrom(rightOperand))); - - // `is y` -> `is not y` - if (syntaxFacts.SupportsNotPattern(semanticModel.SyntaxTree.Options)) + if (operation is IIsTypeOperation isTypeOperation) { - SyntaxNode innerPattern; - if (syntaxFacts.IsAnyName(rightOperand)) - { - // For named types, we need to convert to a constant expression (where the named type becomes a member - // access expression). For other types (predefined, arrays, etc) we can keep this as a type pattern. - // For example: `x is int` can just become `x is not int` where that's a type pattern. But `x is Y` - // will need to become `x is not Y` where that's a constant pattern instead. - var typeExpression = generatorInternal.Type(isTypeOperation.TypeOperand, typeContext: false); - innerPattern = syntaxFacts.IsAnyType(typeExpression) && !syntaxFacts.IsAnyName(typeExpression) - ? generatorInternal.TypePattern(typeExpression) - : generatorInternal.ConstantPattern(typeExpression); - } - else + // `is object` -> `is null` + if (isTypeOperation.TypeOperand.SpecialType is SpecialType.System_Object) + return generatorInternal.IsPatternExpression(leftOperand, operatorToken, generatorInternal.ConstantPattern(generator.NullLiteralExpression().WithTriviaFrom(rightOperand))); + + // `is y` -> `is not y` + if (syntaxFacts.SupportsNotPattern(semanticModel.SyntaxTree.Options)) { - // original form was already not a name (like a predefined type, or array type, etc.). Can just - // use as is as a type pattern. - Debug.Assert(syntaxFacts.IsAnyType(rightOperand)); - innerPattern = generatorInternal.TypePattern(rightOperand); - } + SyntaxNode innerPattern; + if (syntaxFacts.IsAnyName(rightOperand)) + { + // For named types, we need to convert to a constant expression (where the named type becomes a member + // access expression). For other types (predefined, arrays, etc) we can keep this as a type pattern. + // For example: `x is int` can just become `x is not int` where that's a type pattern. But `x is Y` + // will need to become `x is not Y` where that's a constant pattern instead. + var typeExpression = generatorInternal.Type(isTypeOperation.TypeOperand, typeContext: false); + innerPattern = syntaxFacts.IsAnyType(typeExpression) && !syntaxFacts.IsAnyName(typeExpression) + ? generatorInternal.TypePattern(typeExpression) + : generatorInternal.ConstantPattern(typeExpression); + } + else + { + // original form was already not a name (like a predefined type, or array type, etc.). Can just + // use as is as a type pattern. + Debug.Assert(syntaxFacts.IsAnyType(rightOperand)); + innerPattern = generatorInternal.TypePattern(rightOperand); + } - return generatorInternal.IsPatternExpression(leftOperand, operatorToken, generatorInternal.NotPattern(innerPattern.WithTriviaFrom(rightOperand))); + return generatorInternal.IsPatternExpression(leftOperand, operatorToken, generatorInternal.NotPattern(innerPattern.WithTriviaFrom(rightOperand))); + } + } + else + { + return generatorInternal.IsPatternExpression( + leftOperand, operatorToken, generatorInternal.NotPattern(generatorInternal.ConstantPattern(rightOperand))); } } From 50d4ad58d5c6962f3909fa6d30321df192800a89 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 19 Mar 2024 14:35:10 -0700 Subject: [PATCH 7/8] Simplify and share --- .../SyntaxGeneratorExtensions_Negate.cs | 68 ++++++++++--------- 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs index dd14a12a1bc21..6c5048f7773e3 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs @@ -183,49 +183,51 @@ BinaryOperatorKind.ConditionalAnd or newToken, newToken.WithTriviaFrom(operatorToken)); } - else if (syntaxFacts.IsIsTypeExpression(expressionNode) && generatorInternal.SupportsPatterns(semanticModel.SyntaxTree.Options)) + else if (operation is IIsTypeOperation { TypeOperand.SpecialType: SpecialType.System_Object } && generatorInternal.SupportsPatterns(semanticModel.SyntaxTree.Options)) { + // `is object` -> `is null` + return generatorInternal.IsPatternExpression(leftOperand, operatorToken, generatorInternal.ConstantPattern(generator.NullLiteralExpression().WithTriviaFrom(rightOperand))); + } + else if (syntaxFacts.IsIsTypeExpression(expressionNode) && syntaxFacts.SupportsNotPattern(semanticModel.SyntaxTree.Options)) + { + // `is y` -> `is not y` + SyntaxNode innerPattern; if (operation is IIsTypeOperation isTypeOperation) { - // `is object` -> `is null` - if (isTypeOperation.TypeOperand.SpecialType is SpecialType.System_Object) - return generatorInternal.IsPatternExpression(leftOperand, operatorToken, generatorInternal.ConstantPattern(generator.NullLiteralExpression().WithTriviaFrom(rightOperand))); - - // `is y` -> `is not y` - if (syntaxFacts.SupportsNotPattern(semanticModel.SyntaxTree.Options)) + if (syntaxFacts.IsAnyName(rightOperand)) { - SyntaxNode innerPattern; - if (syntaxFacts.IsAnyName(rightOperand)) - { - // For named types, we need to convert to a constant expression (where the named type becomes a member - // access expression). For other types (predefined, arrays, etc) we can keep this as a type pattern. - // For example: `x is int` can just become `x is not int` where that's a type pattern. But `x is Y` - // will need to become `x is not Y` where that's a constant pattern instead. - var typeExpression = generatorInternal.Type(isTypeOperation.TypeOperand, typeContext: false); - innerPattern = syntaxFacts.IsAnyType(typeExpression) && !syntaxFacts.IsAnyName(typeExpression) - ? generatorInternal.TypePattern(typeExpression) - : generatorInternal.ConstantPattern(typeExpression); - } - else - { - // original form was already not a name (like a predefined type, or array type, etc.). Can just - // use as is as a type pattern. - Debug.Assert(syntaxFacts.IsAnyType(rightOperand)); - innerPattern = generatorInternal.TypePattern(rightOperand); - } - - return generatorInternal.IsPatternExpression(leftOperand, operatorToken, generatorInternal.NotPattern(innerPattern.WithTriviaFrom(rightOperand))); + // For named types, we need to convert to a constant expression (where the named type becomes a member + // access expression). For other types (predefined, arrays, etc) we can keep this as a type pattern. + // For example: `x is int` can just become `x is not int` where that's a type pattern. But `x is Y` + // will need to become `x is not Y` where that's a constant pattern instead. + var typeExpression = generatorInternal.Type(isTypeOperation.TypeOperand, typeContext: false); + innerPattern = syntaxFacts.IsAnyType(typeExpression) && !syntaxFacts.IsAnyName(typeExpression) + ? generatorInternal.TypePattern(typeExpression) + : generatorInternal.ConstantPattern(typeExpression); + } + else + { + // original form was already not a name (like a predefined type, or array type, etc.). Can just + // use as is as a type pattern. + Debug.Assert(syntaxFacts.IsAnyType(rightOperand)); + innerPattern = generatorInternal.TypePattern(rightOperand); } } else { - return generatorInternal.IsPatternExpression( - leftOperand, operatorToken, generatorInternal.NotPattern(generatorInternal.ConstantPattern(rightOperand))); + innerPattern = generatorInternal.ConstantPattern(rightOperand); } - } - // Apply the logical not operator if it is not a binary operation and also doesn't support patterns. - return generator.LogicalNotExpression(expressionNode); + return generatorInternal.IsPatternExpression( + leftOperand, + operatorToken, + generatorInternal.NotPattern(innerPattern.WithTriviaFrom(rightOperand))); + } + else + { + // Apply the logical not operator if it is not a binary operation and also doesn't support patterns. + return generator.LogicalNotExpression(expressionNode); + } } private static SyntaxNode GetNegationOfBinaryPattern( From af54a87dde229ab75c510e5c61293aea7ccbb791 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 19 Mar 2024 14:36:52 -0700 Subject: [PATCH 8/8] remove redundant code --- .../Core/Extensions/SyntaxGeneratorExtensions_Negate.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs index 6c5048f7773e3..3e342e41ce85f 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs @@ -209,7 +209,6 @@ BinaryOperatorKind.ConditionalAnd or { // original form was already not a name (like a predefined type, or array type, etc.). Can just // use as is as a type pattern. - Debug.Assert(syntaxFacts.IsAnyType(rightOperand)); innerPattern = generatorInternal.TypePattern(rightOperand); } }