Move more of FindRefs OOP#43914
Conversation
There was a problem hiding this comment.
note: it's unortunate that hte host side still starts with a symbol. However, this is something that is harder to change now. The reason for this is metadata-as-source. The way MAS works is that you start with teh MAS document in the MAS workspace. We find the symbol you're on, then map that back to the normal metadata symbol in the primary workspace. We then do a FAR on that metadata symbol.
@tmat We could likely support this in OOP as well. But it means OOP understanding more about different workspaces (and especially providing the services so those workspaces can map between each other). This was out of scope for this PR, but would be good to have in the future.
There was a problem hiding this comment.
ITheadingContext was unused. So i removed it.
There was a problem hiding this comment.
context and cancellationToken together are redundant (since context has the cancellationToken). I removed the latter, esp. so we don't accidentally pass the wrong one along.
228f237 to
3d1e073
Compare
d6b25b5 to
e59f350
Compare
e59f350 to
f334380
Compare
90165e3 to
a57a76d
Compare
9ea6308 to
a211013
Compare
src/EditorFeatures/Core/FindUsages/AbstractFindUsagesService.cs
Outdated
Show resolved
Hide resolved
dpoeschl
left a comment
There was a problem hiding this comment.
I'm not an OOP expert, but everything I read here makes sense both conceptually and in implementation.
| public Task ReportMessageAsync(string message) | ||
| => _context.ReportMessageAsync(message); | ||
|
|
||
| [Obsolete] |
There was a problem hiding this comment.
I see this same method obsoleted in other places too. Can this one have a message too?
There was a problem hiding this comment.
yup. i will add :)
| } | ||
|
|
||
|
|
||
| public Task FindImplementationsAsync(PinnedSolutionInfo solutionInfo, |
There was a problem hiding this comment.
PinnedSolutionInf [](start = 45, length = 17)
Nit: this is an odd wrapping, i'd expect the first param to be on the next line.
|
LGTM |
…ourceReferenceItem Fixes dotnet#44184 Find References "Kind" column uses `SymbolUsageInfo` for its data. `SymbolUsageInfo` is correctly serialized/deserialized when Find References is invoked via [SymbolFinder.FindReferencesServerCallback](https://github.com/dotnet/roslyn/blob/b8585be8e9ff9e30dbe64980e2c9156ae8b6c2bf/src/Workspaces/Core/Portable/FindSymbols/SymbolFinder.FindReferencesServerCallback.cs#L72-L84), as it uses [SerializableReferenceLocation](https://github.com/dotnet/roslyn/blob/a65653a77b2862329052d84816b5fb324f8bc8bd/src/Workspaces/Core/Portable/Remote/RemoteArguments.cs#L156) which uses `SerializableSymbolUsageInfo`. With dotnet#43914, we added a new [FindUsagesServerCallback](https://github.com/dotnet/roslyn/blob/0852232f315e952fee1dc63a0b374614b55c1c69/src/EditorFeatures/Core/FindUsages/IRemoteFindUsagesService.cs#L32) and switched to it for Find references serialization/deserialization. This callback does not use `SerializableSymbolUsageInfo`, hence loses the SymbolUsageInfo for "Kind" column. This change fixes this part.
The previous model was that FindRefs would call into OOP and OOP would report symbols it found. This worked, but required the host to then do the work to rehydrate those symbols/compilations on the VS side. This is extraneous work we don't need for FAR.
Now, we ask OOP to do the work, and OOP just reports back the results that can be directly shown in the UI. This means no extra work necessary on hte host side. We can just immediately display the work and not cause any symbols/compilations to be created.
Also did teh above for FindImpls.
This should be reviewed one commit at a time.