Find the right symbol when following type forwards #62406
Conversation
| @@ -457,6 +457,50 @@ public class C | |||
| { | |||
| } | |||
| } | |||
| "; | |||
|
|
|||
| await RunTestAsync(async path => | |||
There was a problem hiding this comment.
The diff on this is weird. This test is actually old, just with a few bits of code removed. This wasn't testing type forwards at all, but their presence made me think I was correctly testing them.. which I wasn't!
| } | ||
|
|
||
| [Fact] | ||
| public async Task Net6SdkLayout_TypeForward() |
There was a problem hiding this comment.
This test is actually entirely new, and correctly tests type forwards, in the manner that the .NET sdk uses them. This test fails without the changes in this PR.
src/Features/Core/Portable/PdbSourceDocument/PdbSourceDocumentMetadataAsSourceFileProvider.cs
Outdated
Show resolved
Hide resolved
|
Sorry, had to push more stuff to this. @chuckries found some issues (I was being silly, and not using the full namespace when trying to find types in the cache), and then when doing more testing, I found more issues with type forwards, so I've fixed them too. Basically, I have to reference any DLL that was found on the journey of following type forwards, otherwise symbol key resolution failed. There are still some issues though, and it seems that symbol keys might be a little bit too fussy for my needs. For example, the Thoughts @tmat @jasonmalinowski ? |
|
Ping @tmat @jasonmalinowski for review. It would be good to get your thoughts on my question above, but its not necessary for this PR, so I can file an issue and follow up later if its easier |
Is SymbolKey not able to work with missing type symbols? Seems like we should be able to match missing types if their full names and assembly identities match. |
|
With the changes to symbol key, this just fixes a bug I had in namespace lookups, and adds a bunch of tests. Which is a pretty good result :) |
| // is SymbolDisplayFormat.QualifiedNameOnlyFormat, which this is a copy of. | ||
| var namespaceName = typeSymbol.ContainingNamespace.ToDisplayString(new SymbolDisplayFormat( | ||
| globalNamespaceStyle: SymbolDisplayGlobalNamespaceStyle.Omitted, | ||
| typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces)); |
There was a problem hiding this comment.
Consider storing the format in a static field.
ghost
added this to the 
* upstream/main: (66 commits) Merge release/dev17.3 to main (dotnet#62573) Fix Find the right symbol when following type forwards (dotnet#62406) Remove unused parameter (dotnet#62685) Fix NRE in nullable walker (dotnet#62773) Delete obsolete internal options (dotnet#62746) Optimize away unchecked conversions between nint and nuint types. (dotnet#62774) Delete Inline members Clean up global options usage in tests (dotnet#62692) Update dependencies from https://github.com/dotnet/source-build-externals build 20220719.2 (dotnet#62787) Update src/Workspaces/Remote/ServiceHub/Services/Renamer/RemoteRenamerService.cs No need to roundtrip symbol Unused variable formatting Get document properly Lint Simplify Remove unnecessary callbacks Add telemetry ...
Fixes an issue I found while recording a showcase demo this morning :)