Fix fallback project manager in the presence of linked files or shared projects (#9582)

davidwengier · web-flow · commit cba9a0bf8158 · 2023-11-29T23:49:32.000-08:00
* Add telemetry reporter dependency to the fallback project manager

* Make sure we never bring down project loading if there is a problem with the fallback bits

* Only support fallback documents that are in the project cone

* Make things more efficient, and cross platform

* Validate target paths

* Fix tests linux and mac (even though the fallback project manager never runs on linux or mac)
diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/FallbackProjectManager.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/FallbackProjectManager.cs
@@ -5,6 +5,8 @@
 using System.Collections.Immutable;
 using System.Composition;
 using System.IO;
+using Microsoft.AspNetCore.Razor.Telemetry;
+using Microsoft.AspNetCore.Razor.Utilities;
 using Microsoft.CodeAnalysis.Razor.Workspaces;
 
 namespace Microsoft.CodeAnalysis.Razor.ProjectSystem;
@@ -16,49 +18,58 @@ namespace Microsoft.CodeAnalysis.Razor.ProjectSystem;
 /// </summary>
 [Shared]
 [Export(typeof(FallbackProjectManager))]
-internal sealed class FallbackProjectManager
+[method: ImportingConstructor]
+internal sealed class FallbackProjectManager(
+    ProjectConfigurationFilePathStore projectConfigurationFilePathStore,
+    LanguageServerFeatureOptions languageServerFeatureOptions,
+    ProjectSnapshotManagerAccessor projectSnapshotManagerAccessor,
+    ITelemetryReporter telemetryReporter)
 {
-    private readonly ProjectConfigurationFilePathStore _projectConfigurationFilePathStore;
-    private readonly LanguageServerFeatureOptions _languageServerFeatureOptions;
-    private readonly ProjectSnapshotManagerAccessor _projectSnapshotManagerAccessor;
+    private readonly ProjectConfigurationFilePathStore _projectConfigurationFilePathStore = projectConfigurationFilePathStore;
+    private readonly LanguageServerFeatureOptions _languageServerFeatureOptions = languageServerFeatureOptions;
+    private readonly ProjectSnapshotManagerAccessor _projectSnapshotManagerAccessor = projectSnapshotManagerAccessor;
+    private readonly ITelemetryReporter _telemetryReporter = telemetryReporter;
 
     private ImmutableHashSet<ProjectId> _fallbackProjectIds = ImmutableHashSet<ProjectId>.Empty;
 
-    [ImportingConstructor]
-    public FallbackProjectManager(
-        ProjectConfigurationFilePathStore projectConfigurationFilePathStore,
-        LanguageServerFeatureOptions languageServerFeatureOptions,
-        ProjectSnapshotManagerAccessor projectSnapshotManagerAccessor)
-    {
-        _projectConfigurationFilePathStore = projectConfigurationFilePathStore;
-        _languageServerFeatureOptions = languageServerFeatureOptions;
-        _projectSnapshotManagerAccessor = projectSnapshotManagerAccessor;
-    }
-
     internal void DynamicFileAdded(ProjectId projectId, ProjectKey razorProjectKey, string projectFilePath, string filePath)
     {
-        if (_fallbackProjectIds.Contains(projectId))
+        try
         {
-            // If this is a fallback project, then Roslyn may not track documents in the project, so these dynamic file notifications
-            // are the only way to know about files in the project.
-            AddFallbackDocument(razorProjectKey, filePath, projectFilePath);
+            if (_fallbackProjectIds.Contains(projectId))
+            {
+                // If this is a fallback project, then Roslyn may not track documents in the project, so these dynamic file notifications
+                // are the only way to know about files in the project.
+                AddFallbackDocument(razorProjectKey, filePath, projectFilePath);
+            }
+            else if (_projectSnapshotManagerAccessor.Instance.GetLoadedProject(razorProjectKey) is null)
+            {
+                // We have been asked to provide dynamic file info, which means there is a .razor or .cshtml file in the project
+                // but for some reason our project system doesn't know about the project. In these cases (often when people don't
+                // use the Razor or Web SDK) we spin up a fallback experience for them
+                AddFallbackProject(projectId, filePath);
+            }
         }
-        else if (_projectSnapshotManagerAccessor.Instance.GetLoadedProject(razorProjectKey) is null)
+        catch (Exception ex)
         {
-            // We have been asked to provide dynamic file info, which means there is a .razor or .cshtml file in the project
-            // but for some reason our project system doesn't know about the project. In these cases (often when people don't
-            // use the Razor or Web SDK) we spin up a fallback experience for them
-            AddFallbackProject(projectId, filePath);
+            _telemetryReporter.ReportFault(ex, "Error while trying to add fallback document to project");
         }
     }
 
     internal void DynamicFileRemoved(ProjectId projectId, string projectFilePath, string filePath)
     {
-        if (_fallbackProjectIds.Contains(projectId))
+        try
+        {
+            if (_fallbackProjectIds.Contains(projectId))
+            {
+                // If this is a fallback project, then Roslyn may not track documents in the project, so these dynamic file notifications
+                // are the only way to know about files in the project.
+                RemoveFallbackDocument(projectId, filePath, projectFilePath);
+            }
+        }
+        catch (Exception ex)
         {
-            // If this is a fallback project, then Roslyn may not track documents in the project, so these dynamic file notifications
-            // are the only way to know about files in the project.
-            RemoveFallbackDocument(projectId, filePath, projectFilePath);
+            _telemetryReporter.ReportFault(ex, "Error while trying to remove fallback document from project");
         }
     }
 
@@ -101,15 +112,27 @@ private void AddFallbackProject(ProjectId projectId, string filePath)
     private void AddFallbackDocument(ProjectKey projectKey, string filePath, string projectFilePath)
     {
         var hostDocument = CreateHostDocument(filePath, projectFilePath);
+        if (hostDocument is null)
+        {
+            return;
+        }
+
         var textLoader = new FileTextLoader(filePath, defaultEncoding: null);
         _projectSnapshotManagerAccessor.Instance.DocumentAdded(projectKey, hostDocument, textLoader);
     }
 
-    private static HostDocument CreateHostDocument(string filePath, string projectFilePath)
+    private static HostDocument? CreateHostDocument(string filePath, string projectFilePath)
     {
-        var targetPath = filePath.StartsWith(projectFilePath, FilePathComparison.Instance)
-            ? filePath[projectFilePath.Length..]
-            : filePath;
+        // The compiler only supports paths that are relative to the project root, so filter our files
+        // that don't match
+        var projectPath = FilePathNormalizer.GetNormalizedDirectoryName(projectFilePath);
+        var normalizedFilePath = FilePathNormalizer.Normalize(filePath);
+        if (!normalizedFilePath.StartsWith(projectPath, FilePathComparison.Instance))
+        {
+            return null;
+        }
+
+        var targetPath = filePath[projectPath.Length..];
         var hostDocument = new HostDocument(filePath, targetPath);
         return hostDocument;
     }
@@ -129,6 +152,11 @@ private void RemoveFallbackDocument(ProjectId projectId, string filePath, string
         }
 
         var hostDocument = CreateHostDocument(filePath, projectFilePath);
+        if (hostDocument is null)
+        {
+            return;
+        }
+
         _projectSnapshotManagerAccessor.Instance.DocumentRemoved(razorProjectKey, hostDocument);
     }
 
diff --git a/src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/ProjectSystem/FallbackProjectManagerTest.cs b/src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/ProjectSystem/FallbackProjectManagerTest.cs
@@ -5,6 +5,8 @@
 using System.Linq;
 using Microsoft.AspNetCore.Razor.Language;
 using Microsoft.AspNetCore.Razor.LanguageServer;
+using Microsoft.AspNetCore.Razor.Telemetry;
+using Microsoft.AspNetCore.Razor.Utilities;
 using Microsoft.CodeAnalysis.Razor.ProjectSystem;
 using Moq;
 using Xunit;
@@ -30,7 +32,7 @@ public FallbackProjectManagerTest(ITestOutputHelper testOutputHelper)
 
         var projectSnapshotManagerAccessor = Mock.Of<ProjectSnapshotManagerAccessor>(a => a.Instance == _projectSnapshotManager, MockBehavior.Strict);
 
-        _fallbackProjectManger = new FallbackProjectManager(_projectConfigurationFilePathStore, languageServerFeatureOptions, projectSnapshotManagerAccessor);
+        _fallbackProjectManger = new FallbackProjectManager(_projectConfigurationFilePathStore, languageServerFeatureOptions, projectSnapshotManagerAccessor, NoOpTelemetryReporter.Instance);
     }
 
     [Fact]
@@ -71,6 +73,7 @@ public void DynamicFileAdded_UnknownProject_Adds()
 
         var documentFilePath = Assert.Single(project.DocumentFilePaths);
         Assert.Equal(SomeProjectFile1.FilePath, documentFilePath);
+        Assert.Equal(SomeProjectFile1.TargetPath, project.GetDocument(documentFilePath)!.TargetPath);
     }
 
     [Fact]
@@ -109,14 +112,44 @@ public void DynamicFileAdded_TrackedProject_AddsDocuments()
 
         _fallbackProjectManger.DynamicFileAdded(projectId, SomeProject.Key, SomeProject.FilePath, SomeProjectFile2.FilePath);
 
-        _fallbackProjectManger.DynamicFileAdded(projectId, SomeProject.Key, SomeProject.FilePath, SomeProjectComponentFile1.FilePath);
+        _fallbackProjectManger.DynamicFileAdded(projectId, SomeProject.Key, SomeProject.FilePath, SomeProjectNestedComponentFile3.FilePath);
 
         var project = Assert.Single(_projectSnapshotManager.GetProjects());
 
         Assert.Collection(project.DocumentFilePaths.OrderBy(f => f), // DocumentFilePaths comes from a dictionary, so no sort guarantee
             f => Assert.Equal(SomeProjectFile1.FilePath, f),
-            f => Assert.Equal(SomeProjectComponentFile1.FilePath, f),
-            f => Assert.Equal(SomeProjectFile2.FilePath, f));
+            f => Assert.Equal(SomeProjectFile2.FilePath, f),
+            f => Assert.Equal(SomeProjectNestedComponentFile3.FilePath, f));
+
+        Assert.Equal(SomeProjectFile1.TargetPath, project.GetDocument(SomeProjectFile1.FilePath)!.TargetPath);
+        Assert.Equal(SomeProjectFile2.TargetPath, project.GetDocument(SomeProjectFile2.FilePath)!.TargetPath);
+        // The test data is created with a "\" so when the test runs on linux, and direct string comparison wouldn't work
+        Assert.True(FilePathNormalizer.FilePathsEquivalent(SomeProjectNestedComponentFile3.TargetPath, project.GetDocument(SomeProjectNestedComponentFile3.FilePath)!.TargetPath));
+    }
+
+    [Fact]
+    public void DynamicFileAdded_TrackedProject_IgnoresDocumentFromOutsideCone()
+    {
+        var projectId = ProjectId.CreateNewId();
+        var projectInfo = ProjectInfo.Create(projectId, VersionStamp.Default, "DisplayName", "AssemblyName", LanguageNames.CSharp, filePath: SomeProject.FilePath)
+            .WithCompilationOutputInfo(new CompilationOutputInfo().WithAssemblyPath(Path.Combine(SomeProject.IntermediateOutputPath, "SomeProject.dll")))
+            .WithDefaultNamespace("RootNamespace");
+
+        Workspace.TryApplyChanges(Workspace.CurrentSolution.AddProject(projectInfo));
+
+        _fallbackProjectManger.DynamicFileAdded(projectId, SomeProject.Key, SomeProject.FilePath, SomeProjectFile1.FilePath);
+
+        // These two represent linked files, or shared project items
+        _fallbackProjectManger.DynamicFileAdded(projectId, SomeProject.Key, SomeProject.FilePath, AnotherProjectFile2.FilePath);
+
+        _fallbackProjectManger.DynamicFileAdded(projectId, SomeProject.Key, SomeProject.FilePath, AnotherProjectComponentFile1.FilePath);
+
+        var project = Assert.Single(_projectSnapshotManager.GetProjects());
+
+        Assert.Collection(project.DocumentFilePaths,
+            f => Assert.Equal(SomeProjectFile1.FilePath, f));
+
+        Assert.Equal(SomeProjectFile1.TargetPath, project.GetDocument(SomeProjectFile1.FilePath)!.TargetPath);
     }
 
     [Fact]
diff --git a/src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/Shared/TestProjectData.cs b/src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/Shared/TestProjectData.cs
@@ -28,14 +28,14 @@ static TestProjectData()
         SomeProjectFile1 = new HostDocument(Path.Combine(someProjectPath, "File1.cshtml"), "File1.cshtml", FileKinds.Legacy);
         SomeProjectFile2 = new HostDocument(Path.Combine(someProjectPath, "File2.cshtml"), "File2.cshtml", FileKinds.Legacy);
         SomeProjectImportFile = new HostDocument(Path.Combine(someProjectPath, "_Imports.cshtml"), "_Imports.cshtml", FileKinds.Legacy);
-        SomeProjectNestedFile3 = new HostDocument(Path.Combine(someProjectPath, "Nested", "File3.cshtml"), "Nested\\File1.cshtml", FileKinds.Legacy);
-        SomeProjectNestedFile4 = new HostDocument(Path.Combine(someProjectPath, "Nested", "File4.cshtml"), "Nested\\File2.cshtml", FileKinds.Legacy);
+        SomeProjectNestedFile3 = new HostDocument(Path.Combine(someProjectPath, "Nested", "File3.cshtml"), "Nested\\File3.cshtml", FileKinds.Legacy);
+        SomeProjectNestedFile4 = new HostDocument(Path.Combine(someProjectPath, "Nested", "File4.cshtml"), "Nested\\File4.cshtml", FileKinds.Legacy);
         SomeProjectNestedImportFile = new HostDocument(Path.Combine(someProjectPath, "Nested", "_Imports.cshtml"), "Nested\\_Imports.cshtml", FileKinds.Legacy);
         SomeProjectComponentFile1 = new HostDocument(Path.Combine(someProjectPath, "File1.razor"), "File1.razor", FileKinds.Component);
         SomeProjectComponentFile2 = new HostDocument(Path.Combine(someProjectPath, "File2.razor"), "File2.razor", FileKinds.Component);
         SomeProjectComponentImportFile1 = new HostDocument(Path.Combine(someProjectPath, "_Imports.razor"), "_Imports.razor", FileKinds.Component);
-        SomeProjectNestedComponentFile3 = new HostDocument(Path.Combine(someProjectPath, "Nested", "File3.razor"), "Nested\\File1.razor", FileKinds.Component);
-        SomeProjectNestedComponentFile4 = new HostDocument(Path.Combine(someProjectPath, "Nested", "File4.razor"), "Nested\\File2.razor", FileKinds.Component);
+        SomeProjectNestedComponentFile3 = new HostDocument(Path.Combine(someProjectPath, "Nested", "File3.razor"), "Nested\\File3.razor", FileKinds.Component);
+        SomeProjectNestedComponentFile4 = new HostDocument(Path.Combine(someProjectPath, "Nested", "File4.razor"), "Nested\\File4.razor", FileKinds.Component);
         SomeProjectCshtmlComponentFile5 = new HostDocument(Path.Combine(someProjectPath, "File5.cshtml"), "File5.cshtml", FileKinds.Component);
 
         var anotherProjectPath = Path.Combine(baseDirectory, "AnotherProject");
diff --git a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/ProjectSystem/DefaultRazorDynamicFileInfoProviderTest.cs b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/ProjectSystem/DefaultRazorDynamicFileInfoProviderTest.cs
@@ -8,6 +8,7 @@
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Razor.Language;
 using Microsoft.AspNetCore.Razor.LanguageServer;
+using Microsoft.AspNetCore.Razor.Telemetry;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.Razor;
 using Microsoft.CodeAnalysis.Razor.ProjectSystem;
@@ -56,7 +57,7 @@ public DefaultRazorDynamicFileInfoProviderTest(ITestOutputHelper testOutput)
         var filePathService = new FilePathService(languageServerFeatureOptions);
         var projectSnapshotManagerAccessor = Mock.Of<ProjectSnapshotManagerAccessor>(a => a.Instance == _projectSnapshotManager, MockBehavior.Strict);
         var projectConfigurationFilePathStore = Mock.Of<ProjectConfigurationFilePathStore>(MockBehavior.Strict);
-        var fallbackProjectManager = new FallbackProjectManager(projectConfigurationFilePathStore, languageServerFeatureOptions, projectSnapshotManagerAccessor);
+        var fallbackProjectManager = new FallbackProjectManager(projectConfigurationFilePathStore, languageServerFeatureOptions, projectSnapshotManagerAccessor, NoOpTelemetryReporter.Instance);
 
         _provider = new DefaultRazorDynamicFileInfoProvider(_documentServiceFactory, _editorFeatureDetector, filePathService, projectSnapshotManagerAccessor, fallbackProjectManager);
         _testAccessor = _provider.GetTestAccessor();
