Open terrajobst opened 3 years ago
For the sake of comparison, without the additional newlines:
[set: Obsolete("Nope")]
public string Name { get; set; }
vs.
1.
public string Name { get; [Obsolete("Nope")] set; }
2.
public string Name {
get;
[Obsolete("Nope"] set;
}
3.
public string Name {
get;
[Obsolete("Nope")]
set;
}
I personally have used the first option without the newlines and haven't found it cumbersome to read or write. If the attribute was more verbose I would go with the second option. I can definitely understand having to work on a team that enforces style guidelines that involve more newlines and that feeling annoying.
I assume to target both accessors you'd need to duplicate the attribute?
[get: Obsolete("Nope!")]
[set: Obsolete("Nuh-uh!"]
public string Name { get; set; }
The current situation isn't any better here, just curious.
Cough ... init
... cough
Primary drawbacks is having two syntax options to achieve the same thing
In particular, this syntax option makes it harder to tell at a glance what the attribute applies to. While that’s already also true of, say, assembly-targeted attributes, I find that we should avoid further indirection where not necessary.
In addition, new property targets (e.g., init
, req init
) would also need corresponding attribute targets.
I've had a need for deprecating setters for a long time. Please add this. I think there also could be a need for this for null annotations
I've had a need for deprecating setters for a long time. Please add this. I think there also could be a need for this for null annotations
This is just an alternate syntax form. HaloFour showed the forms available today.
Seems totally unnecessary. public string Name { get; [Obsolete("Nope")] set; }
reads fine, already exists, and is fewer characters than this proposal.
@chucker
In particular, this syntax option makes it harder to tell at a glance what the attribute applies to.
I'm not sure I agree with this. I stare at APIs for a living (note that I say APIs, not code). That basically means I virtually never care for bodies. I grant you that this is a specialized use case but I think everyone experiences this in day to day coding, be that in docs, metadata view in VS, or signature help. In those scenarios, I personally find it unwieldy to have the accessor declarations interspersed with custom attributes. I'd prefer if they were listed on the property as a whole and if they only apply to a specific accessor, targeted using the already existing target syntax for custom attributes.
However, as I said in the proposal I completely concede the point that this mostly a style question and adds zero expressiveness to the language. But I don't see that is an argument against the feature because quite a few language features are added for this very reason.
In addition, new property targets (e.g.,
init
,req init
) would also need corresponding attribute targets.
Good point. I've reworded the proposal to point this out.
fwiw, currently this isn't possible record R([get: Attr] stirng Name);
(unless you redeclare prop definition entirely)
However, as I said in the proposal I completely concede the point that this mostly a style question and adds zero expressiveness to the language. But I don't see that is an argument against the feature because quite a few language features are added for this very reason.
I personally don't agree with this statement. We very, very rarely add new forms for existing features. The bars that such a new form as to pass, in my eyes, are twofold:
IMO (pun kinda intended), this proposal doesn't meet either bar.
A more realistic use case:
[get: MethodImpl(AggressiveInlining)] SomeProp => expression;
A more realistic use case:
[get: MethodImpl(AggressiveInlining)] SomeProp => expression;
This could also be solved with no new target keywords if this was made valid (especially since the docs explain the method target like this method | Method or get and set property accessors
which would hint at this being allowed):
[method: MethodImpl(AggressiveInlining)] SomeProp => expression;
@333fred
- The new form has to be a significant clarity increase over the existing form.
- The old form has to be an extremely common coding pattern.
I think the new proposal with method:
target should meet those criteria better than the original one. There are many use cases in .NET repo, for example, in ReadOnlySpan<T>
:
public int Length
{
[NonVersionable]
get => _length;
}
could have been written as
[method: NonVersionable]
public int Length => _length;
which is also much more clear.
Could you consider this proposal once again?
I genuinely don't find that any more clear. The form with the expression body accessor is clear to me and nice and lightweight.
If it is not more clear, why do we have expression body get-only property anyway, since the form with the expression body accessor is clear enough and nice and lightweight?
Using your jargon, I think attributes and property accessors should be "orthogonal". Design of one of them should not have any effect on the design of the other. The need of having expression bodied property to simplify code exists no matter whether it has attributes or not.
why do we have expression body get-only property anyway
Imo, exactly for this. Because sometimes you want to operate on the accessors, and sometimes not.
Using your jargon, I think attributes and property accessors should be "orthogonal"
I agree. So when attributes have special knowledge about properties so they can operate on an accessor that that violates that orthogonality for me.
Imo, exactly for this. Because sometimes you want to operate on the accessors, and sometimes not.
What do you refer to by "this"? Expression-bodied property is nothing more than a syntax sugar. It just makes code shorter. I don't see why we are allowed to do it without attributes but are not allowed to do it with attribute (given a reasonable syntax).
So when attributes have special knowledge about properties so they can operate on an accessor that that violates that orthogonality for me.
That's not a reason not to do this. Today you can already write:
[field: SomeAttribute]
public int SomeProperty { get; set; }
IMO, field:
and method:
are both internal implementation of a property. If we are allowed to add attributes to the field, why not also to methods?
What do you refer to by "this"?
I mean exactly the problem statement this issue is talking about.
This is exactly the sheet if reason I think expression bodies accessors should exist and be used for.
That's not a reason not to do this.
It is for me. If you want to operate on the method, just use an accessor. It's just get=>
. I don't see any need to change the syntax of that too method:
. It just seems redundant.
In what way are property accessor methods internal implementation details? They are as publicly exposed as the property itself.
In what way are property accessor methods internal implementation details? They are as publicly exposed as the property itself.
That's right, they aren't. But that means that method attributes have more reason to be allowed than field ones, since the latter are an implementation detail (as in, they aren't public and the compiler chooses the name for them).
In what way are property accessor methods internal implementation details
Accessors are not, but accessor methods are. The other comment mentioned that you can't choose a name for the backing field, and that's also true for accessor methods.
In what way are property accessor methods internal implementation details
Accessors are not, but accessor methods are. The other comment mentioned that you can't choose a name for the backing field, and that's also true for accessor methods.
They're public though and the name is always get_
/set_
PropertyName so they're not really implementation detail.
This is exactly the sheet if reason I think expression bodies accessors should exist and be used for.
Because sometimes you want to operate on the accessors, and sometimes not.
If I am understanding correctly, you are saying that expression-bodied accessors exist so that user can add attributes to both the property and the accessor. But that is very confusing. You don't need the syntax sugar of expression-bodied accessor to do that. You can do whatever you want with the old good block syntax.
The only reason for expression-bodied syntax (property, method, accessor) is to simplify code. You cannot deny this. And as I have said for many times, the need to simplify code exists when we have attributes. Just look at .NET runtime repo and see how many times they have to explicitly write the get accessor just to add an attribute.
It's like local functions. Local functions can be written as a normal, non-local method (for captured variables you have to allocate the storage yourself). At first they cannot have attributes, but it does not make sense, because if you admit that local functions would help programmer write better code, it is still true when now the programmer wants to add an attribute. You cannot say, hey, since you have an attribute this time, local functions are not for you. Please modify it to a normal method.
Expression-bodied property is much easier to expand to accessor, but the reasoning is exactly the same.
Accessors are accessor methods. You write get
which implicitly creates get_SomeProperty
. The fact that this name is implicit (not present in the source code) does not make the accessor method an internal implementation detail. The same is true when you write public SomeClass() { }
and get a method called .ctor
.
Accessors are accessor methods
Strictly speaking, they are two distinct concepts, but I admit the difference is subtle.
Accessors are the access point of a property. It is the knowledge that the compiler must know when it converts a property access to a method call.
Accessor methods are the actual method that the accessors point to. I would say it's like C++'s concept of variables and objects. For example, it's valid to have two properties to share the same method as their accessors, and an accessor method can exist even when the property (and its accessors) are removed from the IL. Note that these two do not hold for constructors.
Maybe I am just over-complicating things, but in any case, this is not really related to that programmers want to have expression-bodied property with attributes on accessors (or accessor methods).
The only reason for expression-bodied syntax (property, method, accessor) is to simplify code.
Sure. And am expression bodied accessor is nice and simple to me. I don't see how method:
is beneficial enough over get=>
.
I don't see how method: is beneficial enough over get=>.
It's 2 lines vs 5 lines of code. Definitely much more compact. I think those 3 lines (with 2 of them being the noise) are already beneficial. Expression bodied property vs expression bodied accessor saves exactly the same and I think no one argues it is useless.
It's 2 lines vs 5 lines of code.
I don't know what that means. The language doesn't mandate lines. It sounds like you have a particular style that is very verbose.
Based on your post, I'd right the code as:
public int Length { [NonVersionable] get => _length; }
One line. Simple to read and understand and maintain. I genuinely don't see how method:
is better than this syntax. The difference is:
public int Length { [NonVersionable] get => _length; }
[Method: NonVersionable] public int Length => _length;
Just doesn't seem worth it. It literally seems to save nothing.
It sounds like you have a particular style that is very verbose.
It's not me. It's the dotnet/runtime maintainers, one of the main users of (the latest version of) the C# language. I thought it is a very common style among C# users.
It's the dotnet/runtime maintainers,
You could petition then to change the style here. That would be literally around 100x easier to get rid change to happen :-)
Based on your post, I'd right the code as:
public int Length { [NonVersionable] get => _length; }
One line. Simple to read and understand and maintain. I genuinely don't see how
method:
is better than this syntax. The difference is:public int Length { [NonVersionable] get => _length; } [Method: NonVersionable] public int Length => _length;
Just doesn't seem worth it. It literally seems to save nothing.
I personally find the first one much less readable and it is even more the case when doing longer attributes on a separate line:
// the expression would normally be something that isn't inlined without AggressiveInlining
[method: MethodImpl(MethodImplOptions.AggressiveInlining)]
public int Length => _length;
public int Length { [MethodImpl(MethodImplOptions.AggressiveInlining)] get => return _length; }
That would be literally around 100x easier to get rid change to happen :-)
I don't know what you mean by this.
I think we discuss because we want to get a better experience for programmers. It does not matter whether it is or is not a change or something. And I hope you are not implying that the community can never suggest changes to happen. Otherwise, it would literally be 100000x easier for you to simply close all issues and discussion in these github repos so we just use whatever MS provides, as we had been doing for many years.
To clarify, I mentioned the runtime repo and codes there just to show a very common usage that can be made simpler and easier to read with this minimal feature.
I don't know what you mean by this.
I mean it would be 100 x easier to get a team to change their style choice than to get a language change made becuse you want shorter code than what that code style causes.
and it is even more the case when doing longer attributes on a separate line:
For both the single line and multi line cases the way you can write it today is just much easier to read and understand. So I'm just not seeing any real value here.
To clarify, I mentioned the runtime repo and codes there just to show a very common usage that can be made simpler and easier to read with this minimal feature.
Right. But that's a point about a stylistic decision they've made. My point is simply that it would be easier to get them to stop using that stylistic coding style than it would be to get the language to add a feature to try to make that use case more pleasant for that repo.
My point is simply that it would be easier to get them to stop using that stylistic coding style than it would be to get the language to add a feature to try to make that use case more pleasant for that repo.
It's plausible to me that this would not be true, that convincing them to put get
on the same line as the property is actually impossible but that they would happily accept the hypothetical new language capability. I don't know about dotnet/runtime in particular but I have seen stronger opinions about coding style in some places than about anything else in any place.
I also think the form with long/multiple attributes looks much nicer when attributes are not on the same line as anything else. Given that, the choice (again for consistency with long/multiple attributes) is really between these two things:
[method: MethodImpl(MethodImplOptions.AggressiveInlining)]
public int Length => _length;
public int Length
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
get => _length;
}
I like the one that has => _length
on the same line as Length
and doesn't have brace lines. I would not find it readable to put attributes on the same line as anything else.
Allow using attribute target to express which accessor a custom attribute is applied to
Summary
C# allows putting custom attributes on accessor but it requires to do that by putting them inside the property:
The proposal is to allow get/set as custom attribute targets:
Motivation
This makes property declarations with custom attributes slightly less noisy. One of the values would be documentation and/or metadata view where bodies are generally elided. The current syntax forces some multi-line construct with indentation which gets especially more noisy when multiple custom attributes are applied.
Detailed design
This would be purely syntactic sugar as the underlying encoding in metadata and reflection wouldn't change. This would apply to all kinds of accessors, such as
get
,set
,init
as well asadd
andremove
for events.Drawbacks
Primary drawbacks is having two syntax options to achieve the same thing; it becomes a code style discussion which way is the preferred way.
Alternatives
The only alternative I can think of is simply not doing it. This doesn't change extend the expressiveness of C# but might allow for more advanced properties to read somewhat nicer.
Unresolved questions
Design meetings