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

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

Open
HeinziAT opened this issue Apr 12, 2024 · 3 comments
Labels
VB -> C# Specific to VB -> C# conversion

Comments

@HeinziAT
Copy link

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 HeinziAT added the VB -> C# Specific to VB -> C# conversion label Apr 12, 2024
@HeinziAT
Copy link
Author

HeinziAT commented Apr 12, 2024

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

  1. Create a C# library with the following code:
[DefaultMember("Value")]
public interface I
{ 
    object Value { get; set; }
}
  1. 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
  1. 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.

@HeinziAT HeinziAT changed the title VB -> C#: .Value property access lost on COM object VB -> C#: someInterface.Value becomes someInterface if the interface has the DefaultMember("Value") attribute Apr 12, 2024
@GrahamTheCoder
Copy link
Member

Thanks, looks like something needs changing near here:

// VB doesn't have a specialized node for element access because the syntax is ambiguous. Instead, it just uses an invocation expression or dictionary access expression, then figures out using the semantic model which one is most likely intended.
// https://github.com/dotnet/roslyn/blob/master/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSyntaxFactsService.vb#L768
(var convertedExpression, bool shouldBeElementAccess) = await ConvertInvocationSubExpressionAsync(node, operation, expressionSymbol, expressionReturnType, expr);
if (shouldBeElementAccess)
{
return await CreateElementAccessAsync(node, convertedExpression);
}
if (expressionSymbol != null && expressionSymbol.IsKind(SymbolKind.Property) &&
invocationSymbol != null && invocationSymbol.GetParameters().Length == 0 && node.ArgumentList.Arguments.Count == 0)
{
return convertedExpression; //Parameterless property access
}
var convertedArgumentList = await ConvertArgumentListOrEmptyAsync(node, node.ArgumentList);
if (IsElementAtOrDefaultInvocation(invocationSymbol, expressionSymbol))
{
convertedExpression = GetElementAtOrDefaultExpression(expressionType, convertedExpression);
}

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
VB -> C# Specific to VB -> C# conversion
Projects
None yet
Development

No branches or pull requests

2 participants