Do not offer to simplify interpolations when using formattable strings

src/Analyzers/CSharp/Analyzers/SimplifyInterpolation/CSharpSimplifyInterpolationDiagnosticAnalyzer.cs

@@ -14,14 +14,10 @@
 namespace Microsoft.CodeAnalysis.CSharp.SimplifyInterpolation;
 
 [DiagnosticAnalyzer(LanguageNames.CSharp)]
-internal class CSharpSimplifyInterpolationDiagnosticAnalyzer : AbstractSimplifyInterpolationDiagnosticAnalyzer<
-    InterpolationSyntax, ExpressionSyntax>
+internal sealed class CSharpSimplifyInterpolationDiagnosticAnalyzer
+    : AbstractSimplifyInterpolationDiagnosticAnalyzer<InterpolationSyntax, ExpressionSyntax>
 {
-    protected override IVirtualCharService GetVirtualCharService()
-        => CSharpVirtualCharService.Instance;
-
-    protected override ISyntaxFacts GetSyntaxFacts()
-        => CSharpSyntaxFacts.Instance;
-
-    protected override AbstractSimplifyInterpolationHelpers GetHelpers() => CSharpSimplifyInterpolationHelpers.Instance;
+    protected override ISyntaxFacts SyntaxFacts => CSharpSyntaxFacts.Instance;
+    protected override IVirtualCharService VirtualCharService => CSharpVirtualCharService.Instance;
+    protected override AbstractSimplifyInterpolationHelpers<InterpolationSyntax, ExpressionSyntax> Helpers => CSharpSimplifyInterpolationHelpers.Instance;
 }

src/Analyzers/CSharp/Analyzers/SimplifyInterpolation/CSharpSimplifyInterpolationHelpers.cs

@@ -7,7 +7,8 @@
 
 namespace Microsoft.CodeAnalysis.CSharp.Analyzers.SimplifyInterpolation;
 
-internal sealed class CSharpSimplifyInterpolationHelpers : AbstractSimplifyInterpolationHelpers
+internal sealed class CSharpSimplifyInterpolationHelpers
+    : AbstractSimplifyInterpolationHelpers<InterpolationSyntax, ExpressionSyntax>
 {
     public static CSharpSimplifyInterpolationHelpers Instance { get; } = new();
 

src/Analyzers/CSharp/CodeFixes/SimplifyInterpolation/CSharpSimplifyInterpolationCodeFixProvider.cs

@@ -16,10 +16,13 @@ namespace Microsoft.CodeAnalysis.CSharp.SimplifyInterpolation;
 [method: ImportingConstructor]
 [method: SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
 internal sealed class CSharpSimplifyInterpolationCodeFixProvider() : AbstractSimplifyInterpolationCodeFixProvider<
-    InterpolationSyntax, ExpressionSyntax, InterpolationAlignmentClauseSyntax,
-    InterpolationFormatClauseSyntax, InterpolatedStringExpressionSyntax>
+    InterpolationSyntax,
+    ExpressionSyntax,
+    InterpolationAlignmentClauseSyntax,
+    InterpolationFormatClauseSyntax,
+    InterpolatedStringExpressionSyntax>
 {
-    protected override AbstractSimplifyInterpolationHelpers GetHelpers() => CSharpSimplifyInterpolationHelpers.Instance;
+    protected override AbstractSimplifyInterpolationHelpers<InterpolationSyntax, ExpressionSyntax> Helpers => CSharpSimplifyInterpolationHelpers.Instance;
 
     protected override InterpolationSyntax WithExpression(InterpolationSyntax interpolation, ExpressionSyntax expression)
         => interpolation.WithExpression(expression);

src/Analyzers/CSharp/Tests/SimplifyInterpolation/SimplifyInterpolationTests.cs

@@ -16,13 +16,9 @@
 namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.SimplifyInterpolation;
 
 [Trait(Traits.Feature, Traits.Features.CodeActionsSimplifyInterpolation)]
-public partial class SimplifyInterpolationTests : AbstractCSharpDiagnosticProviderBasedUserDiagnosticTest_NoEditor
+public sealed class SimplifyInterpolationTests(ITestOutputHelper logger)
+    : AbstractCSharpDiagnosticProviderBasedUserDiagnosticTest_NoEditor(logger)
 {
-    public SimplifyInterpolationTests(ITestOutputHelper logger)
-      : base(logger)
-    {
-    }
-
     internal override (DiagnosticAnalyzer, CodeFixProvider) CreateDiagnosticProviderAndFixer(Workspace workspace)
         => (new CSharpSimplifyInterpolationDiagnosticAnalyzer(), new CSharpSimplifyInterpolationCodeFixProvider());
 
@@ -1359,4 +1355,22 @@ void M(bool cond)
             }
             """);
     }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/47956")]
+    public async Task TestNotPassedToFormattableString1()
+    {
+        await TestMissingAsync(
+            """
+            class C
+            {
+                void B() => M($"{args[||].Length.ToString()}");
+
+                void M(FormattableString fs)
+                {
+                    foreach (object o in fs.GetArguments())
+                        Console.WriteLine(o?.GetType());
+                }
+            }
+            """);
+    }
 }

src/Analyzers/Core/Analyzers/SimplifyInterpolation/AbstractSimplifyInterpolationDiagnosticAnalyzer.cs

@@ -2,61 +2,75 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using System.Collections.Immutable;
+using System;
 using Microsoft.CodeAnalysis.CodeStyle;
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.EmbeddedLanguages.VirtualChars;
 using Microsoft.CodeAnalysis.LanguageService;
 using Microsoft.CodeAnalysis.Operations;
+using Microsoft.CodeAnalysis.Shared.Extensions;
 
 namespace Microsoft.CodeAnalysis.SimplifyInterpolation;
 
 internal abstract class AbstractSimplifyInterpolationDiagnosticAnalyzer<
     TInterpolationSyntax,
-    TExpressionSyntax> : AbstractBuiltInUnnecessaryCodeStyleDiagnosticAnalyzer
+    TExpressionSyntax>() : AbstractBuiltInUnnecessaryCodeStyleDiagnosticAnalyzer(IDEDiagnosticIds.SimplifyInterpolationId,
+        EnforceOnBuildValues.SimplifyInterpolation,
+        CodeStyleOptions2.PreferSimplifiedInterpolation,
+        fadingOption: null,
+        new LocalizableResourceString(nameof(AnalyzersResources.Simplify_interpolation), AnalyzersResources.ResourceManager, typeof(AnalyzersResources)),
+        new LocalizableResourceString(nameof(AnalyzersResources.Interpolation_can_be_simplified), AnalyzersResources.ResourceManager, typeof(AnalyzersResources)))
     where TInterpolationSyntax : SyntaxNode
     where TExpressionSyntax : SyntaxNode
 {
-    protected AbstractSimplifyInterpolationDiagnosticAnalyzer()
-       : base(IDEDiagnosticIds.SimplifyInterpolationId,
-              EnforceOnBuildValues.SimplifyInterpolation,
-              CodeStyleOptions2.PreferSimplifiedInterpolation,
-              fadingOption: null,
-              new LocalizableResourceString(nameof(AnalyzersResources.Simplify_interpolation), AnalyzersResources.ResourceManager, typeof(AnalyzersResources)),
-              new LocalizableResourceString(nameof(AnalyzersResources.Interpolation_can_be_simplified), AnalyzersResources.ResourceManager, typeof(AnalyzersResources)))
-    {
-    }
-
-    protected abstract IVirtualCharService GetVirtualCharService();
-
-    protected abstract ISyntaxFacts GetSyntaxFacts();
-
-    protected abstract AbstractSimplifyInterpolationHelpers GetHelpers();
+    protected abstract ISyntaxFacts SyntaxFacts { get; }
+    protected abstract IVirtualCharService VirtualCharService { get; }
+    protected abstract AbstractSimplifyInterpolationHelpers<TInterpolationSyntax, TExpressionSyntax> Helpers { get; }
 
     public override DiagnosticAnalyzerCategory GetAnalyzerCategory()
         => DiagnosticAnalyzerCategory.SemanticSpanAnalysis;
 
     protected override void InitializeWorker(AnalysisContext context)
-        => context.RegisterOperationAction(AnalyzeInterpolation, OperationKind.Interpolation);
+        => context.RegisterCompilationStartAction(
+            context =>
+            {
+                var formattableStringType = context.Compilation.FormattableStringType();
+                context.RegisterOperationAction(context => AnalyzeInterpolation(context, formattableStringType), OperationKind.Interpolation);
+            });
 
-    private void AnalyzeInterpolation(OperationAnalysisContext context)
+    private void AnalyzeInterpolation(
+        OperationAnalysisContext context,
+        INamedTypeSymbol? formattableStringType)
     {
         var option = context.GetAnalyzerOptions().PreferSimplifiedInterpolation;
+
+        // No point in analyzing if the option is off.
         if (!option.Value || ShouldSkipAnalysis(context, option.Notification))
-        {
-            // No point in analyzing if the option is off.
             return;
-        }
 
         var interpolation = (IInterpolationOperation)context.Operation;
-        GetHelpers().UnwrapInterpolation<TInterpolationSyntax, TExpressionSyntax>(
-            GetVirtualCharService(), GetSyntaxFacts(), interpolation, out _, out var alignment, out _,
+
+        // Formattable strings can observe the inner types of the arguments passed to them.  So we can't safely change
+        // to drop ToString in that.
+        if (interpolation.Parent is IInterpolatedStringOperation { Parent: IConversionOperation { Type: { } convertedType } conversion } &&
+            convertedType.Equals(formattableStringType))
+        {
+            // One exception to this is calling directly into FormattableString.Invariant.  That method has known good
+            // behavior that is fine to continue calling into.
+            var isInvariantInvocation =
+                conversion.Parent is IArgumentOperation { Parent: IInvocationOperation invocation } &&
+                invocation.TargetMethod.Name == nameof(FormattableString.Invariant) &&
+                invocation.TargetMethod.ContainingType.Equals(formattableStringType);
+            if (!isInvariantInvocation)
+                return;
+        }
+
+        this.Helpers.UnwrapInterpolation(
+            this.VirtualCharService, this.SyntaxFacts, interpolation, out _, out var alignment, out _,
             out var formatString, out var unnecessaryLocations);
 
         if (alignment == null && formatString == null)
-        {
             return;
-        }
 
         // The diagnostic itself fades the first unnecessary location, and the remaining locations are passed as
         // additional unnecessary locations.

src/Analyzers/Core/Analyzers/SimplifyInterpolation/AbstractSimplifyInterpolationHelpers.cs

@@ -11,30 +11,38 @@
 using Microsoft.CodeAnalysis.EmbeddedLanguages.VirtualChars;
 using Microsoft.CodeAnalysis.LanguageService;
 using Microsoft.CodeAnalysis.Operations;
+using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.Text;
 using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.SimplifyInterpolation;
 
-internal abstract class AbstractSimplifyInterpolationHelpers
+internal abstract class AbstractSimplifyInterpolationHelpers<
+    TInterpolationSyntax,
+    TExpressionSyntax>
+    where TInterpolationSyntax : SyntaxNode
+    where TExpressionSyntax : SyntaxNode
 {
     protected abstract bool PermitNonLiteralAlignmentComponents { get; }
 
     protected abstract SyntaxNode GetPreservedInterpolationExpressionSyntax(IOperation operation);
 
-    public void UnwrapInterpolation<TInterpolationSyntax, TExpressionSyntax>(
-        IVirtualCharService virtualCharService, ISyntaxFacts syntaxFacts, IInterpolationOperation interpolation,
-        out TExpressionSyntax? unwrapped, out TExpressionSyntax? alignment, out bool negate,
-        out string? formatString, out ImmutableArray<Location> unnecessaryLocations)
-            where TInterpolationSyntax : SyntaxNode
-            where TExpressionSyntax : SyntaxNode
+    public void UnwrapInterpolation(
+        IVirtualCharService virtualCharService,
+        ISyntaxFacts syntaxFacts,
+        IInterpolationOperation interpolation,
+        out TExpressionSyntax? unwrapped,
+        out TExpressionSyntax? alignment,
+        out bool negate,
+        out string? formatString,
+        out ImmutableArray<Location> unnecessaryLocations)
     {
         alignment = null;
         negate = false;
         formatString = null;
 
-        var unnecessarySpans = new List<TextSpan>();
+        using var _ = ArrayBuilder<TextSpan>.GetInstance(out var unnecessarySpans);
 
         var expression = Unwrap(interpolation.Expression);
         if (interpolation.Alignment == null)
@@ -73,8 +81,12 @@ public void UnwrapInterpolation<TInterpolationSyntax, TExpressionSyntax>(
     }
 
     private void UnwrapFormatString(
-        IVirtualCharService virtualCharService, ISyntaxFacts syntaxFacts, IOperation expression, out IOperation unwrapped,
-        out string? formatString, List<TextSpan> unnecessarySpans)
+        IVirtualCharService virtualCharService,
+        ISyntaxFacts syntaxFacts,
+        IOperation expression,
+        out IOperation unwrapped,
+        out string? formatString,
+        ArrayBuilder<TextSpan> unnecessarySpans)
     {
         Contract.ThrowIfNull(expression.SemanticModel);
 
@@ -188,10 +200,12 @@ private static TextSpan GetSpanWithinLiteralQuotes(IVirtualCharService virtualCh
             : TextSpan.FromBounds(sequence.First().Span.Start, sequence.Last().Span.End);
     }
 
-    private void UnwrapAlignmentPadding<TExpressionSyntax>(
-        IOperation expression, out IOperation unwrapped,
-        out TExpressionSyntax? alignment, out bool negate, List<TextSpan> unnecessarySpans)
-        where TExpressionSyntax : SyntaxNode
+    private void UnwrapAlignmentPadding(
+        IOperation expression,
+        out IOperation unwrapped,
+        out TExpressionSyntax? alignment,
+        out bool negate,
+        ArrayBuilder<TextSpan> unnecessarySpans)
     {
         if (expression is IInvocationOperation invocation &&
             HasNonImplicitInstance(invocation, out var instance))

src/Analyzers/Core/CodeFixes/SimplifyInterpolation/AbstractSimplifyInterpolationCodeFixProvider.cs

@@ -2,12 +2,9 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using System;
 using System.Collections.Immutable;
-using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
-using Microsoft.CodeAnalysis.CodeActions;
 using Microsoft.CodeAnalysis.CodeFixes;
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Editing;
@@ -33,7 +30,7 @@ internal abstract class AbstractSimplifyInterpolationCodeFixProvider<
     public override ImmutableArray<string> FixableDiagnosticIds { get; } =
         [IDEDiagnosticIds.SimplifyInterpolationId];
 
-    protected abstract AbstractSimplifyInterpolationHelpers GetHelpers();
+    protected abstract AbstractSimplifyInterpolationHelpers<TInterpolationSyntax, TExpressionSyntax> Helpers { get; }
 
     protected abstract TInterpolationSyntax WithExpression(TInterpolationSyntax interpolation, TExpressionSyntax expression);
     protected abstract TInterpolationSyntax WithAlignmentClause(TInterpolationSyntax interpolation, TInterpolationAlignmentClause alignmentClause);
@@ -53,16 +50,16 @@ protected override async Task FixAllAsync(
         var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false);
         var generator = editor.Generator;
         var generatorInternal = document.GetRequiredLanguageService<SyntaxGeneratorInternal>();
-        var helpers = GetHelpers();
+        var helpers = this.Helpers;
 
         foreach (var diagnostic in diagnostics)
         {
-            var loc = diagnostic.AdditionalLocations[0];
-            var interpolation = semanticModel.GetOperation(loc.FindNode(getInnermostNodeForTie: true, cancellationToken), cancellationToken) as IInterpolationOperation;
+            var node = diagnostic.AdditionalLocations[0].FindNode(getInnermostNodeForTie: true, cancellationToken);
+            var interpolation = semanticModel.GetOperation(node, cancellationToken) as IInterpolationOperation;
             if (interpolation?.Syntax is TInterpolationSyntax interpolationSyntax &&
                 interpolationSyntax.Parent is TInterpolatedStringExpressionSyntax interpolatedString)
             {
-                helpers.UnwrapInterpolation<TInterpolationSyntax, TExpressionSyntax>(
+                helpers.UnwrapInterpolation(
                     document.GetRequiredLanguageService<IVirtualCharLanguageService>(),
                     document.GetRequiredLanguageService<ISyntaxFactsService>(),
                     interpolation, out var unwrapped, out var alignment, out var negate, out var formatString, out _);
@@ -71,9 +68,7 @@ protected override async Task FixAllAsync(
                     continue;
 
                 if (alignment != null && negate)
-                {
                     alignment = (TExpressionSyntax)generator.NegateExpression(alignment);
-                }
 
                 editor.ReplaceNode(
                     interpolationSyntax,

src/Analyzers/VisualBasic/Analyzers/SimplifyInterpolation/VisualBasicSimplifyInterpolationDiagnosticAnalyzer.vb

@@ -12,19 +12,11 @@ Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
 
 Namespace Microsoft.CodeAnalysis.VisualBasic.SimplifyInterpolation
     <DiagnosticAnalyzer(LanguageNames.VisualBasic)>
-    Friend Class VisualBasicSimplifyInterpolationDiagnosticAnalyzer
+    Friend NotInheritable Class VisualBasicSimplifyInterpolationDiagnosticAnalyzer
         Inherits AbstractSimplifyInterpolationDiagnosticAnalyzer(Of InterpolationSyntax, ExpressionSyntax)
 
-        Protected Overrides Function GetHelpers() As AbstractSimplifyInterpolationHelpers
-            Return VisualBasicSimplifyInterpolationHelpers.Instance
-        End Function
-
-        Protected Overrides Function GetVirtualCharService() As IVirtualCharService
-            Return VisualBasicVirtualCharService.Instance
-        End Function
-
-        Protected Overrides Function GetSyntaxFacts() As ISyntaxFacts
-            Return VisualBasicSyntaxFacts.Instance
-        End Function
+        Protected Overrides ReadOnly Property SyntaxFacts As ISyntaxFacts = VisualBasicSyntaxFacts.Instance
+        Protected Overrides ReadOnly Property VirtualCharService As IVirtualCharService = VisualBasicVirtualCharService.Instance
+        Protected Overrides ReadOnly Property Helpers As AbstractSimplifyInterpolationHelpers(Of InterpolationSyntax, ExpressionSyntax) = VisualBasicSimplifyInterpolationHelpers.Instance
     End Class
 End Namespace

src/Analyzers/VisualBasic/Analyzers/SimplifyInterpolation/VisualBasicSimplifyInterpolationHelpers.vb

@@ -3,10 +3,11 @@
 ' See the LICENSE file in the project root for more information.
 
 Imports Microsoft.CodeAnalysis.SimplifyInterpolation
+Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
 
 Namespace Microsoft.CodeAnalysis.VisualBasic.SimplifyInterpolation
     Friend NotInheritable Class VisualBasicSimplifyInterpolationHelpers
-        Inherits AbstractSimplifyInterpolationHelpers
+        Inherits AbstractSimplifyInterpolationHelpers(Of InterpolationSyntax, ExpressionSyntax)
 
         Public Shared ReadOnly Property Instance As New VisualBasicSimplifyInterpolationHelpers
 

src/Analyzers/VisualBasic/CodeFixes/SimplifyInterpolation/VisualBasicSimplifyInterpolationCodeFixProvider.vb

@@ -10,7 +10,7 @@ Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
 
 Namespace Microsoft.CodeAnalysis.VisualBasic.SimplifyInterpolation
     <ExportCodeFixProvider(LanguageNames.VisualBasic, Name:=PredefinedCodeFixProviderNames.SimplifyInterpolation), [Shared]>
-    Friend Class VisualBasicSimplifyInterpolationCodeFixProvider
+    Friend NotInheritable Class VisualBasicSimplifyInterpolationCodeFixProvider
         Inherits AbstractSimplifyInterpolationCodeFixProvider(Of
             InterpolationSyntax, ExpressionSyntax, InterpolationAlignmentClauseSyntax,
             InterpolationFormatClauseSyntax, InterpolatedStringExpressionSyntax)
@@ -20,9 +20,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.SimplifyInterpolation
         Public Sub New()
         End Sub
 
-        Protected Overrides Function GetHelpers() As AbstractSimplifyInterpolationHelpers
-            Return VisualBasicSimplifyInterpolationHelpers.Instance
-        End Function
+        Protected Overrides ReadOnly Property Helpers As AbstractSimplifyInterpolationHelpers(Of InterpolationSyntax, ExpressionSyntax) = VisualBasicSimplifyInterpolationHelpers.Instance
 
         Protected Overrides Function WithExpression(interpolation As InterpolationSyntax, expression As ExpressionSyntax) As InterpolationSyntax
             Return interpolation.WithExpression(expression)

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ICompilationExtensions.cs

@@ -99,6 +99,9 @@ public static ImmutableArray<IAssemblySymbol> GetReferencedAssemblySymbols(this
     public static INamedTypeSymbol? ThreadStaticAttributeType(this Compilation compilation)
         => compilation.GetTypeByMetadataName(typeof(ThreadStaticAttribute).FullName!);
 
+    public static INamedTypeSymbol? FormattableStringType(this Compilation compilation)
+        => compilation.GetTypeByMetadataName(typeof(FormattableString).FullName!);
+
     public static INamedTypeSymbol? EventArgsType(this Compilation compilation)
         => compilation.GetTypeByMetadataName(typeof(EventArgs).FullName!);