Proposal: Fine-grained visibility

[YaakovDavis]

Motivation

Often it's desired to have a finer level of control over visibility of type members within a project's boundaries, such as allowing constructors/members to be invoked only from certain contexts.
The existing internal modifier only affects inter-project visibility, but doesn't aid with safety within project boundaries.

Overview

We propose a new top-level declaration, visibility, which defines visibility "contexts". Member/type declarations are then annotated, both at the provider/consumer side to control visibility.

Syntax

In the following example, we want to control the initialization of Node instances.

• A visibility context is defined as follows.

visibility treeInternals;

• A member is declared as visible to a context as follows:

class Node
{
   treeInternals Node(...) //the ctor is visible only to members which see "treeInternals"
   {
   }

   public Node Parent => ...;  //normal public visibility
}

• A class/method can see contextual members if it has the sees clause appended to its declaration:

class Tree sees treeInternals //all methods in Tree can init nodes
{
   ...
}

class TreeUtilities
{
   public Node NewNode() sees treeInternals //can init nodes
   {
       ...
   } 

   public void CalcHeight(Node node) {...} //can't init Nodes
}

• Generic methods can have the sees clause either before or after the where clause:

void Foo<T>() where T : new, sees treeInternals 
{}

void Bar<T>() sees treeInternals, where T : new
{}

Further notes

Standard editor facilities can allow analyzing references to the visibility context (both at the provider/consumer side), allowing the code-base maintainer to ensure that the intended visibility scheme is preserved.

[dmitriyse]

I agree that fine-grained visibility can help to maintain large project and to improve self-documentation.
But we already coding with rocket speed. There are no time to put comments, no time to add fine-grained validation and even less time to maintain fine-grained visibility. We are moving to minimalism.
C# 6.0 adds lots of syntax sugar.
Nuget adds great rocket speed modularity.
Dotnet core adds tools for support multiple runtimes/platform by a single code base.
We are in overflow of code lines.
Github help us to share our code.
Google and internet answers us to a right question in a 5 minute.
VS and R# allow to produce more that 1000 release to gold lines in a day.

So you need to find very strong motivation for this feature to win over other proposal.
Please share more real-life motivation. To fill all your pain.

Also find good reasons that holds you from splitting your large project into multiple project (even more than hundreds it's fairly ok). InternalsVisibleTo can be a good trade-off.

[YaakovDavis]

Caring about safety on design-time, can save you a lot of time along the maintainability road.
That way you won't feel that you have "no time" for proper implementation.

The purpose of "syntax sugar" is to help readability, not to compromise correctness. So relax, and think about what you write :)

[axel-habermaier]

Note that with attributes and analyzers, C# already provides all the necessary features to implement this proposal.

[YaakovDavis]

@axel-habermaier

with attributes and analyzers, C# already provides all the necessary features to implement this proposal

The attributes + analyzers approach is a 2nd-best, but it has the following disadvantages:

1. Users can suppress the analyzer, compromising the codebase safety. (Sure they can change visibility of members as well, but the first type of change isn't visible in source control, while the latter is).
2. AFAIK analyzers can't affect intellisense.
3. An external analyzer will require an expensive analysis at coding-time, thus hurting the editor performance. A built-in approach can potentially optimize the analysis in ways not available to analyzers.
4. Finding references to a specific visibility: although this can be done, the UX will be inferior.

A built-in approach simply offers a better safety mechanism.

[yaakov-h]

An external analyzer will require an expensive analysis at coding-time, thus hurting the editor performance. A built-in approach can simply optimize the analysis in ways not available to analyzers.

This sounds like it needs a lot more justification.

[YaakovDavis]

@yaakov-h
Such an analyzer would need to examine every member access, to ensure that the visibility attributes, if such are defined, are honored. It would need to calculate the symbol of every accessed member, which from my experience, is expensive.
The Analyzers API is designed for general-purposes. Of course the compiler has more information, and more freedom to optimize this using caches, etc.

I think this is trivial, but in any case, I changed the wording slightly to account for your input.

[jnm2]

While I recognize that this proposal isn't likely to be a priority, I've been thinking along the same lines for a long time.

[iam3yal]

I like the idea but not thrilled with the syntax. :)

p.s. This seems like it would impact the whole .NET stack and not just the language, CLR, Reflection APIs, etc... unless it's just a compiler check.

[YaakovDavis]

This seems like it would impact the whole .NET stack

It doesn't have to. The main point is to provide extra safety when working at the source level. At the CLR level, it could be represented using the existing visibility modifiers, possibly with some added attributes, to be used by IDEs.
(Attributes will be needed only if we allow visibility contexts to cross assembly boundaries. If this is allowed, when importing a compiled dll with defined visibilities, the compiler will need to examine those attributes.
I'm leaving this as an open question, though IMO the feature is useful enough even if visibility contexts are scoped per project).

Regarding syntax, you're welcome to suggest a version you'd like :)

[SamPruden]

While I like the general idea of having finer control over visibility, this doesn't feel like it achieves that to me.

When I choose a visibility modifier, I'm specifying where I want this member to be accessible from, and I'm specifying this at the declaration site. If any class can use sees to gain access to any visibility, then you've achieved no actual visibility restriction at all. That member is now public, accessible to any other class that chooses to access it. All that really gives you is a way to organise your Intellisense a little by hiding some members unless you specify that you want to be able to see them.

[YaakovDavis]

@TheOtherSamP

If any class can use sees to gain access to any visibility, then you've achieved no actual visibility restriction at all.

By the same reasoning one could argue that private doesn't restrict, since any user can change it to public.
Of course, users can change visibility, breaking safety. Users can add nasty code that hurts the user in all sorts of ways.
The value this feature brings to the table, is a way to communicate intended access, which is enforceable by the compiler.
As a bonus, you'll be able to find references to visibilities, and see if anyone added a sees clause where it doesn't belong. You can't do this with private.

[dcmeglio]

@YaakovDavis

By the same reasoning one could argue that private doesn't restrict, since any user can change it to public. Of course, users can change visibility, breaking safety. Users can add nasty code that hurts the user in all sorts of ways.

There is a big difference. With private the class determines who can see it (only itself). With your proposal, the sees option allows the caller to decide what he or she sees. Meaning think of it this way. You publish Common.dll, it has some nice private methods I'd love to use. I want to change them to public but I can't, I'm a consumer of Common.dll, not the publisher. Oh well, I must find another way. In your proposal, you publish Common.dll. It has some nice methods that are restricted to onlyForYaakovDavissTeam visibility. I say, "well I'm not really on that team, but I want the methods anyway" so I add sees onlyForYaakovDavissTeam and now I have access. You're letting the consumer decide what he/she can see not the publisher. Hence, no real restriction exists. You will see many people doing sees each,and,every,visibility,scope,provided so that they get everything they could possibly want.

My suggestion would be, if you are encountering such boundaries, it might be better to restructure code. Perhaps the way you have your assemblies split up isn't the best design given the restrictions you need to place.

[YaakovDavis]

@dman2306

Notice that this proposal refers to inter-project boundaries; intra-project boundaries cases are already covered by existing modifiers,

The goal is to enable a visibility that is less restrictive than private, but more than internal. Also, we want it to be contextual & semantic.

sees is an explicit clause, which is part of the code, analyzable by standard editor facilities, and is tracked by source control. These combined, make it IMO a reliable facility for intent communication & verification.

Code restructuring is possible only to a limited extent. Sometimes you end up with complicated hierarchies which could be replaced with much simpler structures using this feature.

[SamPruden]

If you want finer grained control over these things, I would suggest perhaps reversing this idea. I still feel that sees ultimately achieves no real safety or value, but perhaps a visibleto modifier analogous to [InternalsVisibleTo] on an intra-project level would make sense.

class Foo
{
    private int Value { get; set; } visibleto FooWrapper
}

class FooWrapper
{
    public int Value => this.foo.Value;

    private readonly Foo foo;

    FooWrapper(Foo foo)
    {
        this.foo = foo;
    }
}

[YaakovDavis]

visibleto

I considered a similar approach when phrasing the proposal, but found a few issues with it:

1. private int X visibleto SomeType.SomeMember is confusing IMO. Is the member private or visibleto X?
   Having an explicit visibility declaration, gives you a name that you can place in the exact same syntactic place where ordinary visibility modifiers appear (ie. treeInternals X instead of internal X). This makes the original proposal more approachable IMO.
2. What about multiple consumers? visibleto X, visibleto Y, visibleto Z is cumbersome. (You may attempt to solve this by writing visibleto X, Y, Z, but this will collide with other postfix clauses like where).
3. What about overloads? visibleto SomeType.SomeMethod doesn't allow you to specify one. (And even if it does, this becomes verbose very fast).
4. You'll lose analysis capabilities, as you have no visibility declaration. You have no specific name to right-click on to find references, etc.

I think we have a different point of view on this. I see the hidden, contextual members as part of the external implementation, injected to providers by the consumer. I.e. the Tree "injects" a treeInternals constructor into the Node class, where only it (the consumer) can call it.
The hidden member is part of a specific protocol/context, and the provider knows nothing in particular about its visibility, or consumers, except being aware that it belongs to that context.

In your approach, the provider knows specifically about its consumers, having to authorize them explicitly.

The issue I see with your model is what we have a mixed responsibility. On the one hand, the only reason the member exists is for usage by the consumer (as it's hidden to the rest of the code). But suddenly the provider gets to decide who uses it? This is confusing.

In my approach, the consumer declares the context, injects appropriate logic to necessary areas, and decides to see that logic where it needs to.

All in all, I agree that visibleto achieves some of the goals of this proposal.

[SamPruden]

@YaakovDavis

Note that in my version I did not include the SomeMember, visibility is provided to the whole type.

If you really wanted to specify the visibility in the same syntactic position it is currently, you could perhaps define some form of scope construct. This would have additional benefits in how it handles things like property accessors, multiple targets, and reuse.

scope FooValueAccessors
{
    FooWrapper,
    OtherFooWrapper
}

class Foo
{
    // I guess this should also implicitly include private access
    FooValueAccessors int Value { get; private set; }
}

Regarding your point about analysis, I don't really see where you're coming from. My VS (it might be R#) already lets me easily view all uses of a member within the project.

To me, your version is just internal with a bit more boilerplate required for classes that consume it.

[YaakovDavis]

@TheOtherSamP

Can't see how your suggested scope construct is any less "boilerplate" than the visibility declaration.

You've reached the same syntax at the "provider" side. The only difference left now, is how to see members.
visibility + sees IMO reads far more intuitively than scope X { A, B, C}.

[SamPruden]

@YaakovDavis

It's different because it adds value. Permission to access a member should be given out by that member, otherwise it's not permission.

Let's say we're working on a team. Person A is responsible for developing a Node class that maintains some state. The node class has a couple of public methods that mutate its state in a controlled manner while ensuring that state remains valid.

Person B is developing a NodeManager class, and they find that they need to be able to load an arbitrary state into a Node. As that Node's manager, they know a little more about what's going on with it, so they can safely make sensible changes to its state.

Under the visibility approach, Person B would go to Person A and say "Hey, can you give me a SetState(State state) method on Node with a visibility so I can grant myself access?" Person A does this, but suddenly now has to worry about what might happen if some other untrusted class decided to mutate its state in an invalid way. This now has all of the same concerns as making it internal would have brought.

Under the scope {} approach, Person A grants access rights to NodeManager, and to no other classes. Person A can now continue trusting that they have control over Node's state, with the small known and clearly labelled caveat that NodeManager can also set it, but that's okay because we trust NodeManager and we know that Person B knows what they're doing.

You mentioned before that you could use the IDE to find references to a visibility and check that nobody has used sees where they shouldn't. This to me is a good representation of why sees is not good. With scope misuse in that manner is not even a possibility. Furthermore with scope, it's explicitly clear when reading the code (just look at the scope declaration) exactly where it's used.

Perhaps I am not understanding your aims here. What do you see the advantages of your approach being vs just making those members internal?

[YaakovDavis]

Permission to access a member should be given out by that member

Under the scope approach you don't name a specific consumer (as you proposed before with visibleto), but a scope. This is identical to naming a visibility.

We now only differ at the way we specify consumers. I propose to annotate individual consumers using sees, where you propose to have a consumer list under the scope declaration.

We should consider what's more intuitive. IMO, once you lose the visibility keyword, the whole thing ceases to make sense.

I'd say the "consumer list" approach will be more readable if phrased like this:

visibility someInternals visible at OwnerX, OwnerY, OwnerZ;

I prefer individual sees clauses at the consumer side, for the following reason:

I don't see the custom visibility modifier strictly as a permission.
The member can be seen as an implementation piece belonging to some external context, which appears in the declaring type for technical reasons. The member holds various states/capabilities belonging to that external context.
When the sees clauses appears at the consumer site, it makes it immediately clear who owns the contextual members, and for what reasons.

[SamPruden]

@YaakovDavis

Under the scope approach you don't name a specific consumer (as you proposed before with visibleto), but a scope. This is identical to naming a visibility.

We now only differ at the way we specify consumers. I propose to annotate individual consumers using sees, where you propose to have a consumer list under the scope declaration.

Not at all. A scope names the consumer, so in naming the scope you're naming the consumer, just with one layer of indirection.

I still really don't understand why you wouldn't just use internal for these members you're referring to, if you're not using visibility for permissions, then what is it for? If you really do have a very large mess of internal members you're trying to organise then that sounds like an architectural problem rather than a language problem.

[MattDillard]

I have no personal experience with PostSharp, but I understand that product introduces a mechanism for restricting visibility to specific classes or by namespace: http://doc.postsharp.net/control-visibility.

That may provide some inspiration.

[jnm2]

Actual syntax would be nice, but at the end of the day, all I want is to be able to do this:

public sealed class SortedList<T> : IList<T>, IReadOnlyList<T>
{
    [VisibleTo(typeof(SortedListExtensions))]
    private readonly List<T> storage;

    // ...
}

public static class SortedListExtensions
{
    public static int GetInsertionIndex<T, TComparable>(
        this SortedList<T> sortedList,
        TComparable comparableValue) where T : IComparable<TComparable>
    {
        // Access sortedList.list directly to do a binary search
    }
}

@gafter @CyrusNajmabadi is this primarily a language ask or a corefx ask?

[epitka]

After reading the comment to my proposal #732, that referenced this issue, it seems that what you are asking for is similar to what I asked. Please take a look at #732 to see more use cases for this functionality.