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]: Partial properties #6420

Open
1 of 4 tasks
RikkiGibson opened this issue Aug 30, 2022 · 78 comments
Open
1 of 4 tasks

[Proposal]: Partial properties #6420

RikkiGibson opened this issue Aug 30, 2022 · 78 comments
Assignees
Milestone

Comments

@RikkiGibson
Copy link
Contributor

RikkiGibson commented Aug 30, 2022

Partial properties

Summary

Allow the partial modifier on properties to separate declaration and implementation parts, similar to partial methods.

// UserCode.cs
public partial class ViewModel : INotifyPropertyChanged
{
    public event PropertyChangedEventHandler PropertyChanged;

    [NotifyPropertyChanged]
    public partial string UserName { get; set; }
}

// Generated.cs
public partial class ViewModel
{
    private string __generated_userName;

    public partial string UserName
    {
        get => __generated_userName;
        set
        {
            if (value != __generated_userName)
            {
                __generated_userName = value;
                PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(UserName)));
            }
        }
    }
}

Motivation

When we did extended partial methods, we indicated we would like to consider adding support for other kinds of partial members in the future. The community has shown enthusiasm for partial properties in particular.

.NET has a number of scenarios where a property implementation is some kind of boilerplate. One of the most prominent cases is INotifyPropertyChanged, as seen above. Another is dependency properties. There are currently production source generators designed to handle these scenarios. These currently work by having the user write a field and having the generator add the corresponding property.

// UserCode.cs
public partial class ViewModel : INotifyPropertyChanged
{
    public event PropertyChangedEventHandler PropertyChanged;

    [NotifyPropertyChanged]
    private string _userName;
}

// Generated.cs
public partial class ViewModel
{
    public string UserName
    {
        get => /* ... */;
        set
        {
            // ...
        }
    }
}

Under this scheme, users have to become familiar with the conventions for how the generator creates properties based on their fields. Additional workarounds are needed for users to be able to change accessibility, virtual-ness, attributes, or other aspects of the generated property. Also, using features like find-all-references requires navigating to the generated property, instead of being able to just look at the declarations in user code. All of these issues are solved fairly naturally by adding partial properties to the language.

Detailed design

Detailed design has been moved to partial-properties.md.

Design meetings

https://github.com/dotnet/csharplang/blob/main/meetings/2022/LDM-2022-08-31.md#partial-properties
https://github.com/dotnet/csharplang/blob/main/meetings/2022/LDM-2022-11-02.md#partial-properties

@canton7
Copy link

canton7 commented Aug 30, 2022

Apologies if this is considered spam, but I just want to say thank you for working on this, and as an INPC SG author and strong proponent of partial properties, I agree with everything in your proposal 👍

@HaloFour
Copy link
Contributor

HaloFour commented Aug 30, 2022

I still don't understand the distaste the team has expressed in supporting source generators in AOP scenarios properly. This proposal (and the others) feel a lot less intuitive and are significantly less capable. For example, it remains impossible to have custom logic within the accessor method while applying an aspect, and it remains impossible to apply multiple aspects unless they are all supported by the same source generator.

You either have a big cliff at which point you simply can't use source generators at all, or you need to find the one Swiss Army Source Generator to Rule Them All that lets you do everything you could possibly need to ever do in a property accessor, which likely includes twisting the code in awkward ways that would be less intuitive than AOP would be anyway.

@alrz
Copy link
Contributor

alrz commented Aug 31, 2022

The property declarations and their accessor declarations must have the same modifiers

One thing I like about partial classes is that you can add a part just by a partial class declaration no matter what the base is or what modifiers are applied (but if you do include them, they have to match).

I think the same could be applied to partial members so that generators won't need to bother duplicating every detail.

@Sergio0694
Copy link

This looks awesome! 😄

"I think the same could be applied to partial members so that generators won't need to bother duplicating every detail."

Had the same thought - it'd be nice if the partial implementation could just use get/set/init without having to repeat the accessibility modifier. Not a big deal though, even if it was required, not an issue for a generator to be more verbose there.

The only remaining concern/scenario that comes to mind to me is: attributes on fields. That is, since releasing the MVVM Toolkit 8.0 we've had tons of requests about being able to customize how attributes can be added to fields and generated properties. It would be nice if people had some ability to annotate a partial property with attributes that are meant for the backing fields too, and the partial implementation would be given field (which would always be present in that case) with those attributes:

[field: SomeFieldAttribute]
public partial string Name { get; set; }

This way the generator could do eg.:

public partial string Name
{
    get => field;
    set => SetProperty(ref field, value);
}

This would be really important as it'd give users more flexibility and avoid "falling off a cliff" where the second you need a field attribute you're forced to give up generated properties entirely, and going back to manually doing everything. This feature has been requested several times and it's one of the main pain points we've seen for users of our INPC source generator.

@HaloFour
Copy link
Contributor

@Sergio0694

This would be really important as it'd give users more flexibility and avoid "falling off a cliff" where the second you need a field attribute you're forced to give up generated properties entirely, and going back to manually doing everything.

I guess the alternative there would be that the source generator would be expected to detect the field-targeted attributes on the partial property and to copy them correctly to the backing field declarations?

@Sergio0694
Copy link

Yeah that'd also be possible, just generators would have to manually copy all attributes (including arguments), because Roslyn would otherwise just ignore them. There's also the issue of the diagnostic being emitted ("attribute target is invalid here and will be ignored"), but I guess Roslyn could automatically suppress it if you're annotating a partial property 🤔

@333fred
Copy link
Member

333fred commented Aug 31, 2022

but I guess Roslyn could automatically suppress it if you're annotating a partial property 🤔

It wouldn't, but you could.

@michael-hawker
Copy link

@Sergio0694

This would be really important as it'd give users more flexibility and avoid "falling off a cliff" where the second you need a field attribute you're forced to give up generated properties entirely, and going back to manually doing everything.

I guess the alternative there would be that the source generator would be expected to detect the field-targeted attributes on the partial property and to copy them correctly to the backing field declarations?

I mean some attributes can target both fields and properties, so it'd be hard to separate the intent unless there was a way to specify which target the developer intends for the attribute?

@Sergio0694
Copy link

Sergio0694 commented Aug 31, 2022

"it'd be hard to separate the intent unless there was a way to specify which target the developer intends for the attribute?"

That's why I'm saying that developers would use explicit attribute targets in this context 🙂

[Foo] // Goes on the property
[property: Foo] // Redundant, but you can also explicitly target the property if you want
[field: Foo] // Same attribute, but this goes on the field
public partial string Name { get; set; }

@HaloFour
Copy link
Contributor

@michael-hawker

I mean some attributes can target both fields and properties, so it'd be hard to separate the intent unless there was a way to specify which target the developer intends for the attribute?

Right now the attribute targets the property unless explicitly targeted to the field via [field: FooAttribute].

@RikkiGibson
Copy link
Contributor Author

The property declarations and their accessor declarations must have the same modifiers

One thing I like about partial classes is that you can add a part just by a partial class declaration no matter what the base is or what modifiers are applied (but if you do include them, they have to match).

I think the same could be applied to partial members so that generators won't need to bother duplicating every detail.

I'm finding out things I never thought to try with partial classes.. it seems like at least in some cases modifiers are essentially concatenated. SharpLab

// produces `public static class C` in metadata
public partial class C {
    public static void M() {
    }
}

static partial class C { }

It does seem reasonable that users shouldn't have to repeat things which are already known about the member. It also seems like source generators in practice just call UserDeclaration.Modifiers.ToString() inside their template and it tends to go pretty smoothly, though.

The relaxation on modifiers I'd like to see considered categorically for both properties and methods, similar to the relaxation on partial modifier ordering.

@FaustVX
Copy link

FaustVX commented Aug 31, 2022

@RikkiGibson
but it doesn't works for every modifier,
I tried with abstract, but that doesn't work

public partial class C {
    public abstract void M() { // <- error CS0500: 'C.M()' cannot declare a body because it is marked abstract
    }
}

abstract partial class C { }

sharplab.io

@mrpmorris
Copy link

Please consider allowing us to not specify public/virtual/override .

public partial virtual string Name { get; protected set; }

Should work with a partial like this

partial string Name;

Or even

partial object Name; // No need to match type

@CyrusNajmabadi
Copy link
Member

partial object Name; // No need to match type

Which type should we use in that case?

@alrz
Copy link
Contributor

alrz commented Aug 31, 2022

@RikkiGibson

It also seems like source generators in practice just call UserDeclaration.Modifiers.ToString() inside their template and it tends to go pretty smoothly, though.

For properties it's three places to do this.. and it really doesn't add much besides making the compiler happy.

it seems like at least in some cases modifiers are essentially concatenated

Same for ref/readonly on structs - makes me wonder if private and protected should do that. /s

@RikkiGibson
Copy link
Contributor Author

@RikkiGibson but it doesn't works for every modifier, I tried with abstract, but that doesn't work

public partial class C {
    public abstract void M() { // <- error CS0500: 'C.M()' cannot declare a body because it is marked abstract
    }
}

abstract partial class C { }

sharplab.io

I think the method here is behaving as expected for an abstract class. If you use a semicolon body, then it compiles. SharpLab.

@FaustVX
Copy link

FaustVX commented Sep 1, 2022

@RikkiGibson haha Ok 😄

@HaloFour
Copy link
Contributor

HaloFour commented Sep 1, 2022

IMO the signature of the declaration and the implementation should be required to match exactly. The declaration should be the source of truth, given that it is likely what the developer has written manually and what they expect to be the public surface. I think the onus should be on the source generator to emit the matching signature in order to ensure that the source generator is doing what the developer expects them to be doing. A difference in what the source generator emits should result in an error for the sake of sanity checking. If that poses difficult for the source generator I would suggest that the APIs of source generators be improved to make it easier, rather than changing the syntax of the language to make it easier for a mistake to slip through unexpectedly.

@alrz
Copy link
Contributor

alrz commented Sep 1, 2022

While there's no restriction for types as mentioned (some other part could decide on static, for example), for member this could be simply not the case. Meaning, you either have to repeat all modifiers or none at all. That will make it impossible to have unexpected results by making sure nothing about the member can change out of sight.

Further this can only be allowed on the "partial implementation" rather than both ends (implementation and declaration).

@rcbellamy
Copy link

Having written a source generator for my project, in my opinion requiring the source generator to mention access modifiers, etc., is a very big deal. That information is simply never of any relevance to the work that my source generator is created to accomplish (it writes logic, the human-written part defines who can access that logic--simple, right?). It seems like the developers at Microsoft can deal with not requiring the implementation to mention the modifiers once, and save everyone the headache; or they can not bother, and then anyone that ever writes a source generator has to add a significant amount of complexity to their source generator for which many source generator authors will see absolutely no added value. Having Microsoft do it once for everyone seems like a no-brainer.

As for the notion that the solution is to improve the API, I agree that improving the API's documentation would be a very valuable idea. It wouldn't change the part where this requires a significant amount of complexity added to the source generator which many source generators might not benefit from at all. Plus, let's be realistic. It is far easier for Microsoft's developers to make it so that the source generators are not required to specify the modifiers than it is for them to find people that can adequately improve the documentation.

On a side note, @RikkiGibson , could you please provide a quick documentation link regarding UserDeclaration.Modifiers.ToString()? When I google it, this discussion is the only result.

@CyrusNajmabadi
Copy link
Member

and then anyone that ever writes a source generator has to add a significant amount of complexity to their source generator

I don't see there being significant complexity. You can legitimately just reuse the exact same Modifiers property from the original declaration when making your declaration.

@CyrusNajmabadi
Copy link
Member

could you please provide a quick documentation link regarding

https://docs.microsoft.com/en-us/dotnet/api/microsoft.codeanalysis.syntaxnode.tostring?view=roslyn-dotnet-4.3.0

ToString on any node returns the original text (no interpretation or modification) that created it.
ToFullString is the same, except with the leading/trailing trivia around that node as well.

Roslyn does not have abstract syntax trees for our syntax model. We have concrete syntax trees. So they contain every last character used to create them in the original text (no more and no less). ToString/ToFullString just give you those characters back.

So, in teh context of this discussion, producing the modifiers is trivial. You just take the modifiers from the thing you have and pass that node along directly to the tree you're creating (one step). Or, if you're producing text, you just ToString the modifiers and append those (also one step). In both cases it's extremely simple on hte generator side.

@sab39
Copy link

sab39 commented Sep 12, 2022

One consideration - I don't think source generators are can get this granularity of laziness with the current API, but if the modifiers don't have to be repeated, it might eventually be possible to not rerun the source generator at all if only the modifiers are changed. This might have a significant impact if there's a refactoring operation that changes a lot of modifiers at once.

@CyrusNajmabadi
Copy link
Member

This might have a significant impact if there's a refactoring operation that changes a lot of modifiers at once.

The right way to think about incremental-perf is to consider how the generation works on the common editing cases, not hte outliers. In practice, people are editing code bodies most of hte time, and occasionally adding/removing/modifying signatures. The latter is not the common operation, and even if there was "a refactoring operation that changes a lot of modifiers at once", it would be a rare one off that would be greatly subsumed by the normal editing cases which would create your amortized cost.

--

A good analogy here is thinking about how hashtables work. Sure, you might rarely get an O(n) operation when the table needs to resize things. But the vast majority of ops lead to an amortized O(1) cost for the overall structure.

@rcbellamy

This comment was marked as abuse.

@HaloFour
Copy link
Contributor

HaloFour commented Sep 14, 2022

@rcbellamy

It is not trivial until after you learn how to do it.

This applies to literally everything. That's not a reason to affect the language itself. It's infinitely easier to produce documentation with tutorials and helper API. An author of a source generator is not expected to be a beginner with the language or Roslyn framework and IMO it's fully reasonable to put the onus on that developer to emit correct code.

Your statements are also rude, arrogant, and offensive.

You not liking the statement doesn't make it offensive.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Sep 14, 2022

but does not help clarify if UserDeclaration is a class or a member, so I await the link that I requested from

"UserDeclaration" here refers to the property declaration syntax for the partial property.

In this case, it would be an instance of: https://docs.microsoft.com/en-us/dotnet/api/microsoft.codeanalysis.csharp.syntax.propertydeclarationsyntax?view=roslyn-dotnet-4.3.0

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Sep 14, 2022

As for your statement that "producing the modifiers is trivial[,]" you ignore the catch-22. It is not trivial until after you learn how to do it.

Yes. It will be necessary to learn source generators in order to use then effectively. But in this case, the learning is pretty simple and easy. If the modifiers need to be the same, then generating that is actually really nice with Roslyn due to it concrete syntax tree model. Specifically, because they need to be the same, you can accomplish this just by literally reusing the same syntax pieces from the declaration.

Note that you'd have to do this anyways given things like return type, name and whatnot. So it's just as simple to reuse the modifiers as well :-)

That means that there is a significant reduction in the learning curve to be accomplished by removing the complexity,

I recommend using tools like sharplab or the syntax visualizer. They will greatly reduce the learning curve. "Roslyn quoter" is also very useful.

Finally, if you are still facing difficulties, we have a vibrant and helpful community over at discord.gg/csharp that would be happy to help you. Many if the people here are routinely there, along with hundreds of other passionate developers.

@CyrusNajmabadi
Copy link
Member

I had no interest in ever writing one source generator, but I needed it in order to correct deficiencies in Microsoft's work in other aspects of .Net.

Glad to hear that source generators may help you out. Let us know about that experience and how we can make it better. Note that that feedback is best sent to dotnet/Roslyn as that's a compiler/tooling feature. Whereas dotnet/csharplang is specifically for the language design of things. Thanks!

@mrpmorris
Copy link

I vote for skipping this proposal in favour of allowing Interceptors for getters and setters.

Using interceptors will allow adding one Attribute to a class and intercept all properties without further changes. This is what the SwiftUI people are doing with the new macro based approach.

Sticking with the INPC example this means adding for example [Observable] to a class/struct/record would be enough for having all properties setters fire the relevant event.

How would that let me add attributes to a property from a code-generator?

@b-straub
Copy link

How would that let me add attributes to a property from a code-generator?

What would be the use case for adding attributes to a partial property from a SG? Usually a SG is a consumer for attributes.

@HavenDV
Copy link

HavenDV commented Aug 24, 2023

How would that let me add attributes to a property from a code-generator?

What would be the use case for adding attributes to a partial property from a SG? Usually a SG is a consumer for attributes.

Using the DependencyPropertyGenerator example, there are many attributes that the user can additionally define (for example, System.ComponentModel.Category and System.ComponentModel.Description). Now there are separate properties for this inside the generator attribute

@burtonrodman
Copy link

while property interceptors may be a good feature, i agree that they don’t have nearly the flexibility that partial properties would bring. especially given that they act on the call site, i don’t see them covering certain scenarios that may require internal access to the object that a partial property could provide.

@b-straub
Copy link

How would that let me add attributes to a property from a code-generator?

What would be the use case for adding attributes to a partial property from a SG? Usually a SG is a consumer for attributes.

Using the DependencyPropertyGenerator example, there are many attributes that the user can additionally define (for example, System.ComponentModel.Category and System.ComponentModel.Description). Now there are separate properties for this inside the generator attribute

I can't see the difference for your example.

  • with partial properties

code

[global::System.ComponentModel.Category("Category")]
[global::System.ComponentModel.Description("Description")]
public partial bool IsSpinning { get; set; }

SG

public partial bool IsSpinning
{
   get => (bool)GetValue(IsSpinningProperty);
   set => SetValue(IsSpinningProperty, value);
}
  • with interceptor (assume properties will be supported)

code

[global::System.ComponentModel.Category("Category")]
[global::System.ComponentModel.Description("Description")]
public bool IsSpinning { get; set; }

SG

[InterceptsLocation("Program.cs", line: /*L1*/, character: /*C1*/)] // refers to -> public bool IsSpinning { get; set; }
public static bool IsSpinning
{
   get => (bool)GetValue(IsSpinningProperty);
   set => SetValue(IsSpinningProperty, value);
}

@KalleOlaviNiemitalo
Copy link

KalleOlaviNiemitalo commented Aug 27, 2023

Attributes from partial properties would still be useful for code generators that are not Roslyn source generators and perhaps do not read C# source files at all, thus won't know the file names and line numbers to intercept.

@b-straub
Copy link

Attributes from partial properties would still be useful for code generators that are not Roslyn source generators and perhaps do not read C# source files at all, thus won't know the file names and line numbers to intercept.

Not my point. I'm not opposing against partial in general but against the tailoring to SGs needs. SGs have several weaknesses, especially performance and cascading. Extending to more types doesn't make the concept any better.

@Rekkonnect
Copy link
Contributor

That one I really don't understand. SGs already have performance issues, this proposal won't make them slower or anything. And SGs will become faster in the future if they put the effort into enhancing the design.

@b-straub
Copy link

That one I really don't understand. SGs already have performance issues, this proposal won't make them slower or anything. And SGs will become faster in the future if they put the effort into enhancing the design.

I see I didn't explain my concerns very well. My concern is that with each mainly SG iteration of the language, such as the one proposed, the number of SGs will increase. The problem with this increase is the fact that SGs cannot be combined these days, see dotnet/roslyn#57239. When runtime SGs come into play (RegEx, JSON), the problem becomes immediately obvious. Finding a solution to combine SGs is obviously quite difficult, given the input from dotnet/roslyn#57239.

@Rekkonnect
Copy link
Contributor

I see where you come from, even though I have only noticed a spike in popularity from each new SG introduced into the framework, not with each feature that makes SGs friendlier. Making partial properties won't make SGs explode, it'll only help existing SGs that use backing fields to generate the properties, or slapping attributes on classes.

That being said, it's not just SGs that auto-generate code. You can always make your own tool that generates some specific code upon request, and not on every compilation. That one would create specific files that would make use of features that SGs also benefit off of. The main problem we're all facing with SGs is that they run very often, and your PC only lacks some wings before it can fly to the skies.

@KUNGERMOoN
Copy link

KUNGERMOoN commented Oct 3, 2023

Hi, is there any progress on this?
And if not, is there a way I could contribute to help to bring this idea to life?

@jaredpar
Copy link
Member

jaredpar commented Oct 3, 2023

Hi, is there any progress on this?

Not a lot. This is the point in the .NET ship cycle where the team is very heads down on finishing the current version of the language: C# 12 in this case. This feature didn't make that release and hence there isn't going to be much activity on it. This is in strong consideration for C# 13 so once we move back to new feature work it's likely to see some activity. We are at best several weeks out from that right now.

@heartacker
Copy link

want this TO be in NET 8

@ghord
Copy link

ghord commented Nov 22, 2023

Please consider supporting indexers. Currently they are still heavily used in C#, for example when using Range parameters in custom types.

I would like to be able to auto generate indexers from SG based on attributes.

@neuecc
Copy link

neuecc commented Jan 12, 2024

+1 for C# 13.

@rofenix2
Copy link

Probably the most interesting and more useful feature that could be added in a long time to C#. This could bring DDD with source generators to another level and define very complex models in a elegant way without too much typing, making at least the domain model development way faster. +100 to this, i wish this was the top priority right now.

@RoyAwesome
Copy link

I am currently writing a networking library for a video game, and something like this would be very useful. I currently have some code that requires generated OnChanged_PROPERTYNAME calls to be placed in properties and wherever fields are modified to indicate to my system to catch that field on the next network tick, package it into a state update packet, and send it off. Having the ability to auto generate this OnChanged_ function call for a property's setter would be immensely useful.

@robloo
Copy link

robloo commented Apr 2, 2024

Haven't seen any update from Microsoft on this in a while. Everyone doing application development has a use case for this feature -- especially with code generation.

The value add to the C# language for developers is quite high for this. Time saved would be significant.

Do we know the current status? Can this be discussed again in the language meetings?

@heartacker
Copy link

want this TO be in NET 8

hope this is earn in NET9

@thomaslevesque
Copy link
Member

When implementing this feature, please consider also allowing partial events.

@SystematicChaos012
Copy link

Probably the most interesting and more useful feature that could be added in a long time to C#. This could bring DDD with source generators to another level and define very complex models in a elegant way without too much typing, making at least the domain model development way faster. +100 to this, i wish this was the top priority right now.

Yes,If we have this, Event Sourcing between aggregate root and entities will be simple, code also becomes more concise

@Uzbekbekbek
Copy link

One more use-case is docummenting. I want to have all public API for the class in the file separated from the implementation itself. And I want to have good comments with code snippets, etc. something like:

MyClass.cs

public partial class MyClass {
  public int DoSomethingWithNumber(int x) { 
     //do a big job with input to produce output, a lot of lines of code could be there which shouldn't be noised by comments
    return x*Multiplier; 
  }
  public int Multiplier {get;set;}
}

MyClass.API.cs

public partial class MyClass {

  ///<summary>Description</summary>
  ///<example><code>code sample here</code></example>
  public partial int DoSomethingWithNumber(int x);

  ///<summary>Description</summary>
  public partial int Multiplier {get;set;}
}

@DJGosnell
Copy link

One more use-case is docummenting. I want to have all public API for the class in the file separated from the implementation itself. And I want to have good comments with code snippets, etc. something like:...

I think for something like this, Interfaces is a much better system to handle documentation.

@wanggangzero
Copy link

I want it too.

@robloo
Copy link

robloo commented Jul 16, 2024

It's in preview 6 as of a few days ago

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests