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

File-local types #6011

Closed
Tracked by #829 ...
RikkiGibson opened this issue Apr 13, 2022 · 19 comments
Closed
Tracked by #829 ...

File-local types #6011

RikkiGibson opened this issue Apr 13, 2022 · 19 comments
Labels
Proposal Question Question to be discussed in LDM related to a proposal
Milestone

Comments

@RikkiGibson
Copy link
Contributor

This is the next turn of the crank on #5529.

Permit a file modifier on top-level type declarations. The type only exists in the file where it is declared.

// File1.cs
namespace NS;

file class Widget
{
}

// File2.cs
namespace NS;

file class Widget // different symbol than the Widget in File1
{
}

// File3.cs
using NS;

var widget = new Widget(); // error: The type or namespace name 'Widget' could not be found.

Benefits

Removes the need for users to reason out what combinations of access modifiers are permitted with the file modifier on non-type members. Leaves the design space open for non-type file-scoped members to come along later.

Drawbacks

It may be inconvenient to be unable to declare file-scoped methods, properties, etc., even those with private or internal accessibility. The feature may not be viable for some use cases if it can only be adopted at the type level.

Accessibility

No accessibility modifiers can be used in combination with file on a type.

public file class C1 { } // error
internal file class C2 { } // error
file class C3 { } // ok

The implementation guarantees that file types in different files with the same name will be distinct to the runtime. The type's accessibility and name in metadata is implementation-defined. The intention is to permit the compiler to adopt any future access-limitation features in the runtime which are suited to the feature. It's expected that in the initial implementation, an internal accessibility would be used and the name would be mangled based on which file the type is declared in.

Usage in signatures

There is a general need to prevent file types from appearing in member signatures where the file type might not be in scope at the point of usage of the member.

Only allow in members of file types

Perhaps the simplest way to ensure this is to enforce that file types can only appear in signatures or as base types of other file types:

file class FileBase
{
}

public class Derived : FileBase // error
{
    private FileBase M2() => new FileBase() // error
}

file class FileDerived : FileBase // ok
{
    private FileBase M2() => new FileBase() // ok
}

Alternative: allow in members which are only accessible in this file

We may find the above rule is too limiting. In that case, it's also worth considering a rule to instead say that a file scoped type can only appear in a member signature, if that member is only accessible within the same file.

file class Widget
{
}

public class Program
{
    private void M1(Widget w) { } // ok
    internal void M2(Widget w) { } // error
}

partial class Utils
{
    private void M1(Widget w) { } // error
}

Implementation/overrides

file scoped type declarations can implement interfaces, override virtual methods, etc. just like regular type declarations.

file struct Widget : IEquatable<Widget>
{
    public bool Equals(Widget other) => true;
}
@RikkiGibson RikkiGibson added the Proposal Question Question to be discussed in LDM related to a proposal label Apr 13, 2022
@RikkiGibson
Copy link
Contributor Author

One important follow-up question is: what happens when a file type shadows an existing type with the same name.

A major point of the feature is about avoiding name conflicts. In that spirit, it seems like it should be allowed for a file type to shadow a non-file type in a different file.

// File1.cs
namespace NS;
class Widget { }

// File2.cs
namespace NS;
file class Widget { }  // ok. separate from the symbol in File1
// File1.cs
namespace NS;
class Widget { }
file class Widget { }  // error: a type with the same name is already declared in this file.

@DaZombieKiller
Copy link
Contributor

Would it make sense to extend this to members as a general accessibility type? A use-case I can think of is in types such as ZipArchiveEntry/ZipArchive where you have a C++ friend class-like relationship. Only ZipArchive is supposed to call the ZipArchiveEntry constructor, so it is marked internal as a best-effort approach to ensure that.

However, that internal constructor could still be called from elsewhere in the code. If the constructor could be file-scoped, it would be able to look more like:

ZipArchiveEntry.cs:

namespace System.IO.Compression;

public partial class ZipArchiveEntry
{
    // ...
}

ZipArchive.cs:

namespace System.IO.Compression;

public partial class ZipArchiveEntry
{
    file ZipArchiveEntry(ZipArchive archive, ZipCentralDirectoryFileHeader cd)
    {
        // ...
    }
}

public class ZipArchive : IDisposable
{
    // ...
}

@RikkiGibson
Copy link
Contributor Author

RikkiGibson commented Apr 18, 2022

I think we would like to grow it into applying to members eventually. Here are some complications we found:

  • Should file by itself mean something like file internal (usable anywhere in the file), or something like file private (usable in the same class in the same file)?
  • Should we allow explicit access modifiers in combination with file? Which ones should we allow, what do they mean, and how are they encoded? This is what was explored in the rejected proposal File accessibility modifier #5969.

@RikkiGibson RikkiGibson changed the title File-scoped types File types Apr 18, 2022
@RikkiGibson
Copy link
Contributor Author

Renaming to "file types" because we felt "file-scoped types" sounded too much like "file-scoped namespaces".

@RikkiGibson
Copy link
Contributor Author

We will probably rename again to "file-local" types after internal discussion, since we felt "file types" was not specific enough 😄

@alrz
Copy link
Contributor

alrz commented Jul 2, 2022

Doing a lot of work to not say "fileprivate access modifier"? Every time I hear "file types" it sounds like something about IO and I know it isn't. 😅

@RikkiGibson RikkiGibson changed the title File types File-local types Jul 7, 2022
@RikkiGibson
Copy link
Contributor Author

Doing a lot of work to not say "fileprivate access modifier"?

Yes, we are 😄. We think this feature is doing stuff that goes beyond just the concept of accessibility, particularly when it comes to shadowing and name conflicts.

I see that there are several 👎 on the issue. It would be great to know any specific concerns you have. Is it the terminology in use? or maybe the limitations of the feature as specified here? or something else? Thanks.

@alrz
Copy link
Contributor

alrz commented Jul 7, 2022

We think this feature is doing stuff that goes beyond just the concept of accessibility

IMO that's an implementation detail. private on a base class allows shadowing too، doesn't make it beyond a accessibility modifier.

Regardless, I couldn't figure WHY it's limited to types? And is it intentional that there's no example of "file partial class"? (I'd imagine that's a base usage scenario)

All in all, I think it would be actually a lot simpler to look at this as just another accessibility modifier.

@333fred
Copy link
Member

333fred commented Jul 7, 2022

Regardless, I couldn't figure WHY it's limited to types?

So, this proposal started out being named file private types, and led us down the rabbit hole of what does the private mean here, and what would it mean when applied to type members. After several design sessions, we ended up sketching out a world where file can be applied to any member with any accessibility, and it would be the intersection of file local and whatever that accessibility was. There are lots of interesting questions in there though, such as: can a file public member implement an interface member? We decided to scale it back to just file, effectively meaning file internal, on top-level types only, to protect this design space. Particularly as that covers the original ask of the feature, which is source generator support.

@alrz
Copy link
Contributor

alrz commented Jul 7, 2022

If you consider "file" something less accessible than "private" (or "internal" for top-level types) things will get a lot simpler, with NO combinations allowed.

As a user, I wouldn't like to process that many of accessibility levels inside a file.

@WeihanLi
Copy link
Contributor

Still not supported with .NET 7 RC 1?

@ufcpp
Copy link

ufcpp commented Sep 15, 2022

Already supported with .NET 7 Preview 7.

@WeihanLi
Copy link
Contributor

@ufcpp thanks, I build success with the dotnet cli while got an error with VS, and I'm using the latest vs preview(17.4.0 Preview 2.0)
image

@ufcpp
Copy link

ufcpp commented Sep 15, 2022

image

@333fred
Copy link
Member

333fred commented Sep 15, 2022

@WeihanLi are you using ReSharper? That doesn't look like a roslyn error.

@WeihanLi
Copy link
Contributor

are you using ReSharper? That doesn't look like a roslyn error.

@333fred yeah, I'm using ReSharper, it does work when I disable the ReSharper, many thanks

@jcouv jcouv added this to the 11.0 milestone Sep 26, 2022
@jods4
Copy link

jods4 commented Oct 4, 2022

It should be pointed out that the lowering strategy entails surprises for users.

A unique class name is generated, but there are cases where class names matter and an unwary developer would expect file class User to be named User...

// This is gonna end up being named <>_generated_Setting at runtime
file class Setting { }

internal static Setting settings;

public void PersistSettings()
{
  using var db = new DataContext();
  db.Update(settings);
  // Expected SQL, table name is inferred from class name:
  // UPDATE [Setting] SET ...
}

There are work-arounds, for example use case-specific attributes: [Table("Setting")]; or not using file when the name matters, although this might be conflicting with the main use-case of using file when writing code generators.

Maybe a name-preserving strategy would be less surprising, e.g. putting the class in another namespace or class?

// In file Models/User.cs
namespace A 
{
  file class User { }
}

// Generated namespace at build
namespace A.<File>Models_User_cs
{
  internal class User { }
}

// Generated class at build
namespace A
{
  internal static class <File>Models_User_cs
  {
    internal class User { }
  }
}

As an added bonus, the class name would look better in debugger.

These are not perfect either, as they may break reflection code that scans specific namespaces, or that doesn't look at nested classes. Maybe those might be less frequent than relying on class name?

@RikkiGibson
Copy link
Contributor Author

A few thoughts as I have observed feedback and discussion of this feature upon its adoption.

  1. I have seen across the board that users consider file to be an accessibility concept and not a "scoping/visibility" concept as we have currently defined it. When we look at generalizing the file concept to more member kinds, we should also look again at considering file to be a new accessibility modifier rather than being a scoping concept per se. Part of our intention in building out a limited version of this feature to start with was to give ourselves the opportunity to delay making this decision until we understood the problem better and give users an opportunity to provide feedback.
    • Another motivator for the currently shipped design is that we weren't sure whether we wanted "file local" member declarations to be able to implement interface members, override virtual methods with various accessibilities, etc. Hopefully, upon re-examination of the problem space in the future, we'll be able to come to a conclusion on this.
  2. Agree that it would be an improvement in the experience for the member's name itself to somehow match the name used in source from contexts like reflection, etc. This will be a problem for members such as methods which are contained in types and not namespaces, since we won't be able to conceal the file name mangling through a wrapper namespace.

@333fred 333fred added the Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification label Jan 9, 2023
@333fred
Copy link
Member

333fred commented Jan 9, 2023

Closing this out as the main proposal is #5529.

@333fred 333fred closed this as completed Jan 9, 2023
@333fred 333fred removed the Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification label Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Proposal Question Question to be discussed in LDM related to a proposal
Projects
None yet
Development

No branches or pull requests

8 participants