-
Notifications
You must be signed in to change notification settings - Fork 1k
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 (VS 17.11, .NET 9) #6420
Comments
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 👍 |
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. |
One thing I like about partial classes is that you can add a part just by a I think the same could be applied to partial members so that generators won't need to bother duplicating every detail. |
This looks awesome! 😄
Had the same thought - it'd be nice if the partial implementation could just use 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: 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. |
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? |
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 |
It wouldn't, but you could. |
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? |
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; } |
Right now the attribute targets the property unless explicitly targeted to the field via |
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 The relaxation on modifiers I'd like to see considered categorically for both properties and methods, similar to the relaxation on partial modifier ordering. |
@RikkiGibson 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 { } |
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 |
Which type should we use in that case? |
For properties it's three places to do this.. and it really doesn't add much besides making the compiler happy.
Same for |
I think the method here is behaving as expected for an abstract class. If you use a semicolon body, then it compiles. SharpLab. |
@RikkiGibson haha Ok 😄 |
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. |
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). |
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 |
I don't see there being significant complexity. You can legitimately just reuse the exact same |
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. |
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. |
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. |
This comment was marked as abuse.
This comment was marked as abuse.
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.
You not liking the statement doesn't make it offensive. |
"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 |
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 :-)
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. |
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! |
Yes,If we have this, Event Sourcing between aggregate root and entities will be simple, code also becomes more concise |
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;}
} |
I think for something like this, Interfaces is a much better system to handle documentation. |
I want it too. |
It's in preview 6 as of a few days ago |
Partial properties are not functioning in the CLI. The LangVersion property value has been set to preview. Whereas the same works from the VS IDE. To keep it simple, I have defined a partial property and am using the contextual keyword field to refer to the backing field in the implementation. Here's the error message: The name 'field' does not exist in the current context Create a console app, add this code snippet to the Program.cs, and try dotnet run from the CLI. dotnet new console -o PartialProperties --langVersion preview public partial class BaseViewModel
{
public partial string? Title { get; set; }
public partial string? Title
{
get => field;
set => field = value;
}
} |
@egvijayanand your issue is the use of the field keyword, not partial properties. The field keyword is indeed still in preview in .NET 9, and it's intended to be part of .NET 10. |
Technically I agree. The issue is wrt the field keyword. But partial properties can also be implemented with the field keyword as illustrated in the code snippet. That's how SGs are designed to ease the work. And have also indicated the LangVersion property value as preview. Issue is it's not working on the CLI. |
I am facing another issue while trying to initialize a value for the partial property, as it does not permit me to do so. Error message: Only auto-implemented properties can have initializers. Unlike partial methods, where ignoring the implementation leads to the entire method being omitted from the program, the partial property only decouples the definition from the implementation. Is there something I'm missing? Error message: Partial property 'BaseViewModel.Title' must have an implementation part. While using Primary Constructor, inline initialization helps a lot. public partial string Title { get; set; } = string.Empty; |
I am not sure if this is the appropriate place for this request, given the interrelated nature of the features. Please reconsider the proposal to run a piece of code along with the Primary Constructor to address requests like this inline initializations, invoke UI initialization code, and so on. These requests are quite practical @MadsTorgersen |
Appropriate place you should be in is here #140 |
My guess is your .NET SDK is out of date and that's why it's not treating field as a keyword when using LangVersion=preview.
Old partial methods did do this, because they were restricted to returning void and disallowed from having out parameters. So, omitting their calls didn't pose deep challenges for how to define the resulting program semantics. Partial methods that return values, as well as partial properties, are required to have an implementation part, because we can't omit their usage so easily when the implementation part is missing. We're unwilling to define what an omitted call means in a position where its return value is used and so on. Hope this clears things up. |
I am working with the latest preview version (RC2) of the .NET 9 SDK. Please refer to the below screenshot for details. However, the build continues to fail with the same error. Maybe I'll give it a try with the GA version and update. And thank you for clarifying that partial properties simply separate the definition from the implementation. |
One more thing to try. Could you please put |
Compiler version: '4.12.0-3.24473.3 (5ef52ae3)'. Language version: preview. |
Ah, I see. Apparently field keyword just barely didn't make RC2. I didn't realize. Yes, will have to hang in for GA. |
Thanks for the analysis. GA release is due today. Will try with it. |
Unfortunately now that this feature has shipped I realize I can't use it as is. Was there discussion by the LDT on "optional" partial properties? Eg, generated code declares the partial properties, the user code may or may not declare a partial on them in their own file. My use case is having a source generator generate types for database tables with properties representing the columns. A user would only need to actually make their implementation of the partial property if they want to apply some attribute on the source-generated property (which is rare). The other limitation I don't understand is why one half of the partial property must have get/set bodies. This seems rather arbitrary and again complicates my above use case. |
Could the source generator skip emitting the properties if they are already explicitly declared? |
It sounds like you are asking for the ability to declare an implementation part but not a definition part. That's something we could consider adding for both properties and methods.
Could you please elaborate on this? Perhaps including the code you would like to be able to write and what it would mean? |
@HaloFour That's an interesting idea. @RikkiGibson Yes, I think it is potentially useful for both properties and methods. An example: //Source generated file
public partial class MyTable {
public partial string MyProperty { get; set; }
public optional partial string OptionalProperty { get; set; } //Strawman syntax for an "optional partial" member
}
//User-written file
public partial class MyTable {
[Column(TypeName = "nvarchar(max)"]
public partial string MyProperty { get; set; }
//Note the "optional partial" member did not need to be redeclared
} In this example I show both ideas:
|
@RikkiGibson This is to confirm that the CLI build for partial properties works fine in the .NET 9 GA release. |
Partial properties
Summary
Allow the
partial
modifier on properties to separate declaration and implementation parts, similar to partial methods.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.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
The text was updated successfully, but these errors were encountered: