Fix syntax tree creation when modifying source generated documents #78343
Conversation
ghost
added
Area-IDE
untriaged
jasonmalinowski
left a comment
jasonmalinowski
left a comment
There was a problem hiding this comment.
I think there's a bug.
src/Workspaces/Core/Portable/Workspace/Solution/SourceGeneratedDocumentState.cs
Outdated
Show resolved
Hide resolved
src/Workspaces/Core/Portable/Workspace/Solution/SourceGeneratedDocumentState.cs
Outdated
Show resolved
Hide resolved
…it happened outside
| Contract.ThrowIfTrue(sourceText is null && syntaxNode is null); | ||
| Contract.ThrowIfTrue(sourceText is not null && syntaxNode is not null); |
There was a problem hiding this comment.
I like this generally -- it's clear we're always doing exactly one or the other.
jasonmalinowski
left a comment
jasonmalinowski
left a comment
There was a problem hiding this comment.
I like the symmetry now in the code that routes around the text or node now.
src/Workspaces/Core/Portable/Workspace/Solution/SolutionCompilationState.cs
Outdated
Show resolved
Hide resolved
| return this; | ||
| } | ||
|
|
||
| var sourceText = newRoot.GetText(); |
There was a problem hiding this comment.
It still feels to me like we could be avoiding this call if we used the CreateTreeWithLazyText helper in DocumentState.cs, but maybe it's fine. I don't think this is actually realizing the text, right? It's just returning a text that walks the tree as needed?
There was a problem hiding this comment.
We could call that, yes, but at this point the SourceGeneratedDocumentState still needs a fully realised SourceText at construction time. Making it fully lazy is beyond this PR IMO. I haven't looked at how the SourceText property is used, but there is probably a reasonably easy path to, if not fully lazy, at least lazy-as-long-as-its-synchronous.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
3 participants
Fixes dotnet/razor#11787
In my previous PR I naively called
syntaxRoot.SyntaxTreeto get a tree, but turns out that will just happily lazily create one withCSharpLanguageVersion.Default, which breaks things.