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

[Proposal]: Property-Scoped Fields #8633

Open
CyrusNajmabadi opened this issue Nov 19, 2024 · 0 comments
Open

[Proposal]: Property-Scoped Fields #8633

CyrusNajmabadi opened this issue Nov 19, 2024 · 0 comments
Assignees
Milestone

Comments

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Nov 19, 2024

Property-Scoped Fields

(Ported from dotnet/roslyn#850)

Discuss this issue at: #8632

Summary

Allow fields to be scoped within the accessor body of a property or event declaration.

Motivation

Properties often encapsulate constraints on field access or ensure that certain behaviors are invoked. Sometimes it is appropriate for all field access to be performed through the property, including those from within the same class.

These constraints may be unclear or forgotten, leading to subtle bugs during maintenance which are often difficult to diagnose.

Scoping fields to the body of their property/event would allow the designer to more effectively communicate this intent to encapsulate.

Detailed design

Property-scoped fields may be defined above the accessor list within the body of a property or event:

public string MyProperty
{
    string myField;

    get { return myField; }
    set
    {
        myField = value;
        NotifyOfPropertyChange(nameof(MyProperty));
    }
}
public event EventHandler MyHandler
{
    EventHandler handler;

    add
    {
        handler += value;
        Logger.Log("Added handler");
    } 
    remove
    {
        handler -= value;
        Logger.Log("Removed handler");
    }
}

myField and handler are encapsulated within their respective property/event. In each case, both accessors may directly reference the fields. Nothing outside of the property/event scope has direct access to those fields.

Implementation comments from @CyrusNajmabadi:

First, this is just state that's available to this single property that the value is scoped to. In that regard, it's similar to a 'local'. Just a 'local' that is available to all the accessors. As such, this is how i've been designing things from teh language level:

  1. You can have any number of property locals declared with the property.
  2. The property locals can come before/after/interspersed with the actual property accessors.
  3. The property locals are scoped to that property alone. Other members of the class/struct cannot interact with them.
  4. Subclasses can't interact with them (including overrides of the property).
  5. 'var' can be used for these property locals.
  6. Indexers should also be able to use these sorts of property locals.

Now (while this is out of the scope of the actual language change), here's how i would expect to emit them.

  1. These would simply be fields within the same class/struct that the property/indexer is contained within.
  2. The fields would be private.
  3. The fields would be named in such a way that they could never collide with an actual class member. likely something wonky <>_propname_localname

Alternatives

Automatic backing field

This implementation would be an extension of automatic properties. The backing field could be accessed through a keyword field, but only within the scope of the property/event:

public string MyProperty
{
    get { return field; }
    set
    {
        field = value;
        NotifyOfPropertyChange(nameof(MyProperty));
    }
}

This would not allow multiple fields to be encapsulated in the same property.

dotnet/roslyn#7614 has a similar proposal with a different keyword ($state).

Auto-property syntax for custom setter/getter logic

dotnet/roslyn#1551

Semi-auto-properties with setters

dotnet/roslyn#8364

Class-scoped blocks for fields / explicit field usage

dotnet/roslyn#12361

Unresolved questions

A list of questions from @CyrusNajmabadi:

  1. Allow attributes on these locals? It wouldn't be hard to support, and there could be value in putting attributes on the final fields emitted. This does leak through though the implementation of how we deal with property locals.

2. Allow these for events? Yes

  1. How are we going to handle the syntax changes here in our API. It's non trivial to modify the syntactic constructs here in a non-breaking fashion.
  2. We've added 'local functions' to C#7. Should we allow "property local functions" as well?
  3. Should we allow other 'local' constructs? We could have local methods/types/etc. within a property. Technically it would all be possible.

Syntax

It was noted that adding fields above accessors or below accessors (but not interspersed) could be an easier change to the existing compiler design. @lachbaer provided a potential design which would not be a breaking change to the API:

<!-- this Node hase been added, it looks exactly like "FieldDeclarationSyntax" -->
<Node Name="PropertyLocalFieldDeclarationSyntax" Base="BaseFieldDeclarationSyntax">
    <Kind Name="PropertyLocalFieldDeclaration"/>
    <Field Name="AttributeLists" Type="SyntaxList&lt;AttributeListSyntax&gt;" Override="true">
      <PropertyComment>
        <summary>Gets the attribute declaration list.</summary>
      </PropertyComment>
    </Field>
    <Field Name="Modifiers" Type="SyntaxList&lt;SyntaxToken&gt;" Override="true">
      <PropertyComment>
        <summary>Gets the modifier list.</summary>
      </PropertyComment>
    </Field>
    <Field Name="Declaration" Type="VariableDeclarationSyntax" Override="true"/>
    <Field Name="SemicolonToken" Type="SyntaxToken" Override="true">
      <Kind Name="SemicolonToken"/>
    </Field>
  </Node>

  <Node Name="AccessorListSyntax" Base="CSharpSyntaxNode">
    <Kind Name="AccessorList"/>
    <Field Name="OpenBraceToken" Type="SyntaxToken">
      <Kind Name="OpenBraceToken"/>
    </Field>
    <!-- next line has been added -->
    <Field Name="LocalFields" Type="SyntaxList&lt;PropertyLocalFieldDeclarationSyntax&gt;" Optional="true" />
    <Field Name="Accessors" Type="SyntaxList&lt;AccessorDeclarationSyntax&gt;"/>
    <Field Name="CloseBraceToken" Type="SyntaxToken">
      <Kind Name="CloseBraceToken"/>
    </Field>
  </Node>

Field name collisions

Should a property-scoped field be allowed to have the same name as another class member?

Should local identifiers within accessors be allowed to have the same name as a property-scoped field?

Design Meetings

@CyrusNajmabadi CyrusNajmabadi self-assigned this Nov 19, 2024
@dotnet dotnet locked and limited conversation to collaborators Nov 19, 2024
@CyrusNajmabadi CyrusNajmabadi added this to the Backlog milestone Nov 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant