Skip to content

Commit 411981a

Browse files
authored
Perform generator retry logic when there are multiple projects in the solution (#12338)
Fixes #12337 Turns out we shouldn't have ever had the notion of "retry projects", but rather "retry solutions". Or maybe I just thought the requests would come in linearly enough that the lock in the manager would be sufficient? 🤷‍♂️ Sadly, whilst this fixes the error bars, it doesn't do anything about the errors that are reported when we lose the race. That will either come later in another fix (for #12316 perhaps???), or we can just turn on the generator by default which makes this fix, and the whole retry notion, redundant.
2 parents 2e137ac + 2907bca commit 411981a

File tree

5 files changed

+84
-10
lines changed

5 files changed

+84
-10
lines changed

src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/RemoteProjectSnapshot.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ public RemoteDocumentSnapshot GetDocument(TextDocument document)
7373
// of re-running the generator (a "retry project") and the document they passed in is from the original project
7474
// because it comes from the devenv side, so we can be a little lenient here. Since retry projects only exist
7575
// early in a session, we should still catch coding errors with even basic manual testing.
76-
document = _project.IsRetryProject() && _project.GetAdditionalDocument(document.Id) is { } retryDocument
76+
document = _project.Solution.Projects.Any(p => p.IsRetryProject()) &&
77+
_project.GetAdditionalDocument(document.Id) is { } retryDocument
7778
? retryDocument
7879
: throw new ArgumentException(SR.Document_does_not_belong_to_this_project, nameof(document));
7980
}

src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/RemoteSolutionSnapshot.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ public RemoteProjectSnapshot GetProject(Project project)
3030
{
3131
// It might look like we don't know about this project, but we might have actually seen it before but been forced
3232
// to retry running source generators in it, so we handle that specific case here.
33-
project = _solution.GetProject(project.Id) is { } retryProject && retryProject.IsRetryProject()
33+
project = _solution.GetProject(project.Id) is { } retryProject &&
34+
retryProject.Solution.Projects.Any(p => p.IsRetryProject())
3435
? retryProject
3536
: throw new ArgumentException(SR.Project_does_not_belong_to_this_solution, nameof(project));
3637
}

src/Razor/test/Microsoft.AspNetCore.Razor.Test.Common.Cohosting/CohostTestBase.cs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Diagnostics;
6+
using System.Diagnostics.CodeAnalysis;
67
using System.IO;
78
using System.Linq;
89
using System.Text;
@@ -140,9 +141,14 @@ protected virtual TextDocument CreateProjectAndRazorDocument(
140141
}
141142

142143
protected static TextDocument CreateProjectAndRazorDocument(CodeAnalysis.Workspace workspace, ProjectId projectId, bool miscellaneousFile, DocumentId documentId, string documentFilePath, string contents, (string fileName, string contents)[]? additionalFiles, bool inGlobalNamespace)
144+
{
145+
return AddProjectAndRazorDocument(workspace.CurrentSolution, TestProjectData.SomeProject.FilePath, projectId, miscellaneousFile, documentId, documentFilePath, contents, additionalFiles, inGlobalNamespace);
146+
}
147+
148+
protected static TextDocument AddProjectAndRazorDocument(Solution solution, [DisallowNull] string? projectFilePath, ProjectId projectId, bool miscellaneousFile, DocumentId documentId, string documentFilePath, string contents, (string fileName, string contents)[]? additionalFiles, bool inGlobalNamespace)
143149
{
144150
// We simulate a miscellaneous file project by not having a project file path.
145-
var projectFilePath = miscellaneousFile ? null : TestProjectData.SomeProject.FilePath;
151+
projectFilePath = miscellaneousFile ? null : projectFilePath;
146152
var projectName = miscellaneousFile ? "" : Path.GetFileNameWithoutExtension(projectFilePath).AssumeNotNull();
147153

148154
var sgAssembly = typeof(RazorSourceGenerator).Assembly;
@@ -167,7 +173,7 @@ protected static TextDocument CreateProjectAndRazorDocument(CodeAnalysis.Workspa
167173
projectInfo = projectInfo.WithDefaultNamespace(TestProjectData.SomeProject.RootNamespace);
168174
}
169175

170-
var solution = workspace.CurrentSolution.AddProject(projectInfo);
176+
solution = solution.AddProject(projectInfo);
171177

172178
solution = solution
173179
.AddAdditionalDocument(
@@ -221,7 +227,7 @@ @using Microsoft.AspNetCore.Components.Web
221227

222228
var projectBasePath = Path.GetDirectoryName(projectFilePath).AssumeNotNull();
223229
// Normally MS Build targets do this for us, but we're on our own!
224-
foreach (var razorDocument in solution.Projects.Single().AdditionalDocuments)
230+
foreach (var razorDocument in solution.GetRequiredProject(projectId).AdditionalDocuments)
225231
{
226232
if (razorDocument.FilePath is not null &&
227233
razorDocument.FilePath.StartsWith(projectBasePath))

src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostEndpointTestBase.cs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
using Microsoft.CodeAnalysis.Razor.Workspaces;
1616
using Microsoft.CodeAnalysis.Razor.Workspaces.Settings;
1717
using Microsoft.VisualStudio.Composition;
18+
using Microsoft.VisualStudio.Language.CodeCleanUp;
1819
using Microsoft.VisualStudio.Razor.Settings;
1920
using Xunit;
2021
using Xunit.Abstractions;
@@ -87,6 +88,18 @@ private protected override RemoteClientLSPInitializationOptions GetRemoteClientL
8788
private protected virtual TestComposition ConfigureRoslynDevenvComposition(TestComposition composition)
8889
=> composition;
8990

91+
protected TextDocument CreateProjectAndRazorDocument(
92+
string contents,
93+
bool remoteOnly)
94+
{
95+
if (remoteOnly)
96+
{
97+
return base.CreateProjectAndRazorDocument(contents, fileKind: null, documentFilePath: null, additionalFiles: null, inGlobalNamespace: false, miscellaneousFile: false);
98+
}
99+
100+
return this.CreateProjectAndRazorDocument(contents);
101+
}
102+
90103
protected override TextDocument CreateProjectAndRazorDocument(
91104
string contents,
92105
RazorFileKind? fileKind = null,

src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/RetryProjectTest.cs

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33

44
using System;
55
using System.Threading.Tasks;
6+
using Microsoft.AspNetCore.Razor;
67
using Microsoft.AspNetCore.Razor.Language;
78
using Microsoft.AspNetCore.Razor.Test.Common;
9+
using Microsoft.CodeAnalysis;
810
using Microsoft.CodeAnalysis.ExternalAccess.Razor;
911
using Microsoft.CodeAnalysis.Remote.Razor.ProjectSystem;
1012
using Microsoft.CodeAnalysis.Remote.Razor.Resources;
@@ -29,7 +31,6 @@ public async Task HoverRequest_ReturnsResults()
2931
var document = CreateProjectAndRazorDocument(input.Text, RazorFileKind.Component);
3032

3133
// Make sure the source generator has been run while cohosting is off, to simular Roslyn winning the initialization race
32-
//var compilation = await document.Project.GetCompilationAsync(DisposalToken);
3334
Assert.Empty(await document.Project.GetSourceGeneratedDocumentsAsync(DisposalToken));
3435

3536
// Now turn the source generator on, to simulate Razor starting up and initializing OOP
@@ -51,6 +52,62 @@ public async Task HoverRequest_ReturnsResults()
5152
Assert.NotNull(hoverResult);
5253
}
5354

55+
[Fact]
56+
public async Task HoverRequest_MultipleProjects_ReturnsResults()
57+
{
58+
RazorCohostingOptions.UseRazorCohostServer = false;
59+
60+
TestCode input = """
61+
<div></div>
62+
@System.DateTi$$me.Now
63+
""";
64+
// Specify remoteOnly because our test infrastructure isn't set up to mutate solutions otherwise
65+
var document = CreateProjectAndRazorDocument(input.Text, remoteOnly: true);
66+
67+
// Now we create another document, in another project
68+
TestCode otherInput = """
69+
@System.DateTi$$me.Now
70+
<div></div>
71+
""";
72+
var projectId = ProjectId.CreateNewId(debugName: TestProjectData.SomeProject.DisplayName);
73+
var documentFilePath = TestProjectData.AnotherProjectComponentFile1.FilePath;
74+
var documentId = DocumentId.CreateNewId(projectId, debugName: documentFilePath);
75+
var otherDocument = AddProjectAndRazorDocument(document.Project.Solution, TestProjectData.AnotherProject.FilePath, projectId, miscellaneousFile: false, documentId, documentFilePath, otherInput.Text, additionalFiles: null, inGlobalNamespace: false);
76+
77+
// Make sure we have the document from our new fork
78+
document = otherDocument.Project.Solution.GetAdditionalDocument(document.Id).AssumeNotNull();
79+
80+
// Make sure the source generator has been run while cohosting is off, to simular Roslyn winning the initialization race
81+
Assert.Empty(await document.Project.GetSourceGeneratedDocumentsAsync(DisposalToken));
82+
Assert.Empty(await otherDocument.Project.GetSourceGeneratedDocumentsAsync(DisposalToken));
83+
84+
// Now turn the source generator on, to simulate Razor starting up and initializing OOP
85+
RazorCohostingOptions.UseRazorCohostServer = true;
86+
87+
var requestInvoker = new TestHtmlRequestInvoker();
88+
var endpoint = new CohostHoverEndpoint(IncompatibleProjectService, RemoteServiceInvoker, requestInvoker);
89+
90+
// Making this request will cause our solution to be redirected, and project 1 to be a retry project
91+
await MakeHoverRequestAsync(input, document);
92+
93+
// Making this request will use our redirect solution, but project 2 is not a retry project, so would fail normally
94+
await MakeHoverRequestAsync(otherInput, otherDocument);
95+
96+
async Task MakeHoverRequestAsync(TestCode input, TextDocument document)
97+
{
98+
var inputText = await document.GetTextAsync(DisposalToken);
99+
var linePosition = inputText.GetLinePosition(input.Position);
100+
var textDocumentPositionParams = new TextDocumentPositionParams
101+
{
102+
Position = LspFactory.CreatePosition(linePosition),
103+
TextDocument = new TextDocumentIdentifier { DocumentUri = document.CreateDocumentUri() },
104+
};
105+
106+
var hoverResult = await endpoint.GetTestAccessor().HandleRequestAsync(textDocumentPositionParams, document, DisposalToken);
107+
Assert.NotNull(hoverResult);
108+
}
109+
}
110+
54111
[Fact]
55112
public async Task GetRequiredCodeDocument_SucceedsAndReturnsRetryProject()
56113
{
@@ -62,7 +119,6 @@ public async Task GetRequiredCodeDocument_SucceedsAndReturnsRetryProject()
62119
var document = CreateProjectAndRazorDocument(input.Text, RazorFileKind.Component);
63120

64121
// Make sure the source generator has been run while cohosting is off, to simulate Roslyn winning the initialization race
65-
//var compilation = await document.Project.GetCompilationAsync(DisposalToken);
66122
Assert.Empty(await document.Project.GetSourceGeneratedDocumentsAsync(DisposalToken));
67123

68124
// Now turn the source generator on, to simulate Razor starting up and initializing OOP
@@ -89,7 +145,6 @@ public async Task GetSnapshot_ReturnsDifferentSolutionSnapshots()
89145
var document = CreateProjectAndRazorDocument(input.Text, RazorFileKind.Component);
90146

91147
// Make sure the source generator has been run while cohosting is off, to simular Roslyn winning the initialization race
92-
//var compilation = await document.Project.GetCompilationAsync(DisposalToken);
93148
Assert.Empty(await document.Project.GetSourceGeneratedDocumentsAsync(DisposalToken));
94149

95150
var snapshotManager = OOPExportProvider.GetExportedValue<RemoteSnapshotManager>();
@@ -119,7 +174,6 @@ public async Task CohostingOff_Throws()
119174
var document = CreateProjectAndRazorDocument(input.Text, RazorFileKind.Component);
120175

121176
// Make sure the source generator has been run while cohosting is off, to simular Roslyn winning the initialization race
122-
//var compilation = await document.Project.GetCompilationAsync(DisposalToken);
123177
Assert.Empty(await document.Project.GetSourceGeneratedDocumentsAsync(DisposalToken));
124178

125179
var snapshotManager = OOPExportProvider.GetExportedValue<RemoteSnapshotManager>();
@@ -143,7 +197,6 @@ public async Task ProjectChanges_NotRetryProject()
143197
var document = CreateProjectAndRazorDocument(input.Text, RazorFileKind.Component);
144198

145199
// Make sure the source generator has been run while cohosting is off, to simular Roslyn winning the initialization race
146-
//var compilation = await document.Project.GetCompilationAsync(DisposalToken);
147200
Assert.Empty(await document.Project.GetSourceGeneratedDocumentsAsync(DisposalToken));
148201

149202
var snapshotManager = OOPExportProvider.GetExportedValue<RemoteSnapshotManager>();

0 commit comments

Comments
 (0)