Skip to content

Commit 40782d7

Browse files
CyrusNajmabadiCyrusNajmabadi
authored
Merge pull request #64594 from kimsey0/negating-relational-patterns
Change operator when negating relational patterns with numeric values
2 parents 941bdc2 + b843592 commit 40782d7

File tree

12 files changed

+179
-19
lines changed

12 files changed

+179
-19
lines changed

‎src/EditorFeatures/CSharpTest/InvertLogical/InvertLogicalTests.cs‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,86 @@ void M(bool x, int a, object b)
594594
}", parseOptions: CSharp9);
595595
}
596596

597+
[Fact, WorkItem(64558, "https://github.com/dotnet/roslyn/issues/64558")]
598+
public async Task InvertNumericIsGreaterThanPattern1_CSharp9()
599+
{
600+
await TestInRegularAndScriptAsync(
601+
@"class C
602+
{
603+
void M(bool x, int a, object b)
604+
{
605+
var c = a > 10 [||]&& a is > 20;
606+
}
607+
}",
608+
@"class C
609+
{
610+
void M(bool x, int a, object b)
611+
{
612+
var c = !(a <= 10 || a is <= 20);
613+
}
614+
}", parseOptions: CSharp9);
615+
}
616+
617+
[Fact, WorkItem(64558, "https://github.com/dotnet/roslyn/issues/64558")]
618+
public async Task InvertNullableNumericIsGreaterThanPattern1_CSharp9()
619+
{
620+
await TestInRegularAndScriptAsync(
621+
@"class C
622+
{
623+
void M(bool x, int? a, object b)
624+
{
625+
var c = x [||]&& a is > 20;
626+
}
627+
}",
628+
@"class C
629+
{
630+
void M(bool x, int? a, object b)
631+
{
632+
var c = !(!x || a is not > 20);
633+
}
634+
}", parseOptions: CSharp9);
635+
}
636+
637+
[Fact, WorkItem(64558, "https://github.com/dotnet/roslyn/issues/64558")]
638+
public async Task InvertNonNumericIsGreaterThanPattern1_CSharp9()
639+
{
640+
await TestInRegularAndScriptAsync(
641+
@"class C
642+
{
643+
void M(bool x, int a, object b)
644+
{
645+
var c = a > 10 [||]&& b is > 20;
646+
}
647+
}",
648+
@"class C
649+
{
650+
void M(bool x, int a, object b)
651+
{
652+
var c = !(a <= 10 || b is not > 20);
653+
}
654+
}", parseOptions: CSharp9);
655+
}
656+
657+
[Fact, WorkItem(64558, "https://github.com/dotnet/roslyn/issues/64558")]
658+
public async Task InvertInvalidEqualsPattern1_CSharp9()
659+
{
660+
await TestInRegularAndScriptAsync(
661+
@"class C
662+
{
663+
void M(bool x, int a, object b)
664+
{
665+
var c = a > 10 [||]&& a is == 20;
666+
}
667+
}",
668+
@"class C
669+
{
670+
void M(bool x, int a, object b)
671+
{
672+
var c = !(a <= 10 || a is not == 20);
673+
}
674+
}", parseOptions: CSharp9);
675+
}
676+
597677
[Fact, WorkItem(42368, "https://github.com/dotnet/roslyn/issues/42368")]
598678
public async Task InvertIsAndPattern1_CSharp8()
599679
{

‎src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1497,6 +1497,13 @@ public void GetPartsOfUnaryPattern(SyntaxNode node, out SyntaxToken operatorToke
14971497
pattern = unaryPattern.Pattern;
14981498
}
14991499

1500+
public void GetPartsOfRelationalPattern(SyntaxNode node, out SyntaxToken operatorToken, out SyntaxNode expression)
1501+
{
1502+
var relationalPattern = (RelationalPatternSyntax)node;
1503+
operatorToken = relationalPattern.OperatorToken;
1504+
expression = relationalPattern.Expression;
1505+
}
1506+
15001507
public SyntaxNode GetTypeOfTypePattern(SyntaxNode node)
15011508
=> ((TypePatternSyntax)node).Type;
15021509

‎src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxKinds.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ public TSyntaxKind Convert<TSyntaxKind>(int kind) where TSyntaxKind : struct
100100
public int? OrPattern => (int)SyntaxKind.OrPattern;
101101
public int? ParenthesizedPattern => (int)SyntaxKind.ParenthesizedPattern;
102102
public int? RecursivePattern => (int)SyntaxKind.RecursivePattern;
103+
public int? RelationalPattern => (int)SyntaxKind.RelationalPattern;
103104
public int? TypePattern => (int)SyntaxKind.TypePattern;
104105
public int? VarPattern => (int)SyntaxKind.VarPattern;
105106

‎src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,7 @@ void GetPartsOfTupleExpression<TArgumentSyntax>(SyntaxNode node,
465465
void GetPartsOfBinaryPattern(SyntaxNode node, out SyntaxNode left, out SyntaxToken operatorToken, out SyntaxNode right);
466466
void GetPartsOfDeclarationPattern(SyntaxNode node, out SyntaxNode type, out SyntaxNode designation);
467467
void GetPartsOfRecursivePattern(SyntaxNode node, out SyntaxNode? type, out SyntaxNode? positionalPart, out SyntaxNode? propertyPart, out SyntaxNode? designation);
468+
void GetPartsOfRelationalPattern(SyntaxNode node, out SyntaxToken operatorToken, out SyntaxNode expression);
468469
void GetPartsOfUnaryPattern(SyntaxNode node, out SyntaxToken operatorToken, out SyntaxNode pattern);
469470

470471
bool ContainsInterleavedDirective(TextSpan span, SyntaxToken token, CancellationToken cancellationToken);

‎src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFactsExtensions.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -870,6 +870,9 @@ public static bool IsParenthesizedPattern(this ISyntaxFacts syntaxFacts, [NotNul
870870
public static bool IsRecursivePattern(this ISyntaxFacts syntaxFacts, [NotNullWhen(true)] SyntaxNode? node)
871871
=> node?.RawKind == syntaxFacts.SyntaxKinds.RecursivePattern;
872872

873+
public static bool IsRelationalPattern(this ISyntaxFacts syntaxFacts, [NotNullWhen(true)] SyntaxNode? node)
874+
=> node?.RawKind == syntaxFacts.SyntaxKinds.RelationalPattern;
875+
873876
public static bool IsTypePattern(this ISyntaxFacts syntaxFacts, [NotNullWhen(true)] SyntaxNode? node)
874877
=> node?.RawKind == syntaxFacts.SyntaxKinds.TypePattern;
875878

‎src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxKinds.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ internal interface ISyntaxKinds
151151
int? OrPattern { get; }
152152
int? ParenthesizedPattern { get; }
153153
int? RecursivePattern { get; }
154+
int? RelationalPattern { get; }
154155
int? TypePattern { get; }
155156
int? VarPattern { get; }
156157

‎src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1690,6 +1690,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService
16901690
Throw New InvalidOperationException(DoesNotExistInVBErrorMessage)
16911691
End Sub
16921692

1693+
Public Sub GetPartsOfRelationalPattern(node As SyntaxNode, ByRef operatorToken As SyntaxToken, ByRef expression As SyntaxNode) Implements ISyntaxFacts.GetPartsOfRelationalPattern
1694+
Throw New InvalidOperationException(DoesNotExistInVBErrorMessage)
1695+
End Sub
1696+
16931697
Public Sub GetPartsOfDeclarationPattern(node As SyntaxNode, ByRef type As SyntaxNode, ByRef designation As SyntaxNode) Implements ISyntaxFacts.GetPartsOfDeclarationPattern
16941698
Throw New InvalidOperationException(DoesNotExistInVBErrorMessage)
16951699
End Sub

‎src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxKinds.vb‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,9 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService
102102
Public ReadOnly Property OrPattern As Integer? Implements ISyntaxKinds.OrPattern
103103
Public ReadOnly Property ParenthesizedPattern As Integer? Implements ISyntaxKinds.ParenthesizedPattern
104104
Public ReadOnly Property RecursivePattern As Integer? Implements ISyntaxKinds.RecursivePattern
105+
Public ReadOnly Property RelationalPattern As Integer? Implements ISyntaxKinds.RelationalPattern
105106
Public ReadOnly Property TypePattern As Integer? Implements ISyntaxKinds.TypePattern
106107
Public ReadOnly Property VarPattern As Integer? Implements ISyntaxKinds.VarPattern
107-
Public ReadOnly Property IndexerMemberCref As Integer? Implements ISyntaxKinds.IndexerMemberCref
108108

109109
Public ReadOnly Property EndOfFileToken As Integer = SyntaxKind.EndOfFileToken Implements ISyntaxKinds.EndOfFileToken
110110
Public ReadOnly Property AwaitKeyword As Integer = SyntaxKind.AwaitKeyword Implements ISyntaxKinds.AwaitKeyword
@@ -146,5 +146,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService
146146
Public ReadOnly Property Interpolation As Integer = SyntaxKind.Interpolation Implements ISyntaxKinds.Interpolation
147147
Public ReadOnly Property InterpolatedStringExpression As Integer = SyntaxKind.InterpolatedStringExpression Implements ISyntaxKinds.InterpolatedStringExpression
148148
Public ReadOnly Property InterpolatedStringText As Integer = SyntaxKind.InterpolatedStringText Implements ISyntaxKinds.InterpolatedStringText
149+
Public ReadOnly Property IndexerMemberCref As Integer? Implements ISyntaxKinds.IndexerMemberCref
149150
End Class
150151
End Namespace

‎src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/LanguageServices/CSharpSyntaxGeneratorInternal.cs‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,9 @@ public override SyntaxNode IsPatternExpression(SyntaxNode expression, SyntaxToke
156156
isKeyword == default ? SyntaxFactory.Token(SyntaxKind.IsKeyword) : isKeyword,
157157
(PatternSyntax)pattern);
158158

159+
public override SyntaxNode AndPattern(SyntaxNode left, SyntaxNode right)
160+
=> SyntaxFactory.BinaryPattern(SyntaxKind.AndPattern, (PatternSyntax)Parenthesize(left), (PatternSyntax)Parenthesize(right));
161+
159162
public override SyntaxNode ConstantPattern(SyntaxNode expression)
160163
=> SyntaxFactory.ConstantPattern((ExpressionSyntax)expression);
161164

@@ -164,8 +167,17 @@ public override SyntaxNode DeclarationPattern(INamedTypeSymbol type, string name
164167
type.GenerateTypeSyntax(),
165168
SyntaxFactory.SingleVariableDesignation(name.ToIdentifierToken()));
166169

167-
public override SyntaxNode AndPattern(SyntaxNode left, SyntaxNode right)
168-
=> SyntaxFactory.BinaryPattern(SyntaxKind.AndPattern, (PatternSyntax)Parenthesize(left), (PatternSyntax)Parenthesize(right));
170+
public override SyntaxNode LessThanRelationalPattern(SyntaxNode expression)
171+
=> SyntaxFactory.RelationalPattern(SyntaxFactory.Token(SyntaxKind.LessThanToken), (ExpressionSyntax)expression);
172+
173+
public override SyntaxNode LessThanEqualsRelationalPattern(SyntaxNode expression)
174+
=> SyntaxFactory.RelationalPattern(SyntaxFactory.Token(SyntaxKind.LessThanEqualsToken), (ExpressionSyntax)expression);
175+
176+
public override SyntaxNode GreaterThanRelationalPattern(SyntaxNode expression)
177+
=> SyntaxFactory.RelationalPattern(SyntaxFactory.Token(SyntaxKind.GreaterThanToken), (ExpressionSyntax)expression);
178+
179+
public override SyntaxNode GreaterThanEqualsRelationalPattern(SyntaxNode expression)
180+
=> SyntaxFactory.RelationalPattern(SyntaxFactory.Token(SyntaxKind.GreaterThanEqualsToken), (ExpressionSyntax)expression);
169181

170182
public override SyntaxNode NotPattern(SyntaxNode pattern)
171183
=> SyntaxFactory.UnaryPattern(SyntaxFactory.Token(SyntaxKind.NotKeyword), (PatternSyntax)Parenthesize(pattern));

‎src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs‎

Lines changed: 44 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public static SyntaxNode Negate(
4949
bool negateBinary,
5050
CancellationToken cancellationToken)
5151
{
52-
return Negate(generator, generatorInternal, expressionOrPattern, semanticModel, negateBinary, allowSwappingBooleans: true, cancellationToken);
52+
return Negate(generator, generatorInternal, expressionOrPattern, semanticModel, negateBinary, patternValueType: null, cancellationToken);
5353
}
5454

5555
public static SyntaxNode Negate(
@@ -58,7 +58,7 @@ public static SyntaxNode Negate(
5858
SyntaxNode expressionOrPattern,
5959
SemanticModel semanticModel,
6060
bool negateBinary,
61-
bool allowSwappingBooleans,
61+
SpecialType? patternValueType,
6262
CancellationToken cancellationToken)
6363
{
6464
var options = semanticModel.SyntaxTree.Options;
@@ -105,7 +105,7 @@ public static SyntaxNode Negate(
105105
return GetNegationOfBinaryPattern(expressionOrPattern, generator, generatorInternal, semanticModel, cancellationToken);
106106

107107
if (syntaxFacts.IsConstantPattern(expressionOrPattern))
108-
return GetNegationOfConstantPattern(expressionOrPattern, generator, generatorInternal, allowSwappingBooleans);
108+
return GetNegationOfConstantPattern(expressionOrPattern, generator, generatorInternal, patternValueType);
109109

110110
if (syntaxFacts.IsUnaryPattern(expressionOrPattern))
111111
return GetNegationOfUnaryPattern(expressionOrPattern, generator, generatorInternal, syntaxFacts);
@@ -126,9 +126,10 @@ public static SyntaxNode Negate(
126126
return generator.IsTypeExpression(expression, type);
127127
}
128128

129-
// TODO(cyrusn): We could support negating relational patterns in the future. i.e.
130-
//
131-
// not >= 0 -> < 0
129+
if (syntaxFacts.IsRelationalPattern(expressionOrPattern))
130+
{
131+
return GetNegationOfRelationalPattern(expressionOrPattern, generatorInternal, patternValueType);
132+
}
132133

133134
return syntaxFacts.IsAnyPattern(expressionOrPattern)
134135
? generatorInternal.NotPattern(expressionOrPattern)
@@ -207,7 +208,7 @@ private static SyntaxNode GetNegationOfBinaryPattern(
207208
SemanticModel semanticModel,
208209
CancellationToken cancellationToken)
209210
{
210-
// Apply demorgans's law here.
211+
// Apply De Morgan's laws here.
211212
//
212213
// not (a and b) -> not a or not b
213214
// not (a or b) -> not a and not b
@@ -241,11 +242,11 @@ private static SyntaxNode GetNegationOfIsPatternExpression(SyntaxNode isExpressi
241242
if (syntaxFacts.SupportsNotPattern(semanticModel.SyntaxTree.Options))
242243
{
243244
// We do support 'not' patterns. So attempt to push a 'not' pattern into the current is-pattern RHS.
244-
// If the value isn't a Boolean and the pattern `is true/false`, swapping to `is false/true` is incorrect since non-Booleans match neither.
245-
// As an example, `!(new object() is true)` is equivalent to `new object() is not true` but not `new object() is false`.
245+
// We include the type of the value when negating the pattern, since it allows for nicer negations of
246+
// `is true/false` for Boolean values and relational patterns for numeric values.
246247
var operation = semanticModel.GetOperation(isExpression, cancellationToken);
247-
var isValueBoolean = operation is IIsPatternOperation isPatternOperation && isPatternOperation.Value.Type?.SpecialType == SpecialType.System_Boolean;
248-
negatedPattern = generator.Negate(generatorInternal, pattern, semanticModel, negateBinary: true, allowSwappingBooleans: isValueBoolean, cancellationToken);
248+
var valueType = (operation as IIsPatternOperation)?.Value.Type?.SpecialType;
249+
negatedPattern = generator.Negate(generatorInternal, pattern, semanticModel, negateBinary: true, valueType, cancellationToken);
249250
}
250251
else if (syntaxFacts.IsNotPattern(pattern))
251252
{
@@ -272,6 +273,33 @@ private static SyntaxNode GetNegationOfIsPatternExpression(SyntaxNode isExpressi
272273
return generator.LogicalNotExpression(isExpression);
273274
}
274275

276+
private static SyntaxNode GetNegationOfRelationalPattern(
277+
SyntaxNode expressionNode,
278+
SyntaxGeneratorInternal generatorInternal,
279+
SpecialType? patternValueType)
280+
{
281+
if (patternValueType is SpecialType specialType && specialType.IsNumericType())
282+
{
283+
// If we know the value is numeric, we can negate the relational operator.
284+
// This is not valid for non-numeric value since they never match a relational pattern.
285+
// Similarly, it's not valid for nullable values, since null never matches a relational pattern.
286+
// As an example, `!(new object() is < 1)` is equivalent to `new object() is not < 1` but not `new object() is >= 1`.
287+
var syntaxFacts = generatorInternal.SyntaxFacts;
288+
syntaxFacts.GetPartsOfRelationalPattern(expressionNode, out var operatorToken, out var expression);
289+
syntaxFacts.TryGetPredefinedOperator(operatorToken, out var predefinedOperator);
290+
return predefinedOperator switch
291+
{
292+
PredefinedOperator.LessThan => generatorInternal.GreaterThanEqualsRelationalPattern(expression),
293+
PredefinedOperator.LessThanOrEqual => generatorInternal.GreaterThanRelationalPattern(expression),
294+
PredefinedOperator.GreaterThan => generatorInternal.LessThanEqualsRelationalPattern(expression),
295+
PredefinedOperator.GreaterThanOrEqual => generatorInternal.LessThanRelationalPattern(expression),
296+
_ => generatorInternal.NotPattern(expressionNode)
297+
};
298+
}
299+
300+
return generatorInternal.NotPattern(expressionNode);
301+
}
302+
275303
private static bool IsLegalPattern(ISyntaxFacts syntaxFacts, SyntaxNode pattern, bool designatorsLegal)
276304
{
277305
// It is illegal to create a pattern that has a designator under a not-pattern or or-pattern
@@ -440,12 +468,14 @@ private static SyntaxNode GetNegationOfConstantPattern(
440468
SyntaxNode pattern,
441469
SyntaxGenerator generator,
442470
SyntaxGeneratorInternal generatorInternal,
443-
bool allowSwappingBooleans)
471+
SpecialType? patternValueType)
444472
{
445473
var syntaxFacts = generatorInternal.SyntaxFacts;
446474

447-
// If we have `is true/false` just swap that to be `is false/true` if allowed.
448-
if (allowSwappingBooleans)
475+
// If we have `is true/false` and a Boolean value, just swap that to be `is false/true`.
476+
// If the value isn't a Boolean, swapping to `is false/true` is incorrect since non-Booleans match neither.
477+
// As an example, `!(new object() is true)` is equivalent to `new object() is not true` but not `new object() is false`.
478+
if (patternValueType == SpecialType.System_Boolean)
449479
{
450480
var expression = syntaxFacts.GetExpressionOfConstantPattern(pattern);
451481
if (syntaxFacts.IsTrueLiteralExpression(expression))

0 commit comments

Comments
 (0)