Extensions: fix GetSymbolInfo within ExtensionMemberCrefSyntax#81722
Extensions: fix GetSymbolInfo within ExtensionMemberCrefSyntax#81722jcouv merged 5 commits intodotnet:mainfrom
Conversation
jcouv
added
Area-Compilers
Feature - Extension Everything
jcouv
force-pushed
the
extension-cref
branch
2 times, most recently
from
c6f029a to
df2e0f0
Compare
| [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/81710"] | ||
| public void Cref_68() | ||
| { | ||
| var src = """ |
There was a problem hiding this comment.
raw string? #Resolved
There was a problem hiding this comment.
(indented raw string, rather).
There was a problem hiding this comment.
The formatting is intentional
jcouv
force-pushed
the
extension-cref
branch
from
df2e0f0 to
68e8549
Compare
jcouv
force-pushed
the
extension-cref
branch
from
4ce27e5 to
80a02ea
Compare
|
|
||
| <WpfTheory, CombinatorialData> | ||
| <WorkItem("https://github.com/dotnet/roslyn/issues/81710")> | ||
| Public Async Function FindReferences_ExtensionBlockMethod_Cref(kind As TestKind, host As TestHost) As Task |
There was a problem hiding this comment.
nice. i'm glad this just fell out. #Resolved
| var m = ((NameMemberCrefSyntax)extensionCref.Member).Name; | ||
| Assert.Equal("M", m.ToString()); | ||
| AssertEx.Equal("E.extension(int).M(string)", model.GetSymbolInfo(m).Symbol.ToDisplayString()); | ||
| } |
There was a problem hiding this comment.
Consider asserting those found symbols are the same (reference equals) between themselves and also with the E.M symbol found outside the doc comment. #ByDesign
There was a problem hiding this comment.
Hmm, I don't think there's a requirement that the symbols from semantic model be reference equal. Definitions should be reference equal, but that's not about the semantic model and here we're not dealing with definitions in this test anyways.
| <Workspace> | ||
| <Project Language="C#" CommonReferences="true" LanguageVersion="Preview"> | ||
| <Document> | ||
| /// <see cref="E.extension(int).[|M|]()"/> |
There was a problem hiding this comment.
Consider wrapping the code in <![CDATA[]]> instead of xml-encoding parts of it. #ByDesign
There was a problem hiding this comment.
I leaned towards this encoding
| AssertEx.Equal("E.extension(int).M(string)", model.GetSymbolInfo(extensionCref).Symbol.ToDisplayString()); | ||
| var m = ((NameMemberCrefSyntax)extensionCref.Member).Name; | ||
| Assert.Equal("M", m.ToString()); | ||
| AssertEx.Equal("E.extension(int).M(string)", model.GetSymbolInfo(m).Symbol.ToDisplayString()); |
There was a problem hiding this comment.
Thanks. That uncovered another problem
| public void Cref_68() | ||
| { | ||
| var src = """ | ||
| /// <see cref="E.extension(int).M(string)"/> |
|
|
||
| <WpfTheory, CombinatorialData> | ||
| <WorkItem("https://github.com/dotnet/roslyn/issues/81710")> | ||
| Public Async Function FindReferences_ExtensionBlockMethod_Cref(kind As TestKind, host As TestHost) As Task |
There was a problem hiding this comment.
Consider covering all scenarios covered in added compiler test(s)
This thread was resolved, but I do not see any changes made in response. Since these are not compiler tests, I am going to sign off, but I will reactivate the thread in case you resolved it by mistake.
|
Done with review pass (commit 3) |
5 participants
Closes #81710
The problem in #81710 turns out to be a problem with
GetSymbolInfo.GetSymbolInfoshould succeed in finding the extension member not only when given theE.extension(...).Membersyntax but also just theMemberportion of that syntax.FindReferencesin the IDE makes use of that, as it starts by identifying a symbol of interest, then syntactically tracks all the occurrences of the symbol's name, then queries the semantic model for each instance (seeFindReferencesInTokens) to check which instances actually refer to the given symbol.