Roslyn: Add whether IPropertyReferenceOperation is a get or a set

[333fred]

As stated. Currently, you have to walk the parent hierarchy and there are a number of cases. We should add a property to IPropertyReferenceOperation for getting this info.

/cc @AArnott.

[333fred]

@dotnet/analyzer-ioperation.

[CyrusNajmabadi]

What is it if you have it in a nameof? :)

Note: this is relevant as we likely want to do something similar for FindReferences (i.e. bucketing property references by usage). So having common naming/patterns here would be useful. Tagging @jnm2 who was looking at teh FAR side of things.

[AArnott]

What is it if you have it in a nameof? :)

Neither! I currently have a special case to handle that today by recognizing the special nameof method. But I'd rather we not have to hack that. I want to know which accessor is _invoked_. If neither are invoked, I need to know that too.

[AArnott]

Can we get this in the semantic model as well as the IOperation for those of us who aren't using operations?

[AArnott]

FYI I've hit this requirement for several analyzers. My latest need is for VSTHRD010, and my workaround is here: https://github.com/Microsoft/vs-threading/pull/220/files#diff-02b468d833f1d0185fcfabb6fb05f4c4R202

[CyrusNajmabadi]

Neither!

Agreed. I think i'd like there to be a clear enum of cases delineating all the types of reference you might have for a property/field. There's:

1. Read. Console.WriteLine(x.Prop)
2. Written. x.Prop = 4
3. ByName. nameof(x.Prop)
4. ByRef (maybe?). ref int i = ref x.Prop (if we ever get ref properties).

(note: names not final).

[Neme12]

@CyrusNajmabadi why not just 2 properties for whether the getter is invoked and whether the setter is invoked?

in case of x.Prop++, both would be true

[jnm2]

If it's not a property-specific enum, get/set/nameof/ref makes sense for fields. Get/set/nameof makes sense for properties. Add/remove/raise/nameof makes sense for events.

[Neme12]

@jnm what if the getter returns by ref and then you do x.Prop = 5; which only invokes the getter? I think there are two different questions here:

1. which is invoked? getter? setter? both? none?
2. let's say the getter is used. is it used as a read/write/ by ref (same kinds of questions as for fields)?

I think a more appropriate API would be something like:
IsGetterInvoked, IsSetterInvoked (probably needs better names), ReferenceKind where ReferenceKind is read/write/ref/nameof

in the case of x.Prop = 5; where Prop returns by ref and has no setter, you'd have:
IsGetterInvoked: true
IsSetterInvoked: false
ReferenceKind: write

in the case of var y = x.Prop; you'd have:
IsGetterInvoked: true
IsSetterInvoked: false
ReferenceKind: read

in the case of nameof(x.Prop):
IsGetterInvoked: false
IsSetterInvoked: false
ReferenceKind: nameof

[jnm2]

@Neme12 There's no concept of a ref accessor yet, so that's why properties would seem to need get/set/nameof.

But I'm starting to think this doesn't generalize well and should be specific to properties, with a bool for each accessor.

ReferenceKind could have Read, Write, ReadAndWrite, Nameof, Cref. If this question ever came up for events, how would we handle add vs remove vs raise?

[CyrusNajmabadi]

@CyrusNajmabadi why not just 2 properties for whether the getter is invoked and whether the setter is invoked?

So, for nameof, that would be "neither are invoked"?

[Neme12]

@CyrusNajmabadi yes

[CyrusNajmabadi]

But I'm starting to think this doesn't generalize well and should be specific to properties with a bool for each accessor.

I would be ok with that.

ReferenceKind could have Read, Write, ReadAndWrite, Nameof, Cref. If this question ever came up for events, how would we handle add vs remove vs raise?

Also note that we'd want this to be sufficiently generalized to work for any roslyn language.

[Neme12]

@jnm2

@Neme12 There's no concept of a ref accessor yet, so that's why properties would seem to need get/set/nameof.

then how come this code compiles?:
```c#

class C
{
int value;
ref int Value => ref value;

public C()
{
    Value += 1;
}

}
```

[Neme12]

@CyrusNajmabadi
As I said, because of ref, I think there should be at least 3 separate properties:

• 2 booleans for whether getter & setter is invoked
• ReferenceKind which would be an enum of: read/write/ref/nameof/maybe other things as well - ReferenceKind would be exactly the same as for fields

we need this because you can modify the value through the getter thanks to ref, so just asking IsGetterInvoked may not be what you want (depending on the use case)

[jnm2]

@Neme12 That's a ref-returning get accessor. A ref accessor would look like this, if it existed (not convinced it should).

[Neme12]

@jnm2 alright then. I was talking about ref-returning get accessors all along.

[AArnott]

I think this is in line with the foregoing proposals but just to emphasize for the case of ref properties: when a ref property's getter is accessed and assigned to, that's still a getter invocation not a setter invocation. That's important because I need to know which accessor will be executed -- not whether this is a semantic write or read.
But adding more properties to express the semantics of how it's used sounds all good too.

[Neme12]

@AArnott that's why I think both should be exposed (getter/setter invocations and ReferenceKind for semantic read/write) because you may want to ask different questions depending on what you need.

[CyrusNajmabadi]

Yes. that seems reasonable to me. And i'm glad we've had this discussion, esp. as @jnm2 spent a couple of hours on this a few days before.

--

Back to @333fred . I think we should really do what @AArnott recommended here: Can we get this in the semantic model as well as the IOperation for those of us who aren't using operations?

This really does seem like something core that the language shoudl provide, which is hard to figure out today. Customers have asked for it. our own features need it. And it's something the compiler can make quick work out of.

[CyrusNajmabadi]

Maybe something like:

```c#
class SemanticModel {
public SymbolReferenceInfo GetSymbolReferenceInfo(SyntaxNode node);
}

public SymbolReferenceInfo {
ImmutableArray ReferencedSymbols { get; }
ReferenceKind Kind { get; }
}

[Flags]
enum ReferenceKind {
Read,
Write,
Name,
}
```

?

[Neme12]

@CyrusNajmabadi how does this expose getter/setter invocations?

[333fred]

Definitely seems reasonable to me.

[jnm2]

@CyrusNajmabadi What ReferenceKinds should we use for x.Prop++ and <see cref="X.Prop"/>? Write and Name?

[CyrusNajmabadi]

@CyrusNajmabadi how does this expose getter/setter invocations?

Through the ImmutableArray<ISymbol> ReferencedSymbols { get; } array. If you have "this.Prop", then asking for all the ReferencedSymbols on .Prop would give you 1-many results.

[CyrusNajmabadi]

@CyrusNajmabadi What ReferenceKinds should we use for x.Prop++

Read | Write

[jnm2]

Oh, so it's a flags enum. What would Read | Name indicate?

[Neme12]

@CyrusNajmabadi so it would return the getter method symbol? for x.Prop++ it would return getter & setter symbols?

[CyrusNajmabadi]

<see cref="X.Prop"/>

Maybe 'other' :D

Or "normal". Or 'none'. Whatever means 'this is not a special type of reference' :)

[CyrusNajmabadi]

What would Read | Name indicate

I'm not sure htat would ever happen. "Name" would effectively be "nameof" which doens't seem to have any overlap with the other type of references.

[jnm2]

@CyrusNajmabadi For cref, ReferenceKind.Trivia?

[CyrusNajmabadi]

Maybe... :)

[Neme12]

ReferenceKind.Name seems fine for cref too. After all it just uses the name...

[jnm2]

If we started needing to differentiate event accessors, would we just add ReferenceKind.Add, Remove and Raise?

[Neme12]

@CyrusNajmabadi does ReferencedSymbols return getter &setter symbols or the property symbol?

[CyrusNajmabadi]

It might just be that it is:

c# [Flags] enum ReferenceKind { None = 0, Read = 1, Write = 2, }

Where 'none' means 'neither a read nor a write'. So both nameof and crefs are both just 'None'.

[Neme12]

I think ReferenceKind should be pure semantic read/write, NOT getter/setter invocations. Getter/setter/add/remove invocations should be exposed separately. If you have a ref-returning getter and assign to it, ReferenceKind should be Write, but there should be a separate property telling you that only the getter is invoked.

[CyrusNajmabadi]

@CyrusNajmabadi does ReferencedSymbols return getter &setter symbols or the property symbol?

It returns whatever was actually referenced there. In my head it could be:

1. just the property. For example, in a cref/nameof context.
2. the property and the getter. When just doing console.log(x.Prop)
3. the property and the setter. When just doing x.Prop = 1;
4. the property, getter and setter, when just doing x.Prop++

Note: for 2-3 i could be convinced that we could jsut drop the property as well.

[CyrusNajmabadi]

If you have a ref-returning getter and assign to it, ReferenceKind should be Write, but there should be a separate property telling you that only the getter is invoked.

Yes. That's how I expected the API i outlined above to work.

[Neme12]

Therefore you don't need add/remove in ReferenceKind. That would be a question for referenced symbols.

[jnm2]

If you have a ref-returning getter and assign to it, ReferenceKind should be Write, but there should be a separate property telling you that only the getter is invoked.

Yes. That's how I expected the API i outlined above to work.

@CyrusNajmabadi What if you care about differentiating reading through the ref vs storing the ref somewhere to read and write later?

[CyrusNajmabadi]

Agreed. ReferenceKind, currently, would just be read/write. if you had this.ev += ... then GetSymbolReferenceInfo on .ev woudl return the "add" accessor, with 'None' as the ReferenceKind.

[CyrusNajmabadi]

@CyrusNajmabadi What if you care about differentiating reading through the ref vs storing the ref somewhere to read and write later?

Cna you give a full code example of what you mean?

[jnm2]

Agreed. ReferenceKind, currently, would just be read/write. if you had this.ev += ... then GetSymbolReferenceInfo on .ev woudl return the "add" accessor, with 'None' as the ReferenceKind.

Arguably, add and remove could map to Write and raise could map to Read. Using it vs mutating it.

[Neme12]

@CyrusNajmabadi is there an easy way to find out that the symbol is the getter? I'm still thinking if there shoud be a PropertySymbolReferenceInfo : SymbolReferenceInfo which would tell you IsGetterInvocation, IsSetterInvocation + the same for events

[CyrusNajmabadi]

@CyrusNajmabadi is there an easy way to find out that the symbol is the getter?

GetSymbolReferenceInfo(node).ReferencedSymbols.Any(IsGetter)

You could put that behind an extension if you want. The core idea though would be that the API woudl be sufficient to reveal that to you in the data you got back.

Right now, it's very hard to tell. You have to know all the syntactic forms of C# (and several semantic forms of VB). Furthermore, 'ref' throws a big wrench into things (and i'm pretty sure the LS has not been updated properly for those cases).

[Neme12]

@jnm2 has a good point. There should also be ReferenceKind.Ref for when you do:
var y = x.Property where Property has a ref getter.

[CyrusNajmabadi]

Arguably, add and remove could map to Write and raise could map to Read.

I'm not sure i like that. :)

[CyrusNajmabadi]

@jnm2 has a good point. There should also be ReferenceKind.Ref for when you do:
var y = x.Property where Property has a ref getter.

I would be fine with that :)

it also seem appropriate in the simple case of a field/local if you did: Foo(ref local), right? It's neither a direct read or write.

[Neme12]

@jnm2 no it shouldn't because read/write is a very different question than getter/setter invocations because of ref semantics as we've discussed.

[CyrusNajmabadi]

Are we simulating a gitter convo in the most inefficient way possible? :D

[jnm2]

@CyrusNajmabadi What if you care about differentiating reading through the ref vs storing the ref somewhere to read and write later?

Cna you give a full code example of what you mean?

public class C
{
    private int actualStorage;

    public ref int M => ref actualStorage;

    public void Example()
    {
        UseLater(ref M); // ReferenceKind.Read? The ref is copied here but not read through here.

        // ReferenceKind.Read? The ref is read through to the value here.
        int readValue = M;
    }

    public void UseLater(ref int refToUseLater)
    {
        // Either read or write to actualStorage
        int readValue = refToUseLater;
        refToUseLater = 42;
    }
}

[CyrusNajmabadi]

Currently the LS treats "ref" as "i will make the most conservative guess as to what's going on. In other words, if you do "Foo(ref bar)" then we will assume that 'bar' is both a read and a write (as both may happen). Similarly, "Foo(out bar)" is just treated as a write, as that's the only thing guaranteed to happen (which is odd, since under the covers 'bar' may be read, by VB for example).

I would be fine with 'ref x' being treated as 'read/write'. Or just finely enocding it with a specific 'Ref' kind.

[CyrusNajmabadi]

C# // ReferenceKind.Read? The ref is read through to the value here. int readValue = M;

This def feels like a Read to me :) Can you convince me otherwise?

[jnm2]

It's two reads, or rather a get and a dereference.

[CyrusNajmabadi]

```c#
int readValue = refToUseLater;
refToUseLater = 42;

The first usage is a read.  The second usage is a write. 

If you had:

```c#
        ref int readValue = refToUseLater;
        refToUseLater = 42;

Then i could accept the first usage as ReferenceKind.TakeReference (or .Ref, or whatever).

[Neme12]

@jnm2 yes but calling a getter isn't a read. you can find out whether it's the getter that's called from ReferencedSymbols. it's really only a read

[jnm2]

@Neme12 If getting an int from int Prop is a read, getting a ref int from a ref int Prop is the exact same read.

[Neme12]

personally i think ref shouldn't be treated as read+write because the value isn't actually being read or written at all at that point

[CyrusNajmabadi]

c# UseLater(ref M); // ReferenceKind.Read? The ref is copied here but not read through here.

Could be a read, or could be a write. Currently the LS treats this conservatively as a read/write. But the more i see these examples, the more i think it's valuable to have a specific ReferenceKind to represent this.

I think "Read" should mean "an actual read of the value happened here" "Write" shoudl mean "an actual write happneed", and "Ref (name TBD)" means "a reference was taken to this value which was neither a read, nor write".

--

Aside: Shoudl we have the concept of "ReferenceKind.ReadonlyRef"? i.e. a reference was taken, but it's known to be readonly. thus, a write could not happen to this actual variable. Seems reasonable to me. Thoughts?

[jnm2]

personally i think ref shouldn't be treated as read+write because the value isn't actually being read or written at all at that point

I agree. I'd like to treat it as a third category because a ref is the ability to read and write at any point in the future—far more significant than a single write.

[Neme12]

@Neme12 If getting an int from int Prop is a read, getting a ref int from a ref int Prop is the exact same read.

@jnm2 you can't tell whether it's a read/write just knowing it's calling the getter. at that point it's just calling the getter, it's not actually a read until you read from it (eg by storing it in a variable) or taking a ref - then it would be ReferenceKind.Ref etc

[CyrusNajmabadi]

personally i think ref shouldn't be treated as read+write because the value isn't actually being read or written at all at that point

Yes. I think that's appropriate. So, do we have:

c# enum ReferenceKind { None = 0, // The value of the symbol was not used at all. i.e. nameof(), crefs, etc. Read = 1, // value of the reference was read out. Write = 2, // value was written into the reference. ReadOnlyReference = 4, // A readonly reference was taken ReadWriteReference = 8, // A readable and writable reference was taken }

?

[jnm2]

@CyrusNajmabadi ReferenceKind.Ref and ReadonlyRef: I mean sure. At this point, why not have a ReferenceKind.Trivia?

@Neme12 Depends how you look at it. You can't take a reference without reading the place in memory where the reference itself is stored. (In this case, the return value from the ref getter, on the stack.)

[CyrusNajmabadi]

At this point, why not have a ReferenceKind.Trivia?

1. Because that seems trivially syntactic to me.
2. Because that doesn't feel like a language-oriented view of things.
3. Because references really are about values of symbols and how you use them. I'd much rather have a catch-all 'none' which is basically saying: you're not even using the value in any form. Your question doesn't even make sense. Do something else :)

(Note: i dont' feel strongly about this. but i do think that such a model makes the most sense. to me at least :)).

[Neme12]

@CyrusNajmabadi looks good. but I think I would prefer Ref as opposed to Reference given that the whole enum is called 'Reference'Kind, so you have the whole enum having Reference in its name and also Reference as two possible values with these usages of the same word having different meanings.

[CyrusNajmabadi]

@CyrusNajmabadi looks good. but I think I would prefer Ref as opposed to Reference

Yeah. Naming TBD :) I'm just trying to convey the core concepts. Though i do like ReadOnly and ReadWrite in the name, to really convey what sort of reference out that you're getting.

//

Note: there might also be WriteOnly for 'out'. That's certainly the C# view of it. But it's not really accurate as per IL given that the callee may actually read the value. Not sure if that means we should represent as ReadWrite or as WriteOnly.

--

Note: the IDE really likes to know if something was WriteOnly vs ReadWrite. It makes different distinctions. So i think i would like to have:

c# enum ReferenceKind { None = 0, // The value of the symbol was not used at all. i.e. nameof(), crefs, etc. Read = 1, // value of the reference was read out. Write = 2, // value was written into the reference. ReadOnlyReference = 4, // A readonly reference was taken. (i.e. passing to an 'in' parameter) WriteOnlyReference = 8, // A writable reference was taken (i.e. 'out' in C#). ReadWriteReference = 16, // A readable and writable reference was taken. (i.e. 'ref') }

[jnm2]

I'm happy so far.

[jnm2]

Arguably, add and remove could map to Write and raise could map to Read.

I'm not sure i like that. :)

Semantically it still seems the most consistent. Is this reference causing a change or just making use of what's there? That's the kind of question I'm trying to answer when searching for references.

[Neme12]

with events it seems like semantically it should be read/write, even though it might not be true. But I guess that's the same problem as with properties - we have no way of knowing what you're doing in the setter, so even if it's empty, we should always report setter usage as write.

[CyrusNajmabadi]

so, delegates are weird. in that both += and -= can be viewed with different tyeps of read/writes. i.e. due to how delegate invocation lists can work and whatnot. I think it's much clearer that the ReferencedSymbols list just says: you references .add, or .remove, or .raise :)

Basically, wit properties you need the ReferenceKind enum to make it clear how things are being used regardless of which accessors you are getting. With events, the ReferenceKind enum doesn't seem to serve any purpose because all the info you need is in the ReferencedSymbols list.

Right?

[Neme12]

@CyrusNajmabadi yes of course - but the question is what should ReferenceKind be for events?

[jnm2]

@CyrusNajmabadi If the UI exposes this enum as a column to filter, ReferencedSymbols does me no good. Same thing applies to crefs if I'm looking for them.

[CyrusNajmabadi]

@CyrusNajmabadi yes of course - but the question is what should ReferenceKind be for events?

'None' is fine with me.

[CyrusNajmabadi]

@CyrusNajmabadi If the UI exposes this enum as a column to filter, ReferencedSymbols does me no good. Same thing applies to crefs if I'm looking for them.

The UI has no relation to this. This is the underlying symbolic data model. How we present that to users is an entirely different question :)

In other words, i woudl expect the FindRefs symbol model to pass along this data (as well as potentially other data it gleans from teh file). Then it decides how the FindRefs UI should show that data. For example, the FindRefs UI could certainly have categories for things related to accessors. But that doesn't mean the low level symbolic data API needs those same categories :)

[Neme12]

I think if we're using +=, it should be read|write because it's in that assignment position (just as with properties). with invocations, probably None.

[jnm2]

The UI has no relation to this. This is the underlying symbolic data model. How we present that to users is an entirely different question :)

So why bother with ReferenceKind at all, if we aren't interpreting anything? Basically to figure out if there is a ref taken but not immediately used?

[CyrusNajmabadi]

So why bother with ReferenceKind at all, if we aren't interpreting anything?

Because, right now, some things are super hard to tell for any client. i.e. if i tell you "you referenced property P" then figuring out how P was actually used is quite complex and not at all easy for clients to figure out.

[CyrusNajmabadi]

I think if we're using +=, it should be read|write because it's in that assignment position (just as with properties).

I would be ok with this.

[Neme12]

@jnm2 what do you mean by "we aren't interpreting anything"? ReferenceKind seems really useful as getter invocations won't tell you everything. as to which one of these should be exposed in the UI, I don't know. but that's not what we're discussing

[CyrusNajmabadi]

I have no objection to representing += and -= for C# as "read|write" for both. Vb likely needs something for the RaiseEvent statement.

[jnm2]

Because, right now, some things are super hard to tell for any client. i.e. if i tell you "you referenced property P" then figuring out how P was actually used is quite complex and not at all easy for clients to figure out.

Same thing if I'm looking for nameof vs cref. If we don't expose that here, how will that ever be possible?

[Neme12]

@jnm2 that's really easy from the syntax though. and it doesn't really seem like a semantic question

[CyrusNajmabadi]

Same thing if I'm looking for nameof vs cref. If we don't expose that here, how will that ever be possible?

To me, that's trivially determinable syntactically. Note: i'm not hugely opposed to having these be in the referencekind. but i wanted the core set to be the things that are realllllllly hard to tell.

We could add more items there... but it's just not something i'm pushing heavily for.

[CyrusNajmabadi]

For example, here's why references are very hard for properties. Consider this case in VB:

Foo(this.Bar)

What type of reference is 'Bar'? It's really hard to tell. And expecting users to write up all the code to figure this out is basically untenable. people will not get it right at all. On the other hand, determining that you're in nameof/trivia is super simple.

--

Now, again, i'm not super opposed to just having these be in the enum. But it's more like pri3 for me, wheras "Read/Write/ReadableRef/WritableRef/ReadWriteRef" are really necessary to have.

[jnm2]

Makes sense. So if you see ReferenceKind.Read, you know that either 1) a non-ref getter was called or 2) a ref getter was called and the returned ref was immediately read. You can distinguish which by looking for the getter in the ReferencedSymbols and checking the return type.

[jnm2]

but i wanted the core set to be the things that are realllllllly hard to tell.

What about method group references vs calls, and named type references vs constructor calls?

[CyrusNajmabadi]

Yaay for real-time brainstorming sessions!

[jnm2]

https://gitter.im/dotnet/roslyn?at=5a9476146f8b4b9946c623f7

[CyrusNajmabadi]

@Neme12 @jnm2 and i discussed this more in gitter. We've landed here as where we think the codeanalysis API should go:

First, we think there should be:

```c#
public class SemanticModel {
ImmutableArray GetAccessors(SyntaxNode node, CancellationToken token);
}

This member is very similar to GetMemberGroup, but returns the set of accessors invoked at a particular location of code.  The intent would be that you could call it on any node that GetSymbolInfo gave you back an IPropertySymbol/IEventSymbol). For example, given:

```x.Prop++``` this woudl return both the get and set accessors when called on '.Prop'.  For events, (```this.ev +=``` ) this woudl return the .add/.remove/.raise accessor when called on '.ev' depending on the usage.  

Note: for code that does not involve actual accessor, this would return an empty list.  For example ```nameof(this.Prop)``` or ```<see cref="C.Prop```.  No accessors are called in thses contexts, so these lists would be empty.  

--

Second, there should be an api telling a user how a value is actually being used (in the lvalue/rvalue sense) in the code.  i.e. something like:

```c#
enum ReferenceKind {
  None,
  Read, // WriteLine(this.Prop);
  Write,  // this.Prop = 0 
  ReadableReference,  // eg: ```Foo(this.Prop).  void Foo(in T v);``` 
  WritableReference, // eg: Foo(out this.Prop)
}

public class SemanticModel {
   public ReferenceKind GetReferenceKind(SyntaxNode node, CancellationToken token)
}

Interesting cases: this.Prop++ would have kind Read|Write

ref int i = ref this.Prop has kind ReadableReference|WritableReference

Importantly, this api can be called on any actual value in the language. It doesn't need to be called on just a property/field/etc. i.e. if you had a[i]++ you could call this on a[i], and it would return that this was a Read|Write . Similarly, if you did Foo(ref a[i]), then calling this on a[i] would return ReadableReference|WritableReference

For cases where you are not dealing with values, i.e. "nameof(this.Prop)" calling "GetReferenceKind" on Prop would return 'None'.

--

Note: this is about the low level semantic model APIs. If we had these, it would also make sense to expose this stuff higher through IOperation. For example, for IPropertyReference of IEventReference, we should probably "lift" the results of GetAccessors to the node itself, so that IPropertyReference/IEventReference list the actual accessors invoked (if any) at that point in the code.

--

Justifications for these APIs: Right now figuring this sort of thing out is extremely hard to do and is very error prone. Prior to C# 7.0, it was hard, but coudl be done syntactically in C#. But htis required clients to know about all of C#'s syntactic constructs for these sorts of things (i.e. =, +=, ref x, out x, ++, etc. etc.). For VB it was even more difficult. For example, you could have Foo(this.Bar) and have no idea how this was actually being used.

C# 7.0 and onwards made this even more difficult through the use of ref returns and ref expressions. Now figuring out what is going on when you have "this.Prop = 0" is enormously difficult for clients. After all, that could be a write to the property's setter, or it could be a write to a readable/writable reference to a ref-returning property. Many clients have needed this data, and have implemented buggy code to try to figure out it. This includes the LS, features like FindReferences, and other third party analyzers (for example those written in roslyn-analyzers and by @AArnott . We really need core APIs that can answer these questions accurately across both languages.

[CyrusNajmabadi]

Note: another formalization of ReferenceKind that i would be ok with would be:

c# enum ReferenceKind { None, Read, Write, Ref }

Then you would have the following cases for C#:

code | Read | Write | Ref
-- | -- | -- | --
nameof(x)| | |
Foo(x.Prop) | ✔️ | |
x.Prop = 1 | | ✔️ |
x.Prop++ | ✔️ | ✔️ | |
Foo(x.Prop) where void Foo(in T v) | ✔️ | | ✔️
Foo(out x.Prop) | | ✔️ | ✔️
Foo(ref x.Prop) | ✔️ |✔️ |✔️

In other words, you have bits saying if this is a read or a write, and a bit saying if it's a reference. The only combination not allowed is just setting the 'Ref' bit without specifying if it's a read-only, write-only, or read/write reference.

Upon further reflection, we do not like this formalization as a ReadableReference really conveys something different from a Read value.

[CyrusNajmabadi]

Talking it over, i do not like this approach. As i would like "Read" and "Write" to truly mean "teh value was read or written". That will likely be much more intuitive to users. As such, the table would better (IMO) as:

code | Read | Write | ReadableRef | WritableRef
-- | -- | -- | -- | --
nameof(x)| | | |
Foo(x.Prop) | ✔️ | | |
x.Prop = 1 | | ✔️ | |
x.Prop++ | ✔️ | ✔️ | | |
Foo(x.Prop) where void Foo(in T v) | | | ✔️ |
Foo(out x.Prop) | | | | ✔️
Foo(ref x.Prop) | | |✔️ |✔️

[jnm2]

I sort of prefer the usability and intuitiveness of bool IsRead, IsWrite, IsReadableRef, IsWritableRef versus an enum which can have meaningless states.

[jnm2]

Came back to edit my suggestion to Ref instead of Reference, then saw you did the same for the enum. 👍

[AArnott]

@CyrusNajmabadi in this comment I'm not clear as to the ref prop = 5 case since I don't see that listed. A ref property getter that is assigned to is invoking the getter, yet appears as an lvalue. So what would its row on this matrix be?

[Neme12]

@AArnott you mean prop = 5 with a ref-returning getter? ReferenceKind would be Write and GetAccessors would only contain the getter

[CyrusNajmabadi]

ref prop = 5 isn't legal syntax. Do you mean "prop = 5" where prop returned a "ref int"? If so, that woud just be a plain 'Write'. This code isn't taking the ref of anything.

[CyrusNajmabadi]

For C#, 'readable/writable refs' would only come into play if you were in:

1. a ref-expression/ref-argument
2. an out-argument
3. an argument to a method with an 'in' parameter.

For VB, it would be in the cases where you passed something to a 'byref' parameter, though vb would never expose anything that was just "WritableReference". For references VB would always be "ReadableReference or WritableReference".

[AArnott]

Yes, I suspect you're right on all counts. I don't know the whole ref return et. al new features as well. But your answers satisfy me.

[CyrusNajmabadi]

Note: @333fred We've talked about it a bit more, and we think the name "ReferenceKind" and "GetReferenceKind" is a bit of a misnomer. Basically, you're talking about how some value (possibly an lvalue or rvalue) is being used. This is irrespective of if you have references or not.

So maybe it should be something more like:

```c#
// enum, or struct with properties.
public enum ValueUsageInfo {
NotApplicable
Read,
Write,
ReadableRef,
WritableRef,
}

public SemanticModel {
public ValueUsageInfo GetValueUsageInfo(SyntaxNode value, CancellationToken);
}
```

This is the exact same as before, just making it clear it operates on any value, and tells you how you're using it. For lvalues, you could be reading/writing it. or you could be getting a readable/writable reference to it. For rvalues you can essentially only be reading it. For non-value scenarios (i.e. nameof/cref/etc), you get NonApplicable (like Accessibility.NotApplicable) since you're not actually using the value at all.

[Neme12]

@CyrusNajmabadi
To me, None sounds like it means: yes, you called this on the right kind of node - but the value isn't used here (eg. inside nameof) whereas if you say NotApplicable, I would think that would mean that I was using this API inappropriately, on something that can't possibly have a value - therefore this question doesn't even make sense, so NotAppliacable.

[weltkante]

I'm not sure if I missed it in the above discussion, but there's also the case when writing to a readonly property from a constructor (i.e. writing to the backing field). You won't have a set-accessor but still want to determine that you are in this case without running complex inspection against the metadata/syntaxtree.

For what its worth my use-case is an analyzer which wants to look at attributes applied to the getter/setter/field. Turns out its really hard to do that reliably yourself without applying heuristics, in particular since Roslyn isn't very exact in reporting against which IPropertySymbol it compiled, either (#36403)