Support relative paths in interceptors

docs/features/interceptors.md

@@ -68,21 +68,15 @@ File-local declarations of this type (`file class InterceptsLocationAttribute`)
 
 #### File paths
 
-Prior to stable release, we will reach a state where the following rules apply:
+The *referenced syntax tree* of an `[InterceptsLocation]` is determined by normalizing the `filePath` argument value relative to the path of the containing syntax tree of the `[InterceptsLocation]` usage, similar to how paths in `#line` directives are normalized. Let this normalized path be called `normalizedInterceptorPath`. If exactly one syntax tree in the compilation has a normalized path which matches `normalizedInterceptorPath` by ordinal string comparison, that is the *referenced syntax tree*. Otherwise, an error occurs.
 
-1. A *mapped path* of each syntax tree is determined by applying [`/pathmap`](https://learn.microsoft.com/en-us/dotnet/api/microsoft.codeanalysis.commandlinearguments.pathmap?view=roslyn-dotnet-4.7.0) substitution to `SyntaxTree.FilePath`.
-2. The *referenced syntax tree* of an `[InterceptsLocation]` is determined in an equivalent fashion to how the referenced file of a `#line` directive is determined. Specifically, the `filePath` argument value is resolved relative to the path of the containing syntax tree of the `[InterceptsLocation]` usage. Pathmap substitution is applied to the result of this relative resolution. If exactly one syntax tree matches this resolved and mapped path, that is the *referenced syntax tree*.
-    - `#line` directives are not considered when determining the call referenced by an `[InterceptsLocation]` attribute. In other words, the file path, line and column numbers used in `[InterceptsLocation]` are expected to refer to *unmapped* source locations.
-3. If the above comparison does not match exactly one syntax tree, an error occurs.
+`#line` directives are not considered when determining the call referenced by an `[InterceptsLocation]` attribute. In other words, the file path, line and column numbers used in `[InterceptsLocation]` are expected to refer to *unmapped* source locations.
 
-Temporarily, for compatibility purposes, we use the following rules:
+Temporarily, for compatibility purposes, when the initial matching strategy outlined above fails to match any syntax trees, we will fall back to a "compat" matching strategy which works in the following way:
+- A *mapped path* of each syntax tree is determined by applying [`/pathmap`](https://learn.microsoft.com/en-us/dotnet/api/microsoft.codeanalysis.commandlinearguments.pathmap?view=roslyn-dotnet-4.7.0) substitution to `SyntaxTree.FilePath`.
+- For a given `[InterceptsLocation]` usage, the `filePath` argument value is compared to the *mapped path* of each syntax tree using ordinal string comparison. If exactly one syntax tree matches under this comparison, that is the *referenced syntax tree*. Otherwise, an error occurs.
 
-The *referenced syntax tree* of an `[InterceptsLocation]` is determined using the following rules:
-1. A *mapped path* of each syntax tree is determined by applying [`/pathmap`](https://learn.microsoft.com/en-us/dotnet/api/microsoft.codeanalysis.commandlinearguments.pathmap?view=roslyn-dotnet-4.7.0) substitution to `SyntaxTree.FilePath`.
-2. For a given `[InterceptsLocation]` usage, the `filePath` argument value is compared to the *mapped path* of each syntax tree using ordinal string comparison. If exactly one syntax tree matches under this comparison, that is the *referenced syntax tree*.
-3. If no syntax tree matched by the above comparison, then the `filePath` argument value is resolved relative to path of the containing syntax tree of the `[InterceptsLocation]` usage. Pathmap substitution is applied to the result of this relative resolution. If exactly one syntax tree matches this resolved and mapped path, that is the *referenced syntax tree*.
-    - `#line` directives are not considered in either of the above comparisons when determining the call referenced by an `[InterceptsLocation]` attribute. In other words, the line and column numbers used in the attribute are expected to refer to *unmapped* source locations, while the file path is expected to either be unmapped or to have `/pathmap` substitution already applied.
-4. If neither of the above methods of comparison matched exactly one syntax tree, an error occurs.
+Support for the "compat" strategy will be dropped prior to stable release.
 
 #### Position
 

src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs

@@ -154,9 +154,13 @@ internal Conversions Conversions
         /// </summary>
         private readonly ConcurrentCache<TypeSymbol, NamedTypeSymbol> _typeToNullableVersion = new ConcurrentCache<TypeSymbol, NamedTypeSymbol>(size: 100);
 
-        /// <summary>Lazily caches SyntaxTrees by their mapped path. Used to look up the syntax tree referenced by an interceptor. <see cref="TryGetInterceptor"/></summary>
+        /// <summary>Lazily caches SyntaxTrees by their mapped path. Used to look up the syntax tree referenced by an interceptor (temporary compat behavior).</summary>
+        /// <remarks>Must be removed prior to interceptors stable release.</remarks>
         private ImmutableSegmentedDictionary<string, OneOrMany<SyntaxTree>> _mappedPathToSyntaxTree;
 
+        /// <summary>Lazily caches SyntaxTrees by their path. Used to look up the syntax tree referenced by an interceptor.</summary>
+        private ImmutableSegmentedDictionary<string, OneOrMany<SyntaxTree>> _pathToSyntaxTree;
+
         public override string Language
         {
             get
@@ -1042,6 +1046,9 @@ internal override int GetSyntaxTreeOrdinal(SyntaxTree tree)
 
         internal OneOrMany<SyntaxTree> GetSyntaxTreesByMappedPath(string mappedPath)
         {
+            // This method supports a "compat" behavior for interceptor file path resolution.
+            // It must be removed prior to stable release.
+
             // We could consider storing this on SyntaxAndDeclarationManager instead, and updating it incrementally.
             // However, this would make it more difficult for it to be "pay-for-play",
             // i.e. only created in compilations where interceptors are used.
@@ -1057,10 +1064,36 @@ internal OneOrMany<SyntaxTree> GetSyntaxTreesByMappedPath(string mappedPath)
             ImmutableSegmentedDictionary<string, OneOrMany<SyntaxTree>> computeMappedPathToSyntaxTree()
             {
                 var builder = ImmutableSegmentedDictionary.CreateBuilder<string, OneOrMany<SyntaxTree>>();
-                var resolver = Options.SourceReferenceResolver ?? SourceFileResolver.Default;
+                var resolver = Options.SourceReferenceResolver;
+                foreach (var tree in SyntaxTrees)
+                {
+                    var path = resolver?.NormalizePath(tree.FilePath, baseFilePath: null) ?? tree.FilePath;
+                    builder[path] = builder.ContainsKey(path) ? builder[path].Add(tree) : OneOrMany.Create(tree);
+                }
+                return builder.ToImmutable();
+            }
+        }
+
+        internal OneOrMany<SyntaxTree> GetSyntaxTreesByPath(string path)
+        {
+            // We could consider storing this on SyntaxAndDeclarationManager instead, and updating it incrementally.
+            // However, this would make it more difficult for it to be "pay-for-play",
+            // i.e. only created in compilations where interceptors are used.
+            var pathToSyntaxTree = _pathToSyntaxTree;
+            if (pathToSyntaxTree.IsDefault)
+            {
+                RoslynImmutableInterlocked.InterlockedInitialize(ref _pathToSyntaxTree, computePathToSyntaxTree());
+                pathToSyntaxTree = _pathToSyntaxTree;
+            }
+
+            return pathToSyntaxTree.TryGetValue(path, out var value) ? value : OneOrMany<SyntaxTree>.Empty;
+
+            ImmutableSegmentedDictionary<string, OneOrMany<SyntaxTree>> computePathToSyntaxTree()
+            {
+                var builder = ImmutableSegmentedDictionary.CreateBuilder<string, OneOrMany<SyntaxTree>>();
                 foreach (var tree in SyntaxTrees)
                 {
-                    var path = resolver.NormalizePath(tree.FilePath, baseFilePath: null) ?? tree.FilePath;
+                    var path = FileUtilities.GetNormalizedPathOrOriginalPath(tree.FilePath, basePath: null);
                     builder[path] = builder.ContainsKey(path) ? builder[path].Add(tree) : OneOrMany.Create(tree);
                 }
                 return builder.ToImmutable();

src/Compilers/CSharp/Portable/Symbols/Source/SourceMethodSymbolWithAttributes.cs

@@ -1000,56 +1000,56 @@ private void DecodeInterceptsLocationAttribute(DecodeWellKnownAttributeArguments
                 return;
             }
 
-            var referenceResolver = DeclaringCompilation.Options.SourceReferenceResolver ?? SourceFileResolver.Default;
+            var normalizedPath = FileUtilities.GetNormalizedPathOrOriginalPath(attributeFilePath, basePath: SyntaxTree.FilePath);
+            var matchingTrees = DeclaringCompilation.GetSyntaxTreesByPath(normalizedPath);
+            if (matchingTrees.Count > 1)
+            {
+                diagnostics.Add(ErrorCode.ERR_InterceptorNonUniquePath, attributeData.GetAttributeArgumentLocation(filePathParameterIndex), normalizedPath);
+                return;
+            }
 
-            // First, the attributeFilePath might be a mapped path of one of the trees in the compilation.
-            // In this case, it's not intended to be resolved relative to anything, and we won't know if this is the case until we look it up.
-            var matchingTrees = DeclaringCompilation.GetSyntaxTreesByMappedPath(attributeFilePath);
-            if (matchingTrees.Count == 0
-                && referenceResolver.NormalizePath(attributeFilePath, baseFilePath: SyntaxTree.FilePath) is { } normalizedFilePath)
+            if (matchingTrees.Count == 0)
             {
-                // Try again but normalize the given file path relative to the containing file of the attribute application
-                matchingTrees = DeclaringCompilation.GetSyntaxTreesByMappedPath(normalizedFilePath);
+                // Temporary compat behavior: check if 'attributeFilePath' is equivalent to a mapped path of one of the syntax trees in the compilation.
+                matchingTrees = DeclaringCompilation.GetSyntaxTreesByMappedPath(attributeFilePath);
 
-                // Use the normalized path in the following diagnostic messages so the user has a hint of where we are searching.
-                // Relative paths are the happy path so we want them to know how the compiler has resolved the path they gave us.
-                attributeFilePath = normalizedFilePath;
+                if (matchingTrees.Count > 1)
+                {
+                    diagnostics.Add(ErrorCode.ERR_InterceptorNonUniquePath, attributeData.GetAttributeArgumentLocation(filePathParameterIndex), attributeFilePath);
+                    return;
+                }
             }
 
+            // Neither the primary or compat methods of resolving the file path found any match.
             if (matchingTrees.Count == 0)
             {
-                // if we expect '/_/Program.cs':
-
-                // we might get: '\_\Program.cs' <-- slashes not normalized
-                // we might get: '\_/Program.cs' <-- slashes don't match
+                // if we expect '/src/Program.cs':
                 // we might get: 'Program.cs' <-- suffix match
-                // Force normalization of all '\' to '/', but when we recommend a path in the diagnostic message, ensure it will match what we expect if the user decides to use it.
                 var syntaxTrees = DeclaringCompilation.SyntaxTrees;
-                var suffixMatch = syntaxTrees.FirstOrDefault(static (tree, pair)
-                    => mapPath(pair.referenceResolver, tree)
+                var suffixMatch = syntaxTrees.FirstOrDefault(static (tree, attributeFilePathWithForwardSlashes)
+                    => tree.FilePath
                         .Replace('\\', '/')
-                        .EndsWith(pair.attributeFilePath),
-                    (referenceResolver, attributeFilePath: attributeFilePath.Replace('\\', '/')));
+                        .EndsWith(attributeFilePathWithForwardSlashes),
+                    attributeFilePath.Replace('\\', '/'));
                 if (suffixMatch != null)
                 {
+                    var recommendedPath = PathUtilities.IsAbsolute(SyntaxTree.FilePath)
+                        ? PathUtilities.GetRelativePath(PathUtilities.GetDirectoryName(SyntaxTree.FilePath), suffixMatch.FilePath)
+                        : suffixMatch.FilePath;
                     diagnostics.Add(
                         ErrorCode.ERR_InterceptorPathNotInCompilationWithCandidate,
                         attributeData.GetAttributeArgumentLocation(filePathParameterIndex),
                         attributeFilePath,
-                        mapPath(referenceResolver, suffixMatch));
+                        recommendedPath);
                     return;
                 }
 
-                diagnostics.Add(ErrorCode.ERR_InterceptorPathNotInCompilation, attributeData.GetAttributeArgumentLocation(filePathParameterIndex), attributeFilePath);
+                diagnostics.Add(ErrorCode.ERR_InterceptorPathNotInCompilation, attributeData.GetAttributeArgumentLocation(filePathParameterIndex), normalizedPath);
 
                 return;
             }
-            else if (matchingTrees.Count > 1)
-            {
-                diagnostics.Add(ErrorCode.ERR_InterceptorNonUniquePath, attributeData.GetAttributeArgumentLocation(filePathParameterIndex), attributeFilePath);
-                return;
-            }
 
+            Debug.Assert(matchingTrees.Count == 1);
             SyntaxTree? matchingTree = matchingTrees[0];
             // Internally, line and character numbers are 0-indexed, but when they appear in code or diagnostic messages, they are 1-indexed.
             int lineNumberZeroBased = lineNumberOneBased - 1;
@@ -1109,11 +1109,6 @@ private void DecodeInterceptsLocationAttribute(DecodeWellKnownAttributeArguments
 
             DeclaringCompilation.AddInterception(matchingTree.FilePath, lineNumberZeroBased, characterNumberZeroBased, attributeLocation, this);
 
-            static string mapPath(SourceReferenceResolver? referenceResolver, SyntaxTree tree)
-            {
-                return referenceResolver?.NormalizePath(tree.FilePath, baseFilePath: null) ?? tree.FilePath;
-            }
-
             // Caller must free the returned builder.
             ArrayBuilder<string> getNamespaceNames()
             {

src/Compilers/CSharp/Test/Semantic/Semantics/InterceptorsTests.cs

@@ -2614,9 +2614,9 @@ static class D
             """;
         var comp = CreateCompilation(new[] { (source, PlatformInformation.IsWindows ? @"C:\Users\me\projects\Program.cs" : "/Users/me/projects/Program.cs"), s_attributesSource }, parseOptions: RegularWithInterceptors);
         comp.VerifyEmitDiagnostics(
-            // C:\Users\me\projects\Program.cs(21,25): error CS9139: Cannot intercept: compilation does not contain a file with path 'C:\Users\me\projects\projects\Program.cs'.
+            // C:\Users\me\projects\Program.cs(21,25): error CS9140: Cannot intercept: compilation does not contain a file with path 'projects/Program.cs'. Did you mean to use path 'Program.cs'?
             //     [InterceptsLocation("projects/Program.cs", 15, 11)]
-            Diagnostic(ErrorCode.ERR_InterceptorPathNotInCompilation, @"""projects/Program.cs""").WithArguments(PlatformInformation.IsWindows ? @"C:\Users\me\projects\projects\Program.cs" : "/Users/me/projects/projects/Program.cs").WithLocation(21, 25)
+            Diagnostic(ErrorCode.ERR_InterceptorPathNotInCompilationWithCandidate, @"""projects/Program.cs""").WithArguments("projects/Program.cs", "Program.cs").WithLocation(21, 25)
             );
     }
 
@@ -5223,6 +5223,71 @@ static class Interceptors
             Diagnostic(ErrorCode.ERR_InterceptorPathNotInCompilation, @"""../src/Program.cs""").WithArguments("../src/Program.cs").WithLocation(6, 25));
     }
 
+    [Fact]
+    public void OldVersusNewResolutionStrategy()
+    {
+        // relative path resolution will match a file (and the node referenced is not interceptable)
+        // exact mapped resolution will match a *different* file (and the node referenced is interceptable)
+        var source1 = ("""
+            class C1
+            {
+                void M1()
+                {
+                    var _ =
+                        C.Interceptable;
+                }
+            }
+            """, PlatformInformation.IsWindows ? @"C:\src1\file1.cs" : "/src1/file1.cs");
+
+        var directory2 = PlatformInformation.IsWindows ? @"C:\src2\" : "/src2/";
+        var path2 = PlatformInformation.IsWindows ? @"C:\src2\file1.cs" : "/src2/file1.cs";
+        var source2 = ("""
+            class C2
+            {
+                static void Main()
+                {
+                    // var _ =
+                        C.Interceptable();
+                }
+            }
+
+            class C
+            {
+                public static void Interceptable() => throw null!;
+            }
+            """, path2);
+
+        var source3 = ("""
+            using System.Runtime.CompilerServices;
+            using System;
+
+            class Interceptors
+            {
+                [InterceptsLocation("./file1.cs", 6, 15)] // 1
+                public static void Interceptor() => Console.Write(1);
+            }
+            """, PlatformInformation.IsWindows ? @"C:\src1\interceptors.cs" : "/src1/interceptors.cs");
+
+        // Demonstrate that "relative path" resolution happens first by triggering the not interceptable error.
+        var pathMap = ImmutableArray.Create(new KeyValuePair<string, string>(directory2, "./"));
+        var comp = CreateCompilation([source1, source2, source3, s_attributesSource],
+            parseOptions: RegularWithInterceptors,
+            options: TestOptions.DebugExe.WithSourceReferenceResolver(
+                new SourceFileResolver(ImmutableArray<string>.Empty, null, pathMap)));
+        comp.VerifyEmitDiagnostics(
+            // C:\src1\interceptors.cs(6,6): error CS9151: Possible method name 'Interceptable' cannot be intercepted because it is not being invoked.
+            //     [InterceptsLocation("./file1.cs", 6, 15)] // 1
+            Diagnostic(ErrorCode.ERR_InterceptorNameNotInvoked, @"InterceptsLocation(""./file1.cs"", 6, 15)").WithArguments("Interceptable").WithLocation(6, 6));
+
+        // excluding 'source1' from the compilation, we fall back to exact match of mapped path, and interception is successful.
+        var verifier = CompileAndVerify([source2, source3, s_attributesSource],
+            parseOptions: RegularWithInterceptors,
+            options: TestOptions.DebugExe.WithSourceReferenceResolver(
+                new SourceFileResolver(ImmutableArray<string>.Empty, null, pathMap)),
+            expectedOutput: "1");
+        verifier.VerifyDiagnostics();
+    }
+
     [Fact]
     public void InterceptorUnmanagedCallersOnly()
     {

src/Compilers/Core/Portable/FileSystem/FileUtilities.cs

@@ -209,6 +209,11 @@ internal static class FileUtilities
 
         private static readonly char[] s_invalidPathChars = Path.GetInvalidPathChars();
 
+        internal static string GetNormalizedPathOrOriginalPath(string path, string? basePath)
+        {
+            return NormalizeRelativePath(path, basePath, baseDirectory: null) ?? path;
+        }
+
         internal static string? NormalizeRelativePath(string path, string? basePath, string? baseDirectory)
         {
             // Does this look like a URI at all or does it have any invalid path characters? If so, just use it as is.