Extensions: fix GetDocumentationCommentXml with netmodule #80297
Conversation
jcouv
added
Area-Compilers
Feature - Extension Everything
| ExtensionGroupingInfo extensionGroupingInfo = ((SourceMemberContainerTypeSymbol)containingType).GetExtensionGroupingInfo(); | ||
|
|
||
| foreach (ImmutableArray<SourceNamedTypeSymbol> extensions in extensionGroupingInfo.EnumerateMergedExtensionBlocks()) | ||
| if (containingType is SourceMemberContainerTypeSymbol sourceContainingType) |
There was a problem hiding this comment.
Is there any benefit in visiting declarations from added modules? Does this have effect on xml doc comment file? For example, if we stop this component from visiting declarations form added modules (this is probably not the right place to do that though), will any tests fail?
There was a problem hiding this comment.
My understanding is that added modules cannot contribute any doc comments. It was not my goal to address this bigger question in this PR, but adding if (symbol is not SourceNamedTypeSymbol) { return; } in VisitNamedType indeed didn't break any test.
There was a problem hiding this comment.
My understanding is that added modules cannot contribute any doc comments.
If that is what we think, then a proper fix, in my opinion, would be to stop visiting declarations from added modules in this component. Where exactly the change should be made can be discussed off-line. I do not think it should be the change that was tried. Quite possibly it belongs in the caller of this component.
However, if we think that added modules can contribute doc comments , then most likely extensions should contribute as well, and the change in commit 1 is not correct. More specifically, it does prevent the crash, but most likely doesn't result in correct behavior.
|
Done with review pass (commit 1) |
| </doc> | ||
| """, GetDocumentationCommentText(comp)); | ||
| } | ||
|
|
There was a problem hiding this comment.
trailing whitespace
4 participants
Fixes #80294