Skip to content

Commit 5cc79ef

Browse files
davidwengierdavidwengier
authored
Port FallbackProjectManager fix to main (#9589)
2 parents fe8b64b + 70179b3 commit 5cc79ef

File tree

5 files changed

+109
-72
lines changed

5 files changed

+109
-72
lines changed

‎src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/DefaultRazorDynamicFileInfoProvider.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,13 @@ public Task RemoveDynamicFileInfoAsync(ProjectId projectId, string? projectFileP
270270
throw new ArgumentNullException(nameof(filePath));
271271
}
272272

273-
_fallbackProjectManager.DynamicFileRemoved(projectId, projectFilePath, filePath);
273+
var projectKey = TryFindProjectKeyForProjectId(projectId);
274+
if (projectKey is not { } razorProjectKey)
275+
{
276+
return Task.CompletedTask;
277+
}
278+
279+
_fallbackProjectManager.DynamicFileRemoved(projectId, razorProjectKey, projectFilePath, filePath);
274280

275281
// ---------------------------------------------------------- NOTE & CAUTION --------------------------------------------------------------
276282
//

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

Lines changed: 57 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@
22
// Licensed under the MIT license. See License.txt in the project root for license information.
33

44
using System;
5-
using System.Collections.Immutable;
65
using System.Composition;
76
using System.IO;
7+
using Microsoft.AspNetCore.Razor.Telemetry;
8+
using Microsoft.AspNetCore.Razor.Utilities;
89
using Microsoft.CodeAnalysis.Razor.Workspaces;
910

1011
namespace Microsoft.CodeAnalysis.Razor.ProjectSystem;
@@ -16,55 +17,58 @@ namespace Microsoft.CodeAnalysis.Razor.ProjectSystem;
1617
/// </summary>
1718
[Shared]
1819
[Export(typeof(FallbackProjectManager))]
19-
internal sealed class FallbackProjectManager
20+
[method: ImportingConstructor]
21+
internal sealed class FallbackProjectManager(
22+
ProjectConfigurationFilePathStore projectConfigurationFilePathStore,
23+
LanguageServerFeatureOptions languageServerFeatureOptions,
24+
ProjectSnapshotManagerAccessor projectSnapshotManagerAccessor,
25+
ITelemetryReporter telemetryReporter)
2026
{
21-
private readonly ProjectConfigurationFilePathStore _projectConfigurationFilePathStore;
22-
private readonly LanguageServerFeatureOptions _languageServerFeatureOptions;
23-
private readonly ProjectSnapshotManagerAccessor _projectSnapshotManagerAccessor;
24-
25-
private ImmutableHashSet<ProjectId> _fallbackProjectIds = ImmutableHashSet<ProjectId>.Empty;
26-
27-
[ImportingConstructor]
28-
public FallbackProjectManager(
29-
ProjectConfigurationFilePathStore projectConfigurationFilePathStore,
30-
LanguageServerFeatureOptions languageServerFeatureOptions,
31-
ProjectSnapshotManagerAccessor projectSnapshotManagerAccessor)
32-
{
33-
_projectConfigurationFilePathStore = projectConfigurationFilePathStore;
34-
_languageServerFeatureOptions = languageServerFeatureOptions;
35-
_projectSnapshotManagerAccessor = projectSnapshotManagerAccessor;
36-
}
27+
private readonly ProjectConfigurationFilePathStore _projectConfigurationFilePathStore = projectConfigurationFilePathStore;
28+
private readonly LanguageServerFeatureOptions _languageServerFeatureOptions = languageServerFeatureOptions;
29+
private readonly ProjectSnapshotManagerAccessor _projectSnapshotManagerAccessor = projectSnapshotManagerAccessor;
30+
private readonly ITelemetryReporter _telemetryReporter = telemetryReporter;
3731

3832
internal void DynamicFileAdded(ProjectId projectId, ProjectKey razorProjectKey, string projectFilePath, string filePath)
3933
{
40-
var project = _projectSnapshotManagerAccessor.Instance.GetLoadedProject(razorProjectKey);
41-
if (_fallbackProjectIds.Contains(projectId))
34+
try
4235
{
43-
// The project might have started as a fallback project, but it might have been upgraded by our getting CPS info
44-
// about it. In that case, leave the CPS bits to do the work
36+
var project = _projectSnapshotManagerAccessor.Instance.GetLoadedProject(razorProjectKey);
4537
if (project is ProjectSnapshot { HostProject: FallbackHostProject })
4638
{
4739
// If this is a fallback project, then Roslyn may not track documents in the project, so these dynamic file notifications
4840
// are the only way to know about files in the project.
4941
AddFallbackDocument(razorProjectKey, filePath, projectFilePath);
5042
}
43+
else if (project is null)
44+
{
45+
// We have been asked to provide dynamic file info, which means there is a .razor or .cshtml file in the project
46+
// but for some reason our project system doesn't know about the project. In these cases (often when people don't
47+
// use the Razor or Web SDK) we spin up a fallback experience for them
48+
AddFallbackProject(projectId, filePath);
49+
}
5150
}
52-
else if (project is null)
51+
catch (Exception ex)
5352
{
54-
// We have been asked to provide dynamic file info, which means there is a .razor or .cshtml file in the project
55-
// but for some reason our project system doesn't know about the project. In these cases (often when people don't
56-
// use the Razor or Web SDK) we spin up a fallback experience for them
57-
AddFallbackProject(projectId, filePath);
53+
_telemetryReporter.ReportFault(ex, "Error while trying to add fallback document to project");
5854
}
5955
}
6056

61-
internal void DynamicFileRemoved(ProjectId projectId, string projectFilePath, string filePath)
57+
internal void DynamicFileRemoved(ProjectId projectId, ProjectKey razorProjectKey, string projectFilePath, string filePath)
6258
{
63-
if (_fallbackProjectIds.Contains(projectId))
59+
try
60+
{
61+
var project = _projectSnapshotManagerAccessor.Instance.GetLoadedProject(razorProjectKey);
62+
if (project is ProjectSnapshot { HostProject: FallbackHostProject })
63+
{
64+
// If this is a fallback project, then Roslyn may not track documents in the project, so these dynamic file notifications
65+
// are the only way to know about files in the project.
66+
RemoveFallbackDocument(projectId, filePath, projectFilePath);
67+
}
68+
}
69+
catch (Exception ex)
6470
{
65-
// If this is a fallback project, then Roslyn may not track documents in the project, so these dynamic file notifications
66-
// are the only way to know about files in the project.
67-
RemoveFallbackDocument(projectId, filePath, projectFilePath);
71+
_telemetryReporter.ReportFault(ex, "Error while trying to remove fallback document from project");
6872
}
6973
}
7074

@@ -82,11 +86,6 @@ private void AddFallbackProject(ProjectId projectId, string filePath)
8286
return;
8387
}
8488

85-
if (!ImmutableInterlocked.Update(ref _fallbackProjectIds, (set, id) => set.Add(id), project.Id))
86-
{
87-
return;
88-
}
89-
9089
var rootNamespace = project.DefaultNamespace;
9190

9291
// We create this as a fallback project so that other parts of the system can reason about them - eg we don't do code
@@ -107,15 +106,27 @@ private void AddFallbackProject(ProjectId projectId, string filePath)
107106
private void AddFallbackDocument(ProjectKey projectKey, string filePath, string projectFilePath)
108107
{
109108
var hostDocument = CreateHostDocument(filePath, projectFilePath);
109+
if (hostDocument is null)
110+
{
111+
return;
112+
}
113+
110114
var textLoader = new FileTextLoader(filePath, defaultEncoding: null);
111115
_projectSnapshotManagerAccessor.Instance.DocumentAdded(projectKey, hostDocument, textLoader);
112116
}
113117

114-
private static HostDocument CreateHostDocument(string filePath, string projectFilePath)
118+
private static HostDocument? CreateHostDocument(string filePath, string projectFilePath)
115119
{
116-
var targetPath = filePath.StartsWith(projectFilePath, FilePathComparison.Instance)
117-
? filePath[projectFilePath.Length..]
118-
: filePath;
120+
// The compiler only supports paths that are relative to the project root, so filter our files
121+
// that don't match
122+
var projectPath = FilePathNormalizer.GetNormalizedDirectoryName(projectFilePath);
123+
var normalizedFilePath = FilePathNormalizer.Normalize(filePath);
124+
if (!normalizedFilePath.StartsWith(projectPath, FilePathComparison.Instance))
125+
{
126+
return null;
127+
}
128+
129+
var targetPath = filePath[projectPath.Length..];
119130
var hostDocument = new HostDocument(filePath, targetPath);
120131
return hostDocument;
121132
}
@@ -135,6 +146,11 @@ private void RemoveFallbackDocument(ProjectId projectId, string filePath, string
135146
}
136147

137148
var hostDocument = CreateHostDocument(filePath, projectFilePath);
149+
if (hostDocument is null)
150+
{
151+
return;
152+
}
153+
138154
_projectSnapshotManagerAccessor.Instance.DocumentRemoved(razorProjectKey, hostDocument);
139155
}
140156

@@ -154,21 +170,4 @@ private void RemoveFallbackDocument(ProjectId projectId, string filePath, string
154170

155171
return project;
156172
}
157-
158-
internal TestAccessor GetTestAccessor()
159-
{
160-
return new TestAccessor(this);
161-
}
162-
163-
internal readonly struct TestAccessor
164-
{
165-
private readonly FallbackProjectManager _instance;
166-
167-
internal TestAccessor(FallbackProjectManager instance)
168-
{
169-
_instance = instance;
170-
}
171-
172-
internal ImmutableHashSet<ProjectId> ProjectIds => _instance._fallbackProjectIds;
173-
}
174173
}

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

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
using System.Linq;
66
using Microsoft.AspNetCore.Razor.Language;
77
using Microsoft.AspNetCore.Razor.LanguageServer;
8+
using Microsoft.AspNetCore.Razor.Telemetry;
9+
using Microsoft.AspNetCore.Razor.Utilities;
810
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
911
using Moq;
1012
using Xunit;
@@ -30,7 +32,7 @@ public FallbackProjectManagerTest(ITestOutputHelper testOutputHelper)
3032

3133
var projectSnapshotManagerAccessor = Mock.Of<ProjectSnapshotManagerAccessor>(a => a.Instance == _projectSnapshotManager, MockBehavior.Strict);
3234

33-
_fallbackProjectManger = new FallbackProjectManager(_projectConfigurationFilePathStore, languageServerFeatureOptions, projectSnapshotManagerAccessor);
35+
_fallbackProjectManger = new FallbackProjectManager(_projectConfigurationFilePathStore, languageServerFeatureOptions, projectSnapshotManagerAccessor, NoOpTelemetryReporter.Instance);
3436
}
3537

3638
[Fact]
@@ -46,7 +48,8 @@ public void DynamicFileAdded_KnownProject_DoesNothing()
4648

4749
_fallbackProjectManger.DynamicFileAdded(projectId, hostProject.Key, SomeProject.FilePath, SomeProjectFile1.FilePath);
4850

49-
Assert.Empty(_fallbackProjectManger.GetTestAccessor().ProjectIds);
51+
var project = Assert.Single(_projectSnapshotManager.GetProjects());
52+
Assert.IsNotType<FallbackHostProject>(((ProjectSnapshot)project).HostProject);
5053
}
5154

5255
[Fact]
@@ -61,9 +64,6 @@ public void DynamicFileAdded_UnknownProject_Adds()
6164

6265
_fallbackProjectManger.DynamicFileAdded(projectId, SomeProject.Key, SomeProject.FilePath, SomeProjectFile1.FilePath);
6366

64-
var actualId = Assert.Single(_fallbackProjectManger.GetTestAccessor().ProjectIds);
65-
Assert.Equal(projectId, actualId);
66-
6767
var project = Assert.Single(_projectSnapshotManager.GetProjects());
6868
Assert.Equal("DisplayName", project.DisplayName);
6969
Assert.Equal("RootNamespace", project.RootNamespace);
@@ -72,6 +72,7 @@ public void DynamicFileAdded_UnknownProject_Adds()
7272

7373
var documentFilePath = Assert.Single(project.DocumentFilePaths);
7474
Assert.Equal(SomeProjectFile1.FilePath, documentFilePath);
75+
Assert.Equal(SomeProjectFile1.TargetPath, project.GetDocument(documentFilePath)!.TargetPath);
7576
}
7677

7778
[Fact]
@@ -110,14 +111,44 @@ public void DynamicFileAdded_TrackedProject_AddsDocuments()
110111

111112
_fallbackProjectManger.DynamicFileAdded(projectId, SomeProject.Key, SomeProject.FilePath, SomeProjectFile2.FilePath);
112113

113-
_fallbackProjectManger.DynamicFileAdded(projectId, SomeProject.Key, SomeProject.FilePath, SomeProjectComponentFile1.FilePath);
114+
_fallbackProjectManger.DynamicFileAdded(projectId, SomeProject.Key, SomeProject.FilePath, SomeProjectNestedComponentFile3.FilePath);
114115

115116
var project = Assert.Single(_projectSnapshotManager.GetProjects());
116117

117118
Assert.Collection(project.DocumentFilePaths.OrderBy(f => f), // DocumentFilePaths comes from a dictionary, so no sort guarantee
118119
f => Assert.Equal(SomeProjectFile1.FilePath, f),
119-
f => Assert.Equal(SomeProjectComponentFile1.FilePath, f),
120-
f => Assert.Equal(SomeProjectFile2.FilePath, f));
120+
f => Assert.Equal(SomeProjectFile2.FilePath, f),
121+
f => Assert.Equal(SomeProjectNestedComponentFile3.FilePath, f));
122+
123+
Assert.Equal(SomeProjectFile1.TargetPath, project.GetDocument(SomeProjectFile1.FilePath)!.TargetPath);
124+
Assert.Equal(SomeProjectFile2.TargetPath, project.GetDocument(SomeProjectFile2.FilePath)!.TargetPath);
125+
// The test data is created with a "\" so when the test runs on linux, and direct string comparison wouldn't work
126+
Assert.True(FilePathNormalizer.FilePathsEquivalent(SomeProjectNestedComponentFile3.TargetPath, project.GetDocument(SomeProjectNestedComponentFile3.FilePath)!.TargetPath));
127+
}
128+
129+
[Fact]
130+
public void DynamicFileAdded_TrackedProject_IgnoresDocumentFromOutsideCone()
131+
{
132+
var projectId = ProjectId.CreateNewId();
133+
var projectInfo = ProjectInfo.Create(projectId, VersionStamp.Default, "DisplayName", "AssemblyName", LanguageNames.CSharp, filePath: SomeProject.FilePath)
134+
.WithCompilationOutputInfo(new CompilationOutputInfo().WithAssemblyPath(Path.Combine(SomeProject.IntermediateOutputPath, "SomeProject.dll")))
135+
.WithDefaultNamespace("RootNamespace");
136+
137+
Workspace.TryApplyChanges(Workspace.CurrentSolution.AddProject(projectInfo));
138+
139+
_fallbackProjectManger.DynamicFileAdded(projectId, SomeProject.Key, SomeProject.FilePath, SomeProjectFile1.FilePath);
140+
141+
// These two represent linked files, or shared project items
142+
_fallbackProjectManger.DynamicFileAdded(projectId, SomeProject.Key, SomeProject.FilePath, AnotherProjectFile2.FilePath);
143+
144+
_fallbackProjectManger.DynamicFileAdded(projectId, SomeProject.Key, SomeProject.FilePath, AnotherProjectComponentFile1.FilePath);
145+
146+
var project = Assert.Single(_projectSnapshotManager.GetProjects());
147+
148+
Assert.Collection(project.DocumentFilePaths,
149+
f => Assert.Equal(SomeProjectFile1.FilePath, f));
150+
151+
Assert.Equal(SomeProjectFile1.TargetPath, project.GetDocument(SomeProjectFile1.FilePath)!.TargetPath);
121152
}
122153

123154
[Fact]

‎src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/Shared/TestProjectData.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,14 @@ static TestProjectData()
2828
SomeProjectFile1 = new HostDocument(Path.Combine(someProjectPath, "File1.cshtml"), "File1.cshtml", FileKinds.Legacy);
2929
SomeProjectFile2 = new HostDocument(Path.Combine(someProjectPath, "File2.cshtml"), "File2.cshtml", FileKinds.Legacy);
3030
SomeProjectImportFile = new HostDocument(Path.Combine(someProjectPath, "_Imports.cshtml"), "_Imports.cshtml", FileKinds.Legacy);
31-
SomeProjectNestedFile3 = new HostDocument(Path.Combine(someProjectPath, "Nested", "File3.cshtml"), "Nested\\File1.cshtml", FileKinds.Legacy);
32-
SomeProjectNestedFile4 = new HostDocument(Path.Combine(someProjectPath, "Nested", "File4.cshtml"), "Nested\\File2.cshtml", FileKinds.Legacy);
31+
SomeProjectNestedFile3 = new HostDocument(Path.Combine(someProjectPath, "Nested", "File3.cshtml"), "Nested\\File3.cshtml", FileKinds.Legacy);
32+
SomeProjectNestedFile4 = new HostDocument(Path.Combine(someProjectPath, "Nested", "File4.cshtml"), "Nested\\File4.cshtml", FileKinds.Legacy);
3333
SomeProjectNestedImportFile = new HostDocument(Path.Combine(someProjectPath, "Nested", "_Imports.cshtml"), "Nested\\_Imports.cshtml", FileKinds.Legacy);
3434
SomeProjectComponentFile1 = new HostDocument(Path.Combine(someProjectPath, "File1.razor"), "File1.razor", FileKinds.Component);
3535
SomeProjectComponentFile2 = new HostDocument(Path.Combine(someProjectPath, "File2.razor"), "File2.razor", FileKinds.Component);
3636
SomeProjectComponentImportFile1 = new HostDocument(Path.Combine(someProjectPath, "_Imports.razor"), "_Imports.razor", FileKinds.Component);
37-
SomeProjectNestedComponentFile3 = new HostDocument(Path.Combine(someProjectPath, "Nested", "File3.razor"), "Nested\\File1.razor", FileKinds.Component);
38-
SomeProjectNestedComponentFile4 = new HostDocument(Path.Combine(someProjectPath, "Nested", "File4.razor"), "Nested\\File2.razor", FileKinds.Component);
37+
SomeProjectNestedComponentFile3 = new HostDocument(Path.Combine(someProjectPath, "Nested", "File3.razor"), "Nested\\File3.razor", FileKinds.Component);
38+
SomeProjectNestedComponentFile4 = new HostDocument(Path.Combine(someProjectPath, "Nested", "File4.razor"), "Nested\\File4.razor", FileKinds.Component);
3939
SomeProjectCshtmlComponentFile5 = new HostDocument(Path.Combine(someProjectPath, "File5.cshtml"), "File5.cshtml", FileKinds.Component);
4040

4141
var anotherProjectPath = Path.Combine(baseDirectory, "AnotherProject");

‎src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/ProjectSystem/DefaultRazorDynamicFileInfoProviderTest.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Threading.Tasks;
99
using Microsoft.AspNetCore.Razor.Language;
1010
using Microsoft.AspNetCore.Razor.LanguageServer;
11+
using Microsoft.AspNetCore.Razor.Telemetry;
1112
using Microsoft.CodeAnalysis;
1213
using Microsoft.CodeAnalysis.Razor;
1314
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
@@ -57,7 +58,7 @@ public DefaultRazorDynamicFileInfoProviderTest(ITestOutputHelper testOutput)
5758
var projectSnapshotManagerAccessor = Mock.Of<ProjectSnapshotManagerAccessor>(a => a.Instance == _projectSnapshotManager, MockBehavior.Strict);
5859

5960
var projectConfigurationFilePathStore = Mock.Of<ProjectConfigurationFilePathStore>(MockBehavior.Strict);
60-
var fallbackProjectManager = new FallbackProjectManager(projectConfigurationFilePathStore, languageServerFeatureOptions, projectSnapshotManagerAccessor);
61+
var fallbackProjectManager = new FallbackProjectManager(projectConfigurationFilePathStore, languageServerFeatureOptions, projectSnapshotManagerAccessor, NoOpTelemetryReporter.Instance);
6162

6263
_provider = new DefaultRazorDynamicFileInfoProvider(_documentServiceFactory, _editorFeatureDetector, filePathService, projectSnapshotManagerAccessor, fallbackProjectManager);
6364
_testAccessor = _provider.GetTestAccessor();

0 commit comments

Comments
 (0)