Use of `EditorBrowsableState.Never` leads language service to claim it's gone

[AArnott]

Due to the Never flag used on a couple of these enum values:

https://github.com/dotnet/runtime/blob/20ab2fa963934b5823e0b8afbba858169dd27169/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs#L288-L298

When coding against multiple frameworks, Roslyn claims the APIs are simply missing:

Can we get these Never settings to be changed to Advanced instead?

Desired API Changes

 public enum ComInterfaceType 
 { 
-    [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] 
-    [System.ObsoleteAttribute("Support for IDispatch may be unavailable in future releases.")] 
     InterfaceIsDual = 0, 
     InterfaceIsIUnknown = 1, 
-    [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] 
-    [System.ObsoleteAttribute("Support for IDispatch may be unavailable in future releases.")] 
     InterfaceIsIDispatch = 2, 
     InterfaceIsIInspectable = 3, 
 }

[AArnott]

CC: @AaronRobinsonMSFT

[danmoseley]

@terrajobst

[terrajobst]

I have zero issues with changing the editor browsable state for these APIs. But before we're changing the APIs to accommodate tools I'd like to understand the IntelliSense behavior.

@jaredpar

Is this a side effect of how the combined language service works in multi-targeted projects? If so, can we fix this?

[jaredpar]

@jasonmalinowski

[jasonmalinowski]

The "this is only available on one side" logic is implemented by computing the list for both targets; it merges all the times together and if it didn't appear on one side or the other then the warning is given. You could imagine that we could instead merge and compute the warnings prior to throwing out the things for EditorBrowsableState.Never, although I'm not sure how much work that would take. I would have two things to think about:

1. EditorBrowsableState.Never is the IDE's ultimate back compat nightmare: any time we change this, somebody else will complain instead. At this point, we don't touch it unless we have a really good reason to. This is something we'd have to consider carefully, and I could imagine somebody else might come up with a reason we simply can't change this.
2. The IDE behavior looks like a feature here for other scenarios. I could totally imagine somebody having a multitargeted library they built that they're updating for .NET Core, and there's a corner of the library that they must deprecate -- say it's some corner that was something that isn't supported in .NET Core, and they don't really care to support it either. I could imagine them using [Obsolete] plus an EditorBrowsableState.Never as a way to say "in this target, this feature is no longer supported" so the consumer of a library could get a better error message that says "this is now dead, go use this other thing over here" rather than a generic "method doesn't exist" compiler error. Put another way, if in say .NET 42 you decided that .NET 43 was going to finally deprecate say InterfaceIsDual, would you wish for the current behavior?

[jasonmalinowski]

@dpoeschl might also know more about the behavior here as well.

[terrajobst]

The "this is only available on one side" logic is implemented by computing the list for both targets; it merges all the times together and if it didn't appear on one side or the other then the warning is given.

That's what I thought. I don't think the current behavior is desirable. You show a warning in IntelliSense that never materializes as a diagnostic when building. As this issue demonstrates, this is confusing because IntelliSense claims an API to be unavailable when in fact it is available but hidden. If the API is marked as EditorBrowsable.Never across all configured TFMs, then hiding the API entirely in the merged IntelliSense makes sense to me. But even in that case, the completion window wouldn't show a warning icon.

1. EditorBrowsableState.Never is the IDE's ultimate back compat nightmare: any time we change this, somebody else will complain instead. At this point, we don't touch it unless we have a really good reason to. This is something we'd have to consider carefully, and I could imagine somebody else might come up with a reason we simply can't change this.

That's why I'd like to avoid having to change the API declarations.

2. The IDE behavior looks like a feature here for other scenarios.

I don't buy the described scenario. If the person obsoletes the API for one TFM, then the consumer will get a warning when building for that TFM. Keep in mind that when people use multi-targeting they often do so because they need to support multiple versions of the same TFM. In that scenario you're often forced to call API that was obsoleted/hidden in a later TFM because the replacement isn't available in the earlier TFM. In that scenario, not showing me the API is the opposite of being helpful.

[AArnott]

We have the ObsoleteAttribute API to mark things as deprecated. Making them "Never" visible is an extreme tool to use for obsoleting something. The irony of doing so on these particular enum values is that IDispatch is the default value. How can .NET Core deprecate the default setting? It's ironic that .NET Core hides the enum value that is default anyway, making it difficult to express the default explicitly. But not setting it at all (by omitting the InterfaceType attribute) doesn't get you closer to your goal because that too is the deprecated setting.

So what's the plan here? How can IDispatch even be deprecated to begin with?

[terrajobst]

I’m not sure why it’s marked as hidden. I’ll take a look tomorrow.

FWIW, I agree with you that In 99% of cases obsoletion and hiding shouldn’t be combined.

[jozefizso]

Fixing this will greatly improve the quality of IntelliSense we get when we develop Office Add-in and other COM interop code which we plan to target for .NET Framework and .NET Core.

[reflectronic]

Are these even supposed to be deprecated? There is an issue about some COM features that mistakenly were marked obsolete.

[terrajobst]

OK, I've created https://github.com/dotnet/roslyn/issues/40370 to track the potential for changing how IntelliSense deals with hidden APIs. Now let's keep this thread focused on the why these particular APIs are hidden/obsoleted.

[AaronRobinsonMSFT]

These APIs were marked hidden/obsolete when the original port to coreclr was made. At that time we had no COM support so it made sense. In 3.0, we have that support so they are technically still valid. Issue https://github.com/dotnet/corefx/issues/40376 was created based on my comments at https://github.com/dotnet/coreclr/issues/26207#issuecomment-522070936.

At a minimum these APIs shouldn't marked System.ComponentModel.EditorBrowsableState.Never and ideally, since it plays badly with code quality metrics, the ObsoleteAttribute should be removed.

[Dennis-Petrov]

@AaronRobinsonMSFT , started to port our COM-object from NETfx to NETCore, got Support for IDispatch may be unavailable in future releases warning. Found this thread after a couple of hours of web search. Tons of posts which tells that NETCore 3+ has COM support, even the official docs tutorial, but nothing mentions IDispatch. So, if one wants to use late binding for NETCore 3+ COM-object from VBA, SAP ERP or something similar, it is valid to use ComInterfaceType.InterfaceIsDual, and it will work, isn't it?

[AaronRobinsonMSFT]

@Dennis-Petrov IDispatch is supported and .NET Core does support scenarios involving VBA. There are some caveats however. Since there is no supported TlbExp tool for .NET Core, a Type Library will not be created by the runtime. This is simple to circumvent since the interface can be defined in IDL and a Type Library created using the MIDL compiler. The scenario is definitely not as direct as .NET Framework but all the IDispatch runtime support from .NET Framework is available in .NET Core.

See https://github.com/dotnet/core-setup/issues/7800 for an end-to-end with VBA and the current issues.

[Dennis-Petrov]

@AaronRobinsonMSFT , thanks a lot!

[AArnott]

Since there is no supported TlbExp tool for .NET Core

@AaronRobinsonMSFT Why not just use tlbexp.exe from a .NET Framework targeted DLL? If you expect a DLL to have COM compatible types, wouldn't that assembly likely be readily targetable to net472 and then you can use the existing tools?

[Dennis-Petrov]

@AArnott, in my particular case COM object is just a thin legacy wrapper around large piece of functionality. It depends on number of NETStandard2.1/NETCore 3.1 libraries. Actually, absence of type library is a headache. I'm considering available options... Create IDL manually, compile it with MIDL compiler? How to register that TLB on target machine?

[AaronRobinsonMSFT]

@AArnott That is a valid point. Since all that is needed is a TLB it is possible to simply take the .NET Framework DLL and generate the TLB to represent the contracts. The only caveat is the automatic generation of class interfaces, which is not supported in .NET Core and unlikely to be brought back. This reduces to the idea that if one defines all their interfaces in C# compiles for .NET Framework and then runs the assembly through TlbExp, the resulting TLB should have a contract that is projectable from both a .NET Framework and .NET Core COM server.

[terrajobst]

@AaronRobinsonMSFT are you OK with these changes then?

 public enum ComInterfaceType 
 { 
-    [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] 
-    [System.ObsoleteAttribute("Support for IDispatch may be unavailable in future releases.")] 
     InterfaceIsDual = 0, 
     InterfaceIsIUnknown = 1, 
-    [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] 
-    [System.ObsoleteAttribute("Support for IDispatch may be unavailable in future releases.")] 
     InterfaceIsIDispatch = 2, 
     InterfaceIsIInspectable = 3, 
 }

[AaronRobinsonMSFT]

@terrajobst Yep, that makes sense to me.

[clairernovotny]

Ping? I just hit this with 5.0p4. ComEventInterface is also marked as obsolete when I don't believe it should be?

[terrajobst]

Video

Looks good as proposed

 public enum ComInterfaceType 
 { 
-    [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] 
-    [System.ObsoleteAttribute("Support for IDispatch may be unavailable in future releases.")] 
     InterfaceIsDual = 0, 
     InterfaceIsIUnknown = 1, 
-    [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] 
-    [System.ObsoleteAttribute("Support for IDispatch may be unavailable in future releases.")] 
     InterfaceIsIDispatch = 2, 
     InterfaceIsIInspectable = 3, 
 }

[AArnott]

Reactivating because the proposed and accepted API change was made, then lost. See? The attributes are still there:

https://github.com/dotnet/runtime/blob/d5b1a7f5fea6469e82a612d7de6f584667506b87/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs#L149-L158

[AArnott]

@AaronRobinsonMSFT Can you re-apply your change?

[AArnott]

It's not just the language service discoverability issues. These attributes are leading to CS0618 warnings on legitimate code.

[AaronRobinsonMSFT]

@AArnott That is a different API. This issue was for ComInterfaceType. It seems we missed updating ClassInterfaceType. I'll send out a new PR.