Analyzer: Recommend upgrading to CompositeFormat for enhanced performance

[geeknoid]

.NET 8 introduces the CompositeFormat abstraction which can substantially improve performance of code that does a lot of formatting (we have some services that spend their day doing this). Having a performance-centric analyzer discovering uses of String.Format/StringBuilder.AppendFormat and recommending updates to CompositeFormat would help the ecosystem evolve. A fixer here should be relatively simple.

Category : Performance
Severity : Hidden

[geeknoid]

Here's the core part of our existing analyzer around recommending string format -> composite format. It's not a full analyzer class since it's built around some higher-level convenience wrappers we use for analyzers, but it has the core logic.

internal sealed class StringFormat
{
    public StringFormat(CallAnalyzer.Registrar reg)
    {
        foreach (var method in reg.Compilation.GetSpecialType(SpecialType.System_String).GetMembers("Format").OfType<IMethodSymbol>())
        {
            reg.RegisterMethod(method, Handle);
        }

        var type = reg.Compilation.GetTypeByMetadataName("System.Text.StringBuilder");
        if (type != null)
        {
            foreach (var method in type.GetMembers("AppendFormat").OfType<IMethodSymbol>())
            {
                reg.RegisterMethod(method, Handle);
            }
        }

        static void Handle(OperationAnalysisContext context, IInvocationOperation op)
        {
            var format = GetFormatArgument(op);
            if (format.ChildNodes().First().IsKind(SyntaxKind.StringLiteralExpression))
            {
                var properties = new Dictionary<string, string?>();
                if (op.TargetMethod.Name == "Format")
                {
                    properties.Add("StringFormat", null);
                }

                var diagnostic = Diagnostic.Create(DiagDescriptors.StringFormat, op.Syntax.GetLocation(), properties.ToImmutableDictionary());
                context.ReportDiagnostic(diagnostic);
            }

            static SyntaxNode GetFormatArgument(IInvocationOperation invocation)
            {
                var sm = invocation.SemanticModel!;
                var arguments = invocation.Arguments;
                var typeInfo = sm.GetTypeInfo(arguments[0].Syntax.ChildNodes().First());

                // This check is needed to identify exactly which argument of string.Format is the format argument
                // The format might be passed as first or second argument
                // if there are more than 1 arguments and first argument is IFormatProvider then format is second argument otherwise it is first
                if (arguments.Length > 1 && typeInfo.Type != null && typeInfo.Type.AllInterfaces.Any(i => i.MetadataName == "IFormatProvider"))
                {
                    return arguments[1].Syntax;
                }

                return arguments[0].Syntax;
            }
        }
    }

[geeknoid]

And here's a corresponding fixer:

[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(StringFormatFixer))]
[Shared]
public sealed class StringFormatFixer : CodeFixProvider
{
    private const string TargetClass = "CompositeFormat";
    private const string TargetMethod = "Format";
    private const string VariableName = "_sf";
    private const int ArgumentsToSkip = 2;
    private static readonly IdentifierNameSyntax _textNamespace = SyntaxFactory.IdentifierName("Microsoft.R9.Extensions.Text");

    /// <inheritdoc/>
    public override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(DiagDescriptors.StringFormat.Id);

    /// <inheritdoc/>
    public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;

    /// <inheritdoc/>
    public override Task RegisterCodeFixesAsync(CodeFixContext context)
    {
        var diagnostics = context.Diagnostics.First();
        context.RegisterCodeFix(
               CodeAction.Create(
                   title: Resources.StringFormatTitle,
                   createChangedDocument: cancellationToken => ApplyFixAsync(context.Document, diagnostics.Location, diagnostics.Properties, cancellationToken),
                   equivalenceKey: nameof(Resources.StringFormatTitle)),
               context.Diagnostics);

        return Task.CompletedTask;
    }

    private static async Task<Document> ApplyFixAsync(Document document, Location diagnosticLocation, IReadOnlyDictionary<string, string?> properties, CancellationToken cancellationToken)
    {
        var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
        if (editor.OriginalRoot.FindNode(diagnosticLocation.SourceSpan) is InvocationExpressionSyntax expression)
        {
            var classDeclaration = GetTypeDeclaration(expression);
            if (classDeclaration != null)
            {
                var (format, argList) = GetFormatAndArguments(editor, expression);
                var formatKind = format.ChildNodes().First().Kind();

                if (formatKind is SyntaxKind.StringLiteralExpression)
                {
                    var (identifier, field) = GetFieldDeclaration(editor, classDeclaration, format);
                    var invocation = properties.ContainsKey("StringFormat")
                        ? CreateInvocationExpression(editor, identifier, argList.Arguments, expression)
                        : GetStringBuilderExpression(editor, identifier, argList.Arguments, expression);
                    ApplyChanges(editor, expression, invocation, classDeclaration, field);
                }
            }
        }

        return editor.GetChangedDocument();
    }

    private static TypeDeclarationSyntax? GetTypeDeclaration(SyntaxNode node)
    {
        return node.FirstAncestorOrSelf<TypeDeclarationSyntax>(n => n.IsKind(SyntaxKind.ClassDeclaration) || n.IsKind(SyntaxKind.StructDeclaration));
    }

    private static (string identifier, FieldDeclarationSyntax? field) GetFieldDeclaration(SyntaxEditor editor, SyntaxNode classDeclaration, SyntaxNode format)
    {
        var members = classDeclaration.DescendantNodes().OfType<FieldDeclarationSyntax>();
        int numberOfMembers = 1;

        var strExp = format.ToString();

        var arguments = SyntaxFactory.Argument(SyntaxFactory.ParseExpression(strExp));
        HashSet<string> fields = new HashSet<string>();

        foreach (var member in members)
        {
            var fieldName = member.DescendantNodes().OfType<VariableDeclaratorSyntax>().First().Identifier.ToString();
            _ = fields.Add(fieldName);

            if (member.Declaration.Type.ToString() == "CompositeFormat")
            {
                if (member.DescendantNodes().OfType<ObjectCreationExpressionSyntax>().First().ArgumentList!.Arguments.First().ToString() == strExp)
                {
                    return (member.DescendantNodes().OfType<VariableDeclaratorSyntax>().First().Identifier.ToString(), null);
                }

                numberOfMembers++;
            }
        }

        string variableName;
        do
        {
            variableName = $"{VariableName}{numberOfMembers}";
            numberOfMembers++;
        }
        while (!IsFieldNameAvailable(fields, variableName));

        return (variableName, editor.Generator.FieldDeclaration(
                        variableName,
                        SyntaxFactory.ParseTypeName(TargetClass),
                        Accessibility.Private,
                        DeclarationModifiers.Static | DeclarationModifiers.ReadOnly,
                        SyntaxFactory.ObjectCreationExpression(
                                SyntaxFactory.Token(SyntaxKind.NewKeyword),
                                SyntaxFactory.IdentifierName(TargetClass),
                                SyntaxFactory.ArgumentList().AddArguments(arguments),
                                null)) as FieldDeclarationSyntax);
    }

    private static bool IsFieldNameAvailable(ICollection<string> fields, string field)
    {
        return !fields.Contains(field);
    }

    private static (ArgumentSyntax argument, ArgumentListSyntax argumentList) GetFormatAndArguments(DocumentEditor editor, InvocationExpressionSyntax invocation)
    {
        var arguments = invocation.ArgumentList.Arguments;
        var first = arguments[0];
        var typeInfo = editor.SemanticModel.GetTypeInfo(first.ChildNodes().First());
        SeparatedSyntaxList<ArgumentSyntax> separatedList;
        if (arguments.Count > 1 && typeInfo.Type!.AllInterfaces.Any(i => i.MetadataName == "IFormatProvider"))
        {
            separatedList = SyntaxFactory.SingletonSeparatedList(first).AddRange(arguments.Skip(ArgumentsToSkip));
            return (arguments[1], SyntaxFactory.ArgumentList(separatedList));
        }

        var nullArgument = SyntaxFactory.Argument(SyntaxFactory.LiteralExpression(SyntaxKind.NullLiteralExpression));
        separatedList = SyntaxFactory.SingletonSeparatedList(nullArgument).AddRange(arguments.Skip(1));
        return (first, SyntaxFactory.ArgumentList(separatedList));
    }

    private static SyntaxNode CreateInvocationExpression(SyntaxEditor editor, string identifierName, IEnumerable<SyntaxNode> arguments, SyntaxNode invocation)
    {
        var gen = editor.Generator;
        var identifier = gen.IdentifierName(identifierName);
        var memberAccessExpression = gen.MemberAccessExpression(identifier, TargetMethod);
        return gen.InvocationExpression(memberAccessExpression, arguments).WithTriviaFrom(invocation);
    }

    private static void ApplyChanges(SyntaxEditor editor, SyntaxNode oldInvocation, SyntaxNode newInvocation, SyntaxNode classDeclaration, SyntaxNode? field)
    {
        if (field != null)
        {
            editor.AddMember(classDeclaration, field);
        }

        editor.ReplaceNode(oldInvocation, newInvocation);
        editor.TryAddUsingDirective(_textNamespace);
    }

    private static SyntaxNode GetStringBuilderExpression(SyntaxEditor editor, string identifierName, IEnumerable<ArgumentSyntax> arguments, InvocationExpressionSyntax invocation)
    {
        var gen = editor.Generator;
        var identifier = gen.IdentifierName(identifierName);
        var memberAccessExpression = gen.Argument(identifier);
        var list = SyntaxFactory.SingletonSeparatedList(memberAccessExpression).AddRange(arguments);
        return invocation.WithArgumentList(SyntaxFactory.ArgumentList(list));
    }
}

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

---

.NET 8 introduces the CompositeFormat abstraction which can substantially improve performance of code that does a lot of formatting (we have some services that spend their day doing this). Having a performance-centric analyzer discovering uses of String.Format/StringBuilder.AppendFormat and recommending updates to CompositeFormat would help the ecosystem evolve. A fixer here should be relatively simple.

Author:	geeknoid
Assignees:	-
Labels:	area-System.Runtime, untriaged, code-analyzer, code-fixer, needs-area-label
Milestone:	-

[jeffhandley]

Triage note: We could consider having the/a fixer create the CompositeFormat lazily upon the first execution so that the static startup cost penalty isn't paid for exception-related call sites (or other call sites that might not get hit).