RDG file name mismatch between SyntaxTree.FilePath and [CallerFilePath] causes runtime KeyNotFoundException

[halter73]

SyntaxTree.FilePath and [CallerFilePath] can give different filenames for the same compilation on our own public AzDO PR build pipeline which ultimately causes runtime KeyNotFoundExceptions in code relying on RDG.

Example runtime stack trace:

System.Collections.Generic.KeyNotFoundException : The given key '(/_/src/Identity/Core/src/IdentityApiEndpointRouteBuilderExtensions.cs, 38)' was not present in the dictionary.
Stack trace
   at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
   at Microsoft.AspNetCore.Http.Generated.<GeneratedRouteBuilderExtensions_g>F69328E0708B4B584C5AACA22FE2C51A1CF192D6622828F613FC57C583CA77B63__GeneratedRouteBuilderExtensionsCore.MapCore(IEndpointRouteBuilder routes, String pattern, Delegate handler, IEnumerable`1 httpMethods, String filePath, Int32 lineNumber) in /_/src/Identity/Core/src/Microsoft.AspNetCore.Http.RequestDelegateGenerator/Microsoft.AspNetCore.Http.RequestDelegateGenerator.RequestDelegateGenerator/GeneratedRouteBuilderExtensions.g.cs:line 306
   at Microsoft.AspNetCore.Builder.GenerateRouteBuilderEndpoints.MapPost(IEndpointRouteBuilder endpoints, String pattern, Func`3 handler, String filePath, Int32 lineNumber) in /_/src/Identity/Core/src/Microsoft.AspNetCore.Http.RequestDelegateGenerator/Microsoft.AspNetCore.Http.RequestDelegateGenerator.RequestDelegateGenerator/GeneratedRouteBuilderExtensions.g.cs:line 47
   at Microsoft.AspNetCore.Routing.IdentityApiEndpointRouteBuilderExtensions.MapIdentityApi[TUser](IEndpointRouteBuilder endpoints) in /_/src/Identity/Core/src/IdentityApiEndpointRouteBuilderExtensions.cs:line 38
   at Microsoft.AspNetCore.Identity.FunctionalTests.MapIdentityTests.CreateAppAsync[TUser,TContext](Action`1 configureServices) in /_/src/Identity/test/Identity.FunctionalTests/MapIdentityTests.cs:line 216
   at Microsoft.AspNetCore.Identity.FunctionalTests.MapIdentityTests.CanReadBearerTokenFromQueryString() in /_/src/Identity/test/Identity.FunctionalTests/MapIdentityTests.cs:line 155
--- End of stack trace from previous location ---

https://dev.azure.com/dnceng-public/public/_build/results?buildId=253772&view=ms.vss-test-web.build-test-results-tab&runId=4932292&resultId=111661&paneView=debug

The disassembly of Microsoft.AspNetCore.Identity.dll from the PR shows RDG used "D:\a_work\1\s\src\Identity\Core\src\IdentityApiEndpointRouteBuilderExtensions.cs" from SyntaxTree.FilePath as the filename for the source key:

    file static class GeneratedRouteBuilderExtensionsCore
    {
        private static readonly Dictionary<(string, int), (MetadataPopulator, RequestDelegateFactoryFunc)> map = new()
        {
            [(@"D:\a\_work\1\s\src\Identity\Core\src\IdentityApiEndpointRouteBuilderExtensions.cs", 38)] = (
                (methodInfo, options) =>

aspnetcore/src/Http/Http.Extensions/gen/StaticRouteHandlerModel/Endpoint.cs

Lines 164 to 170 in 58940e3

	private static (string, int) GetLocation(IInvocationOperation operation)
	{
	var filePath = operation.Syntax.SyntaxTree.FilePath;
	var span = operation.Syntax.SyntaxTree.GetLineSpan(operation.Syntax.Span);
	var lineNumber = span.StartLinePosition.Line + 1;
	return (filePath, lineNumber);
	}

But Roslyn used a different filename at the call site of the generated MapPost method for its [CallerFilePath] parameter. It used "/_/src/Identity/Core/src/IdentityApiEndpointRouteBuilderExtensions.cs" instead inside the same assembly:

    public static IEndpointConventionBuilder MapIdentityApi<TUser>(this IEndpointRouteBuilder endpoints) where TUser : class, new()
    {
        ArgumentNullException.ThrowIfNull(endpoints);
        var routeGroup = endpoints.MapGroup("");
        routeGroup.MapPost("/register",
            new Func<RegisterRequest, IServiceProvider, System.Threading.Tasks.Task<Results<Ok, ValidationProblem>>>(<>c__0<TUser>.<>9.<MapIdentityApi>b__0_0),
            "/_/src/Identity/Core/src/IdentityApiEndpointRouteBuilderExtensions.cs", 38);

[mitchdenny]

It looks to me like one was generated on a Linux machine, and the other was generated on a Windows machine. This might be a hole in our lookup strategy for interception. In the case o MapIdentityApi(...), its producing a DLL which is built on Linux and we are using it in a test which is presumably being built on Windows.

At this point I would say that our approach can't support what you are trying to do here with MapIdentityApi.

/cc @captainsafia

[halter73]

It looks to me like one was generated on a Linux machine, and the other was generated on a Windows machine. This might be a hole in our lookup strategy for interception. In the case o MapIdentityApi(...), its producing a DLL which is built on Linux and we are using it in a test which is presumably being built on Windows.

I don't think this is a Windows vs Linux thing. That's why I made such a big point in the issue about how both file paths strings are being compiled into the same into the same Microsoft.AspNetCore.Identity.dll assembly. This is all being built on our Windows CI machine. The test project is not referencing RDG at all. It's simply using the endpoints defined by MapIdentityApi(...) using RDG that have already been compiled into the non-test Microsoft.AspNetCore.Identity.dll assembly.

This is not just a test issue. This breaks anyone trying to call MapIdentityApi(...) using the build produced by our CI. It's true that the path originating because of the [CallerFilePath] looks like a Linux path in this case because of the forward slashes, but I'm thinking something else must be causing this. Locally when I build with Windows, I see two Windows-looking paths with drive letters and backslashes.

@jaredpar @cston Do you have any idea why SyntaxTree.FilePath and [CallerFilePath] could give different filenames for the same compilation ?

[jaredpar]

Do you have any idea why SyntaxTree.FilePath and [CallerFilePath] could give different filenames for the same compilation ?

This can happen with /pathmap is in play. The purpose of that argument is to allow builds to normalize build paths between machines. It changes the paths we emit into binaries:

• SyntaxTree.FilePath: this is always the physical file path (assuming there was one). It doesn't respect path map as it's telling the tooling where the file exists on disk. It's not directly emitted to the binary or PDB.
• [CallerFilePath]: this is directly part of the binary and hence respects path map.

[halter73]

Is there a way to reliably get something that matches what the [CallerFilePath] produces from the SyntaxTree or another API in an analyzer?

[jaredpar]

There is not a public API for this. 😦

Short term you can either reflect into PathUtilities.NormalizePathPrefix or just copy / paste the code into your generator. It's a fairly trivial function. Longer term we should get this added to the public API surface area. It's a very reasonable API to want.

@RikkiGibson can you get a bug filed for that + track it through API review?

[RikkiGibson]

SyntaxTree.GetDisplayPath is the method responsible for this. It happens to be implemented in terms of public APIs.

        /// <summary>
        /// Returns a path for particular location in source that is presented to the user. 
        /// </summary>
        /// <remarks>
        /// Used for implementation of <see cref="System.Runtime.CompilerServices.CallerFilePathAttribute"/> 
        /// or for embedding source paths in error messages.
        /// 
        /// Unlike Dev12 we do account for #line and #ExternalSource directives when determining value for 
        /// <see cref="System.Runtime.CompilerServices.CallerFilePathAttribute"/>.
        /// </remarks>
        internal string GetDisplayPath(TextSpan span, SourceReferenceResolver? resolver)
        {
            var mappedSpan = GetMappedLineSpan(span);
            if (resolver == null || mappedSpan.Path.IsEmpty())
            {
                return mappedSpan.Path;
            }


            return resolver.NormalizePath(mappedSpan.Path, baseFilePath: mappedSpan.HasMappedPath ? FilePath : null) ?? mappedSpan.Path;
        }

This also affects interceptors a bit, and relates to this prototype comment in the interceptors feature spec.

PROTOTYPE(ic): editorconfig support matches paths in cross-platform fashion (e.g. normalizing slashes). We should revisit how that works and consider if the same matching strategy should be used instead of ordinal comparison.

I think interceptors should apply pathmap substitution but not #line substitution. I convinced myself we should not do #line substitution. Mainly because it complicates the process of figuring out which location an [InterceptsLocation] is referring to. If things go wrong and you try navigating to the path/line/character your generator created to figure it out, it will take you to the "original document", not to the C# source being compiled, and it will be quite difficult to tell where you went wrong. The method name token the interceptor needs to receive may not map precisely to a token in the original document, depending on how precisely the spans in the original document are broken down, and just accidental things about how text lines up between the original and mapped spans.

Usually, pathmap is just about eliminating machine-specific elements of paths, so even if the error we give only contains a mapped path, it's still pretty easy to use it to navigate to the appropriate file in your project.

[RikkiGibson]

• SyntaxTree.FilePath: this is always the physical file path (assuming there was one). It doesn't respect path map as it's telling the tooling where the file exists on disk. It's not directly emitted to the binary or PDB.

Whyyyyy did I convince myself that SyntaxTree.FilePath have already applied /pathmap when they are created 🙂