Roslyn: EditorBrowsable(EditorBrowsableState.Never) in VS 2015 does not work as well as it does in VS 2013

[TyreeJackson]

Problem

Consider the following projects and classes:

ProjectA

_is not included in the solution_

namespace ProjectA
{

    using System;
    using System.ComponentModel;

    public class BaseClass
    {
        [EditorBrowsable(EditorBrowsableState.Never)]
        public new Type GetType() { return base.GetType(); }

        [EditorBrowsable(EditorBrowsableState.Never)]
        public new static bool Equals(object objA, object objB) { return Object.Equals(objA, objB); }

        [EditorBrowsable(EditorBrowsableState.Never)]
        public override bool Equals(object obj)  { return base.Equals(obj); }

        [EditorBrowsable(EditorBrowsableState.Never)]
        public override int GetHashCode() { return base.GetHashCode(); }

        [EditorBrowsable(EditorBrowsableState.Never)]
        protected new object MemberwiseClone() { return base.MemberwiseClone(); }

        [EditorBrowsable(EditorBrowsableState.Never)]
        public new static bool ReferenceEquals(object objA, object objB) { return Object.ReferenceEquals(objA, objB); }

        [EditorBrowsable(EditorBrowsableState.Never)]
        public override string ToString() { return base.ToString(); }

        public static void DoSomeBasicThing() {}

    }

}

namespace ProjectA
{

    public class ClassA : BaseClass
    {

        public static void InitializeClassA() { }

        public void DoSomethingA() {}

    }

}

ProjectB

_is included in the solution; has a binary reference to ProjectA_

namespace ProjectB
{

    using System.ComponentModel;
    using ProjectA;

    [EditorBrowsable(EditorBrowsableState.Never)]
    public class ClassB : BaseClass, IInterfaceA<ClassB>
    {

        public void DoSomethingB()
        {
            var objectA = new ClassA();
        }

    }

}

ProjectC

_is included in the solution; has a binary reference to ProjectA; has a project reference to ProjectB_

namespace ProjectC
{

    using ProjectB;

    public class ClassC 
    {

        public void TestEm()
        {
            var objectB  = new ClassB();
        }

    }

}

In Visual Studio 2013 the EditorBrowsable attribute is honored correctly for members of types defined in assemblies that are included using a binary reference. However, in Visual Studio 2015, some of the methods defined on the System.Object [_static Equals_, _static ReferenceEquals_, and _GetType_] appear in the Intellisense menu despite being marked with the EditorBrowsable attribute with the EditorBrowsableState.Never argument in a sub class via new hiding methods.

Here are some screenshots:

| Visual Studio 2013 _- viewing ProjectA.ClassA's Intellisense menu from ProjectB.ClassB:_ | Visual Studio 2015 _- viewing ProjectA.ClassA's Intellisense menu from ProjectB.ClassB:_ |
| --- | --- |
| | |

| Visual Studio 2013 _- viewing objectA's Intellisense menu from ProjectB.ClassB:_ | Visual Studio 2015 _- viewing objectA's Intellisense menu from ProjectB.ClassB:_ |
| --- | --- |
| | |

| Visual Studio 2013 _- viewing ProjectB.ClassB's Intellisense menu from ProjectC.ClassC:_ | Visual Studio 2015 _- viewing ProjectB.ClassB's Intellisense menu from ProjectC.ClassC:_ |
| --- | --- |
| | |

| Visual Studio 2013 _- viewing objectB's Intellisense menu from ProjectC.ClassC:_ | Visual Studio 2015 _- viewing objectB's Intellisense menu from ProjectC.ClassC:_ |
| --- | --- |
| | |

_Note that this is the additional problem I mentioned in #4358 in the footnote of this comment._

[hmahajanHM]

Is this fixed? I can still see this bug on VS 2015 Professional Update 1 build 14.0.24720.00

[jcansdale]

The NUnit team is suffering from this issue when developing our Assert API. We'd like to hide Equals, to prevent users from accidentally entering the following:

``` c#
Assert.Equals(a, b);
Assert.That(a, Is.Equals...

When they should have entered:

``` c#
Assert.AreEqual(a, b);
Assert.That(a, Is.EqualTo(b));

The following appears to work fine in Visual Studio 2013:

``` c#
[EditorBrowsable(EditorBrowsableState.Never)]
new public static bool Equals(object ob1, object ob2)
{
throw new NotImplementedException("I'm afraid I can't do that");
}

    [EditorBrowsable(EditorBrowsableState.Never)]
    new public static bool ReferenceEquals(object ob1, object ob2)
    {
        throw new NotImplementedException("I'm afraid I can't do that");
    }

```

But no longer works in Visual Studio 2015:

You can find the issue here:
https://github.com/nunit/nunit/issues/1726#issuecomment-255089011

See also:
https://twitter.com/jcansdale/status/788704315695398912 😉

[Pilchie]

Also reported in https://developercommunity.visualstudio.com/content/problem/32768/editorbrowsable-attribute-not-respected-by-intelli.html.

[kornelijepetak]

I don't understand why this is so hard to fix?
It should be a piece of cake now that Intellisense already got filtering by types, methods, properties, etc.

Is this a VS thing, or Roslyn? Which system is responsible for Intellisense population?

[sebastianbk]

So that's up with this? It was added to the 2.0 milestone but appears to still be an issue. Is there any work for a fix going on regarding this bug?

[ryo0ka]

Any update?

[CyrusNajmabadi]

@ryo0ka PRs welcome :)

[sharwell]

@dpoeschl and I are interested in seeing this completed, but it doesn't currently fix into our scheduling. I'm hoping a community user picks it up so it can ship in 15.5.

Requirements

• Define the intended behavior
  
  
  
  • Target item is in the current project
  
  
  • Target item is found via a project reference
  
  
  • Target item is found via an assembly (metadata) reference
  
  
• Locally experiment with Visual Studio 2013 to identify any differences between the intended behavior and the behavior you observe in Visual Studio 2013
• Post results of intended behavior and Visual Studio 2013 experiments here for review/sign-off (feel free to copy from the original bug report comment where it helps, but review text for accuracy)
• Implement the completion filtering according to the intended behavior
• Implement tests covering each of the intended behavior scenarios

Time requirement

This will probably take a new user up to a week or more, or an experienced user a few days.

Work would need to start within the next week to ensure time for this to ship in 15.5 (the earliest release where it would currently be possible).

Interested?

If you are interested, please let us know. If more than one person is interested, that would be acceptable for this issue. In particular, it will be good to have more than one person test the intended behavior in Visual Studio 2013, and more than one person review the test cases to ensure they accurately represent the intended behavior so we do not regress in the future.

[kzu]

As far as intended behavior, I'd say the members with EditorBrowsableState.Never should never be show, regardless of how the member was discovered (current project, project reference or assembly reference). An improvement would be to show it anyway for the first two, but I'd say it's very much a nice too have with very low pri.

[apmorton]

@JieCarolHu @sharwell what is the status of this? I am interested in helping to get this issue fixed.

@JieCarolHu it looks like you have done some work on this in the past (#25290), but the PR is closed and appears dead for several months. Is there still work on this? Was it killed because of the required public API changes?

The work in #25290, although incomplete, does appear to fix some cursory unit tests I wrote up for this issue. What is the blocker on finishing this work? Presumably approval or buy-in from whomever owns the relevant public APIs?

[MikeLimaSierra]

After more than 4 years this is still an issue with VS 2017.
Can we expect a fix or is it just that low a priority that nobody has time for it?

[CyrusNajmabadi]

Can we expect a fix or is it just that low a priority that nobody has time for it?

@MikeLimaSierra In this case, i believe this would be taken as a community contribution if someone was interested. However, what's important (to me at least) is that a design be created that doesn't break very specific and intentional design changes that were intentionally made to this feature.

Specifically, it was intentional, and based on enough feedback, that the EditorBrowable attribute have different behavior for your current project versus references. This was because there were many teams that effectively wanted to have members that they would see, but that they wanted to hide from others.

So, the design of the attribute was updated to very intentionally operate different within projects in a solution vs metadata references pulled in. The intuition being: if this is your own solution, you understand the design and want to have access to your own members, but when distributing as a library you want to be able to control how others view your assembly.

If we can fix these issues while keeping the above design intact, I'm personally happy.

[sharwell]

@CyrusNajmabadi The bug here is unrelated to the situation you describe, which is covered by #37478. This bug is about a failure of the Roslyn implementation to also eliminate methods _hidden_ by a method whenever it eliminates a method with this attribute.

[Scottj1s]

Ping... it sounds like a conservative fix - one that does not change the original constraints of the attribute, but simply restores VS 2013 behavior - has been validated. What's the status of releasing this fix? Many code generators would benefit greatly.

[CyrusNajmabadi]

@Scottj1s There has been no forward progress on this issue. Sorry!

[TyreeJackson]

Can we expect a fix or is it just that low a priority that nobody has time for it?

@MikeLimaSierra In this case, i believe this would be taken as a community contribution if someone was interested. However, what's important (to me at least) is that a design be created that doesn't break very specific and intentional design changes that were intentionally made to this feature.

Specifically, it was intentional, and based on enough feedback, that the EditorBrowable attribute have different behavior for your current project versus references. This was because there were many teams that effectively wanted to have members that _they_ would see, but that they wanted to hide from others.

So, the design of the attribute was updated to _very intentionally_ operate different within projects in a solution vs metadata references pulled in. The intuition being: if this is your own solution, you understand the design and want to have access to your own members, but when distributing as a library you want to be able to control how _others_ view your assembly.

If we can fix these issues while keeping the above design intact, I'm personally happy.

Can we have a new EditorBrowsableState.Never_We_Really_Mean_It_This_Time state for those of us who don't want to see members even in their own projects? Or perhaps a more appropriate option would be to add a EditorBrowsableState.InternalOnly for those who want to see the member in their projects but not outside of their project. It's strange to change the meaning of Never after so many years of respecting it.

[Scottj1s]

I'm not opposed to a Never_We_Really_Mean_It_This_Time enhancement, as long as it doesn't hold hostage just getting the regression fixed. The former has a higher acceptance bar (new feature), so should be tracked and implemented as a separate issue.

[CyrusNajmabadi]

I've marked this up for grabs. We would be ok taking in a contribution as per the rules that Sam has laid out. Thanks!

[stevenbrix]

We seemed to have traded one evil for another and at the same time introduced a breaking change. Making changes like this doesn't seem like good engineering practice to me.

@Scottj1s @TyreeJackson, something like EditorBrowsableState.Never_We_Really_Mean_It_This_Time feels like the incorrect solution to a problem that never should've existed. I imagine if we went back in time before this breaking change was introduced, we wouldn't come up with this solution.

I understanding wanting to preserve the current behavior of EditorBrowsableState.Never for those that are now dependent on it. But I would vote for an MSBuild project property that can used to opt-in to this behavior:

<PropertyGroup>
   <EditorBrowsableNeverReallyMeansInternalOnly>true</EditorBrowsableNeverReallyMeansInternalOnly>
</PropertyGroup>

Then we can have a more correct solution using your suggestion of EditorBrowsableState.InternalOnly. This should behave identically to the internal keyword, meaning that assemblies who are granted internal access via InternalsVisibleTo would also see metadata tagged with EditorBrowsableState.InternalOnly.

[CyrusNajmabadi]

As above, we woudl be willing to take a contribution here that matches the change that Sam laid out.