Roslyn: Request: Find All References broken down by use

[HaloFour]

This is a feature supported in Red Gate Reflector that I think would be very useful within Visual Studio.

I'd like to see Find References be able to differentiate between the use of the reference and allow either breaking down the references by their use or isolating a specific use type:

1. Properties by accessor method (get vs. set)
   

2. Fields, locals and parameters by whether they are used or assigned
   

3. Types by where they are instantiated (via instance constructor invocation), exposed (via non-private field, property or method parameter) or used
   

I am currently working on a project where many fields are exposed as protected or as internal and are frequently referenced directly by other types. I am trying to discern where those fields are being mutated and in quite a few cases Find All References is identifying over a 100 direct references of each of those fields, but scanning through the results I might identify only a couple assignments. Doing this manually is both tedious and very likely error prone.

[jnm2]

I want something similar but different. ReSharper has right click > "Find usages advanced..." feature where I can find all usages of a property getter or a property setter or the whole property. I lean heavily on this and would like to see it in VS.

Even more useful to me would be the same thing for fields: find all reads of a field or all writes of a field, but don't show me both at the same time and make me wade through a ton of false positives to get to what I need to see. (nameof wouldn't count and ref would trigger either. out would count as a write.)

[CyrusNajmabadi]

Even more useful to me would be the same thing for fields: find all reads of a field or all writes of a field, but don't show me both at the same time and make me wade through a ton of false positives to get to what I need to see. (nameof wouldn't count and ref would trigger either. out would count as a write.)

We have the data for this internally. It could be expose as a new column in the VS FAR window. You could hten use that column to 'group by', as you can with the other columns.

[jnm2]

I guess that wouldn't be bad, but say you had four classifications: field write, field read, field readandwrite (or ref), and field metadata (nameof). I can sort them but what I really want to do is filter to one of them. Sorting still means I'm scrolling past dozens of hits I don't care about, though the scanning is much nicer with the new column to look at. Grouping still means I have to collapse and expand etc.

[HaloFour]

@CyrusNajmabadi

That would certainly scratch my immediate itch if that information was available for both fields and properties.

I think that there is value to the other references types, too, particularly those around where a type is exposed or instantiated. When refactoring a public type, e.g. hiding it behind an interface, it would help to have a quick way to identify where that type might leak out of the assembly.

[CyrusNajmabadi]

I think that there is value to the other references types, too, particularly those around where a type is exposed or instantiated.

You can get instantiation information simply by looking at the references to the type's constructors (which we automatically cascade to when doing a FindReferences on a type). Is that not sufficient?

[spottedmahn]

Related User Voice Request: Find All assignments

A feature similar to "Find all references", but only includes assignments.

[HaloFour]

@CyrusNajmabadi

You can get instantiation information simply by looking at the references to the type's constructors (which we automatically cascade to when doing a FindReferences on a type). Is that not sufficient?

It's alright, but it's not great. When grouping by definition you can see the constructor invocations but they're all separate from one another. And it looks like any static member invocation or similar use bubbles up the top of the list above the constructors, so you'd likely have to dig through that first.

[jnm2]

Fyi– I made it this far on 15.6 without ReSharper, but there's no way I can finish the task I have at hand without going back to ReSharper. I have to look through 223 hits, the overwhelming majority of which are usages of the getter, while the tool window title is 'set_ThisProperty' references.

Edit: as it turns out, 222 of the hits were usages of the getter 😁

[jnm2]

This is why I reinstalled ReSharper again the other day.

[CyrusNajmabadi]

We have the information in our FindRefs API. It's here:
https://github.com/dotnet/roslyn/blob/d4dab355b96955aca5b4b0ebf6282575fad78ba8/src/Workspaces/Core/Portable/FindSymbols/ReferenceLocation.cs#L42

@jnm2 Want to just hook it up to the UI?

[jnm2]

Where's the UI source?

[CyrusNajmabadi]

https://github.com/dotnet/roslyn/blob/master/src/VisualStudio/Core/Def/Implementation/FindReferences/StreamingFindUsagesPresenter.cs

[CyrusNajmabadi]

Let me know if you have any questions. Gitter or here work for me.

[jnm2]

Cool! It's on my list.

[jnm2]

@CyrusNajmabadi So what I really want is more accurate behavior like we've been used to (not to make a big deal of it) with ReSharper. If I ask for all references of a property or event, I want all references of that property or event. If I ask for all references of an accessor method, I only want results for that accessor.

Don't worry though; you won't accidentally search for an accessor when you meant to search for a property. To do that you'd have to be searching from the definition of the accessor:

Along the way I discovered that searching for an event accessor is broken.

This does not preclude the possibility of adding a reference classification column. I'd want this accessor-level search precision to be added either way.

Implementation (need to figure out tests): https://github.com/dotnet/roslyn/compare/master...jnm2:accessor_specific_search(UI_only)

Do you have suggestions?

[CyrusNajmabadi]

One way this would work IMO, would be to actually not have the properties, or references to it. Instead, just cascade between the accessors and give results corresponding to both. You would always get all of them. but if you only wanted one you would just not collapse the getters (or setters) group.

[CyrusNajmabadi]

Actually, probably scratch that suggestion. this is similar to how we do NamedTypes vs. Constructors. If you search for the named-type, you get all references (including references to them in consturctor calls). but if you do a FAR for a constructor, you just find references to that constructor.

So we should likely do the same here. THe property should cascade to the accessors. The accessor shoudl not cascade to the property. Note: i perused the code, and it looks like that might be what is already happening. So it's possible some other part of the process is mapping accessors to properties.

[jinujoseph]

related to https://github.com/dotnet/roslyn/issues/24877

[mavasani]

I believe the primary request here has been addressed by https://github.com/dotnet/roslyn/pull/30155.

New FAR column allows classification, sorting, filtering, etc. by reference kind (this will show up in Dev16 Preview2)

@HaloFour Does this meet your request?

[HaloFour]

@mavasani

That looks really nice and solves most of my use cases!

[jinujoseph]

@jnm2 does #30155 addresses your requirement

[jnm2]

@jinujoseph tl;dr yes.

https://github.com/dotnet/roslyn/pull/28231 addresses what I was asking for in this thread, summarized in the pictures here: https://github.com/dotnet/roslyn/issues/22545#issuecomment-366847223. That's for when I know I want the getter or the setter and nothing else.

https://github.com/dotnet/roslyn/pull/30155 addresses rarer (but just as cool) scenarios: field usages, and searches when I don't know ahead of time that I only want to see one accessor of a property.

[jnm2]

@mavasani Super unimportant, but is NameOnly going to show up pascal cased in the final UI?

[mavasani]

Super unimportant, but is NameOnly going to show up pascal cased in the final UI?

This is how it has been checked in, but we can surely make a change if it seems appropriate.

[jinujoseph]

Thanks for the confirmation @jnm2 , @HaloFour
I will go-ahead and close this issue (refer #28231 & #30155 )
If there are more improvements to be made on top of the current experience , pls feel free to file an issue

[jnm2]

@mavasani It seems like a leaking implementation detail ("look, we used an enum"). I think the style guide for UI elements is sentence casing.

(UI text section in https://docs.microsoft.com/style-guide/text-formatting/formatting-common-text-elements, sections in https://docs.microsoft.com/style-guide/procedures-instructions/formatting-text-in-instructions.)

[CyrusNajmabadi]

@jnm2 is right. this should be one of:

1. 'Name'
2. 'Name only'
3. 'Name Only'

it def should not be NameOnly :)

[jnm2]

*I pay attention now that your PR is closed (sorry about that 😳)

[jinujoseph]

@jnm2 no worries , we can address this as a follow-up PR