Correct logic for conversion objWithOverloads Is Nothing - fixes #591

Author: GrahamTheCoder

CHANGELOG.md

@@ -9,7 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 * Fix file extension and location of single converted file [#589](https://github.com/icsharpcode/CodeConverter/issues/589)
 
 ### VB -> C#
-
+* Correct logic for conversion "objectWithOverloadedEquals Is Nothing" [#591](https://github.com/icsharpcode/CodeConverter/issues/591)
 
 ### C# -> VB
 

CodeConverter/CSharp/BuiltInVisualBasicOperatorSubstitutions.cs

@@ -49,18 +49,20 @@ public async Task<ExpressionSyntax> ConvertNothingComparisonOrNullAsync(VBSyntax
                 if (!(exprNode is VBSyntax.BinaryExpressionSyntax node) || !node.IsKind(VBasic.SyntaxKind.IsExpression, VBasic.SyntaxKind.EqualsExpression, VBasic.SyntaxKind.IsNotExpression, VBasic.SyntaxKind.NotEqualsExpression)) {
                     return null;
                 }
-                ExpressionSyntax otherArgument;
+
+                VBSyntax.ExpressionSyntax vbOtherArg;
                 if (node.Left.IsKind(VBasic.SyntaxKind.NothingLiteralExpression)) {
-                    otherArgument = (ExpressionSyntax)await ConvertIsOrIsNotExpressionArgAsync(node.Right);
+                    vbOtherArg = node.Right;
                 } else if (node.Right.IsKind(VBasic.SyntaxKind.NothingLiteralExpression)) {
-                    otherArgument = (ExpressionSyntax)await ConvertIsOrIsNotExpressionArgAsync(node.Left);
+                    vbOtherArg = node.Left;
                 } else {
                     return null;
                 }
-
-                var isReference = node.IsKind(VBasic.SyntaxKind.IsExpression, VBasic.SyntaxKind.IsNotExpression);
+                var csOtherArg = (ExpressionSyntax)await ConvertIsOrIsNotExpressionArgAsync(vbOtherArg);
+                var couldHaveOverloadedOperators = _semanticModel.GetTypeInfo(vbOtherArg).Type.SpecialType == SpecialType.None;
+                var isReferenceComparison = node.IsKind(VBasic.SyntaxKind.IsExpression, VBasic.SyntaxKind.IsNotExpression);
                 var notted = node.IsKind(VBasic.SyntaxKind.IsNotExpression, VBasic.SyntaxKind.NotEqualsExpression) || negateExpression;
-                return notted ? CommonConversions.NotNothingComparison(otherArgument, isReference) : CommonConversions.NothingComparison(otherArgument, isReference);
+                return notted ? CommonConversions.NotNothingComparison(csOtherArg, isReferenceComparison) : CommonConversions.NothingComparison(csOtherArg, isReferenceComparison, couldHaveOverloadedOperators);
             }
 
             private async Task<CSharpSyntaxNode> ConvertIsOrIsNotExpressionArgAsync(VBSyntax.ExpressionSyntax binaryExpressionArg)

CodeConverter/CSharp/CommonConversions.cs

@@ -658,16 +658,17 @@ public static CSSyntax.ParameterListSyntax CreateParameterList(IEnumerable<Synta
         public static CSSyntax.BinaryExpressionSyntax NotNothingComparison(ExpressionSyntax otherArgument, bool isReferenceType)
         {
             if (isReferenceType) {
-                return SyntaxFactory.BinaryExpression(CSSyntaxKind.IsExpression, otherArgument, SyntaxFactory.PredefinedType(SyntaxFactory.Token(CSSyntaxKind.ObjectKeyword)));
+                return SyntaxFactory.BinaryExpression(CSSyntaxKind.IsExpression, otherArgument, ValidSyntaxFactory.ObjectType);
             }
-            return SyntaxFactory.BinaryExpression(CSSyntaxKind.NotEqualsExpression, otherArgument, SyntaxFactory.LiteralExpression(CSSyntaxKind.DefaultLiteralExpression));
+            return SyntaxFactory.BinaryExpression(CSSyntaxKind.NotEqualsExpression, otherArgument, ValidSyntaxFactory.DefaultExpression);
         }
 
-        public static ExpressionSyntax NothingComparison(ExpressionSyntax otherArgument, bool isReferenceType)
+        public static ExpressionSyntax NothingComparison(ExpressionSyntax otherArgument, bool isReferenceType, bool couldHaveOverloadedOperators)
         {
             // Old project style doesn't support is pattern expressions (or indeed anything beyond c#7.3), so can't use "x is null"
-            var literalKind = isReferenceType ? CSSyntaxKind.NullLiteralExpression : CSSyntaxKind.DefaultLiteralExpression;
-            return SyntaxFactory.BinaryExpression(CSSyntaxKind.EqualsExpression, otherArgument, SyntaxFactory.LiteralExpression(literalKind));
+            var literal = isReferenceType ? ValidSyntaxFactory.NullExpression : (ExpressionSyntax)ValidSyntaxFactory.DefaultExpression;
+            if (isReferenceType && couldHaveOverloadedOperators) otherArgument = SyntaxFactory.ParenthesizedExpression(SyntaxFactory.CastExpression(ValidSyntaxFactory.ObjectType, otherArgument));
+            return SyntaxFactory.BinaryExpression(CSSyntaxKind.EqualsExpression, otherArgument, literal);
         }
     }
 }

CodeConverter/CSharp/ValidSyntaxFactory.cs

@@ -99,5 +99,9 @@ public static DeclarationPatternSyntax VarPattern(SyntaxToken varName)
             return SyntaxFactory.DeclarationPattern(
                 ValidSyntaxFactory.VarType, SyntaxFactory.SingleVariableDesignation(varName));
         }
+
+        public static PredefinedTypeSyntax ObjectType => SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.ObjectKeyword));
+        public static LiteralExpressionSyntax NullExpression => SyntaxFactory.LiteralExpression(SyntaxKind.NullLiteralExpression);
+        public static LiteralExpressionSyntax DefaultExpression => SyntaxFactory.LiteralExpression(SyntaxKind.DefaultLiteralExpression);
     }
 }

Tests/CSharp/ExpressionTests/ExpressionTests.cs

@@ -1347,7 +1347,7 @@ public partial class Foo
 
     protected void OnBar(EventArgs e)
     {
-        if (Bar == null)
+        if ((object)Bar == null)
         {
             Debug.WriteLine(""No subscriber"");
         }

Tests/TestData/SelfVerifyingTests/VBToCS/ObjectEqualityTests.vb

@@ -0,0 +1,47 @@
+Imports System
+Imports System.Collections.Generic
+Imports System.Linq
+Imports Xunit
+
+
+Friend Class CTst
+    Public Shared Operator <>(ByVal aCls1 As CTst, ByVal aCls2 As CTst) As Boolean
+        Assert.Empty("<> operator should not be called")
+        Return True
+    End Operator
+    Public Shared Operator =(ByVal aCls1 As CTst, ByVal aCls2 As CTst) As Boolean
+        Assert.Empty("= operator should not be called")
+        Return True
+    End Operator
+End Class
+
+Public Class ObjectEqualityTests
+
+    <Fact>
+    Public Sub ComparingToNothingDoesNotCallOperatorOverload()
+        Dim Test1 As CTst
+
+        If Not Test1 Is Nothing Then
+            Assert.Empty("First branch should not be taken")
+        ElseIf Test1 IsNot Nothing Then
+            Assert.Empty("Second branch should not be taken")
+        ElseIf Test1 Is Nothing Then
+            Assert.True(Test1 Is Nothing)
+            Assert.False(Not Test1 Is Nothing)
+            Assert.False(Test1 IsNot Nothing)
+        Else
+            Assert.Empty("Else branch should not be taken")
+        End If
+    End Sub
+
+    <Fact>
+    Public Sub GenericCanBeAssignedNothing()
+        Generic(5, False)
+        Generic("5", True)
+    End Sub
+
+    Private Sub Generic(Of T)(input As T, isReference As Boolean)
+        input = Nothing
+        If isReference Then Assert.True(input Is Nothing) Else Assert.True(input IsNot Nothing)
+    End Sub
+End Class