Correct conversion for equality of overloaded types - fixes #594

Author: GrahamTheCoder

CHANGELOG.md

@@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 ### VB -> C#
 * Correct logic for conversion "objectWithOverloadedEquals Is Nothing" [#591](https://github.com/icsharpcode/CodeConverter/issues/591)
 * Coercing enum to a string now correctly uses its numeric value [#590](https://github.com/icsharpcode/CodeConverter/issues/590)
+* Correct conversion for equality of overloaded types [#594](https://github.com/icsharpcode/CodeConverter/issues/594)
 
 ### C# -> VB
 

CodeConverter/CSharp/BuiltInVisualBasicOperatorSubstitutions.cs

@@ -44,31 +44,58 @@ public BuiltInVisualBasicOperatorSubstitutions(CommentConvertingVisitorWrapper t
                 _typeConversionAnalyzer = typeConversionAnalyzer;
             }
 
-            public async Task<ExpressionSyntax> ConvertNothingComparisonOrNullAsync(VBSyntax.ExpressionSyntax exprNode, bool negateExpression = false)
+            public async Task<ExpressionSyntax> ConvertReferenceOrNothingComparisonOrNullAsync(VBSyntax.ExpressionSyntax exprNode, bool negateExpression = false)
             {
-                if (!(exprNode is VBSyntax.BinaryExpressionSyntax node) || !node.IsKind(VBasic.SyntaxKind.IsExpression, VBasic.SyntaxKind.EqualsExpression, VBasic.SyntaxKind.IsNotExpression, VBasic.SyntaxKind.NotEqualsExpression)) {
+                if (!(exprNode is VBSyntax.BinaryExpressionSyntax node) ||
+                    !node.IsKind(VBasic.SyntaxKind.IsExpression, VBasic.SyntaxKind.EqualsExpression, VBasic.SyntaxKind.IsNotExpression, VBasic.SyntaxKind.NotEqualsExpression)) {
                     return null;
                 }
+                
+                var notted =
+                    node.IsKind(VBasic.SyntaxKind.IsNotExpression, VBasic.SyntaxKind.NotEqualsExpression) ||
+                    negateExpression;
+                var isReferenceComparison = node.IsKind(VBasic.SyntaxKind.IsExpression, VBasic.SyntaxKind.IsNotExpression);
+
+                if (ArgComparedToNull(node) is {} vbOtherArg) {
+                    var csOtherArg = await ConvertIsOrIsNotExpressionArgAsync(vbOtherArg);
+                    return notted
+                        ? CommonConversions.NotNothingComparison(csOtherArg, isReferenceComparison)
+                        : CommonConversions.NothingComparison(csOtherArg, isReferenceComparison);
+                }
 
-                VBSyntax.ExpressionSyntax vbOtherArg;
-                if (node.Left.IsKind(VBasic.SyntaxKind.NothingLiteralExpression)) {
-                    vbOtherArg = node.Right;
-                } else if (node.Right.IsKind(VBasic.SyntaxKind.NothingLiteralExpression)) {
-                    vbOtherArg = node.Left;
-                } else {
-                    return null;
+                if (isReferenceComparison) {
+
+                    var lhs = await ConvertIsOrIsNotExpressionArgAsync(node.Left);
+                    var rhs = await ConvertIsOrIsNotExpressionArgAsync(node.Right);
+
+                    var equalityCheck = new KnownMethod(nameof(System), nameof(Object), nameof(object.ReferenceEquals))
+                        .Invoke(_visualBasicEqualityComparison.ExtraUsingDirectives,
+                            ConvertTo(node.Left, lhs, SpecialType.System_Object), rhs);
+                    return notted
+                        ? SyntaxFactory.PrefixUnaryExpression(SyntaxKind.LogicalNotExpression, equalityCheck)
+                        : equalityCheck;
+                }
+                return null;
+            }
+
+            private static VBSyntax.ExpressionSyntax ArgComparedToNull(VBSyntax.BinaryExpressionSyntax node)
+            {
+                if (node.Left.IsKind(VBasic.SyntaxKind.NothingLiteralExpression))
+                {
+                    return node.Right;
                 }
-                var csOtherArg = (ExpressionSyntax)await ConvertIsOrIsNotExpressionArgAsync(vbOtherArg);
-                var typeSymbol = _semanticModel.GetTypeInfo(vbOtherArg).Type;
-                var isReferenceComparison = typeSymbol?.IsValueType != true || node.IsKind(VBasic.SyntaxKind.IsExpression, VBasic.SyntaxKind.IsNotExpression);
-                var notted = node.IsKind(VBasic.SyntaxKind.IsNotExpression, VBasic.SyntaxKind.NotEqualsExpression) || negateExpression;
-                return notted ? CommonConversions.NotNothingComparison(csOtherArg, isReferenceComparison) : CommonConversions.NothingComparison(csOtherArg, isReferenceComparison);
+                else if (node.Right.IsKind(VBasic.SyntaxKind.NothingLiteralExpression))
+                {
+                    return node.Left;
+                }
+
+                return null;
             }
 
-            private async Task<CSharpSyntaxNode> ConvertIsOrIsNotExpressionArgAsync(VBSyntax.ExpressionSyntax binaryExpressionArg)
+            private async Task<ExpressionSyntax> ConvertIsOrIsNotExpressionArgAsync(VBSyntax.ExpressionSyntax binaryExpressionArg)
             {
-                return await ConvertMyGroupCollectionPropertyGetWithUnderlyingFieldAsync(binaryExpressionArg)
-                       ?? await binaryExpressionArg.AcceptAsync(_triviaConvertingVisitor);
+                return (ExpressionSyntax) (await ConvertMyGroupCollectionPropertyGetWithUnderlyingFieldAsync(binaryExpressionArg)
+                                           ?? await binaryExpressionArg.AcceptAsync(_triviaConvertingVisitor));
             }
 
             private async Task<ExpressionSyntax> ConvertMyGroupCollectionPropertyGetWithUnderlyingFieldAsync(SyntaxNode node)
@@ -104,9 +131,9 @@ private async Task<ExpressionSyntax> ConvertToPowOperatorAsync(VBSyntax.BinaryEx
                     .Invoke(_visualBasicEqualityComparison.ExtraUsingDirectives, lhs, rhs);
             }
 
-            private ExpressionSyntax ConvertTo(VBSyntax.ExpressionSyntax node, ExpressionSyntax lhs, SpecialType targetType)
+            private ExpressionSyntax ConvertTo(VBSyntax.ExpressionSyntax vbNode, ExpressionSyntax csNode, SpecialType targetType)
             {
-                return _typeConversionAnalyzer.AddExplicitConversion(node, lhs, forceTargetType: _semanticModel.Compilation.GetSpecialType(targetType));
+                return _typeConversionAnalyzer.AddExplicitConversion(vbNode, csNode, forceTargetType: _semanticModel.Compilation.GetSpecialType(targetType));
             }
 
             /// <remarks>No need to implement these since this is only called for things that are already decimal and hence will resolve operator in C#</remarks>
@@ -173,7 +200,8 @@ public async Task<ExpressionSyntax> ConvertRewrittenBinaryOperatorOrNullAsync(VB
                 switch (opKind) {
                     case BinaryOperatorKind.IsExpression:
                     case BinaryOperatorKind.IsNotExpression: {
-                            if (await ConvertNothingComparisonOrNullAsync(node) is CSharpSyntaxNode nothingComparison) return (ExpressionSyntax) nothingComparison;
+                            if (await ConvertReferenceOrNothingComparisonOrNullAsync(node) is { } nothingComparison) return nothingComparison;
+                            
                             break;
                         }
 

CodeConverter/CSharp/ExpressionNodeVisitor.cs

@@ -679,7 +679,7 @@ private ExpressionSyntax AsBool(VBSyntax.UnaryExpressionSyntax node, ExpressionS
 
         private async Task<ExpressionSyntax> NegateAndSimplifyOrNullAsync(VBSyntax.UnaryExpressionSyntax node, ExpressionSyntax expr)
         {
-            if (await _operatorConverter.ConvertNothingComparisonOrNullAsync(node.Operand.SkipIntoParens(), true) is { } nothingComparison) {
+            if (await _operatorConverter.ConvertReferenceOrNothingComparisonOrNullAsync(node.Operand.SkipIntoParens(), true) is { } nothingComparison) {
                 return nothingComparison;
             } else if (expr is BinaryExpressionSyntax bes && bes.OperatorToken.IsKind(SyntaxKind.EqualsToken)) {
                 return bes.WithOperatorToken(SyntaxFactory.Token(SyntaxKind.ExclamationEqualsToken));

CodeConverter/CSharp/IOperatorConverter.cs

@@ -6,7 +6,7 @@ namespace ICSharpCode.CodeConverter.CSharp
 {
     public interface IOperatorConverter
     {
-        Task<Microsoft.CodeAnalysis.CSharp.Syntax.ExpressionSyntax> ConvertNothingComparisonOrNullAsync(Microsoft.CodeAnalysis.VisualBasic.Syntax.ExpressionSyntax exprNode, bool negateExpression = false);
+        Task<Microsoft.CodeAnalysis.CSharp.Syntax.ExpressionSyntax> ConvertReferenceOrNothingComparisonOrNullAsync(Microsoft.CodeAnalysis.VisualBasic.Syntax.ExpressionSyntax exprNode, bool negateExpression = false);
         Task<Microsoft.CodeAnalysis.CSharp.Syntax.ExpressionSyntax> ConvertRewrittenBinaryOperatorOrNullAsync(Microsoft.CodeAnalysis.VisualBasic.Syntax.BinaryExpressionSyntax node, bool inExpressionLambda = false);
     }
 }

Tests/CSharp/ExpressionTests/BinaryExpressionTests.cs

@@ -47,8 +47,8 @@ await TestConversionVisualBasicToCSharpAsync(@"Class TestClass
 End Class", @"
 internal partial class TestClass
 {
-    private bool bIs = new object() == new object();
-    private bool bIsNot = new object() != new object();
+    private bool bIs = ReferenceEquals(new object(), new object());
+    private bool bIsNot = !ReferenceEquals(new object(), new object());
     private int bLeftShift = 1 << 3;
     private int bRightShift = 8 >> 3;
 }");

Tests/TestData/MultiFileCharacterization/VBToCSResults/ConvertWholeSolution/VbNetStandardLib/My Project/MyNamespace.Static.2.Designer.cs

@@ -5,13 +5,13 @@ Imports Xunit
 
 
 Friend Class CTst
+    Public Shared Property GetOperatorResult As Func(Of CTst, CTst, Boolean)
+
     Public Shared Operator <>(ByVal aCls1 As CTst, ByVal aCls2 As CTst) As Boolean
-        Assert.Empty("<> operator should not be called")
-        Return True
+        Return Not GetOperatorResult()(aCls1, aCls2)
     End Operator
     Public Shared Operator =(ByVal aCls1 As CTst, ByVal aCls2 As CTst) As Boolean
-        Assert.Empty("= operator should not be called")
-        Return True
+        Return GetOperatorResult()(aCls1, aCls2)
     End Operator
 End Class
 
@@ -21,6 +21,10 @@ Public Class ObjectEqualityTests
     Public Sub ComparingToNothingDoesNotCallOperatorOverload()
         Dim Test1 As CTst
 
+        CTst.GetOperatorResult = Function(a, b)
+                                     Assert.Empty("operator should not be called")
+                                     Return False
+                                 End Function
         If Not Test1 Is Nothing Then
             Assert.Empty("First branch should not be taken")
         ElseIf Test1 IsNot Nothing Then
@@ -34,6 +38,7 @@ Public Class ObjectEqualityTests
         End If
     End Sub
 
+
     <Fact>
     Public Sub GenericCanBeAssignedNothing()
         Generic(5, False)
@@ -44,4 +49,23 @@ Public Class ObjectEqualityTests
         input = Nothing
         If isReference Then Assert.True(input Is Nothing) Else Assert.True(input IsNot Nothing)
     End Sub
+
+    <Fact>
+    Public Sub ConstrainedGenericCanBeEquatedToNothing()
+        Dim callCount = 0
+        CTst.GetOperatorResult = Function(a, b)
+                                     callCount += 1
+                                     Return Object.ReferenceEquals(a, b)
+                                 End Function
+        GenericEquatable(New CTst)
+        Assert.Equal(callCount, 4)
+    End Sub
+
+    Private Sub GenericEquatable(Of T As CTst)(input As T)
+        Assert.False(input = Nothing)
+        Assert.True(input <> Nothing)
+        input = Nothing
+        Assert.True(input = Nothing)
+        Assert.False(input <> Nothing)
+    End Sub
 End Class