Skip to content

Commit f5ee183

Browse files
DustinCampbellDustinCampbell
committed
Clean up OpenDocumentGenerator and BackgroundDocumentGeneator a bit
- Change ProjectSnapshot.GetRelatedDocuments(DocumentSnapshot) to GetRelatedDocumentFIlePaths(string) that takes a document file path and returns document file paths. This avoids creating DocumentSnapshots unnecessarily to call this method or for the result. - Make OpenDocumentGenerator and BackgroundDocumentGenerator ProjectManager_Changed effectively match. - Rename BackgroundDocumentGenerator.Enqueue to EnqueueIfNecessary to match OpenDocumentGeneator. - Merge ProjectAdded and ProjectChanged cases in BackgroundDocumentGenerator since they match. - Add ProjectAdded case in OpenDocumentGenerator. This is probably a no-op, but it matches BackgroundDocumentGenerator. - Add _solutionClosing flag to OpenDocumentGenerator to match BackgroundDocumentGenerator.
1 parent 6fc18f2 commit f5ee183

File tree

5 files changed

+78
-72
lines changed

5 files changed

+78
-72
lines changed

‎src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/OpenDocumentGenerator.cs‎

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,12 @@ internal partial class OpenDocumentGenerator : IRazorStartupService, IDisposable
3737
private readonly CancellationTokenSource _disposeTokenSource;
3838
private readonly HashSet<DocumentKey> _workerSet;
3939

40+
// Note: This is likely to always be false. Only the Visual Studio ProjectSnapshotManager
41+
// is notified of the solution opening and closing, so the language server shouldn't
42+
// update this value. However, this may change at some point and keeping the check here means
43+
// that the logic between this class and the Visual Studio BackgroundDocumentGenerator are in sync.
44+
private bool _solutionIsClosing;
45+
4046
public OpenDocumentGenerator(
4147
IEnumerable<IDocumentProcessedListener> listeners,
4248
ProjectSnapshotManager projectManager,
@@ -80,6 +86,12 @@ private async ValueTask ProcessBatchAsync(ImmutableArray<DocumentKey> items, Can
8086
return;
8187
}
8288

89+
// If the solution is closing, avoid any in-progress work.
90+
if (_solutionIsClosing)
91+
{
92+
return;
93+
}
94+
8395
if (!_projectManager.TryGetDocument(key, out var document))
8496
{
8597
continue;
@@ -103,16 +115,20 @@ private async ValueTask ProcessBatchAsync(ImmutableArray<DocumentKey> items, Can
103115

104116
private void ProjectManager_Changed(object? sender, ProjectChangeEventArgs args)
105117
{
106-
// Don't do any work if the solution is closing
118+
// We don't want to do any work on solution close.
107119
if (args.IsSolutionClosing)
108120
{
121+
_solutionIsClosing = true;
109122
return;
110123
}
111124

125+
_solutionIsClosing = false;
126+
112127
_logger.LogDebug($"Got a project change of type {args.Kind} for {args.ProjectKey.Id}");
113128

114129
switch (args.Kind)
115130
{
131+
case ProjectChangeKind.ProjectAdded:
116132
case ProjectChangeKind.ProjectChanged:
117133
{
118134
var newProject = args.Newer.AssumeNotNull();
@@ -136,12 +152,9 @@ private void ProjectManager_Changed(object? sender, ProjectChangeEventArgs args)
136152

137153
EnqueueIfNecessary(new(newProject.Key, documentFilePath));
138154

139-
if (newProject.TryGetDocument(documentFilePath, out var document))
155+
foreach (var relatedDocumentFilePath in newProject.GetRelatedDocumentFilePaths(documentFilePath))
140156
{
141-
foreach (var relatedDocument in newProject.GetRelatedDocuments(document))
142-
{
143-
EnqueueIfNecessary(relatedDocument.Key);
144-
}
157+
EnqueueIfNecessary(new(newProject.Key, relatedDocumentFilePath));
145158
}
146159

147160
break;
@@ -153,16 +166,14 @@ private void ProjectManager_Changed(object? sender, ProjectChangeEventArgs args)
153166
var oldProject = args.Older.AssumeNotNull();
154167
var documentFilePath = args.DocumentFilePath.AssumeNotNull();
155168

156-
if (oldProject.TryGetDocument(documentFilePath, out var document))
169+
// For removals use the old snapshot to find related documents to update if they exist
170+
// in the new snapshot.
171+
172+
foreach (var relatedDocumentFilePath in oldProject.GetRelatedDocumentFilePaths(documentFilePath))
157173
{
158-
foreach (var relatedDocument in oldProject.GetRelatedDocuments(document))
174+
if (newProject.ContainsDocument(relatedDocumentFilePath))
159175
{
160-
var relatedDocumentFilePath = relatedDocument.FilePath;
161-
162-
if (newProject.TryGetDocument(relatedDocumentFilePath, out var newRelatedDocument))
163-
{
164-
EnqueueIfNecessary(newRelatedDocument.Key);
165-
}
176+
EnqueueIfNecessary(new(newProject.Key, relatedDocumentFilePath));
166177
}
167178
}
168179

@@ -171,9 +182,13 @@ private void ProjectManager_Changed(object? sender, ProjectChangeEventArgs args)
171182

172183
case ProjectChangeKind.ProjectRemoved:
173184
{
174-
// No-op. We don't need to enqueue recompilations if the project is being removed
185+
// No-op. We don't need to compile anything if the project is being removed
175186
break;
176187
}
188+
189+
default:
190+
Assumed.Unreachable($"Unknown {nameof(ProjectChangeKind)}: {args.Kind}");
191+
break;
177192
}
178193

179194
void EnqueueIfNecessary(DocumentKey documentKey)

‎src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/ProjectSnapshot.cs‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -98,13 +98,18 @@ bool IProjectSnapshot.TryGetDocument(string filePath, [NotNullWhen(true)] out ID
9898
}
9999

100100
/// <summary>
101-
/// If the provided document is an import document, gets the other documents in the project
102-
/// that include directives specified by the provided document. Otherwise returns an empty
103-
/// list.
101+
/// If the provided document file path references an import document, gets the other
102+
/// documents in the project that include directives specified by the provided document.
103+
/// Otherwise returns an empty array.
104104
/// </summary>
105-
public ImmutableArray<DocumentSnapshot> GetRelatedDocuments(DocumentSnapshot document)
105+
public ImmutableArray<string> GetRelatedDocumentFilePaths(string documentFilePath)
106106
{
107-
var targetPath = document.TargetPath;
107+
if (!_state.Documents.TryGetValue(documentFilePath, out var documentState))
108+
{
109+
return [];
110+
}
111+
112+
var targetPath = documentState.HostDocument.TargetPath;
108113

109114
if (!_state.ImportsToRelatedDocuments.TryGetValue(targetPath, out var relatedDocuments))
110115
{
@@ -113,13 +118,13 @@ public ImmutableArray<DocumentSnapshot> GetRelatedDocuments(DocumentSnapshot doc
113118

114119
lock (_gate)
115120
{
116-
using var builder = new PooledArrayBuilder<DocumentSnapshot>(capacity: relatedDocuments.Count);
121+
using var builder = new PooledArrayBuilder<string>(capacity: relatedDocuments.Count);
117122

118123
foreach (var relatedDocumentFilePath in relatedDocuments)
119124
{
120-
if (TryGetDocument(relatedDocumentFilePath, out var relatedDocument))
125+
if (ContainsDocument(relatedDocumentFilePath))
121126
{
122-
builder.Add(relatedDocument);
127+
builder.Add(relatedDocumentFilePath);
123128
}
124129
}
125130

‎src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/DynamicFiles/BackgroundDocumentGenerator.cs‎

Lines changed: 17 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ protected virtual async Task ProcessDocumentAsync(DocumentSnapshot document, Can
9393
UpdateFileInfo(document);
9494
}
9595

96-
public virtual void Enqueue(DocumentKey documentKey)
96+
public virtual void EnqueueIfNecessary(DocumentKey documentKey)
9797
{
9898
if (_disposeTokenSource.IsCancellationRequested)
9999
{
@@ -125,10 +125,10 @@ protected virtual async ValueTask ProcessBatchAsync(ImmutableArray<DocumentKey>
125125
return;
126126
}
127127

128-
// If the solution is closing, escape any in-progress work
128+
// If the solution is closing, avoid any in-progress work.
129129
if (_solutionIsClosing)
130130
{
131-
break;
131+
return;
132132
}
133133

134134
if (!_projectManager.TryGetDocument(key, out var document))
@@ -184,7 +184,7 @@ private void UpdateFileInfo(DocumentSnapshot document)
184184

185185
private void ProjectManager_Changed(object sender, ProjectChangeEventArgs args)
186186
{
187-
// We don't want to do any work on solution close
187+
// We don't want to do any work on solution close.
188188
if (args.IsSolutionClosing)
189189
{
190190
_solutionIsClosing = true;
@@ -196,24 +196,13 @@ private void ProjectManager_Changed(object sender, ProjectChangeEventArgs args)
196196
switch (args.Kind)
197197
{
198198
case ProjectChangeKind.ProjectAdded:
199-
{
200-
var newProject = args.Newer.AssumeNotNull();
201-
202-
foreach (var documentFilePath in newProject.DocumentFilePaths)
203-
{
204-
Enqueue(new(newProject.Key, documentFilePath));
205-
}
206-
207-
break;
208-
}
209-
210199
case ProjectChangeKind.ProjectChanged:
211200
{
212201
var newProject = args.Newer.AssumeNotNull();
213202

214203
foreach (var documentFilePath in newProject.DocumentFilePaths)
215204
{
216-
Enqueue(new(newProject.Key, documentFilePath));
205+
EnqueueIfNecessary(new(newProject.Key, documentFilePath));
217206
}
218207

219208
break;
@@ -225,32 +214,30 @@ private void ProjectManager_Changed(object sender, ProjectChangeEventArgs args)
225214
var newProject = args.Newer.AssumeNotNull();
226215
var documentFilePath = args.DocumentFilePath.AssumeNotNull();
227216

228-
if (newProject.TryGetDocument(documentFilePath, out var document))
229-
{
230-
Enqueue(document.Key);
217+
EnqueueIfNecessary(new(newProject.Key, documentFilePath));
231218

232-
foreach (var relatedDocument in newProject.GetRelatedDocuments(document))
233-
{
234-
Enqueue(relatedDocument.Key);
235-
}
219+
foreach (var relatedDocumentFilePath in newProject.GetRelatedDocumentFilePaths(documentFilePath))
220+
{
221+
EnqueueIfNecessary(new(newProject.Key, relatedDocumentFilePath));
236222
}
237223

238224
break;
239225
}
240226

241227
case ProjectChangeKind.DocumentRemoved:
242228
{
243-
// For removals use the old snapshot to find the removed document, so we can figure out
244-
// what the imports were in the new snapshot.
245229
var newProject = args.Newer.AssumeNotNull();
246230
var oldProject = args.Older.AssumeNotNull();
247231
var documentFilePath = args.DocumentFilePath.AssumeNotNull();
248232

249-
if (oldProject.TryGetDocument(documentFilePath, out var document))
233+
// For removals use the old snapshot to find related documents to update if they exist
234+
// in the new snapshot.
235+
236+
foreach (var relatedDocumentFilePath in oldProject.GetRelatedDocumentFilePaths(documentFilePath))
250237
{
251-
foreach (var relatedDocument in newProject.GetRelatedDocuments(document))
238+
if (newProject.ContainsDocument(relatedDocumentFilePath))
252239
{
253-
Enqueue(relatedDocument.Key);
240+
EnqueueIfNecessary(new(newProject.Key, relatedDocumentFilePath));
254241
}
255242
}
256243

@@ -264,7 +251,8 @@ private void ProjectManager_Changed(object sender, ProjectChangeEventArgs args)
264251
}
265252

266253
default:
267-
throw new InvalidOperationException($"Unknown ProjectChangeKind {args.Kind}");
254+
Assumed.Unreachable($"Unknown {nameof(ProjectChangeKind)}: {args.Kind}");
255+
break;
268256
}
269257
}
270258
}

‎src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/ProjectSystem/ProjectSnapshotTest.cs‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -57,16 +57,15 @@ public void ProjectSnapshot_CachesDocumentSnapshots()
5757
public void GetRelatedDocuments_NonImportDocument_ReturnsEmpty()
5858
{
5959
// Arrange
60-
var state = ProjectState.Create(s_hostProject, CompilerOptions, ProjectEngineFactoryProvider)
60+
var state = ProjectState
61+
.Create(s_hostProject, CompilerOptions, ProjectEngineFactoryProvider)
6162
.WithProjectWorkspaceState(s_projectWorkspaceState)
6263
.AddEmptyDocument(s_documents[0]);
6364

64-
var snapshot = new ProjectSnapshot(state);
65-
66-
var document = snapshot.GetRequiredDocument(s_documents[0].FilePath);
65+
var project = new ProjectSnapshot(state);
6766

6867
// Act
69-
var documents = snapshot.GetRelatedDocuments(document);
68+
var documents = project.GetRelatedDocumentFilePaths(s_documents[0].FilePath);
7069

7170
// Assert
7271
Assert.Empty(documents);
@@ -76,23 +75,22 @@ public void GetRelatedDocuments_NonImportDocument_ReturnsEmpty()
7675
public void GetRelatedDocuments_ImportDocument_ReturnsRelated()
7776
{
7877
// Arrange
79-
var state = ProjectState.Create(s_hostProject, CompilerOptions, ProjectEngineFactoryProvider)
78+
var state = ProjectState
79+
.Create(s_hostProject, CompilerOptions, ProjectEngineFactoryProvider)
8080
.WithProjectWorkspaceState(s_projectWorkspaceState)
8181
.AddEmptyDocument(s_documents[0])
8282
.AddEmptyDocument(s_documents[1])
8383
.AddEmptyDocument(TestProjectData.SomeProjectImportFile);
8484

85-
var snapshot = new ProjectSnapshot(state);
86-
87-
var document = snapshot.GetRequiredDocument(TestProjectData.SomeProjectImportFile.FilePath);
85+
var project = new ProjectSnapshot(state);
8886

8987
// Act
90-
var documents = snapshot.GetRelatedDocuments(document);
88+
var relatedDocumentFilePaths = project.GetRelatedDocumentFilePaths(TestProjectData.SomeProjectImportFile.FilePath);
9189

9290
// Assert
9391
Assert.Collection(
94-
documents.OrderBy(d => d.FilePath),
95-
d => Assert.Equal(s_documents[0].FilePath, d.FilePath),
96-
d => Assert.Equal(s_documents[1].FilePath, d.FilePath));
92+
relatedDocumentFilePaths.Sort(),
93+
path => Assert.Equal(s_documents[0].FilePath, path),
94+
path => Assert.Equal(s_documents[1].FilePath, path));
9795
}
9896
}

‎src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/DocumentGenerator/BackgroundDocumentGeneratorTest.cs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ await projectManager.UpdateAsync(updater =>
131131
using var generator = new TestBackgroundDocumentGenerator(projectManager, s_fallbackProjectManager, _dynamicFileInfoProvider, loggerFactoryMock.Object);
132132

133133
// Act & Assert
134-
generator.Enqueue(documentKey1);
134+
generator.EnqueueIfNecessary(documentKey1);
135135

136136
await generator.WaitUntilCurrentBatchCompletesAsync();
137137
}
@@ -172,7 +172,7 @@ await projectManager.UpdateAsync(updater =>
172172
using var generator = new TestBackgroundDocumentGenerator(projectManager, s_fallbackProjectManager, _dynamicFileInfoProvider, loggerFactoryMock.Object);
173173

174174
// Act & Assert
175-
generator.Enqueue(documentKey1);
175+
generator.EnqueueIfNecessary(documentKey1);
176176

177177
await generator.WaitUntilCurrentBatchCompletesAsync();
178178
}
@@ -198,7 +198,7 @@ await projectManager.UpdateAsync(updater =>
198198
// Act & Assert
199199

200200
// Enqueue some work.
201-
generator.Enqueue(documentKey1);
201+
generator.EnqueueIfNecessary(documentKey1);
202202

203203
// Wait for the work to complete.
204204
await generator.WaitUntilCurrentBatchCompletesAsync();
@@ -237,7 +237,7 @@ await projectManager.UpdateAsync(updater =>
237237
// Act & Assert
238238

239239
// First, enqueue some work.
240-
generator.Enqueue(documentKey1);
240+
generator.EnqueueIfNecessary(documentKey1);
241241

242242
// Wait for the work to complete.
243243
await generator.WaitUntilCurrentBatchCompletesAsync();
@@ -246,7 +246,7 @@ await projectManager.UpdateAsync(updater =>
246246
Assert.Single(generator.CompletedWork, documentKey1);
247247

248248
// Enqueue more work.
249-
generator.Enqueue(documentKey2);
249+
generator.EnqueueIfNecessary(documentKey2);
250250

251251
// Wait for the work to complete.
252252
await generator.WaitUntilCurrentBatchCompletesAsync();
@@ -397,11 +397,11 @@ protected override async ValueTask ProcessBatchAsync(ImmutableArray<DocumentKey>
397397
await base.ProcessBatchAsync(items, token);
398398
}
399399

400-
public override void Enqueue(DocumentKey documentKey)
400+
public override void EnqueueIfNecessary(DocumentKey documentKey)
401401
{
402402
PendingWork.Add(documentKey);
403403

404-
base.Enqueue(documentKey);
404+
base.EnqueueIfNecessary(documentKey);
405405
}
406406

407407
protected override Task ProcessDocumentAsync(DocumentSnapshot document, CancellationToken cancellationToken)

0 commit comments

Comments
 (0)