Higher Level SyntaxProvider APIs for incremental generators

[chsienki]

Background and Motivation

An extremely common pattern seen in source generators involves inserting an attribute via PostInitialization, then finding all symbols that are marked with that attribute. There are currently several different ways to acheive this, and we often hear from customers that they would like a 'canonical' compiler approved method. While we provide examples in the cookbook, it seems like we have an opportunity to provide a set of APIs to do this directly which will allow customers to fall into the 'pit of success' and allow us to potentially optimize further as we know exactly what they are trying to acheive.

Proposed API

namespace Microsoft.CodeAnalysis
{
    public readonly struct SyntaxValueProvider
    {
+       /// <summary>
+       /// Creates an <see cref="IncrementalValuesProvider{T}"/> that can provide a transform over all <see cref="SyntaxNode"/>s marked with a specified attribute.
+       /// </summary>
+       /// <param name="attributeFQN">The FullyQualifiedName of the attribute used to search for nodes that are annotated with it.</param>
+       /// <param name="transform">A function that performs the transfomation of the found nodes</param>
+       /// <returns>An <see cref="IncrementalValuesProvider{T}"/> that provides the results of the transformation</returns>
+       public IncrementalValuesProvider<T> FromAttribute<T>(string attributeFQN, Func<GeneratorSyntaxContext, AttributeData attributeData, CancellationToken, T> transform);
}

Usage Examples

    // regular post initalization injected attribute
    ctx.RegisterPostInitializationOutput((c) => c.AddSource("myAttribute", @"using System;
namespace AutoNotify
{
   [AttributeUsage(AttributeTargets.Field, Inherited = false, AllowMultiple = false)]
   sealed class AutoNotifyAttribute : Attribute
   {
       public AutoNotifyAttribute()
       {
       }
       public string PropertyName { get; set; }
   }
"));
    
    // search by existing / post init injected attribute
    var x = ctx.SyntaxProvider.FromAttribute("AutoNotify.AutoNotifyAttribute", (syntaxContext, attribData, ct) => ...);

Error handling / resolution

There are multiple possible resolution strategies to the attribute look up (should it exist in source only? can it exist in a library?) and we'll take an opinionated stance on this, rather than provide overloads that provide these different behaviours. If an author wants full control they can use the exisiting lower level primitives as today.

Open Questions

Should AttributeData be on the Context or passed as another lambda parameter?

[chsienki]

API Review notes

Drop the generated attribute and just use post init

It seems less useful to be able to simultaneously supply and find the attribute, when we already have PostInit for this purpose, and it would be doing an implicit post init anyway which is a little bit magic. Given that it'll often require dropping to PostInit anyway to control the behavior, lets drop this API for now. We can re-introduce it (taking a full attribute string) if needed down the line.

Also answers Attribute Usage / Visibility

Error Handling / Lookup

Lets be opininated. You can always do it manually if you choose to do so.

We should provide the AttributeData that was found

It seems useful to be able to lookup the attribute that was found. We should provide the attribute data to the caller.

Open Question should this be on the context, or as another param?

Naming

The name is bad, and we should rename

CreateByAttribute with generic TAttribute?

No. Mixing runtime and compile time types is always a complicated idea. Lets not do this.

[333fred]

API Review Notes

Last time, we decided to have AttributeData on the callback. Where to put it?

• We generally do Contexts here, let's keep doing that

This thing is not a provider, but makes providers. That's a factory, and that's pretty well known. We'll go with SyntaxProviderFactory.

namespace Microsoft.CodeAnalysis
{
    public readonly struct SyntaxProviderFactory
    {
+       /// <summary>
+       /// Creates an <see cref="IncrementalValuesProvider{T}"/> that can provide a transform over all <see cref="SyntaxNode"/>s marked with a specified attribute.
+       /// </summary>
+       /// <param name="attributeFQN">The FullyQualifiedName of the attribute used to search for nodes that are annotated with it.</param>
+       /// <param name="transform">A function that performs the transfomation of the found nodes</param>
+       /// <returns>An <see cref="IncrementalValuesProvider{T}"/> that provides the results of the transformation</returns>
+       public IncrementalValuesProvider<T> FromAttribute<T>(string attributeFQN, Func<GeneratorAttributeSyntaxContext, AttributeData attributeData, CancellationToken, T> transform);
}

-        public IncrementalValuesProvider<T> CreateSyntaxProvider<T>(Func<SyntaxNode, CancellationToken, bool> predicate, Func<GeneratorSyntaxContext, CancellationToken, T> transform)
+        public IncrementalValuesProvider<T> FromPredicate<T>(Func<SyntaxNode, CancellationToken, bool> predicate, Func<GeneratorSyntaxContext, CancellationToken, T> transform)
    }
+   public readonly struct GeneratorAttributeSyntaxContext
+   {
+       ... Existing properties from GeneratorSyntaxContext
+       public AttributeData AttributeData { get; }
+   }
}

[chsienki]

We probably want a way to filter syntax nodes without realizing the red tree too. We've observed in some scenarios, such as very large files, that the syntax filter becomes a bottleneck. If we could for instance, have a simple filter by kind, we could potentially do it without the need to realize any red nodes.

[AraHaan]

Here is a fun story:

1. Generator generates attributes for itself to use (a few of them taking a single boolean parameter)
2. User uses generated attributes from the Generator to mark items for the generator to write more code for.
3. Generator uses SyntaxReciever to read the attributes and their parameters.
4. Generator crashes due to the obtained Attribute's AttributeData instance does not contain any items for it's ConstructorArguments or even TypeArguments (I think it should be filled in with 1 when they pass in 1) to satisfy the generator requirements and fix it). Which means I found a roslyn bug. 😄

[CyrusNajmabadi]

Going to bring back to review with a slight tweak (to help another simple case).

[CyrusNajmabadi]

API:

public readonly struct GeneratorAttributeSyntaxContext<TSyntaxNode>
    where TSyntaxNode : SyntaxNode
{
    public TSyntaxNode Node { get; }
    public SemanticModel SemanticModel { get; }
    public AttributeData AttributeData { get; }

    internal GeneratorAttributeSyntaxContext(TSyntaxNode node, SemanticModel semanticModel, AttributeData attributeData)
    {
        Node = node;
        SemanticModel = semanticModel;
        AttributeData = attributeData;
    }
}

    /// <summary>
    /// Returns all syntax nodes of type <typeparamref name="T"/> if that node has an attribute on it that binds to a
    /// <see cref="INamedTypeSymbol"/> with the same fully-qualified metadata as the provided <paramref
    /// name="fullyQualifiedMetadataName"/>. <paramref name="fullyQualifiedMetadataName"/> should be the
    /// fully-qualified, metadata name of the attribute, including the <c>Attribute</c> suffix.  For example
    /// <c>System.CLSCompliantAttribute</c> for <see cref="System.CLSCompliantAttribute"/>.
    /// <para>This provider understands <see langword="using"/> aliases and will find matches even when the attribute
    /// references an alias name.  For example, given:
    /// <code>
    /// using XAttribute = System.CLSCompliantAttribute;
    /// [X]
    /// class C { }
    /// </code>
    /// Then
    /// <c>context.SyntaxProvider.CreateSyntaxProviderForAttribute&lt;ClassDeclarationSyntax&gt;(typeof(CLSCompliantAttribute).FullName)</c>
    /// will find the <c>C</c> class.</para>
    /// </summary>
    /// <remarks>
    /// The <typeparamref name="T"/> should be given the type of the syntax node that owns the <see
    /// cref="T:Microsoft.CodeAnalysis.CSharp.Syntax.AttributeListSyntax"/> that contains the matching attribute.  For
    /// the example above, that would be a <see cref="T:Microsoft.CodeAnalysis.CSharp.Syntax.ClassDeclarationSyntax"/>.
    /// <see cref="SyntaxNode"/> can be used as the type argument to return every syntax node of any type that has such
    /// a matching attribute on it.
    /// </remarks>
    public IncrementalValuesProvider<T> ForAttributeWithMetadataName<T>(string fullyQualifiedMetadataName) where T : SyntaxNode;

    /// <summary>
    /// Creates an <see cref="IncrementalValuesProvider{T}"/> that can provide a transform over all <typeparamref
    /// name="TSyntaxNode"/>s if that node has an attribute on it that binds to a <see cref="INamedTypeSymbol"/> with
    /// the same fully-qualified metadata as the provided <paramref name="fullyQualifiedMetadataName"/>. <paramref
    /// name="fullyQualifiedMetadataName"/> should be the fully-qualified, metadata name of the attribute, including the
    /// <c>Attribute</c> suffix.  For example <c>System.CLSCompliantAttribute</c> for <see
    /// cref="System.CLSCompliantAttribute"/>.
    /// </summary>
    public IncrementalValuesProvider<TResult> ForAttributeWithMetadataName<TSyntaxNode, TResult>(
        string fullyQualifiedMetadataName,
        Func<GeneratorAttributeSyntaxContext<TSyntaxNode>, AttributeData, CancellationToken, TResult> transform)
        where TSyntaxNode : SyntaxNode;

[333fred]

API Review

Overall the approach has promise, but we have some notes to look at before finalizing.

• AttributeData is provided in multiple places.
• GeneratorAttributeSyntaxContext might want to provide the target symbol
• If we keep the syntax generic parameter, a better name might be TAttributeTarget
• Our analyzers don't use generics, they use SyntaxKinds. Maybe we should do the same: it will allow filtering to multiple declaration kinds, and potentially make calling more ergonomic by allowing the user to leave off the generic arguments.
  • We might also be able use SymbolKinds instead, to allow language agnosticism?
• The non-transform overload should be removed. We don't think it's necessary, and could lead users down a pit of failure by causing more recalculation later in the pipeline.

[CyrusNajmabadi]

<3 @333fred . Thanks!

[AraHaan]

@CyrusNajmabadi Thanks for the new apis, mind opening an issue for my Source Generator as well so I can remember to migrate to it as well when Visual Studio and the .NET SDK starts using a version of roslyn that ships with this new API?

[alrz]

Could this work with a base attributes?

What I'm trying to do is to define a base attribute to plug code generation to the target symbol:

[MyImplementor]
public partial void Method();

[MyDecorator]
public virtual void Method();

[MyInterceptor]
public void Method()

class MyAttribute : SomeBaseAttribute, IInterceptor, IDecorator, IImplementor
{
	// called for each invocation,
	// adds context (intercept attributes) + <new method body>
	public void Intercept(methodInvocation, codeProductionContext) { ... }

	// adds context (namespace, type) + override void Method() { <added code> base.Method(); <added code> } 
	public void Decorate(targetVirtualMethod, codeProductionContext) { ... }

	// adds context (namespace, type) + partial void Method() { <method body> } 
	public void Implement(targetPartialMethod, codeProductionContext) { ... }
}

Something like [Implement(typeof(MyImplementor))] or using generics could work but it's a little bit verbose :)

[333fred]

Unfortunately supporting that would obviate all the perf benefits.

[alrz]

I think calling ForAttributeWithMetadataName for each attribute doesn't scale as well. Perhaps if ForAttributeWithMetadataName could accept multiple names at once the pipeline could be more efficient?

[CyrusNajmabadi]

FAWMN should scale well. What's the issue you're concerned with there?

[alrz]

It feels like it could reduce the provider instances required, but I guess that doesn't matter much.

[alrz]

@CyrusNajmabadi

I managed to gather attributes that should be looked up, but those would be still inside a provider.

I think for that, this overload could be helpful:

public IncrementalValuesProvider<T> ForAttributeWithMetadataName<T>(
        IncrementalValuesProvider<string> names, // get all the values
        Func<SyntaxNode, CancellationToken, bool> predicate,
        Func<GeneratorAttributeSyntaxContext, CancellationToken, T> transform)

Would it be possible to implement this?

[CyrusNajmabadi]

wait. the attributes themselves are not known ahead of time? What scenario is this?

[alrz]

the attributes themselves are not known ahead of time?

They are, but they could be dynamically declared in a separate assembly.
I think the only alternative is to declare one generator per each.

public abstract class InterceptorGenerator<TAttribute> : IIncrementalGenerator
    where TAttribute : Attribute, IInterceptor

[Generator] public class InterceptorGenerator1 : InterceptorGenerator<InterceptorAttribute1>;
[Generator] public class InterceptorGeneraote2 : InterceptorGenerator<InterceptorAttribute2>;

[CyrusNajmabadi]

They are, but they could be dynamically declared in a separate assembly.

I'm not sure why that matters. All that matters here is the FQN, which would be known ahead of time. So i'm not sure why multiple value-providers need to feel into ForAttributeWithMetadataName. (or why even a single value provided would feed into it :)).

I can see the desire to pass in multiple FQNs into ForAttributeWithMetadataName. However, that is trivially solvable with an extension method that accompishes the same with a few Combines and a SelectMany :)