Skip to content

Best friends to limit friend access #1519

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

Closed
TomFinley opened this issue Nov 3, 2018 · 0 comments
Closed

Best friends to limit friend access #1519

TomFinley opened this issue Nov 3, 2018 · 0 comments
Assignees
Labels
API Issues pertaining the friendly API

Comments

@TomFinley
Copy link
Contributor

TomFinley commented Nov 3, 2018

While we stabilize the API, it is best if we keep hidden as much as possible, since once v1.0 the barrier to changing the public types on a publicly released library will be high. For that reason the thinking is to keep the scope of what we publicly release as limited as possible. If we suppose that usage of the library vs. component authorship inside the library, one obvious way to reduce the surface area is to ideally hide (or, in the worst case, render useless) all the items related to component authorship.

The problem is, over the years we have accumulated have a fairly huge library, so there are a tremendous number of types, and a fair amount of infrastructure. We cannot make that infrastructure internal simply and put everything in one big assembly, because some components or their dependencies are quite large, so we want to keep them in separate nugets, which means separate assemblies. But, that means public classes.

We could make the core infrastructure assemblies friends of all leaf assemblies via the InternalsVisibleTo attribute, but that would render all internal classes usable from the leaf assemblies, which again is not what we want from a code maintainability perspective.

We really want to have a mechanism that distinguishes internal to the assembly, and internal to the library. There is no concept in .NET, and based on conversations with the .NET folks there seems to be little belief such a thing might be useful in any other situation, so we'll roll our own.

The best idea we have settled on so far is that we set things to be friends as described above, make things internal, but for these internal items we want to share, make sure that there is an attribute (provisionally called BestFriend) on the item, and if not, the code analyzer we have internal to ML.NET will complain.

So, e.g.:

internal sealed class Foo { }

[BestFriend]
internal sealed class Bar
{
    [BestFriend]
    internal int A { get; }
    internal int B { get; }
}

public abstract class Biz
{
    [BestFriend]
    private internal Biz() { }
}

In this scenario, if we consider some hypothetical other project code trying to access this, access to Foo and Bar.B would be restricted (despite being friends!), but access to Bar, Bar.A, and a class inheriting from Biz would be possible, but only from our assemblies.

Note here, Biz itself is still public since we very often have user-accessible classes descend from some class we would like to be "internal," but kind of can't, at least, not without introducing a significant absurd amount of wrapping. This level of "visible but unusable" though we should try to keep to a minimum.

You could imagine this attribute being an opt-out rather than opt-in, but opt-in seems a bit more reasonable since we want to treat these cross-assembly sharings as being somehow special, and it would be nice if in PRs it becomes obvious when there's something like this going on.

The work then is:

  1. Introduce two attributes, one this BestFriend attribute, the other an assembly level attribute whereby an assembly can indicate it wants to be subject to these best-friend controls. (Some assemblies like CpuMath that are purely intended for component authors would perhaps share everything without the need for this attribute.)

  2. Write an analyzer that complains when there is a cross-assembly friend access, when the item does not contain this attribute.

Obviously one of these is slightly harder than the other. 😄 Note that this work merely covers the creation of the infrastructure, not the use of the infrastructure. That later work will be considerably more involved and spread across many files, and so will certainly be structured across many PRs.

/cc @danmosemsft @eerhardt @Zruty0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API Issues pertaining the friendly API
Projects
None yet
Development

No branches or pull requests

1 participant