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 1 commit
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,66 @@ 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(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 @@ -102,6 +102,7 @@ public TSyntaxKind Convert<TSyntaxKind>(int kind) where TSyntaxKind : struct
public int? RecursivePattern => (int)SyntaxKind.RecursivePattern;
public int? TypePattern => (int)SyntaxKind.TypePattern;
public int? VarPattern => (int)SyntaxKind.VarPattern;
public int? RelationalPattern => (int)SyntaxKind.RelationalPattern;
kimsey0 marked this conversation as resolved.
Show resolved Hide resolved

public int EndOfFileToken => (int)SyntaxKind.EndOfFileToken;
public int AwaitKeyword => (int)SyntaxKind.AwaitKeyword;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,7 @@ void GetPartsOfTupleExpression<TArgumentSyntax>(SyntaxNode node,
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 GetPartsOfUnaryPattern(SyntaxNode node, out SyntaxToken operatorToken, out SyntaxNode pattern);
void GetPartsOfRelationalPattern(SyntaxNode node, out SyntaxToken operatorToken, out SyntaxNode expression);
kimsey0 marked this conversation as resolved.
Show resolved Hide resolved

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,9 @@ public static bool IsTypePattern(this ISyntaxFacts syntaxFacts, [NotNullWhen(tru
public static bool IsVarPattern(this ISyntaxFacts syntaxFacts, [NotNullWhen(true)] SyntaxNode? node)
=> node?.RawKind == syntaxFacts.SyntaxKinds.VarPattern;

public static bool IsRelationalPattern(this ISyntaxFacts syntaxFacts, [NotNullWhen(true)] SyntaxNode? node)
=> node?.RawKind == syntaxFacts.SyntaxKinds.RelationalPattern;
kimsey0 marked this conversation as resolved.
Show resolved Hide resolved

#endregion

#region statements
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ internal interface ISyntaxKinds
int? RecursivePattern { get; }
int? TypePattern { get; }
int? VarPattern { get; }
int? RelationalPattern { get; }
kimsey0 marked this conversation as resolved.
Show resolved Hide resolved

#endregion

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1689,6 +1689,9 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService
Public Sub GetPartsOfUnaryPattern(node As SyntaxNode, ByRef operatorToken As SyntaxToken, ByRef pattern As SyntaxNode) Implements ISyntaxFacts.GetPartsOfUnaryPattern
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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService
Public ReadOnly Property RecursivePattern As Integer? Implements ISyntaxKinds.RecursivePattern
Public ReadOnly Property TypePattern As Integer? Implements ISyntaxKinds.TypePattern
Public ReadOnly Property VarPattern As Integer? Implements ISyntaxKinds.VarPattern
Public ReadOnly Property RelationalPattern As Integer? Implements ISyntaxKinds.RelationalPattern
Public ReadOnly Property IndexerMemberCref As Integer? Implements ISyntaxKinds.IndexerMemberCref
kimsey0 marked this conversation as resolved.
Show resolved Hide resolved

Public ReadOnly Property EndOfFileToken As Integer = SyntaxKind.EndOfFileToken Implements ISyntaxKinds.EndOfFileToken
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,18 @@ public override SyntaxNode TypePattern(SyntaxNode type)
public override SyntaxNode UnaryPattern(SyntaxToken operatorToken, SyntaxNode pattern)
=> SyntaxFactory.UnaryPattern(operatorToken, (PatternSyntax)Parenthesize(pattern));

public override SyntaxNode LessThanPattern(SyntaxNode expression)
kimsey0 marked this conversation as resolved.
Show resolved Hide resolved
=> SyntaxFactory.RelationalPattern(SyntaxFactory.Token(SyntaxKind.LessThanToken), (ExpressionSyntax)expression);

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

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

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

#endregion
}
}
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.GreaterThanEqualsPattern(expression),
PredefinedOperator.LessThanOrEqual => generatorInternal.GreaterThanPattern(expression),
PredefinedOperator.GreaterThan => generatorInternal.LessThanEqualsPattern(expression),
PredefinedOperator.GreaterThanOrEqual => generatorInternal.LessThanPattern(expression),
_ => throw ExceptionUtilities.UnexpectedValue(predefinedOperator)
kimsey0 marked this conversation as resolved.
Show resolved Hide resolved
};
}

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 @@ -107,6 +107,10 @@ public SyntaxNode LocalDeclarationStatement(SyntaxToken name, SyntaxNode initial
public abstract SyntaxNode ParenthesizedPattern(SyntaxNode pattern);
public abstract SyntaxNode TypePattern(SyntaxNode type);
public abstract SyntaxNode UnaryPattern(SyntaxToken operatorToken, SyntaxNode pattern);
public abstract SyntaxNode LessThanPattern(SyntaxNode expression);
public abstract SyntaxNode LessThanEqualsPattern(SyntaxNode expression);
public abstract SyntaxNode GreaterThanPattern(SyntaxNode expression);
public abstract SyntaxNode GreaterThanEqualsPattern(SyntaxNode expression);
kimsey0 marked this conversation as resolved.
Show resolved Hide resolved

#endregion
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,22 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeGeneration
Throw New NotImplementedException()
End Function

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

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

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

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

#End Region
End Class
End Namespace