Add public API to obtain a location specifier for a call

[RikkiGibson]

Continuation of #72093

Need for generators to obtain a location specifier for a call

We are making adjustments to the interceptors design in order to improve the following aspects:

• Portability of generated code. (i.e. the ability to generate code on one machine and compile the generated code on another machine.)
• Simplicity for generator authors, by introducing public API accessible for generators, in order to get the correct argument(s) for [InterceptsLocation], with minimal guesswork.
• Evolvability of the representation of locations, transparent to generator authors, by introducing an [InterceptsLocation(int, string)] constructor overload, the encoding of which can be updated over time.

Proposed API

We plan to add the following well-known constructor to InterceptsLocationAttribute:

 namespace System.Runtime.CompilerServices;

 public class InterceptsLocationAttribute
 {
     public InterceptsLocationAttribute(string filePath, int line, int character) { } // plan to drop support prior to stable release of interceptors feature
+    public InterceptsLocationAttribute(int version, string data) { }
 }

We propose adding the following public API to Roslyn:

 namespace Microsoft.CodeAnalysis
 {
     public static class CSharpExtensions
     {
+        /// <summary>
+        /// If 'node' cannot be intercepted, returns null. Otherwise, returns an InterceptableLocation instance which can be used to intercept the call denoted by 'node'.
+        /// </summary>
+        [Experimental(RoslynExperiments.Interceptors)]
+        public InterceptableLocation? GetInterceptableLocation(this SemanticModel model, InvocationExpressionSyntax node);
     }
 
     namespace CSharp
     {
+        /// <summary>Location data which can be used to intercept a call.</summary>
+        [Experimental(RoslynExperiments.Interceptors)]
+        public sealed class InterceptableLocation : IEquatable<InterceptableLocation>
+        {
+            /// <summary>The version of the InterceptsLocationAttribute data encoding. Used as an argument for the parameter 'version' of InterceptsLocationAttribute.</summary>
+            public int Version { get; }
+
+            /// <summary>Opaque data which references an interceptable call. Used as an argument for the parameter 'data' of InterceptsLocationAttribute.</summary>
+            public string Data { get; }
+
+            /// <summary>Gets a human-readable location description for the interceptable call.</summary>
+            public string GetDisplayLocation();
+
+            /// <summary>Returns a source code fragment of the form `[global::System.Runtime.CompilerServices.InterceptsLocationAttribute(version, data)]`.</summary>
+            public string GetInterceptsLocationAttributeSyntax();
+        }
+    }
 }

• We declare this as an extension method because we want to use C#-specific types in the signature (a la Add public API to determine if a call is being intercepted #72093).
• We use SemanticModel as the receiver type of the extension, because we want to have the ability to depend on compilation-level information in the location encoding.

The InterceptsLocation(string, int, int) constructor will coexist with the new constructor while we work to get partner generators onboarded to it. Prior to stable release of the interceptors feature, support for InterceptsLocation(string, int, int) will be dropped.

Usage Examples

In order to produce an interceptor method like the following:

// C:\project\src\Program.cs(6,17)
[InterceptsLocation(1, "yPU5+1/pMuRHlz+XbnIQwQYAAAARAAAAUHJvZ3JhbS5jcw==")]
public static void Interceptor(this ReceiverType receiver, ParamType param) { ... }

We may employ a generator implemented like the following:

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;

[Generator(LanguageNames.CSharp)]
public class MyGenerator : IIncrementalGenerator
{
    record InterceptorInfo(InterceptableLocation location, object data);

    public void Initialize(IncrementalGeneratorInitializationContext context)
    {
        var interceptorInfos = context.SyntaxProvider.CreateSyntaxProvider(
            predicate: IsInterceptableCall,
            transform: (GeneratorSyntaxContext context, CancellationToken token) => {
                var interceptableLocation = context.GetInterceptableLocation((InvocationExpressionSyntax)context.Node);
                if (interceptableLocation is null)
                {
                    return null; // generator wants to intercept call, but host thinks call is not interceptable. bug.
                }

                // generator is careful to propagate only equatable data (i.e., not syntax nodes or symbols).
                return new InterceptorInfo(interceptableLocation, GetData(context));
            })
            .Where(info => info != null)
            .Collect();

        context.RegisterSourceOutput(interceptorInfos, (context, interceptorInfos) => {
            var builder = new StringBuilder();
            // builder boilerplate..
            foreach (var interceptorInfo in interceptorInfos)
            {
                var (location, data) = interceptorInfo;
                builder.Add($$"""
                    // {{location.GetDisplayLocation()}}
                    [InterceptsLocation({{location.Version}}, "{{location.Data}}")]
                    public static void Interceptor(this ReceiverType receiver, ParamType param)
                    {
                        {{GetMethodBody(data)}}
                    }
                    """);

                // alternatively, author can use the convenience method, which fully qualifies the attribute name:
                // e.g. `[global::System.Runtime.CompilerServices.InterceptsLocationAttribute(...)]`.
                builder.Add($$"""
                    // {{location.GetDisplayLocation()}}
                    {{location.GetInterceptsLocationAttributeSyntax()}}
                    public static void Interceptor(this ReceiverType receiver, ParamType param)
                    {
                        {{GetMethodBody(data)}}
                    }
                    """);
            }
            // builder boilerplate..

            context.AddSource(builder.ToString(), hintName: "MyInterceptors.cs");
        });
    }
}

Alternative Designs

• Do not add a new constructor to [InterceptsLocation], and instead expose an API which obtains the attribute arguments for the existing constructor. This would likely require knowing the output directory for the particular generator and accepting a hintName in order to produce a relative path.

 namespace Microsoft.CodeAnalysis;

 public readonly struct GeneratorSyntaxContext
 {
     public void AddSource(string hintName, string source);
+    /// <summary>If 'expression' cannot be intercepted, returns null. Otherwise, returns a location specifier which can be used to intercept 'expression'.</summary>
+    /// <remarks>Currently, only 'InvocationExpressionSyntax' can be intercepted.</remarks>
+    public (string filePath, int line, int character)? GetInterceptsLocationArguments(ExpressionSyntax expression, string interceptorFileHintName);
 }

• Do not expose any API for obtaining InterceptsLocation arguments. Instead, document the location encoding and have generator authors create them on their own.

Risks

LDM has not yet reviewed the proposed checksum-based approach. LDM will review the design while there is time to address any feedback before shipping the feature. If the feedback requires adjusting the approach (e.g. the particular signature of the attribute constructor), then it is possible that corresponding public API changes will need to also be made prior to shipping.

[jaredpar]

@333fred, @RikkiGibson is this on the schedule for API review? 2 weeks with no activity so would assume it went through review by now.

[RikkiGibson]

We filed this immediately after the previous API review meeting because it was the part of the proposal for that meeting which we ran out of time to review. Expecting we will review it this Thursday.

[333fred]

API Review

• Do we need to put the API on the GeneratorSyntaxContext, or should it go in a more broad location?
• What if the user doesn't know the hintName yet in this stage of the pipeline?
• Important scenario for generators is that users can switch on EmitGeneratedFiles, then check
  in the files. That won't seem to work here?
  • Compiler provides an option to control where the generated source is emitted
  • Could we just specify from the root of the project?
  • This would potentially also allow us to not need the hintName of the generated file, which could simplify the API.
• Could we have GetInterceptsLocationSpecifier return the entire globally-qualified attribute?
  • Seems odd that we'd save the user typing and escaping the string, but then still require them to specify the attribute?
  • Users may be trying to make the attribute look prettier, handling imports and the like.
  • Perhaps we may want to have a general "give me a string literal that represents this string value" API
• Can we have "handle" based approach that the compiler has more knowledge of? So that if only the location
  changes, the compiler can understand this, and not rerun everything?
  • Complex topic. Maybe possible, but pretty hard.
  • We may also want to look into a ForMethodWithName approach for interceptors which could further optimize this path.

[jaredpar]

Important scenario for generators is that users can switch on EmitGeneratedFiles, then check
in the files. That won't seem to work here?

What about this doesn't work if the user checks in files? The intent of this design is that the file path for generated files is the same whether or not EmitGeneratedFiles is specified in the build. Flipping the switch on should change only if they are written to disk, not where they are written to disk.

Yes the user can change the location by manipulating <CompilerGeneratedFilesOutputPath> directly but that doesn't change the invariant above. Essentially the path should still match.

Do we need to put the API on the GeneratorSyntaxContext, or should it go in a more broad location?

What data is needed to make this decision?

What if the user doesn't know the hintName yet in this stage of the pipeline?

Redesign your pipeline to know it.

[333fred]

What about this doesn't work if the user checks in files?

This was addressed during the meeting, that's why there's a bullet point under that describing the option for controlling where files are emitted.

What data is needed to make this decision?

We likely should examine real interceptor pipelines that don't have a constant hintName.

Redesign your pipeline to know it.

Not possible. This is the start node of the pipeline. There is no way to flow information into this stage from other parts of a pipeline.

[jaredpar]

This was addressed during the meeting, that's why there's a bullet point under that describing the option for controlling where files are emitted.

What was addressed? What specific scenarios do not work?

[333fred]

What specific scenarios do not work?

None. As I said, this concern was addressed during the meeting.

[roji]

Including the method as an extension on GeneratorSyntaxContext pushes it toward only being usable from the transform callback of CreateSyntaxProvider, versus being able to be used from any other context type from a different built-in provider. However, it appears that this is the most suitable provider to use when the generator needs to generate code based on presence of invocations in user source code.

On this - if I understand correctly, that means that creating a location would only be possible in a source generator; other code which generates code but isn't a Roslyn source generator wouldn't be able to use it. The EF approach notably does this - code generation happens in a standalone program (and/or possibly code executed as an MSBuild task on publish), since there are various blockers on generating the interceptors in a Roslyn source generator.

In other words, it would be useful (at least for EF) if there were no dependency on source generator APIs here.

[RikkiGibson]

Updated the proposal toward the checksum-based approach that came out of recent discussions.

[333fred]

API Review

• Why do we need Version and Data?
  • Because we want to allow users to pretty-print the attribute
• We should document the guarantees around Data:
  • Will need no escaping to use as string content
  • Specify the type of string (regular, verbatim, etc)
• Should we make InterceptableLocation an abstract class now so that we can add new internal derived types later?
  • Likely not a breaking change given how roslyn emits method calls, but fine to do anyways.
  • Make the constructor internal or private protected so it can't be inherited publicly
• Move GetInterceptsLocationAttributeSyntax to an extension?
  • Should be fine, it's essentially a helper.

Conclusion: Modulo the move to an abstract type and GetInterceptsLocationAttributeSyntax, this is approved.

[jaredpar]

Will need no escaping to use as string content

Not following this comment. Can you elaborate a bit?

[333fred]

Not following this comment. Can you elaborate a bit?

The guarantee is that Data can be surrounded with quotes and need no further manipulation to be a legal C# string.

[RikkiGibson]

In implementation we found a need to pass a CancellationToken as part of getting the file content hash. I will update the proposal accordingly. Seems like a fairly small/standard change. Let me know if there are any concerns.

[RikkiGibson]

Heads up @mgravell about this change to the experimental feature design, as it looks like DapperAOT is using interceptors.

[RikkiGibson]

Implemented in #72814

[Tornhoof]

That file hash, is it line endings sensitive?
If I understand the PR correctly it's a xx128 hash, so I'll get two different hashes on a win vs Linux build machine?
Won't that break things like deterministic builds?

[RikkiGibson]

If you use different line endings you will get different content hashes.

We don't expect builds to be deterministic across OSes. since, for example, the PDBs already contain have checksums in SHA256.

However, in order to build correctly in a different environment after generating, you will need to take care to not change the line endings. For sources being checked out from git, this is fairly straightforward to achieve through config options.