Tooling: Don't throw exceptions when generating code for file rooted outside of project (#11864)

Fixes [AzDO
2477693](https://devdiv.visualstudio.com/DevDiv/_queries/edit/2477693/?queryId=25fb5e55-8826-41b9-849a-30c1cf2d9f51)

It is a perfectly valid scenario to use a file rooted outside of the
project containing it. For example, a NuGet package might add Razor
files as content along with "TargetPath" metadata to a project. However,
recent changes broke this scenario.

The incorrect change was to start calling
`RazorProjectFileSystem.GetItem(...)` with the physical file path of a
document. This is almost never correct because
`DefaultRazorProjectFileSystem.GetItem(...)` will throw if the path
passed in is rooted outside of the project root. Instead, it's more
correct to pass in the document's target path, which is essentially a
"logical path" to the file under the project root. When the target path
is passed in, `DefaultRazorProjectFileSystem.GetItem(...)` will return a
file path under the project root, which is what is needed by tooling to
compute the applicable import file paths.

I've fixed this issue comprehensively by correcting three other issues
that I found along the way.

1. `DefaultRazorProjecFileSystem.NormalizeAndEnsureValidPath(...)`
shouldn't join two rooted paths. Instead, if the path passed is rooted
outside of the project root, it should just return the path. This was
noticed because a rooted path was being passed in when it shouldn't have
been.
2. `ProjectStateUpdate` shouldn't throw when trying to send "success"
telemetry on a failure.
3. Ensure that tooling calls `RazorProjectFileSystem.GetItem(...)` with
a document's target path rather than its physical file path.

----
CI Build:
https://dev.azure.com/dnceng/internal/_build/results?buildId=2710729&view=results
Test Insertion:
https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/637065

Author: phil-allen-msft

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorProjectFileSystem.cs

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
+using Microsoft.AspNetCore.Razor.Utilities;
 
 namespace Microsoft.AspNetCore.Razor.Language;
 
@@ -93,6 +94,23 @@ protected override string NormalizeAndEnsureValidPath(string path)
 
         var normalizedPath = path.Replace('\\', '/');
 
+        // On Windows, check to see if this is a rooted file path. If it is, just return it.
+        // This covers the following cases:
+        //
+        // 1. It is rooted within the project root. That's valid and we would have checked
+        //    specifically for that case below.
+        // 2. It is rooted outside of the project root. That's invalid, and we don't want to
+        //    concatenate it with the project root. That would potentially produce an invalid
+        //    Windows path like 'C:/project/C:/other-project/some-file.cshtml'.
+        //
+        // Note that returning a path that is rooted outside of the project root will cause
+        // the GetItem(...) method to throw, but it could be overridden by a descendant file
+        // system.
+        if (PlatformInformation.IsWindows && PathUtilities.IsPathFullyQualified(path))
+        {
+            return normalizedPath;
+        }
+
         // Check if the given path is an absolute path. It is absolute if...
         //
         // 1. It is a network share path and starts with a '//' (e.g. //server/some/network/folder) or...

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/CompilationHelpers.cs

@@ -20,7 +20,7 @@ internal static async Task<RazorCodeDocument> GenerateCodeDocumentAsync(
     {
         var importSources = await GetImportSourcesAsync(document, projectEngine, cancellationToken).ConfigureAwait(false);
         var tagHelpers = await document.Project.GetTagHelpersAsync(cancellationToken).ConfigureAwait(false);
-        var source = await document.GetSourceAsync(projectEngine, cancellationToken).ConfigureAwait(false);
+        var source = await document.GetSourceAsync(cancellationToken).ConfigureAwait(false);
 
         var generator = new CodeDocumentGenerator(projectEngine, compilerOptions);
         return generator.Generate(source, document.FileKind, importSources, tagHelpers, cancellationToken);
@@ -33,15 +33,18 @@ internal static async Task<RazorCodeDocument> GenerateDesignTimeCodeDocumentAsyn
     {
         var importSources = await GetImportSourcesAsync(document, projectEngine, cancellationToken).ConfigureAwait(false);
         var tagHelpers = await document.Project.GetTagHelpersAsync(cancellationToken).ConfigureAwait(false);
-        var source = await document.GetSourceAsync(projectEngine, cancellationToken).ConfigureAwait(false);
+        var source = await document.GetSourceAsync(cancellationToken).ConfigureAwait(false);
 
         var generator = new CodeDocumentGenerator(projectEngine, RazorCompilerOptions.None);
         return generator.GenerateDesignTime(source, document.FileKind, importSources, tagHelpers, cancellationToken);
     }
 
     private static async Task<ImmutableArray<RazorSourceDocument>> GetImportSourcesAsync(IDocumentSnapshot document, RazorProjectEngine projectEngine, CancellationToken cancellationToken)
     {
-        var projectItem = projectEngine.FileSystem.GetItem(document.FilePath, document.FileKind);
+        // We don't use document.FilePath when calling into GetItem(...) because
+        // it could be rooted outside of the project root. document.TargetPath should
+        // represent the logical relative path within the project root.
+        var projectItem = projectEngine.FileSystem.GetItem(document.TargetPath, document.FileKind);
 
         using var importProjectItems = new PooledArrayBuilder<RazorProjectItem>();
         projectEngine.CollectImports(projectItem, ref importProjectItems.AsRef());

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/IDocumentSnapshotExtensions.cs

@@ -11,14 +11,12 @@ namespace Microsoft.CodeAnalysis.Razor.ProjectSystem;
 
 internal static class IDocumentSnapshotExtensions
 {
-    public static async Task<RazorSourceDocument> GetSourceAsync(this IDocumentSnapshot document, RazorProjectEngine projectEngine, CancellationToken cancellationToken)
+    public static async Task<RazorSourceDocument> GetSourceAsync(
+        this IDocumentSnapshot document,
+        CancellationToken cancellationToken)
     {
-        var projectItem = document is { FilePath: string filePath, FileKind: var fileKind }
-            ? projectEngine.FileSystem.GetItem(filePath, fileKind)
-            : null;
-
         var text = await document.GetTextAsync(cancellationToken).ConfigureAwait(false);
-        var properties = RazorSourceDocumentProperties.Create(document.FilePath, projectItem?.RelativePhysicalPath);
+        var properties = RazorSourceDocumentProperties.Create(document.FilePath, document.TargetPath);
         return RazorSourceDocument.Create(text, properties);
     }
 

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/Legacy/ILegacyDocumentSnapshot.cs

@@ -13,5 +13,6 @@ namespace Microsoft.CodeAnalysis.Razor.ProjectSystem.Legacy;
 /// </remarks>
 internal interface ILegacyDocumentSnapshot
 {
+    string TargetPath { get; }
     RazorFileKind FileKind { get; }
 }

src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/Discovery/ProjectStateUpdater.cs

@@ -311,9 +311,26 @@ private void ReleaseSemaphore(ProjectKey projectKey)
                 .ConfigureAwait(false);
             watch.Stop();
 
-            // don't report success if the work was cancelled
+            // Don't report success if the work was cancelled
             cancellationToken.ThrowIfCancellationRequested();
 
+            // Don't report success if the call failed.
+            // If the ImmutableArray that was returned is default, then the call failed.
+
+            if (tagHelpers.IsDefault)
+            {
+                _telemetryReporter.ReportEvent("taghelperresolve/end", Severity.Normal,
+                new("id", telemetryId),
+                new("result", "error"));
+
+                _logger.LogError($"""
+                    Tag helper discovery failed.
+                    Project: {projectSnapshot.FilePath}
+                    """);
+
+                return (null, configuration);
+            }
+
             _telemetryReporter.ReportEvent("taghelperresolve/end", Severity.Normal,
                 new("id", telemetryId),
                 new("ellapsedms", watch.ElapsedMilliseconds),

src/Razor/src/Microsoft.VisualStudio.LegacyEditor.Razor/ImportDocumentManager.cs

@@ -97,10 +97,16 @@ public void OnUnsubscribed(IVisualStudioDocumentTracker documentTracker)
     private static ImmutableArray<RazorProjectItem> GetPhysicalImportItems(string filePath, ILegacyProjectSnapshot projectSnapshot)
     {
         var projectEngine = projectSnapshot.GetProjectEngine();
-        var documentSnapshot = projectSnapshot.GetDocument(filePath);
-        var projectItem = projectEngine.FileSystem.GetItem(filePath, documentSnapshot?.FileKind);
 
-        return projectEngine.GetImports(projectItem, static i => i.PhysicalPath is not null);
+        // If we can get the document, use it's target path to find the project item
+        // to avoid GetItem(...) throwing an exception if the file path is rooted outside
+        // of the project. If we can't get the document, we'll just go ahead and use
+        // the file path, since it's probably OK.
+        var projectItem = projectSnapshot.GetDocument(filePath) is { } document
+            ? projectEngine.FileSystem.GetItem(document.TargetPath, document.FileKind)
+            : projectEngine.FileSystem.GetItem(filePath, fileKind: null);
+
+        return projectEngine.GetImports(projectItem, static i => i is not DefaultImportProjectItem);
     }
 
     private void FileChangeTracker_Changed(object sender, FileChangeEventArgs args)

src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/TestRazorProjectService.cs

@@ -6,9 +6,11 @@
 using Microsoft.AspNetCore.Razor.Language;
 using Microsoft.AspNetCore.Razor.LanguageServer.Common;
 using Microsoft.AspNetCore.Razor.Test.Common;
+using Microsoft.CodeAnalysis.Razor;
 using Microsoft.CodeAnalysis.Razor.Logging;
 using Microsoft.CodeAnalysis.Razor.ProjectSystem;
 using Microsoft.CodeAnalysis.Razor.Serialization;
+using Microsoft.CodeAnalysis.Razor.Utilities;
 using Moq;
 
 namespace Microsoft.AspNetCore.Razor.LanguageServer.ProjectSystem;
@@ -37,13 +39,19 @@ private static IRazorProjectInfoDriver CreateProjectInfoDriver()
         return mock.Object;
     }
 
-    public async Task AddDocumentToPotentialProjectsAsync(string textDocumentPath, CancellationToken cancellationToken)
+    public async Task AddDocumentToPotentialProjectsAsync(string filePath, CancellationToken cancellationToken)
     {
-        var document = new DocumentSnapshotHandle(
-            textDocumentPath, textDocumentPath, FileKinds.GetFileKindFromPath(textDocumentPath));
-
-        foreach (var projectSnapshot in _projectManager.FindPotentialProjects(textDocumentPath))
+        foreach (var projectSnapshot in _projectManager.FindPotentialProjects(filePath))
         {
+            var projectDirectory = FilePathNormalizer.GetNormalizedDirectoryName(projectSnapshot.FilePath);
+            var normalizedFilePath = FilePathNormalizer.Normalize(filePath);
+
+            var targetPath = normalizedFilePath.StartsWith(projectDirectory, FilePathComparison.Instance)
+                ? normalizedFilePath[projectDirectory.Length..]
+                : normalizedFilePath;
+
+            var document = new DocumentSnapshotHandle(filePath, targetPath, FileKinds.GetFileKindFromPath(filePath));
+
             var projectInfo = projectSnapshot.ToRazorProjectInfo();
 
             projectInfo = projectInfo with

src/Shared/Microsoft.AspNetCore.Razor.Test.Common/ConditionalFactAttribute.cs

@@ -121,6 +121,11 @@ public static class Is
     /// </summary>
     public const string FreeBSD = nameof(FreeBSD);
 
+    /// <summary>
+    ///  Only execute if the current operating system platform is Unix-based.
+    /// </summary>
+    public const string AnyUnix = nameof(AnyUnix);
+
     public static class Not
     {
         /// <summary>
@@ -142,6 +147,12 @@ public static class Not
         ///  Only execute if the current operating system platform is not FreeBSD.
         /// </summary>
         public const string FreeBSD = $"!{nameof(FreeBSD)}";
+
+        /// <summary>
+        ///  Only execute if the current operating system platform is not Unix-based.
+        /// </summary>
+        public const string AnyUnix = $"!{nameof(AnyUnix)}";
+
     }
 }
 
@@ -157,6 +168,9 @@ private static FrozenDictionary<string, Func<bool>> CreateConditionMap()
         Add(Is.Linux, static () => PlatformInformation.IsLinux);
         Add(Is.MacOS, static () => PlatformInformation.IsMacOS);
         Add(Is.FreeBSD, static () => PlatformInformation.IsFreeBSD);
+        Add(Is.AnyUnix, static () => PlatformInformation.IsLinux ||
+                                     PlatformInformation.IsMacOS ||
+                                     PlatformInformation.IsFreeBSD);
 
         return map.ToFrozenDictionary();
 

src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared.Test/PathUtilitiesTests.cs

@@ -42,6 +42,84 @@ public void GetExtension_Span(string path, string expected)
         Assert.Equal(!string.IsNullOrEmpty(expected), PathUtilities.HasExtension(path.AsSpan()));
     }
 
+    // The tests below are derived from the .NET Runtime:
+    // - https://github.com/dotnet/runtime/blob/91195a7948a16c769ccaf7fd8ca84b1d210f6841/src/libraries/System.Runtime/tests/System.Runtime.Extensions.Tests/System/IO/Path.IsPathFullyQualified.cs
+
+    [Fact]
+    public static void IsPathFullyQualified_NullArgument()
+    {
+        Assert.Throws<ArgumentNullException>(() => PathUtilities.IsPathFullyQualified(null!));
+    }
+
+    [Fact]
+    public static void IsPathFullyQualified_Empty()
+    {
+        Assert.False(PathUtilities.IsPathFullyQualified(""));
+        Assert.False(PathUtilities.IsPathFullyQualified(ReadOnlySpan<char>.Empty));
+    }
+
+    [ConditionalTheory(Is.Windows)]
+    [InlineData("/")]
+    [InlineData(@"\")]
+    [InlineData(".")]
+    [InlineData("C:")]
+    [InlineData("C:foo.txt")]
+    public static void IsPathFullyQualified_Windows_Invalid(string path)
+    {
+        Assert.False(PathUtilities.IsPathFullyQualified(path));
+        Assert.False(PathUtilities.IsPathFullyQualified(path.AsSpan()));
+    }
+
+    [ConditionalTheory(Is.Windows)]
+    [InlineData(@"\\")]
+    [InlineData(@"\\\")]
+    [InlineData(@"\\Server")]
+    [InlineData(@"\\Server\Foo.txt")]
+    [InlineData(@"\\Server\Share\Foo.txt")]
+    [InlineData(@"\\Server\Share\Test\Foo.txt")]
+    [InlineData(@"C:\")]
+    [InlineData(@"C:\foo1")]
+    [InlineData(@"C:\\")]
+    [InlineData(@"C:\\foo2")]
+    [InlineData(@"C:/")]
+    [InlineData(@"C:/foo1")]
+    [InlineData(@"C://")]
+    [InlineData(@"C://foo2")]
+    public static void IsPathFullyQualified_Windows_Valid(string path)
+    {
+        Assert.True(PathUtilities.IsPathFullyQualified(path));
+        Assert.True(PathUtilities.IsPathFullyQualified(path.AsSpan()));
+    }
+
+    [ConditionalTheory(Is.AnyUnix)]
+    [InlineData(@"\")]
+    [InlineData(@"\\")]
+    [InlineData(".")]
+    [InlineData("./foo.txt")]
+    [InlineData("..")]
+    [InlineData("../foo.txt")]
+    [InlineData(@"C:")]
+    [InlineData(@"C:/")]
+    [InlineData(@"C://")]
+    public static void IsPathFullyQualified_Unix_Invalid(string path)
+    {
+        Assert.False(PathUtilities.IsPathFullyQualified(path));
+        Assert.False(PathUtilities.IsPathFullyQualified(path.AsSpan()));
+    }
+
+    [ConditionalTheory(Is.AnyUnix)]
+    [InlineData("/")]
+    [InlineData("/foo.txt")]
+    [InlineData("/..")]
+    [InlineData("//")]
+    [InlineData("//foo.txt")]
+    [InlineData("//..")]
+    public static void IsPathFullyQualified_Unix_Valid(string path)
+    {
+        Assert.True(PathUtilities.IsPathFullyQualified(path));
+        Assert.True(PathUtilities.IsPathFullyQualified(path.AsSpan()));
+    }
+
     private static void AssertEqual(ReadOnlySpan<char> expected, ReadOnlySpan<char> actual)
     {
         if (!actual.SequenceEqual(expected))

src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/PathUtilities.cs

@@ -36,7 +36,7 @@ public static ReadOnlySpan<char> GetExtension(ReadOnlySpan<char> path)
             {
                 return i != length - 1
                     ? path[i..length]
-                    :  [];
+                    : [];
             }
 
             if (IsDirectorySeparator(ch))
@@ -70,5 +70,95 @@ public static bool HasExtension(ReadOnlySpan<char> path)
     private static bool IsDirectorySeparator(char ch)
         => ch == Path.DirectorySeparatorChar ||
           (PlatformInformation.IsWindows && ch == Path.AltDirectorySeparatorChar);
+
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    private static bool IsValidDriveChar(char value)
+        => (uint)((value | 0x20) - 'a') <= (uint)('z' - 'a');
+#endif
+
+    public static bool IsPathFullyQualified(string path)
+    {
+        ArgHelper.ThrowIfNull(path);
+
+        return IsPathFullyQualified(path.AsSpan());
+    }
+
+    public static bool IsPathFullyQualified(ReadOnlySpan<char> path)
+    {
+#if NET
+        return Path.IsPathFullyQualified(path);
+#else
+        if (PlatformInformation.IsWindows)
+        {
+            // Derived from .NET Runtime:
+            // - https://github.com/dotnet/runtime/blob/c7c961a330395152e5ec4000032cd3204ceb4a10/src/libraries/Common/src/System/IO/PathInternal.Windows.cs#L250-L274
+
+            if (path.Length < 2)
+            {
+                // It isn't fixed, it must be relative.  There is no way to specify a fixed
+                // path with one character (or less).
+                return false;
+            }
+
+            if (IsDirectorySeparator(path[0]))
+            {
+                // There is no valid way to specify a relative path with two initial slashes or
+                // \? as ? isn't valid for drive relative paths and \??\ is equivalent to \\?\
+                return path[1] == '?' || IsDirectorySeparator(path[1]);
+            }
+
+            // The only way to specify a fixed path that doesn't begin with two slashes
+            // is the drive, colon, slash format- i.e. C:\
+            return (path.Length >= 3)
+                && (path[1] == Path.VolumeSeparatorChar)
+                && IsDirectorySeparator(path[2])
+                // To match old behavior we'll check the drive character for validity as the path is technically
+                // not qualified if you don't have a valid drive. "=:\" is the "=" file's default data stream.
+                && IsValidDriveChar(path[0]);
+        }
+        else
+        {
+            // Derived from .NET Runtime:
+            // - https://github.com/dotnet/runtime/blob/c7c961a330395152e5ec4000032cd3204ceb4a10/src/libraries/Common/src/System/IO/PathInternal.Unix.cs#L77-L82
+
+            // This is much simpler than Windows where paths can be rooted, but not fully qualified (such as Drive Relative)
+            // As long as the path is rooted in Unix it doesn't use the current directory and therefore is fully qualified.
+            return IsPathRooted(path);
+        }
+#endif
+    }
+
+    public static bool IsPathRooted(string path)
+    {
+#if NET
+        return Path.IsPathRooted(path);
+#else
+        return IsPathRooted(path.AsSpan());
+#endif
+    }
+
+    public static bool IsPathRooted(ReadOnlySpan<char> path)
+    {
+#if NET
+        return Path.IsPathRooted(path);
+
+#else
+        if (PlatformInformation.IsWindows)
+        {
+            // Derived from .NET Runtime
+            // - https://github.com/dotnet/runtime/blob/850c0ab4519b904a28f2d67abdaba1ac78c955ff/src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs#L271-L276
+
+            var length = path.Length;
+            return (length >= 1 && IsDirectorySeparator(path[0]))
+                || (length >= 2 && IsValidDriveChar(path[0]) && path[1] == Path.VolumeSeparatorChar);
+        }
+        else
+        {
+            // Derived from .NET Runtime
+            // - https://github.com/dotnet/runtime/blob/850c0ab4519b904a28f2d67abdaba1ac78c955ff/src/libraries/System.Private.CoreLib/src/System/IO/Path.Unix.cs#L132-L135
+
+            return path.StartsWith(Path.DirectorySeparatorChar);
+        }
 #endif
     }
+}