VB -> C#: `someInterface.Value` becomes `someInterface` if the interface has the `DefaultMember("Value")` attribute

HeinziAT commented 2 months ago

VB.Net input code

Requires a reference to "Microsoft Outlook 16.0 Object Library".

Imports Microsoft.Office.Interop.Outlook

Module Module1
    Sub Main()
    End Sub

    Sub Test(item As MailItem)
        Dim p = item.UserProperties.Find("abc")
        Console.WriteLine(p.Value)
    End Sub
End Module

Erroneous output

using System;
using Microsoft.Office.Interop.Outlook;

namespace VbCsRepro
{

    static class Module1
    {
        public static void Main()
        {
        }

        public static void Test(MailItem item)
        {
            var p = item.UserProperties.Find("abc");
            Console.WriteLine(p);
        }
    }
}

Expected output

Console.WriteLine(p) should be Console.WriteLine(p.Value).

Details

Product in use: VS extension
Version in use: 9.2.5.0
Most cases have been caught by compile-time errors, but some haven't. For example, newProperty.Value = oldProperty.Value becomes newProperty = oldProperty, which (unfortunately) compiles, but does something completely different.

HeinziAT commented 2 months ago

OK, I did some more debugging and managed to create a repro example without an Outlook reference:

Create a C# library with the following code:

[DefaultMember("Value")]
public interface I
{ 
object Value { get; set; }
}

Create a VB.NET project referencing this library with the following code:

Module Module1
Sub Main()
End Sub

Sub Test(p As I)
    Console.WriteLine(p.Value)
End Sub
End Module

Convert the VB file to C#.

Expected result: Console.WriteLine(p.Value);

Actual result: Console.WriteLine(p);

Apparently, the DefaultMemberAttribute make the converter think that it can just remove property accessors for that property. So the problem is not specific to COM.

GrahamTheCoder commented 2 months ago

Thanks, looks like something needs changing near here: https://github.com/icsharpcode/CodeConverter/blob/a978d1b9ac7c01ab90ff38acc005f9ee08286f15/CodeConverter/CSharp/ExpressionNodeVisitor.cs#L1033-L1052

In VB, it's a compile error to mark a property with no required parameters as Default. The attribute can be manually added of course, but it doesn't have the same effect on the converter as the C# attribute does apparently. My understanding was that C# only emitted that attribute to help VB users consume the code more idiomatically, so I'm a bit confused why the outlook library would be using this attribute in a way that VB doesn't support.

HeinziAT commented 2 months ago

My understanding was that C# only emitted that attribute to help VB users consume the code more idiomatically, so I'm a bit confused why the outlook library would be using this attribute in a way that VB doesn't support.

Good question. My guess is that whatever creates those COM interop assemblies translates COM default properties (DISPID 0, used e.g. for "Let coercion" in VB6/VBA/VBScript) by marking them as DefaultMemberAttribute. I don't really see the point of this, because (fortunately) no .NET language I am familiar with supports Let coercion.

For those unfamiliar with it, this is Let coercion in VB6/VBA/VBScript:

Sub Test(p As UserProperty)
    Dim v As Variant

    v = p       ' v now contains p.Value (Let coercion)
    Set v = p   ' v now references p
End Sub

Thanks, looks like something needs changing near here

My first instinct was that that invocationSymbol is null, but p.Value() (with unnecessary, but legal parenthesis) gets converted to p as well.

On a slightly related note, if we have the following VB library

Public Interface I
    Default Property Value(i As Int32) As String
End Interface

and a client uses NameOf(I.Value) (VB), this currently becomes nameof(I) (C#), silently introducing a bug. If we fix the underlying issue, we will get nameof(I.Value), which is better: It's still wrong (since C# cannot "see" the name of the indexer), but it's a compile-time error instead of a silent behavioral change.

icsharpcode / CodeConverter