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

[EditorBrowsable(EditorBrowsableState.Never)] does not hide System.Object members from types in Nuget packages #61912

Closed
dotfede opened this issue Jun 15, 2022 · 9 comments
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead

Comments

@dotfede
Copy link

dotfede commented Jun 15, 2022

Version Used:

Visual Studio 17.2.4
.NET SDK 6.0.301

Steps to Reproduce:

1 - dotnet new console
2 - dotnet add package FluentInterfaceExample
3 - Open the newly created project in Visual Studio
4 - Inside program.cs, paste the following code:

FluentInterfaceExample.FluentExample1 e1;
FluentInterfaceExample.FluentExample2 e2;

5 - Below that, type e1. to bring up Intellisense for e1 and e2. to bring up Intellisense for e2

6 - Observe the following:

image

image

Note that these types are defined in FluentInterfaceExample Nuget package (source), and implement IFluentInterface (source), which shadows the following System.Object members as follows:

        [EditorBrowsable(EditorBrowsableState.Never)]
        Type GetType();

        [EditorBrowsable(EditorBrowsableState.Never)]
        int GetHashCode();

        [EditorBrowsable(EditorBrowsableState.Never)]
        string ToString();

        [EditorBrowsable(EditorBrowsableState.Never)]
        bool Equals(object obj);

Also note that methods FluentExample1.HiddenMethod1() and FluentExample2.HiddenMethod2(), both marked with [EditorBrowsable(EditorBrowsableState.Never)] are properly hidden from Intellisense.

Expected Behavior:

System.Object members mentioned above should not be visible in Intellisense.

Actual Behavior:

System.Object members mentioned above are visible in Intellisense.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Jun 15, 2022
@CyrusNajmabadi
Copy link
Member

The members of IFluentInterface are indeed hidden (you're not seeing IFluentInterface.Equals in the list). Having one member's attributes hide another member from intellisense has never been supported afaict.

@dotfede
Copy link
Author

dotfede commented Jun 15, 2022

@CyrusNajmabadi that is exactly what IFluentInterface was designed for.

See the screenshot in their readme which shows precisely that all 4 members of System.Object aren't shown in Intellisense.

Also see this issue that states that this worked properly in older VS versions and is no longer working in newer versions.

@CyrusNajmabadi
Copy link
Member

@fberasategui for the linked issue, we would be open to taking a community contribution here.

@ichthus1604
Copy link

ichthus1604 commented Jun 15, 2022

@CyrusNajmabadi to clarify, is all the information discussed in #4434 relevant to this particular problem?

This problem is affecting us as well since long ago.

From your comment:

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.

It seems that current behavior in the case of Nuget references does not match your description.

I would be willing to contribute, but please help me clarify the situation here because I'm not sure the code by proposed @sharwell would fix this particular situation, and if it does, what's missing or expected (other than that code) in a PR that would get merged to fix this.

@CyrusNajmabadi
Copy link
Member

It seems that current behavior in the case of Nuget references does not match your description.

Can you clarify this? Code from a nuget package should be considered to be external and we should hide items from them when referenced from a project. We should not do the same if the attribute is on a source member that you are referencing as a source-project. This was an intentional change made based on lots of feedback from teams that only wanted hiding from external consumers, not hiding from themselves.

@ichthus1604
Copy link

@CyrusNajmabadi

We should not do the same if the attribute is on a source member that you are referencing as a source-project

That's correct. I don't think anyone can argue against that. But the current behavior is not what you describe in the other case:

Code from a nuget package should be considered to be external and we should hide items from them when referenced from a project

As shown above, types from a Nuget package are currently not hiding shadowed members (specifically) marked with the attribute. I assume this is a more general problem (not specifically related to System.Object), but that is by far the greatest offender.

How would a PR have to look, and what would it have to include in order to get merged to fix this? What else apart from @sharwell's code is required?

@CyrusNajmabadi
Copy link
Member

How would a PR have to look, and what would it have to include in order to get merged to fix this?

It would likely need to implement the acceptable behavior (as specified in that linked issue), and come with reasonable (e.g. not insanely complex) code to implement that logic, as well as tests to demonstrate the new behavior and how it works for non-source references vs source references.

@sharwell
Copy link
Member

Duplicate of #4434

@sharwell sharwell marked this as a duplicate of #4434 Jun 15, 2022
@sharwell sharwell closed this as not planned Won't fix, can't repro, duplicate, stale Jun 15, 2022
@sharwell
Copy link
Member

sharwell commented Jun 15, 2022

The information in 4434 should still be relevant. The reason why this was difficult in the past is we didn't have a good way in the compiler API to access the symbols which a symbol hides (as opposed to overrides, which does have an API).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

No branches or pull requests

4 participants