Merge pull request dotnet#4 from jnm2/simplifyInterpolation

Add tests

src/EditorFeatures/CSharpTest/SimplifyInterpolation/SimplifyInterpolationTests.cs

@@ -0,0 +1,203 @@
+// 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.Threading.Tasks;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp.SimplifyInterpolation;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Diagnostics;
+using Microsoft.CodeAnalysis.Test.Utilities;
+using Xunit;
+
+namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.SimplifyInterpolation
+{
+    [Trait(Traits.Feature, Traits.Features.CodeActionsSimplifyInterpolation)]
+    public partial class SimplifyInterpolationTests : AbstractCSharpDiagnosticProviderBasedUserDiagnosticTest
+    {
+        internal override (DiagnosticAnalyzer, CodeFixProvider) CreateDiagnosticProviderAndFixer(Workspace workspace)
+            => (new CSharpSimplifyInterpolationDiagnosticAnalyzer(), new CSharpSimplifyInterpolationCodeFixProvider());
+
+        [Fact]
+        public async Task ToString_with_no_parameter()
+        {
+            await TestInRegularAndScriptAsync(
+@"class C
+{
+    void M(int someValue)
+    {
+        _ = $""prefix {someValue{|Unnecessary:[||].ToString()|}} suffix"";
+    }
+}",
+@"class C
+{
+    void M(int someValue)
+    {
+        _ = $""prefix {someValue} suffix"";
+    }
+}");
+        }
+
+        [Fact]
+        public async Task ToString_with_string_literal_parameter()
+        {
+            await TestInRegularAndScriptAsync(
+@"class C
+{
+    void M(int someValue)
+    {
+        _ = $""prefix {someValue{|Unnecessary:[||].ToString(""|}some format code{|Unnecessary:"")|}} suffix"";
+    }
+}",
+@"class C
+{
+    void M(int someValue)
+    {
+        _ = $""prefix {someValue:some format code} suffix"";
+    }
+}");
+        }
+
+        [Fact]
+        public async Task ToString_with_escape_sequences()
+        {
+            await TestInRegularAndScriptAsync(
+@"class C
+{
+    void M(System.DateTime someValue)
+    {
+        _ = $""prefix {someValue{|Unnecessary:[||].ToString(""|}\\d \""d\""{|Unnecessary:"")|}} suffix"";
+    }
+}",
+@"class C
+{
+    void M(int someValue)
+    {
+        _ = $""prefix {someValue:\\d \""d\""} suffix"";
+    }
+}");
+        }
+
+        [Fact]
+        public async Task ToString_with_verbatim_string_literal_parameter()
+        {
+            await TestInRegularAndScriptAsync(
+@"class C
+{
+    void M(int someValue)
+    {
+        _ = $""prefix {someValue{|Unnecessary:[||].ToString(@""|}some format code{|Unnecessary:"")|}} suffix"";
+    }
+}",
+@"class C
+{
+    void M(int someValue)
+    {
+        _ = $""prefix {someValue:some format code} suffix"";
+    }
+}");
+        }
+
+        [Fact]
+        public async Task ToString_with_verbatim_escape_sequences_inside_verbatim_interpolated_string()
+        {
+            await TestInRegularAndScriptAsync(
+@"class C
+{
+    void M(System.DateTime someValue)
+    {
+        _ = $@""prefix {someValue{|Unnecessary:[||].ToString(@""|}\d """"d""""{|Unnecessary:"")|}} suffix"";
+    }
+}",
+@"class C
+{
+    void M(int someValue)
+    {
+        _ = $@""prefix {someValue:\d """"d""""} suffix"";
+    }
+}");
+        }
+
+        [Fact]
+        public async Task ToString_with_verbatim_escape_sequences_inside_non_verbatim_interpolated_string()
+        {
+            await TestInRegularAndScriptAsync(
+@"class C
+{
+    void M(System.DateTime someValue)
+    {
+        _ = $""prefix {someValue{|Unnecessary:[||].ToString(@""|}\d """"d""""{|Unnecessary:"")|}} suffix"";
+    }
+}",
+@"class C
+{
+    void M(int someValue)
+    {
+        _ = $""prefix {someValue:\\d \""d\""} suffix"";
+    }
+}");
+        }
+
+        [Fact]
+        public async Task ToString_with_non_verbatim_escape_sequences_inside_verbatim_interpolated_string()
+        {
+            await TestInRegularAndScriptAsync(
+@"class C
+{
+    void M(System.DateTime someValue)
+    {
+        _ = $@""prefix {someValue{|Unnecessary:[||].ToString(""|}\\d \""d\""{|Unnecessary:"")|}} suffix"";
+    }
+}",
+@"class C
+{
+    void M(int someValue)
+    {
+        _ = $@""prefix {someValue:\d """"d""""} suffix"";
+    }
+}");
+        }
+
+        [Fact]
+        public async Task ToString_with_string_constant_parameter()
+        {
+            await TestMissingInRegularAndScriptAsync(
+@"class C
+{
+    void M(int someValue)
+    {
+        const string someConst = ""some format code"";
+        _ = $""prefix {someValue[||].ToString(someConst)} suffix"";
+    }
+}");
+        }
+
+        [Fact]
+        public async Task ToString_with_character_literal_parameter()
+        {
+            await TestMissingInRegularAndScriptAsync(
+@"class C
+{
+    void M(C someValue)
+    {
+        _ = $""prefix {someValue[||].ToString('f')} suffix"";
+    }
+
+    public string ToString(object obj) => null;
+}");
+        }
+
+        [Fact]
+        public async Task ToString_with_format_provider()
+        {
+            // (If someone is explicitly specifying culture, an implicit form should not be encouraged.)
+
+            await TestMissingInRegularAndScriptAsync(
+@"class C
+{
+    void M(int someValue)
+    {
+        _ = $""prefix {someValue[||].ToString(""some format code"", System.Globalization.CultureInfo.CurrentCulture)} suffix"";
+    }
+}");
+        }
+    }
+}

src/EditorFeatures/TestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs

@@ -4,6 +4,7 @@
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Linq;
+using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis;
@@ -37,6 +38,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 +49,7 @@ internal TestParameters(
                 int index = 0,
                 CodeActionPriority? priority = null,
                 bool retainNonFixableDiagnostics = false,
+                bool includeDiagnosticsOutsideSelection = false,
                 string title = null)
             {
                 this.parseOptions = parseOptions;
@@ -56,23 +59,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 +380,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 +417,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)
+                .ToList();
+
+            if (expectedSpans.Length != diagnosticsWithTag.Count)
+            {
+                AssertEx.Fail(BuildFailureMessage(expectedSpans, diagnosticTag, markupKey, initialMarkupWithoutSpans, diagnosticsWithTag));
+            }
+
+            for (var i = 0; i < Math.Min(expectedSpans.Length, diagnosticsWithTag.Count); 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,
+            List<Diagnostic> diagnosticsWithTag)
+        {
+            var message = $"Expected {expectedSpans.Length} diagnostic spans with custom tag '{diagnosticTag}', but there were {diagnosticsWithTag.Count}.";
+
+            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 +763,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(