Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change operator when negating relational patterns with numeric values #64594

Merged
merged 6 commits into from
Oct 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,86 @@ void M(bool x, int a, object b)
}", parseOptions: CSharp9);
}

[Fact, WorkItem(64558, "https://github.com/dotnet/roslyn/issues/64558")]
public async Task InvertNumericIsGreaterThanPattern1_CSharp9()
{
await TestInRegularAndScriptAsync(
@"class C
{
void M(bool x, int a, object b)
{
var c = a > 10 [||]&& a is > 20;
kimsey0 marked this conversation as resolved.
Show resolved Hide resolved
}
}",
@"class C
{
void M(bool x, int a, object b)
{
var c = !(a <= 10 || a is <= 20);
}
}", parseOptions: CSharp9);
}

[Fact, WorkItem(64558, "https://github.com/dotnet/roslyn/issues/64558")]
public async Task InvertNullableNumericIsGreaterThanPattern1_CSharp9()
{
await TestInRegularAndScriptAsync(
@"class C
{
void M(bool x, int? a, object b)
{
var c = x [||]&& a is > 20;
}
}",
@"class C
{
void M(bool x, int? a, object b)
{
var c = !(!x || a is not > 20);
}
}", parseOptions: CSharp9);
}

[Fact, WorkItem(64558, "https://github.com/dotnet/roslyn/issues/64558")]
public async Task InvertNonNumericIsGreaterThanPattern1_CSharp9()
{
await TestInRegularAndScriptAsync(
@"class C
{
void M(bool x, int a, object b)
{
var c = a > 10 [||]&& b is > 20;
}
}",
@"class C
{
void M(bool x, int a, object b)
{
var c = !(a <= 10 || b is not > 20);
}
}", parseOptions: CSharp9);
}

[Fact, WorkItem(64558, "https://github.com/dotnet/roslyn/issues/64558")]
public async Task InvertInvalidEqualsPattern1_CSharp9()
{
await TestInRegularAndScriptAsync(
@"class C
{
void M(bool x, int a, object b)
{
var c = a > 10 [||]&& a is == 20;
}
}",
@"class C
{
void M(bool x, int a, object b)
{
var c = !(a <= 10 || a is not == 20);
}
}", parseOptions: CSharp9);
}

[Fact, WorkItem(42368, "https://github.com/dotnet/roslyn/issues/42368")]
public async Task InvertIsAndPattern1_CSharp8()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1497,6 +1497,13 @@ public void GetPartsOfUnaryPattern(SyntaxNode node, out SyntaxToken operatorToke
pattern = unaryPattern.Pattern;
}

public void GetPartsOfRelationalPattern(SyntaxNode node, out SyntaxToken operatorToken, out SyntaxNode expression)
{
var relationalPattern = (RelationalPatternSyntax)node;
operatorToken = relationalPattern.OperatorToken;
expression = relationalPattern.Expression;
}

public SyntaxNode GetTypeOfTypePattern(SyntaxNode node)
=> ((TypePatternSyntax)node).Type;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ public TSyntaxKind Convert<TSyntaxKind>(int kind) where TSyntaxKind : struct
public int? OrPattern => (int)SyntaxKind.OrPattern;
public int? ParenthesizedPattern => (int)SyntaxKind.ParenthesizedPattern;
public int? RecursivePattern => (int)SyntaxKind.RecursivePattern;
public int? RelationalPattern => (int)SyntaxKind.RelationalPattern;
public int? TypePattern => (int)SyntaxKind.TypePattern;
public int? VarPattern => (int)SyntaxKind.VarPattern;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,7 @@ void GetPartsOfTupleExpression<TArgumentSyntax>(SyntaxNode node,
void GetPartsOfBinaryPattern(SyntaxNode node, out SyntaxNode left, out SyntaxToken operatorToken, out SyntaxNode right);
void GetPartsOfDeclarationPattern(SyntaxNode node, out SyntaxNode type, out SyntaxNode designation);
void GetPartsOfRecursivePattern(SyntaxNode node, out SyntaxNode? type, out SyntaxNode? positionalPart, out SyntaxNode? propertyPart, out SyntaxNode? designation);
void GetPartsOfRelationalPattern(SyntaxNode node, out SyntaxToken operatorToken, out SyntaxNode expression);
kimsey0 marked this conversation as resolved.
Show resolved Hide resolved
void GetPartsOfUnaryPattern(SyntaxNode node, out SyntaxToken operatorToken, out SyntaxNode pattern);

bool ContainsInterleavedDirective(TextSpan span, SyntaxToken token, CancellationToken cancellationToken);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,9 @@ public static bool IsParenthesizedPattern(this ISyntaxFacts syntaxFacts, [NotNul
public static bool IsRecursivePattern(this ISyntaxFacts syntaxFacts, [NotNullWhen(true)] SyntaxNode? node)
=> node?.RawKind == syntaxFacts.SyntaxKinds.RecursivePattern;

public static bool IsRelationalPattern(this ISyntaxFacts syntaxFacts, [NotNullWhen(true)] SyntaxNode? node)
=> node?.RawKind == syntaxFacts.SyntaxKinds.RelationalPattern;

public static bool IsTypePattern(this ISyntaxFacts syntaxFacts, [NotNullWhen(true)] SyntaxNode? node)
=> node?.RawKind == syntaxFacts.SyntaxKinds.TypePattern;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ internal interface ISyntaxKinds
int? OrPattern { get; }
int? ParenthesizedPattern { get; }
int? RecursivePattern { get; }
int? RelationalPattern { get; }
int? TypePattern { get; }
int? VarPattern { get; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1690,6 +1690,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService
Throw New InvalidOperationException(DoesNotExistInVBErrorMessage)
End Sub

Public Sub GetPartsOfRelationalPattern(node As SyntaxNode, ByRef operatorToken As SyntaxToken, ByRef expression As SyntaxNode) Implements ISyntaxFacts.GetPartsOfRelationalPattern
kimsey0 marked this conversation as resolved.
Show resolved Hide resolved
Throw New InvalidOperationException(DoesNotExistInVBErrorMessage)
End Sub

Public Sub GetPartsOfDeclarationPattern(node As SyntaxNode, ByRef type As SyntaxNode, ByRef designation As SyntaxNode) Implements ISyntaxFacts.GetPartsOfDeclarationPattern
Throw New InvalidOperationException(DoesNotExistInVBErrorMessage)
End Sub
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,9 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService
Public ReadOnly Property OrPattern As Integer? Implements ISyntaxKinds.OrPattern
Public ReadOnly Property ParenthesizedPattern As Integer? Implements ISyntaxKinds.ParenthesizedPattern
Public ReadOnly Property RecursivePattern As Integer? Implements ISyntaxKinds.RecursivePattern
Public ReadOnly Property RelationalPattern As Integer? Implements ISyntaxKinds.RelationalPattern
Public ReadOnly Property TypePattern As Integer? Implements ISyntaxKinds.TypePattern
Public ReadOnly Property VarPattern As Integer? Implements ISyntaxKinds.VarPattern
Public ReadOnly Property IndexerMemberCref As Integer? Implements ISyntaxKinds.IndexerMemberCref

Public ReadOnly Property EndOfFileToken As Integer = SyntaxKind.EndOfFileToken Implements ISyntaxKinds.EndOfFileToken
Public ReadOnly Property AwaitKeyword As Integer = SyntaxKind.AwaitKeyword Implements ISyntaxKinds.AwaitKeyword
Expand Down Expand Up @@ -146,5 +146,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService
Public ReadOnly Property Interpolation As Integer = SyntaxKind.Interpolation Implements ISyntaxKinds.Interpolation
Public ReadOnly Property InterpolatedStringExpression As Integer = SyntaxKind.InterpolatedStringExpression Implements ISyntaxKinds.InterpolatedStringExpression
Public ReadOnly Property InterpolatedStringText As Integer = SyntaxKind.InterpolatedStringText Implements ISyntaxKinds.InterpolatedStringText
Public ReadOnly Property IndexerMemberCref As Integer? Implements ISyntaxKinds.IndexerMemberCref
End Class
End Namespace
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,9 @@ public override SyntaxNode IsPatternExpression(SyntaxNode expression, SyntaxToke
isKeyword == default ? SyntaxFactory.Token(SyntaxKind.IsKeyword) : isKeyword,
(PatternSyntax)pattern);

public override SyntaxNode AndPattern(SyntaxNode left, SyntaxNode right)
=> SyntaxFactory.BinaryPattern(SyntaxKind.AndPattern, (PatternSyntax)Parenthesize(left), (PatternSyntax)Parenthesize(right));

public override SyntaxNode ConstantPattern(SyntaxNode expression)
=> SyntaxFactory.ConstantPattern((ExpressionSyntax)expression);

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

public override SyntaxNode AndPattern(SyntaxNode left, SyntaxNode right)
=> SyntaxFactory.BinaryPattern(SyntaxKind.AndPattern, (PatternSyntax)Parenthesize(left), (PatternSyntax)Parenthesize(right));
public override SyntaxNode LessThanRelationalPattern(SyntaxNode expression)
=> SyntaxFactory.RelationalPattern(SyntaxFactory.Token(SyntaxKind.LessThanToken), (ExpressionSyntax)expression);

public override SyntaxNode LessThanEqualsRelationalPattern(SyntaxNode expression)
=> SyntaxFactory.RelationalPattern(SyntaxFactory.Token(SyntaxKind.LessThanEqualsToken), (ExpressionSyntax)expression);

public override SyntaxNode GreaterThanRelationalPattern(SyntaxNode expression)
=> SyntaxFactory.RelationalPattern(SyntaxFactory.Token(SyntaxKind.GreaterThanToken), (ExpressionSyntax)expression);

public override SyntaxNode GreaterThanEqualsRelationalPattern(SyntaxNode expression)
=> SyntaxFactory.RelationalPattern(SyntaxFactory.Token(SyntaxKind.GreaterThanEqualsToken), (ExpressionSyntax)expression);

public override SyntaxNode NotPattern(SyntaxNode pattern)
=> SyntaxFactory.UnaryPattern(SyntaxFactory.Token(SyntaxKind.NotKeyword), (PatternSyntax)Parenthesize(pattern));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public static SyntaxNode Negate(
bool negateBinary,
CancellationToken cancellationToken)
{
return Negate(generator, generatorInternal, expressionOrPattern, semanticModel, negateBinary, allowSwappingBooleans: true, cancellationToken);
return Negate(generator, generatorInternal, expressionOrPattern, semanticModel, negateBinary, patternValueType: null, cancellationToken);
}

public static SyntaxNode Negate(
Expand All @@ -58,7 +58,7 @@ public static SyntaxNode Negate(
SyntaxNode expressionOrPattern,
SemanticModel semanticModel,
bool negateBinary,
bool allowSwappingBooleans,
SpecialType? patternValueType,
CancellationToken cancellationToken)
{
var options = semanticModel.SyntaxTree.Options;
Expand Down Expand Up @@ -105,7 +105,7 @@ public static SyntaxNode Negate(
return GetNegationOfBinaryPattern(expressionOrPattern, generator, generatorInternal, semanticModel, cancellationToken);

if (syntaxFacts.IsConstantPattern(expressionOrPattern))
return GetNegationOfConstantPattern(expressionOrPattern, generator, generatorInternal, allowSwappingBooleans);
return GetNegationOfConstantPattern(expressionOrPattern, generator, generatorInternal, patternValueType);

if (syntaxFacts.IsUnaryPattern(expressionOrPattern))
return GetNegationOfUnaryPattern(expressionOrPattern, generator, generatorInternal, syntaxFacts);
Expand All @@ -126,9 +126,10 @@ public static SyntaxNode Negate(
return generator.IsTypeExpression(expression, type);
}

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

return syntaxFacts.IsAnyPattern(expressionOrPattern)
? generatorInternal.NotPattern(expressionOrPattern)
Expand Down Expand Up @@ -207,7 +208,7 @@ private static SyntaxNode GetNegationOfBinaryPattern(
SemanticModel semanticModel,
CancellationToken cancellationToken)
{
// Apply demorgans's law here.
// Apply De Morgan's laws here.
//
// not (a and b) -> not a or not b
// not (a or b) -> not a and not b
Expand Down Expand Up @@ -241,11 +242,11 @@ private static SyntaxNode GetNegationOfIsPatternExpression(SyntaxNode isExpressi
if (syntaxFacts.SupportsNotPattern(semanticModel.SyntaxTree.Options))
{
// We do support 'not' patterns. So attempt to push a 'not' pattern into the current is-pattern RHS.
// 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.
// As an example, `!(new object() is true)` is equivalent to `new object() is not true` but not `new object() is false`.
// We include the type of the value when negating the pattern, since it allows for nicer negations of
// `is true/false` for Boolean values and relational patterns for numeric values.
var operation = semanticModel.GetOperation(isExpression, cancellationToken);
var isValueBoolean = operation is IIsPatternOperation isPatternOperation && isPatternOperation.Value.Type?.SpecialType == SpecialType.System_Boolean;
negatedPattern = generator.Negate(generatorInternal, pattern, semanticModel, negateBinary: true, allowSwappingBooleans: isValueBoolean, cancellationToken);
var valueType = (operation as IIsPatternOperation)?.Value.Type?.SpecialType;
kimsey0 marked this conversation as resolved.
Show resolved Hide resolved
negatedPattern = generator.Negate(generatorInternal, pattern, semanticModel, negateBinary: true, valueType, cancellationToken);
}
else if (syntaxFacts.IsNotPattern(pattern))
{
Expand All @@ -272,6 +273,33 @@ private static SyntaxNode GetNegationOfIsPatternExpression(SyntaxNode isExpressi
return generator.LogicalNotExpression(isExpression);
}

private static SyntaxNode GetNegationOfRelationalPattern(
SyntaxNode expressionNode,
SyntaxGeneratorInternal generatorInternal,
SpecialType? patternValueType)
{
if (patternValueType is SpecialType specialType && specialType.IsNumericType())
{
// If we know the value is numeric, we can negate the relational operator.
// This is not valid for non-numeric value since they never match a relational pattern.
// Similarly, it's not valid for nullable values, since null never matches a relational pattern.
// As an example, `!(new object() is < 1)` is equivalent to `new object() is not < 1` but not `new object() is >= 1`.
var syntaxFacts = generatorInternal.SyntaxFacts;
syntaxFacts.GetPartsOfRelationalPattern(expressionNode, out var operatorToken, out var expression);
syntaxFacts.TryGetPredefinedOperator(operatorToken, out var predefinedOperator);
kimsey0 marked this conversation as resolved.
Show resolved Hide resolved
return predefinedOperator switch
{
PredefinedOperator.LessThan => generatorInternal.GreaterThanEqualsRelationalPattern(expression),
PredefinedOperator.LessThanOrEqual => generatorInternal.GreaterThanRelationalPattern(expression),
PredefinedOperator.GreaterThan => generatorInternal.LessThanEqualsRelationalPattern(expression),
PredefinedOperator.GreaterThanOrEqual => generatorInternal.LessThanRelationalPattern(expression),
_ => generatorInternal.NotPattern(expressionNode)
};
}

return generatorInternal.NotPattern(expressionNode);
}

private static bool IsLegalPattern(ISyntaxFacts syntaxFacts, SyntaxNode pattern, bool designatorsLegal)
{
// It is illegal to create a pattern that has a designator under a not-pattern or or-pattern
Expand Down Expand Up @@ -440,12 +468,14 @@ private static SyntaxNode GetNegationOfConstantPattern(
SyntaxNode pattern,
SyntaxGenerator generator,
SyntaxGeneratorInternal generatorInternal,
bool allowSwappingBooleans)
SpecialType? patternValueType)
{
var syntaxFacts = generatorInternal.SyntaxFacts;

// If we have `is true/false` just swap that to be `is false/true` if allowed.
if (allowSwappingBooleans)
// If we have `is true/false` and a Boolean value, just swap that to be `is false/true`.
// If the value isn't a Boolean, swapping to `is false/true` is incorrect since non-Booleans match neither.
// As an example, `!(new object() is true)` is equivalent to `new object() is not true` but not `new object() is false`.
if (patternValueType == SpecialType.System_Boolean)
{
var expression = syntaxFacts.GetExpressionOfConstantPattern(pattern);
if (syntaxFacts.IsTrueLiteralExpression(expression))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,12 @@ public SyntaxNode LocalDeclarationStatement(SyntaxToken name, SyntaxNode initial
public abstract SyntaxNode IsPatternExpression(SyntaxNode expression, SyntaxToken isToken, SyntaxNode pattern);

public abstract SyntaxNode AndPattern(SyntaxNode left, SyntaxNode right);
public abstract SyntaxNode DeclarationPattern(INamedTypeSymbol type, string name);
public abstract SyntaxNode ConstantPattern(SyntaxNode expression);
public abstract SyntaxNode DeclarationPattern(INamedTypeSymbol type, string name);
public abstract SyntaxNode GreaterThanRelationalPattern(SyntaxNode expression);
public abstract SyntaxNode GreaterThanEqualsRelationalPattern(SyntaxNode expression);
public abstract SyntaxNode LessThanRelationalPattern(SyntaxNode expression);
public abstract SyntaxNode LessThanEqualsRelationalPattern(SyntaxNode expression);
public abstract SyntaxNode NotPattern(SyntaxNode pattern);
public abstract SyntaxNode OrPattern(SyntaxNode left, SyntaxNode right);
public abstract SyntaxNode ParenthesizedPattern(SyntaxNode pattern);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeGeneration
Throw New NotImplementedException()
End Function

Public Overrides Function AndPattern(left As SyntaxNode, right As SyntaxNode) As SyntaxNode
Throw New NotImplementedException()
End Function

Public Overrides Function ConstantPattern(expression As SyntaxNode) As SyntaxNode
Throw New NotImplementedException()
End Function
Expand All @@ -164,7 +168,19 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeGeneration
Throw New NotImplementedException()
End Function

Public Overrides Function AndPattern(left As SyntaxNode, right As SyntaxNode) As SyntaxNode
Public Overrides Function LessThanRelationalPattern(expression As SyntaxNode) As SyntaxNode
Throw New NotImplementedException()
End Function

Public Overrides Function LessThanEqualsRelationalPattern(expression As SyntaxNode) As SyntaxNode
Throw New NotImplementedException()
End Function

Public Overrides Function GreaterThanRelationalPattern(expression As SyntaxNode) As SyntaxNode
Throw New NotImplementedException()
End Function

Public Overrides Function GreaterThanEqualsRelationalPattern(expression As SyntaxNode) As SyntaxNode
Throw New NotImplementedException()
End Function

Expand Down