Skip to content

Allow modification of source generated documents #77587

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
35c369c
Allow WithText to be called on source generated documents
davidwengier Mar 13, 2025
ff977e0
Allow multiple freezing and fully rollback on unfreeze
davidwengier Mar 13, 2025
b305748
Allow WithSyntaxRoot to work on source generated documents
davidwengier Mar 13, 2025
a4741c0
Remove irrelevant work items
davidwengier Mar 13, 2025
d72827f
Preserve the actual modified root so that annotations flow forward
davidwengier Mar 13, 2025
c465ec3
Consider source generated document changes when computing code action…
davidwengier Mar 13, 2025
fbb41aa
Consider source generated documents in code action cleanup
davidwengier Mar 13, 2025
7553622
Introduce a helper method
davidwengier Mar 13, 2025
a41720b
Missed a spot
davidwengier Mar 13, 2025
fd92422
Correctness
davidwengier Mar 13, 2025
3ba9684
Expand doc
davidwengier Mar 19, 2025
c98b091
Add optional parameter to GetRequiredDocument
davidwengier Mar 19, 2025
bbdef4f
Use helper
davidwengier Mar 19, 2025
1f4564f
Doc
davidwengier Mar 19, 2025
5260136
Don't wrap frozen state trackers and create long chains
davidwengier Mar 19, 2025
81cb8a9
Fix another spot to allow something to work for Razor
davidwengier Mar 20, 2025
ed74582
Remove helper method, and just use the async option where needed
davidwengier Mar 20, 2025
5f0dd64
The rest of the PR feedback
davidwengier Mar 20, 2025
fcd2d4d
Update src/Workspaces/Core/Portable/Workspace/Solution/SolutionCompil…
davidwengier Mar 20, 2025
582edc2
Merge branch 'main' into TryingToGetUpdatedSourceGeneratedDocuments
davidwengier Mar 22, 2025
07df90a
PR feedback
davidwengier Mar 23, 2025
5a075b0
Fix
davidwengier Mar 24, 2025
703d071
Docs and throws
davidwengier Mar 24, 2025
95ff002
Merge remote-tracking branch 'upstream/main' into TryingToGetUpdatedS…
CyrusNajmabadi Mar 28, 2025
b5a8879
Simplify
CyrusNajmabadi Mar 28, 2025
e4dfe81
Simplify
CyrusNajmabadi Mar 28, 2025
8d08511
Indentation
CyrusNajmabadi Mar 28, 2025
8bc17a8
Add optimization back
CyrusNajmabadi Mar 28, 2025
837b368
Share code
CyrusNajmabadi Mar 28, 2025
fd086b1
PR Feedback
davidwengier Mar 29, 2025
3a68fc2
Merge remote-tracking branch 'upstream/release/dev17.15' into TryingT…
davidwengier Apr 1, 2025
645611f
PR feedback
davidwengier Apr 1, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ internal sealed override async Task<CompletionDescription> GetDescriptionWorkerA

async Task<CompletionDescription?> TryGetDescriptionAsync(DocumentId documentId)
{
var relatedDocument = document.Project.Solution.GetRequiredDocument(documentId);
var relatedDocument = await document.Project.Solution.GetRequiredDocumentAsync(documentId, includeSourceGenerated: true, cancellationToken).ConfigureAwait(false);
var context = await Utilities.CreateSyntaxContextWithExistingSpeculativeModelAsync(relatedDocument, position, cancellationToken).ConfigureAwait(false) as TSyntaxContext;
Contract.ThrowIfNull(context);
var symbols = await TryGetSymbolsForContextAsync(completionContext: null, context, options, cancellationToken).ConfigureAwait(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,17 @@ await AddTextDocumentEditsAsync(
newSolution.GetDocument,
solution.GetDocument).ConfigureAwait(false);

// Razor calls through our code action handlers with documents that come from the Razor source generator
// Those changes are not visible in project changes, because they happen in the compilation state, so we
// make sure to pull changes out from that too. Changed source generated documents are "frozen" because
// their content no longer comes from the source generator, so thats our cue to know when to handle.
// Changes to non-frozen documents don't need to be included, because changes to the origin document would
// cause the generator to re-generate the same changed content.
await AddTextDocumentEditsAsync(
changes.GetExplicitlyChangedSourceGeneratedDocuments(),
newSolution.GetRequiredSourceGeneratedDocumentForAlreadyGeneratedId,
solution.GetRequiredSourceGeneratedDocumentForAlreadyGeneratedId).ConfigureAwait(false);

// Changed analyzer config documents
await AddTextDocumentEditsAsync(
projectChanges.SelectMany(pc => pc.GetChangedAnalyzerConfigDocuments()),
Expand Down
14 changes: 9 additions & 5 deletions src/Workspaces/Core/Portable/CodeActions/CodeAction_Cleanup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,9 @@ internal static ImmutableArray<DocumentId> GetAllChangedOrAddedDocumentIds(
.GetProjectChanges()
.SelectMany(p => p.GetChangedDocuments(onlyGetDocumentsWithTextChanges: true).Concat(p.GetAddedDocuments()))
.Concat(solutionChanges.GetAddedProjects().SelectMany(p => p.DocumentIds))
.ToImmutableArray();
return documentIds;
.Concat(solutionChanges.GetExplicitlyChangedSourceGeneratedDocuments());

return documentIds.ToImmutableArray();
}

internal static async Task<Solution> CleanSyntaxAndSemanticsAsync(
Expand All @@ -89,7 +90,10 @@ internal static async Task<Solution> CleanSyntaxAndSemanticsAsync(
using var _ = ArrayBuilder<(DocumentId documentId, CodeCleanupOptions options)>.GetInstance(documentIds.Length, out var documentIdsAndOptions);
foreach (var documentId in documentIds)
{
var document = changedSolution.GetRequiredDocument(documentId);
// We include source generated documents here for Razor, which uses them. In that scenario the cleaned document is compared to the
// original to create a set of changes for the LSP client, and part of that will include mapping the changes back to the Razor document,
// so whilst it would seem like cleaning source generated documents is a waste of time, it's sometimes not.
var document = await changedSolution.GetRequiredDocumentAsync(documentId, includeSourceGenerated: true, cancellationToken).ConfigureAwait(false);

// Only care about documents that support syntax. Non-C#/VB files can't be cleaned.
if (document.SupportsSyntaxTree)
Expand All @@ -114,7 +118,7 @@ internal static async ValueTask<Document> CleanupDocumentAsync(Document document
CodeAnalysisProgress.None,
cancellationToken).ConfigureAwait(false);

return cleanedSolution.GetRequiredDocument(document.Id);
return await cleanedSolution.GetRequiredDocumentAsync(document.Id, includeSourceGenerated: true, cancellationToken).ConfigureAwait(false);
}

private static async Task<Solution> RunAllCleanupPassesInOrderAsync(
Expand Down Expand Up @@ -152,7 +156,7 @@ async Task<Solution> RunParallelCleanupPassAsync(
var (documentId, options) = documentIdAndOptions;

// Fetch the current state of the document from this fork of the solution.
var document = solution.GetRequiredDocument(documentId);
var document = await solution.GetRequiredDocumentAsync(documentId, includeSourceGenerated: true, cancellationToken).ConfigureAwait(false);
Contract.ThrowIfFalse(document.SupportsSyntaxTree, "GetDocumentIdsAndOptionsAsync should only be returning documents that support syntax");

// Now, perform the requested cleanup pass on it.
Expand Down
24 changes: 22 additions & 2 deletions src/Workspaces/Core/Portable/Workspace/Solution/Document.cs
Original file line number Diff line number Diff line change
Expand Up @@ -388,13 +388,33 @@ public Document WithSourceCodeKind(SourceCodeKind kind)
/// Creates a new instance of this document updated to have the text specified.
/// </summary>
public Document WithText(SourceText text)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels a bit weird this method isn't just virtual rather than having the base type explicitly deal with the source-generated case, but it's fine I guess.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you commented this before, and I think it's a good idea, but making this virtual would be a public API change that needs to go through that process, right? So a follow up would be a lot easier

=> this.Project.Solution.WithDocumentText(this.Id, text, PreservationMode.PreserveIdentity).GetRequiredDocument(Id);
{
var solution = this.Project.Solution.WithDocumentText(this.Id, text, PreservationMode.PreserveIdentity);

if (Id.IsSourceGenerated)
{
// We just modified the text of the generated document, so it should be available synchronously, and throwing is appropriate if it isn't.
return solution.GetRequiredSourceGeneratedDocumentForAlreadyGeneratedId(Id);
}

return solution.GetRequiredDocument(Id);
}

/// <summary>
/// Creates a new instance of this document updated to have a syntax tree rooted by the specified syntax node.
/// </summary>
public Document WithSyntaxRoot(SyntaxNode root)
=> this.Project.Solution.WithDocumentSyntaxRoot(this.Id, root, PreservationMode.PreserveIdentity).GetRequiredDocument(Id);
{
var solution = this.Project.Solution.WithDocumentSyntaxRoot(this.Id, root, PreservationMode.PreserveIdentity);

if (Id.IsSourceGenerated)
{
// We just modified the text of the generated document, so it should be available synchronously, and throwing is appropriate if it isn't.
return solution.GetRequiredSourceGeneratedDocumentForAlreadyGeneratedId(Id);
}

return solution.GetRequiredDocument(Id);
}

/// <summary>
/// Creates a new instance of this document updated to have the specified name.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ internal DocumentState UpdateTree(SyntaxNode newRoot, PreservationMode mode)
}
}

private VersionStamp GetNewTreeVersionForUpdatedTree(SyntaxNode newRoot, VersionStamp newTextVersion, PreservationMode mode)
protected VersionStamp GetNewTreeVersionForUpdatedTree(SyntaxNode newRoot, VersionStamp newTextVersion, PreservationMode mode)
{
RoslynDebug.Assert(TreeSource != null);

Expand All @@ -566,7 +566,7 @@ private VersionStamp GetNewTreeVersionForUpdatedTree(SyntaxNode newRoot, Version
return oldRoot.IsEquivalentTo(newRoot, topLevel: true) ? oldTreeAndVersion.Version : newTextVersion;
}

private VersionStamp GetNewerVersion()
protected VersionStamp GetNewerVersion()
{
if (TextAndVersionSource.TryGetValue(LoadTextOptions, out var textAndVersion))
{
Expand Down
3 changes: 1 addition & 2 deletions src/Workspaces/Core/Portable/Workspace/Solution/Project.cs
Original file line number Diff line number Diff line change
Expand Up @@ -332,8 +332,7 @@ internal SourceGeneratedDocument GetOrCreateSourceGeneratedDocument(SourceGenera
/// </summary>
/// <remarks>
/// This is only safe to call if you already have seen the SyntaxTree or equivalent that indicates the document state has already been
/// generated. This method exists to implement <see cref="Solution.GetDocument(SyntaxTree?)"/> and is best avoided unless you're doing something
/// similarly tricky like that.
/// generated. This method is best avoided unless you manually ensure you realise the generated document before calling this method.
/// </remarks>
internal SourceGeneratedDocument? TryGetSourceGeneratedDocumentForAlreadyGeneratedId(DocumentId documentId)
{
Expand Down
26 changes: 22 additions & 4 deletions src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1616,7 +1616,8 @@ public Solution WithDocumentText(IEnumerable<DocumentId?> documentIds, SourceTex
internal Document WithFrozenSourceGeneratedDocument(
SourceGeneratedDocumentIdentity documentIdentity, DateTime generationDateTime, SourceText text)
{
var newCompilationState = CompilationState.WithFrozenSourceGeneratedDocuments([(documentIdentity, generationDateTime, text)]);
// SyntaxNode is null here because it will be computed on demand. Other APIs, like Document.WithSyntaxRoot, specify it.
var newCompilationState = CompilationState.WithFrozenSourceGeneratedDocuments([(documentIdentity, generationDateTime, text, syntaxNode: null)]);
var newSolution = WithCompilationState(newCompilationState);

var newDocumentState = newCompilationState.TryGetSourceGeneratedDocumentStateForAlreadyGeneratedId(documentIdentity.DocumentId);
Expand All @@ -1627,7 +1628,7 @@ internal Document WithFrozenSourceGeneratedDocument(
}

internal Solution WithFrozenSourceGeneratedDocuments(ImmutableArray<(SourceGeneratedDocumentIdentity documentIdentity, DateTime generationDateTime, SourceText text)> documents)
=> WithCompilationState(CompilationState.WithFrozenSourceGeneratedDocuments(documents));
=> WithCompilationState(CompilationState.WithFrozenSourceGeneratedDocuments(documents.SelectAsArray(d => (d.documentIdentity, d.generationDateTime, d.text, (SyntaxNode?)null))));

/// <inheritdoc cref="SolutionCompilationState.UpdateSpecificSourceGeneratorExecutionVersions"/>
internal Solution UpdateSpecificSourceGeneratorExecutionVersions(SourceGeneratorExecutionVersionMap sourceGeneratorExecutionVersionMap)
Expand Down Expand Up @@ -1740,9 +1741,26 @@ private void CheckContainsDocument(DocumentId documentId)
throw new ArgumentNullException(nameof(documentId));
}

if (!ContainsDocument(documentId))
// For source generated documents we expect them to be already generated to use any of the APIs that call this
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first thing the WithDocumentTexts etc. methods do is check that the document Id exists in the solution, so that failed when the Id was source generated.

if (documentId.IsSourceGenerated && ContainsSourceGeneratedDocument(documentId))
{
throw new InvalidOperationException(WorkspaceExtensionsResources.The_solution_does_not_contain_the_specified_document);
return;
}

if (ContainsDocument(documentId))
{
return;
}

throw new InvalidOperationException(WorkspaceExtensionsResources.The_solution_does_not_contain_the_specified_document);

bool ContainsSourceGeneratedDocument(DocumentId documentId)
{
var project = this.GetProject(documentId.ProjectId);
if (project is null)
return false;

return project.TryGetSourceGeneratedDocumentForAlreadyGeneratedId(documentId) is not null;
}
}

Expand Down
30 changes: 30 additions & 0 deletions src/Workspaces/Core/Portable/Workspace/Solution/SolutionChanges.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis;

Expand Down Expand Up @@ -80,4 +83,31 @@ public IEnumerable<AnalyzerReference> GetRemovedAnalyzerReferences()
}
}
}

/// <summary>
/// Gets changed source generated document ids that were modified with <see cref="Solution.WithFrozenSourceGeneratedDocuments(System.Collections.Immutable.ImmutableArray{ValueTuple{SourceGeneratedDocumentIdentity, DateTime, Text.SourceText}})"/>
/// </summary>
/// <remarks>
/// It is possible for a source generated document to be "frozen" without it existing in the solution, and in that case
/// this method will not return that document. This only returns changes to source generated documents, hence they had
/// to already be observed in the old solution.
/// </remarks>
internal IEnumerable<DocumentId> GetExplicitlyChangedSourceGeneratedDocuments()
{
if (_newSolution.CompilationState.FrozenSourceGeneratedDocumentStates.IsEmpty)
return [];

using var _ = ArrayBuilder<SourceGeneratedDocumentState>.GetInstance(out var oldStateBuilder);
foreach (var (id, _) in _newSolution.CompilationState.FrozenSourceGeneratedDocumentStates.States)
{
var oldState = _oldSolution.CompilationState.TryGetSourceGeneratedDocumentStateForAlreadyGeneratedId(id);
oldStateBuilder.AddIfNotNull(oldState);
}

var oldStates = new TextDocumentStates<SourceGeneratedDocumentState>(oldStateBuilder);
return _newSolution.CompilationState.FrozenSourceGeneratedDocumentStates.GetChangedStateIds(
oldStates,
ignoreUnchangedContent: true,
ignoreUnchangeableDocuments: false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,18 @@ public ICompilationTracker WithDoNotCreateCreationPolicy()
: new WithFrozenSourceGeneratedDocumentsCompilationTracker(underlyingTracker, _replacementDocumentStates);
}

/// <summary>
/// Updates the frozen source generated documents states being tracked
/// </summary>
/// <remarks>
/// NOTE: This does not merge the states currently tracked, it simply replaces them. If merging is desired, it should be done
/// by the caller.
/// </remarks>
public ICompilationTracker WithReplacementDocumentStates(TextDocumentStates<SourceGeneratedDocumentState> replacementDocumentStates)
{
return new WithFrozenSourceGeneratedDocumentsCompilationTracker(this.UnderlyingTracker, replacementDocumentStates);
}

public async Task<Compilation> GetCompilationAsync(SolutionCompilationState compilationState, CancellationToken cancellationToken)
{
// Fast path if we've definitely already done this before
Expand Down
Loading
Loading