FindAllReferences and QuickInfo need a dual-view of tuples

[jcouv]

If you select the declaration of local a, its usages get properly highlighted:

But if you highlight the usage of a in a tuple, then its declaration is not properly highlighted:

I think the root problem is that when you highlight a member of a tuple (including one on the left of a deconstruction), there are two views you could take: (1) you're pointing at the expression of that member, or (2) you're pointing at a member of the tuple.

Currently, QuickInfo takes the second view (see screenshots below).

There is a similar problem with anonymous types:

FYI @CyrusNajmabadi

Relates to #20116

[jcouv]

@dpoeschl @CyrusNajmabadi Do you have any tips on this?

FindReferences relies on SemanticModelExtensions.GetSemanticInfo. On (Ali$$ce, 2), GetSemanticInfo returns the tuple field as the declared symbol, and the variable "Alice" as one of the referenced symbols. But FindReferences (which relies on SymbolFinder.FindSymbolAtPositionAsync) ends up only using the declared symbol (it needs to pick a single symbol, rather than a set of them).

One option would be to say that (Ali$$ce, 2) isn't declaring the tuple field symbol. Then we'd get the following behaviors, which is a little better than today:

    void M(int {|Definition:[|Alice|]|})
    {
        var tuple = ([|Al$$ice|], 2);
        M(tuple.Alice);
        M(tuple.Item1);
    }

    void M(int Alice)
    {
        var tuple = ({|Definition:[|Ali$$ce|]|}: Alice, 2);
        M(tuple.[|Alice|]);
        M(tuple.[|Item1|]);
    }

I don't see a way to actually implement a dual view, as the following would be weird (involves two definitions):

    void M(int {|Definition:[|Alice|]|})
    {
        var tuple = ({|Definition:[|Al$$ice|]|}, 2);
        M(tuple.[|Alice|]);
        M(tuple.[|Item1|]);
    }

[CyrusNajmabadi]

But FindReferences (which relies on SymbolFinder.FindSymbolAtPositionAsync) ends up only using the declared symbol (it needs to pick a single symbol, rather than a set of them).

Why can't we change that to accept multiple symbols? :)

[CyrusNajmabadi]

I feel like i would prefer:

 void M(int {|Definition:[|Alice|]|})
    {
        var tuple = ([|Al$$ice|], 2);
        M(tuple.[|Alice|]);
        M(tuple.[|Item1|]);
    }

And i would also be ok with:

 void M(int {|Definition:[|Alice|]|})
    {
        var tuple = ({|Definition:[|Al$$ice|]|}, 2);
        M(tuple.[|Alice|]);
        M(tuple.[|Item1|]);
    }

Basically, these should all always be selected whenever you are on any one of them. Which are definitions vs references is less vital to me. For somethign that is both a reference and a definition, i think it's fine to thing of it as a reference for presentation purposes.

[CyrusNajmabadi]

I don't see a way to actually implement a dual view, as the following would be weird (involves two definitions):

Doesn't seem weird to me. It's just cascading. Which we do in many other places as well :)

[CyrusNajmabadi]

ends up only using the declared symbol (it needs to pick a single symbol, rather than a set of them).

Can we handle this through cascading, where FindRefs on some symbol could 'cascade' to find-refs on a tuple-field? And vice versa?

[jcouv]

@CyrusNajmabadi I took at stab at this (still buggy and WIP) in #24363.
Based on my understanding of the problem, I don't think this can be handled by cascading. Cascading functions on the basis of a symbol, but the desired behavior for tuples depends not just on the symbol (a tuple field) but also the current position (are you pointing at the definition of that field or not).
For instance, if you're pointing at t.Alice, the local called Alice which is used in var t = (Alice, Bob); should not considered "referenced".

So the key here is that when looking for symbols at a certain position, we may come back with two symbols instead of one. Then FAR can list the references to both symbols separately:

[CyrusNajmabadi]

but the desired behavior for tuples depends not just on the symbol (a tuple field) but also the current position (are you pointing at the definition of that field or not)

To be pedantic: is that something that has been explicitly verified to be the desired behavior? in general, we haven't wanted the set of highlight-refs entities to be dependent on position.

[jcouv]

For the record, here are the design notes for tracking and renaming references to tuple elements: #14115 (comment) (it's just a summary, I'll try to dig up the more detailed meeting notes)

[xparadoxical]

Just found this issue in a developercommunity VS ticket. I wanted to see how many places would be affected by a change of type of a record property, and to my surprise, nothing was found. My case uses record deconstruction:

record R(bool B, char C);
class C { public static R R; }
var (b, c) = C.R; //'C.R' not found as a reference to R.C