Merge pull request #40070 from CyrusNajmabadi/simplifyInterpolation

Add feature to simplify interpolation expressions when possible

Author: Andrew Hall

src/EditorFeatures/CSharpTest/SimplifyInterpolation/SimplifyInterpolationTests.cs

@@ -353,6 +353,9 @@ public void CSharp_VerifyIDEDiagnosticSeveritiesAreConfigurable()
 # IDE0070
 dotnet_diagnostic.IDE0070.severity = %value%
 
+# IDE0071
+dotnet_style_prefer_simplified_interpolation = true:suggestion
+
 # IDE0072
 dotnet_diagnostic.IDE0072.severity = %value%
 
@@ -487,6 +490,9 @@ public void VisualBasic_VerifyIDEDiagnosticSeveritiesAreConfigurable()
 # IDE0070
 dotnet_diagnostic.IDE0070.severity = %value%
 
+# IDE0071
+dotnet_style_prefer_simplified_interpolation = true:suggestion
+
 # IDE1006
 dotnet_diagnostic.IDE1006.severity = %value%
 
@@ -835,6 +841,9 @@ No editorconfig based code style option
 # IDE0070
 No editorconfig based code style option
 
+# IDE0071, PreferSimplifiedInterpolation
+dotnet_style_prefer_simplified_interpolation = true:suggestion
+
 # IDE0072
 No editorconfig based code style option
 
@@ -1002,6 +1011,9 @@ No editorconfig based code style option
 # IDE0070
 No editorconfig based code style option
 
+# IDE0071, PreferSimplifiedInterpolation
+dotnet_style_prefer_simplified_interpolation = true:suggestion
+
 # IDE1006
 No editorconfig based code style option
 

src/EditorFeatures/Test/Diagnostics/IDEDiagnosticIDConfigurationTests.cs

@@ -4,17 +4,16 @@
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Linq;
+using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CodeActions;
-using Microsoft.CodeAnalysis.CodeFixes.Suppression;
 using Microsoft.CodeAnalysis.CodeStyle;
 using Microsoft.CodeAnalysis.Editor.Implementation.Preview;
 using Microsoft.CodeAnalysis.Editor.UnitTests.Extensions;
 using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
 using Microsoft.CodeAnalysis.Options;
-using Microsoft.CodeAnalysis.Remote;
 using Microsoft.CodeAnalysis.Shared.Utilities;
 using Microsoft.CodeAnalysis.Test.Utilities;
 using Microsoft.CodeAnalysis.Text;
@@ -37,6 +36,7 @@ public struct TestParameters
             internal readonly int index;
             internal readonly CodeActionPriority? priority;
             internal readonly bool retainNonFixableDiagnostics;
+            internal readonly bool includeDiagnosticsOutsideSelection;
             internal readonly string title;
 
             internal TestParameters(
@@ -47,6 +47,7 @@ internal TestParameters(
                 int index = 0,
                 CodeActionPriority? priority = null,
                 bool retainNonFixableDiagnostics = false,
+                bool includeDiagnosticsOutsideSelection = false,
                 string title = null)
             {
                 this.parseOptions = parseOptions;
@@ -56,23 +57,27 @@ internal TestParameters(
                 this.index = index;
                 this.priority = priority;
                 this.retainNonFixableDiagnostics = retainNonFixableDiagnostics;
+                this.includeDiagnosticsOutsideSelection = includeDiagnosticsOutsideSelection;
                 this.title = title;
             }
 
             public TestParameters WithParseOptions(ParseOptions parseOptions)
-                => new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, title: title);
+                => new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, retainNonFixableDiagnostics, includeDiagnosticsOutsideSelection, title);
 
             public TestParameters WithOptions(IDictionary<OptionKey, object> options)
-                => new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, title: title);
+                => new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, retainNonFixableDiagnostics, includeDiagnosticsOutsideSelection, title);
 
             public TestParameters WithFixProviderData(object fixProviderData)
-                => new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, title: title);
+                => new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, retainNonFixableDiagnostics, includeDiagnosticsOutsideSelection, title);
 
             public TestParameters WithIndex(int index)
-                => new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, title: title);
+                => new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, retainNonFixableDiagnostics, includeDiagnosticsOutsideSelection, title);
 
             public TestParameters WithRetainNonFixableDiagnostics(bool retainNonFixableDiagnostics)
-                => new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, title: title, retainNonFixableDiagnostics: retainNonFixableDiagnostics);
+                => new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, retainNonFixableDiagnostics, includeDiagnosticsOutsideSelection, title);
+
+            public TestParameters WithIncludeDiagnosticsOutsideSelection(bool includeDiagnosticsOutsideSelection)
+                => new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, retainNonFixableDiagnostics, includeDiagnosticsOutsideSelection, title);
         }
 
         protected abstract string GetLanguage();
@@ -373,17 +378,35 @@ private async Task TestAsync(
             CodeActionPriority? priority,
             TestParameters parameters)
         {
+            MarkupTestFile.GetSpans(
+                initialMarkup.NormalizeLineEndings(),
+                out var initialMarkupWithoutSpans, out IDictionary<string, ImmutableArray<TextSpan>> initialSpanMap);
+
+            const string UnnecessaryMarkupKey = "Unnecessary";
+            var unnecessarySpans = initialSpanMap.GetOrAdd(UnnecessaryMarkupKey, _ => ImmutableArray<TextSpan>.Empty);
+
             MarkupTestFile.GetSpans(
                 expectedMarkup.NormalizeLineEndings(),
-                out var expected, out IDictionary<string, ImmutableArray<TextSpan>> spanMap);
+                out var expected, out IDictionary<string, ImmutableArray<TextSpan>> expectedSpanMap);
 
-            var conflictSpans = spanMap.GetOrAdd("Conflict", _ => ImmutableArray<TextSpan>.Empty);
-            var renameSpans = spanMap.GetOrAdd("Rename", _ => ImmutableArray<TextSpan>.Empty);
-            var warningSpans = spanMap.GetOrAdd("Warning", _ => ImmutableArray<TextSpan>.Empty);
-            var navigationSpans = spanMap.GetOrAdd("Navigation", _ => ImmutableArray<TextSpan>.Empty);
+            var conflictSpans = expectedSpanMap.GetOrAdd("Conflict", _ => ImmutableArray<TextSpan>.Empty);
+            var renameSpans = expectedSpanMap.GetOrAdd("Rename", _ => ImmutableArray<TextSpan>.Empty);
+            var warningSpans = expectedSpanMap.GetOrAdd("Warning", _ => ImmutableArray<TextSpan>.Empty);
+            var navigationSpans = expectedSpanMap.GetOrAdd("Navigation", _ => ImmutableArray<TextSpan>.Empty);
 
             using (var workspace = CreateWorkspaceFromOptions(initialMarkup, parameters))
             {
+                // Ideally this check would always run, but there are several hundred tests that would need to be
+                // updated with {|Unnecessary:|} spans.
+                if (unnecessarySpans.Any())
+                {
+                    var allDiagnostics = await GetDiagnosticsWorkerAsync(workspace, parameters
+                        .WithRetainNonFixableDiagnostics(true)
+                        .WithIncludeDiagnosticsOutsideSelection(true));
+
+                    TestDiagnosticTags(allDiagnostics, unnecessarySpans, WellKnownDiagnosticTags.Unnecessary, UnnecessaryMarkupKey, initialMarkupWithoutSpans);
+                }
+
                 var (_, action) = await GetCodeActionsAsync(workspace, parameters);
                 await TestActionAsync(
                     workspace, expected, action,
@@ -392,6 +415,85 @@ await TestActionAsync(
             }
         }
 
+        private static void TestDiagnosticTags(
+            ImmutableArray<Diagnostic> diagnostics,
+            ImmutableArray<TextSpan> expectedSpans,
+            string diagnosticTag,
+            string markupKey,
+            string initialMarkupWithoutSpans)
+        {
+            var diagnosticsWithTag = diagnostics
+                .Where(d => d.Descriptor.CustomTags.Contains(diagnosticTag))
+                .OrderBy(s => s.Location.SourceSpan.Start)
+                .ToImmutableArray();
+
+            if (expectedSpans.Length != diagnosticsWithTag.Length)
+            {
+                AssertEx.Fail(BuildFailureMessage(expectedSpans, diagnosticTag, markupKey, initialMarkupWithoutSpans, diagnosticsWithTag));
+            }
+
+            for (var i = 0; i < expectedSpans.Length; i++)
+            {
+                var actual = diagnosticsWithTag[i].Location.SourceSpan;
+                var expected = expectedSpans[i];
+                Assert.Equal(expected, actual);
+            }
+        }
+
+        private static string BuildFailureMessage(
+            ImmutableArray<TextSpan> expectedSpans,
+            string diagnosticTag,
+            string markupKey,
+            string initialMarkupWithoutSpans,
+            ImmutableArray<Diagnostic> diagnosticsWithTag)
+        {
+            var message = $"Expected {expectedSpans.Length} diagnostic spans with custom tag '{diagnosticTag}', but there were {diagnosticsWithTag.Length}.";
+
+            if (expectedSpans.Length == 0)
+            {
+                message += $" If a diagnostic span tagged '{diagnosticTag}' is expected, surround the span in the test markup with the following syntax: {{|Unnecessary:...}}";
+
+                var segments = new List<(int originalStringIndex, string segment)>();
+
+                foreach (var diagnostic in diagnosticsWithTag)
+                {
+                    var documentOffset = initialMarkupWithoutSpans.IndexOf(diagnosticsWithTag.First().Location.SourceTree.ToString());
+                    if (documentOffset == -1) continue;
+
+                    segments.Add((documentOffset + diagnostic.Location.SourceSpan.Start, "{|" + markupKey + ":"));
+                    segments.Add((documentOffset + diagnostic.Location.SourceSpan.End, "|}"));
+                }
+
+                if (segments.Any())
+                {
+                    message += Environment.NewLine
+                        + "Example:" + Environment.NewLine
+                        + Environment.NewLine
+                        + InsertSegments(initialMarkupWithoutSpans, segments);
+                }
+            }
+
+            return message;
+        }
+
+        private static string InsertSegments(string originalString, IEnumerable<(int originalStringIndex, string segment)> segments)
+        {
+            var builder = new StringBuilder();
+
+            var positionInOriginalString = 0;
+
+            foreach (var (originalStringIndex, segment) in segments.OrderBy(s => s.originalStringIndex))
+            {
+                builder.Append(originalString, positionInOriginalString, originalStringIndex - positionInOriginalString);
+                builder.Append(segment);
+
+                positionInOriginalString = originalStringIndex;
+            }
+
+            builder.Append(originalString, positionInOriginalString, originalString.Length - positionInOriginalString);
+            return builder.ToString();
+        }
+
         internal async Task<Tuple<Solution, Solution>> TestActionAsync(
             TestWorkspace workspace, string expected,
             CodeAction action,
@@ -659,7 +761,7 @@ internal static IDictionary<OptionKey, object> OptionsSet(
         /// <summary>
         /// Tests all the code actions for the given <paramref name="input"/> string.  Each code
         /// action must produce the corresponding output in the <paramref name="outputs"/> array.
-        /// 
+        ///
         /// Will throw if there are more outputs than code actions or more code actions than outputs.
         /// </summary>
         protected Task TestAllInRegularAndScriptAsync(

src/EditorFeatures/TestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs

@@ -141,7 +141,8 @@ internal async override Task<IEnumerable<Diagnostic>> GetDiagnosticsAsync(
             }
 
             var testDriver = new TestDiagnosticAnalyzerDriver(document.Project, provider);
-            var diagnostics = (await testDriver.GetAllDiagnosticsAsync(document, span)).ToImmutableArray();
+            var filterSpan = parameters.includeDiagnosticsOutsideSelection ? (TextSpan?)null : span;
+            var diagnostics = (await testDriver.GetAllDiagnosticsAsync(document, filterSpan)).ToImmutableArray();
             AssertNoAnalyzerExceptionDiagnostics(diagnostics);
 
             var fixer = providerAndFixer.Item2;

src/EditorFeatures/TestUtilities/Diagnostics/AbstractDiagnosticProviderBasedUserDiagnosticTest.cs

@@ -54,7 +54,7 @@ private TestDiagnosticAnalyzerService CreateDiagnosticAnalyzerService(Project pr
         private async Task<IEnumerable<Diagnostic>> GetDiagnosticsAsync(
             Project project,
             Document document,
-            TextSpan span,
+            TextSpan? filterSpan,
             bool getDocumentDiagnostics,
             bool getProjectDiagnostics)
         {
@@ -66,7 +66,12 @@ private async Task<IEnumerable<Diagnostic>> GetDiagnosticsAsync(
             if (getDocumentDiagnostics)
             {
                 var dxs = await _diagnosticAnalyzerService.GetDiagnosticsAsync(project.Solution, project.Id, document.Id, _includeSuppressedDiagnostics);
-                documentDiagnostics = await CodeAnalysis.Diagnostics.Extensions.ToDiagnosticsAsync(dxs.Where(d => d.HasTextSpan && d.GetTextSpan().IntersectsWith(span)), project, CancellationToken.None);
+                documentDiagnostics = await CodeAnalysis.Diagnostics.Extensions.ToDiagnosticsAsync(
+                    filterSpan is null
+                        ? dxs.Where(d => d.HasTextSpan)
+                        : dxs.Where(d => d.HasTextSpan && d.GetTextSpan().IntersectsWith(filterSpan.Value)),
+                    project,
+                    CancellationToken.None);
             }
 
             if (getProjectDiagnostics)
@@ -86,9 +91,9 @@ private async Task<IEnumerable<Diagnostic>> GetDiagnosticsAsync(
             return allDiagnostics;
         }
 
-        public Task<IEnumerable<Diagnostic>> GetAllDiagnosticsAsync(Document document, TextSpan span)
+        public Task<IEnumerable<Diagnostic>> GetAllDiagnosticsAsync(Document document, TextSpan? filterSpan)
         {
-            return GetDiagnosticsAsync(document.Project, document, span, getDocumentDiagnostics: true, getProjectDiagnostics: true);
+            return GetDiagnosticsAsync(document.Project, document, filterSpan, getDocumentDiagnostics: true, getProjectDiagnostics: true);
         }
 
         public async Task<IEnumerable<Diagnostic>> GetAllDiagnosticsAsync(Project project)
@@ -125,7 +130,7 @@ public Task<IEnumerable<Diagnostic>> GetDocumentDiagnosticsAsync(Document docume
 
         public Task<IEnumerable<Diagnostic>> GetProjectDiagnosticsAsync(Project project)
         {
-            return GetDiagnosticsAsync(project, document: null, span: default, getDocumentDiagnostics: false, getProjectDiagnostics: true);
+            return GetDiagnosticsAsync(project, document: null, filterSpan: default, getDocumentDiagnostics: false, getProjectDiagnostics: true);
         }
 
         private async Task SynchronizeGlobalAssetToRemoteHostIfNeededAsync(Workspace workspace)

src/EditorFeatures/TestUtilities/Diagnostics/TestDiagnosticAnalyzerDriver.cs

@@ -0,0 +1,59 @@
+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+
+using System.Composition;
+using System.Text;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.SimplifyInterpolation;
+
+namespace Microsoft.CodeAnalysis.CSharp.SimplifyInterpolation
+{
+    [ExportCodeFixProvider(LanguageNames.CSharp), Shared]
+    internal class CSharpSimplifyInterpolationCodeFixProvider : AbstractSimplifyInterpolationCodeFixProvider<
+        InterpolationSyntax, ExpressionSyntax, InterpolationAlignmentClauseSyntax,
+        InterpolationFormatClauseSyntax, InterpolatedStringExpressionSyntax>
+    {
+        protected override InterpolationSyntax WithExpression(InterpolationSyntax interpolation, ExpressionSyntax expression)
+            => interpolation.WithExpression(expression);
+
+        protected override InterpolationSyntax WithAlignmentClause(InterpolationSyntax interpolation, InterpolationAlignmentClauseSyntax alignmentClause)
+            => interpolation.WithAlignmentClause(alignmentClause);
+
+        protected override InterpolationSyntax WithFormatClause(InterpolationSyntax interpolation, InterpolationFormatClauseSyntax formatClause)
+            => interpolation.WithFormatClause(formatClause);
+
+        protected override string Escape(InterpolatedStringExpressionSyntax interpolatedString, string formatString)
+        {
+            var result = new StringBuilder();
+            if (interpolatedString.StringStartToken.Kind() == SyntaxKind.InterpolatedVerbatimStringStartToken)
+            {
+                foreach (var c in formatString)
+                {
+                    // in a verbatim string, the only char we have to escape is the double-quote char
+                    if (c == '"')
+                    {
+                        result.Append(c);
+                    }
+
+                    result.Append(c);
+                }
+            }
+            else
+            {
+                // In a normal string we have to escape quotes and we have to escape an 
+                // escape character itself.
+                foreach (var c in formatString)
+                {
+                    if (c == '"' || c == '\\')
+                    {
+                        result.Append('\\');
+                    }
+
+                    result.Append(c);
+                }
+            }
+
+            return result.ToString();
+        }
+    }
+}

src/Features/CSharp/Portable/SimplifyInterpolation/CSharpSimplifyInterpolationCodeFixProvider.cs

@@ -0,0 +1,18 @@
+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+
+using Microsoft.CodeAnalysis.CSharp.EmbeddedLanguages.VirtualChars;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.EmbeddedLanguages.VirtualChars;
+using Microsoft.CodeAnalysis.SimplifyInterpolation;
+
+namespace Microsoft.CodeAnalysis.CSharp.SimplifyInterpolation
+{
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    internal class CSharpSimplifyInterpolationDiagnosticAnalyzer : AbstractSimplifyInterpolationDiagnosticAnalyzer<
+        InterpolationSyntax, ExpressionSyntax>
+    {
+        protected override IVirtualCharService GetVirtualCharService()
+            => CSharpVirtualCharService.Instance;
+    }
+}

src/Features/CSharp/Portable/SimplifyInterpolation/CSharpSimplifyInterpolationDiagnosticAnalyzer.cs

@@ -115,6 +115,8 @@ internal static class IDEDiagnosticIds
 
         public const string UseSystemHashCode = "IDE0070";
 
+        public const string SimplifyInterpolationId = "IDE0071";
+
         public const string PopulateSwitchExpressionDiagnosticId = "IDE0072";
 
         // Analyzer error Ids