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:

runtime/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs

Lines 288 to 298 in 20ab2fa

	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,
	}

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.