Merge pull request #1181 from sharwell/faded-locations

Add validation for additional unnecessary locations

src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest`1.cs

@@ -12,6 +12,7 @@ using System.Linq;
 using System.Reflection;
 using System.Runtime.CompilerServices;
 using System.Text;
+using System.Text.RegularExpressions;
 using System.Threading;
 using System.Threading.Tasks;
 using DiffPlex;
@@ -34,6 +35,7 @@ namespace Microsoft.CodeAnalysis.Testing
         where TVerifier : IVerifier, new()
     {
         private static readonly ConditionalWeakTable<Diagnostic, object> NonLocalDiagnostics = new ConditionalWeakTable<Diagnostic, object>();
+        private static readonly Regex EncodedIndicesSyntax = new Regex(@"^\s*\[\s*((?<Index>[0-9]+)\s*(,\s*(?<Index>[0-9]+)\s*)*)?\]\s*$", RegexOptions.ExplicitCapture | RegexOptions.Compiled);
 
         /// <summary>
         /// Gets the default verifier for the test.
@@ -558,6 +560,23 @@ namespace Microsoft.CodeAnalysis.Testing
                 else
                 {
                     VerifyDiagnosticLocation(analyzers, actual.diagnostic, expected, actual.diagnostic.Location, expected.Spans[0], verifier);
+                    int[] unnecessaryIndices = { };
+                    if (actual.diagnostic.Properties.TryGetValue(WellKnownDiagnosticTags.Unnecessary, out var encodedUnnecessaryLocations))
+                    {
+                        verifier.True(actual.diagnostic.Descriptor.CustomTags.Contains(WellKnownDiagnosticTags.Unnecessary), "Diagnostic reported extended unnecessary locations, but the descriptor is not marked as unnecessary code.");
+                        var match = EncodedIndicesSyntax.Match(encodedUnnecessaryLocations);
+                        verifier.True(match.Success, $"Expected encoded unnecessary locations to be a valid JSON array of non-negative integers: {encodedUnnecessaryLocations}");
+                        unnecessaryIndices = match.Groups["Index"].Captures.OfType<Capture>().Select(capture => int.Parse(capture.Value)).ToArray();
+                        verifier.NotEmpty(nameof(unnecessaryIndices), unnecessaryIndices);
+                        foreach (var index in unnecessaryIndices)
+                        {
+                            if (index < 0 || index >= actual.diagnostic.AdditionalLocations.Count)
+                            {
+                                verifier.Fail($"All unnecessary indices in the diagnostic must be valid indices in AdditionalLocations [0-{actual.diagnostic.AdditionalLocations.Count}): {encodedUnnecessaryLocations}");
+                            }
+                        }
+                    }
+
                     if (!expected.Options.HasFlag(DiagnosticOptions.IgnoreAdditionalLocations))
                     {
                         var additionalLocations = actual.diagnostic.AdditionalLocations.ToArray();
@@ -565,9 +584,19 @@ namespace Microsoft.CodeAnalysis.Testing
                         message = FormatVerifierMessage(analyzers, actual.diagnostic, expected, $"Expected {expected.Spans.Length - 1} additional locations but got {additionalLocations.Length} for Diagnostic:");
                         verifier.Equal(expected.Spans.Length - 1, additionalLocations.Length, message);
 
-                        for (var j = 0; j < additionalLocations.Length; ++j)
+                        for (var j = 0; j < additionalLocations.Length; j++)
                         {
                             VerifyDiagnosticLocation(analyzers, actual.diagnostic, expected, additionalLocations[j], expected.Spans[j + 1], verifier);
+                            var isActualUnnecessary = unnecessaryIndices.Contains(j);
+                            var isExpectedUnnecessary = expected.Spans[j + 1].Options.HasFlag(DiagnosticLocationOptions.UnnecessaryCode);
+                            if (isExpectedUnnecessary)
+                            {
+                                verifier.True(isActualUnnecessary, $"Expected diagnostic additional location index \"{j}\" to be marked unnecessary, but was not.");
+                            }
+                            else
+                            {
+                                verifier.False(isActualUnnecessary, $"Expected diagnostic additional location index \"{j}\" to not be marked unnecessary, but was instead marked unnecessary.");
+                            }
                         }
                     }
                 }
@@ -815,18 +844,17 @@ namespace Microsoft.CodeAnalysis.Testing
         private static string FormatDiagnostics(ImmutableArray<DiagnosticAnalyzer> analyzers, string defaultFilePath, params Diagnostic[] diagnostics)
         {
             var builder = new StringBuilder();
-            for (var i = 0; i < diagnostics.Length; ++i)
+            foreach (var diagnostic in diagnostics)
             {
-                var diagnosticsId = diagnostics[i].Id;
-                var location = diagnostics[i].Location;
+                var location = diagnostic.Location;
 
-                builder.Append("// ").AppendLine(diagnostics[i].ToString());
+                builder.Append("// ").AppendLine(diagnostic.ToString());
 
-                var applicableAnalyzer = analyzers.FirstOrDefault(a => a.SupportedDiagnostics.Any(dd => dd.Id == diagnosticsId));
+                var applicableAnalyzer = analyzers.FirstOrDefault(a => a.SupportedDiagnostics.Any(dd => dd.Id == diagnostic.Id));
                 if (applicableAnalyzer != null)
                 {
                     var analyzerType = applicableAnalyzer.GetType();
-                    var rule = location != Location.None && location.IsInSource && applicableAnalyzer.SupportedDiagnostics.Length == 1 ? string.Empty : $"{analyzerType.Name}.{diagnosticsId}";
+                    var rule = location != Location.None && location.IsInSource && applicableAnalyzer.SupportedDiagnostics.Length == 1 ? string.Empty : $"{analyzerType.Name}.{diagnostic.Id}";
 
                     if (location == Location.None || !location.IsInSource)
                     {
@@ -841,11 +869,11 @@ namespace Microsoft.CodeAnalysis.Testing
                 else
                 {
                     builder.Append(
-                        diagnostics[i].Severity switch
+                        diagnostic.Severity switch
                         {
-                            DiagnosticSeverity.Error => $"{nameof(DiagnosticResult)}.{nameof(DiagnosticResult.CompilerError)}(\"{diagnostics[i].Id}\")",
-                            DiagnosticSeverity.Warning => $"{nameof(DiagnosticResult)}.{nameof(DiagnosticResult.CompilerWarning)}(\"{diagnostics[i].Id}\")",
-                            var severity => $"new {nameof(DiagnosticResult)}(\"{diagnostics[i].Id}\", {nameof(DiagnosticSeverity)}.{severity})",
+                            DiagnosticSeverity.Error => $"{nameof(DiagnosticResult)}.{nameof(DiagnosticResult.CompilerError)}(\"{diagnostic.Id}\")",
+                            DiagnosticSeverity.Warning => $"{nameof(DiagnosticResult)}.{nameof(DiagnosticResult.CompilerWarning)}(\"{diagnostic.Id}\")",
+                            var severity => $"new {nameof(DiagnosticResult)}(\"{diagnostic.Id}\", {nameof(DiagnosticSeverity)}.{severity})",
                         });
                 }
 
@@ -855,14 +883,27 @@ namespace Microsoft.CodeAnalysis.Testing
                 }
                 else
                 {
-                    AppendLocation(diagnostics[i].Location);
-                    foreach (var additionalLocation in diagnostics[i].AdditionalLocations)
+                    // The unnecessary code designator is ignored for the primary diagnostic location.
+                    AppendLocation(diagnostic.Location, isUnnecessary: false);
+
+                    int[] unnecessaryIndices = { };
+                    if (diagnostic.Properties.TryGetValue(WellKnownDiagnosticTags.Unnecessary, out var encodedUnnecessaryLocations))
                     {
-                        AppendLocation(additionalLocation);
+                        var match = EncodedIndicesSyntax.Match(encodedUnnecessaryLocations);
+                        if (match.Success)
+                        {
+                            unnecessaryIndices = match.Groups["Index"].Captures.OfType<Capture>().Select(capture => int.Parse(capture.Value)).ToArray();
                         }
                     }
 
-                var arguments = diagnostics[i].Arguments();
+                    for (var i = 0; i < diagnostic.AdditionalLocations.Count; i++)
+                    {
+                        var additionalLocation = diagnostic.AdditionalLocations[i];
+                        AppendLocation(additionalLocation, isUnnecessary: unnecessaryIndices.Contains(i));
+                    }
+                }
+
+                var arguments = diagnostic.Arguments();
                 if (arguments.Count > 0)
                 {
                     builder.Append($".{nameof(DiagnosticResult.WithArguments)}(");
@@ -870,7 +911,7 @@ namespace Microsoft.CodeAnalysis.Testing
                     builder.Append(")");
                 }
 
-                if (diagnostics[i].IsSuppressed())
+                if (diagnostic.IsSuppressed())
                 {
                     builder.Append($".{nameof(DiagnosticResult.WithIsSuppressed)}(true)");
                 }
@@ -881,13 +922,16 @@ namespace Microsoft.CodeAnalysis.Testing
             return builder.ToString();
 
             // Local functions
-            void AppendLocation(Location location)
+            void AppendLocation(Location location, bool isUnnecessary)
             {
                 var lineSpan = location.GetLineSpan();
                 var pathString = location.IsInSource && lineSpan.Path == defaultFilePath ? string.Empty : $"\"{lineSpan.Path}\", ";
                 var linePosition = lineSpan.StartLinePosition;
                 var endLinePosition = lineSpan.EndLinePosition;
-                builder.Append($".WithSpan({pathString}{linePosition.Line + 1}, {linePosition.Character + 1}, {endLinePosition.Line + 1}, {endLinePosition.Character + 1})");
+                var unnecessaryArgument = isUnnecessary
+                    ? $", {nameof(DiagnosticLocationOptions)}.{nameof(DiagnosticLocationOptions.UnnecessaryCode)}"
+                    : string.Empty;
+                builder.Append($".WithSpan({pathString}{linePosition.Line + 1}, {linePosition.Character + 1}, {endLinePosition.Line + 1}, {endLinePosition.Character + 1}{unnecessaryArgument})");
             }
         }
 

src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/DiagnosticLocationOptions.cs

@@ -37,5 +37,10 @@ namespace Microsoft.CodeAnalysis.Testing
         /// </list>
         /// </summary>
         InterpretAsMarkupKey = 2,
+
+        /// <summary>
+        /// The diagnostic location is marked as unnecessary code.
+        /// </summary>
+        UnnecessaryCode = 4,
     }
 }

src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/PublicAPI.Unshipped.txt

@@ -63,6 +63,7 @@ Microsoft.CodeAnalysis.Testing.DiagnosticLocationOptions
 Microsoft.CodeAnalysis.Testing.DiagnosticLocationOptions.IgnoreLength = 1 -> Microsoft.CodeAnalysis.Testing.DiagnosticLocationOptions
 Microsoft.CodeAnalysis.Testing.DiagnosticLocationOptions.InterpretAsMarkupKey = 2 -> Microsoft.CodeAnalysis.Testing.DiagnosticLocationOptions
 Microsoft.CodeAnalysis.Testing.DiagnosticLocationOptions.None = 0 -> Microsoft.CodeAnalysis.Testing.DiagnosticLocationOptions
+Microsoft.CodeAnalysis.Testing.DiagnosticLocationOptions.UnnecessaryCode = 4 -> Microsoft.CodeAnalysis.Testing.DiagnosticLocationOptions
 Microsoft.CodeAnalysis.Testing.DiagnosticOptions
 Microsoft.CodeAnalysis.Testing.DiagnosticOptions.IgnoreAdditionalLocations = 1 -> Microsoft.CodeAnalysis.Testing.DiagnosticOptions
 Microsoft.CodeAnalysis.Testing.DiagnosticOptions.IgnoreSeverity = 2 -> Microsoft.CodeAnalysis.Testing.DiagnosticOptions

tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing.UnitTests/DiagnosticLocationTests.cs

@@ -245,6 +245,120 @@ namespace Microsoft.CodeAnalysis.Testing
             new DefaultVerifier().EqualOrDiff(expected, exception.Message);
         }
 
+        [Fact]
+        public async Task TestAdditionalUnnecessaryLocations()
+        {
+            await new CSharpAnalyzerTest<HighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer>
+            {
+                TestCode = @"class TestClass {|#0:{|} {|#1:}|}",
+                ExpectedDiagnostics =
+                {
+                    Diagnostic<HighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer>().WithLocation(0).WithLocation(1, DiagnosticLocationOptions.UnnecessaryCode),
+                },
+            }.RunAsync();
+        }
+
+        [Fact]
+        public async Task TestAdditionalUnnecessaryLocationsIgnored()
+        {
+            await new CSharpAnalyzerTest<HighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer>
+            {
+                TestCode = @"class TestClass {|#0:{|} {|#1:}|}",
+                ExpectedDiagnostics =
+                {
+                    Diagnostic<HighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer>().WithLocation(0).WithOptions(DiagnosticOptions.IgnoreAdditionalLocations),
+                },
+            }.RunAsync();
+        }
+
+        [Fact]
+        public async Task TestAdditionalUnnecessaryLocationRequiresExpectedMarkedUnnecessary()
+        {
+            var exception = await Assert.ThrowsAsync<InvalidOperationException>(async () =>
+            {
+                await new CSharpAnalyzerTest<HighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer>
+                {
+                    TestCode = @"class TestClass {|#0:{|} {|#1:}|}",
+                    ExpectedDiagnostics =
+                    {
+                        Diagnostic<HighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer>().WithLocation(0).WithLocation(1),
+                    },
+                }.RunAsync();
+            });
+
+            var expected =
+                """
+                Expected diagnostic additional location index "0" to not be marked unnecessary, but was instead marked unnecessary.
+                """.ReplaceLineEndings();
+            new DefaultVerifier().EqualOrDiff(expected, exception.Message);
+        }
+
+        [Fact]
+        public async Task TestAdditionalUnnecessaryLocationRequiresDescriptorMarkedUnnecessary()
+        {
+            var exception = await Assert.ThrowsAsync<InvalidOperationException>(async () =>
+            {
+                await new CSharpAnalyzerTest<HighlightBraceSpanWithEndMarkedUnnecessaryButDescriptorNotAnalyzer>
+                {
+                    TestCode = @"class TestClass {|#0:{|} {|#1:}|}",
+                    ExpectedDiagnostics =
+                    {
+                        Diagnostic<HighlightBraceSpanWithEndMarkedUnnecessaryButDescriptorNotAnalyzer>().WithLocation(0).WithLocation(1, DiagnosticLocationOptions.UnnecessaryCode),
+                    },
+                }.RunAsync();
+            });
+
+            var expected =
+                """
+                Diagnostic reported extended unnecessary locations, but the descriptor is not marked as unnecessary code.
+                """.ReplaceLineEndings();
+            new DefaultVerifier().EqualOrDiff(expected, exception.Message);
+        }
+
+        [Fact]
+        public async Task TestAdditionalUnnecessaryLocationNotJsonArray()
+        {
+            var exception = await Assert.ThrowsAsync<InvalidOperationException>(async () =>
+            {
+                await new CSharpAnalyzerTest<HighlightBraceSpanWithEndMarkedUnnecessaryNotJsonArrayAnalyzer>
+                {
+                    TestCode = @"class TestClass {|#0:{|} {|#1:}|}",
+                    ExpectedDiagnostics =
+                    {
+                        Diagnostic<HighlightBraceSpanWithEndMarkedUnnecessaryNotJsonArrayAnalyzer>().WithLocation(0).WithLocation(1, DiagnosticLocationOptions.UnnecessaryCode),
+                    },
+                }.RunAsync();
+            });
+
+            var expected =
+                """
+                Expected encoded unnecessary locations to be a valid JSON array of non-negative integers: Text
+                """.ReplaceLineEndings();
+            new DefaultVerifier().EqualOrDiff(expected, exception.Message);
+        }
+
+        [Fact]
+        public async Task TestAdditionalUnnecessaryLocationIndexOutOfRange()
+        {
+            var exception = await Assert.ThrowsAsync<InvalidOperationException>(async () =>
+            {
+                await new CSharpAnalyzerTest<HighlightBraceSpanWithEndMarkedUnnecessaryIndexOutOfRangeAnalyzer>
+                {
+                    TestCode = @"class TestClass {|#0:{|} {|#1:}|}",
+                    ExpectedDiagnostics =
+                    {
+                        Diagnostic<HighlightBraceSpanWithEndMarkedUnnecessaryIndexOutOfRangeAnalyzer>().WithLocation(0).WithLocation(1, DiagnosticLocationOptions.UnnecessaryCode),
+                    },
+                }.RunAsync();
+            });
+
+            var expected =
+                """
+                All unnecessary indices in the diagnostic must be valid indices in AdditionalLocations [0-1): [1]
+                """.ReplaceLineEndings();
+            new DefaultVerifier().EqualOrDiff(expected, exception.Message);
+        }
+
         [DiagnosticAnalyzer(LanguageNames.CSharp)]
         private class HighlightBracePositionAnalyzer : AbstractHighlightBracesAnalyzer
         {
@@ -264,6 +378,55 @@ namespace Microsoft.CodeAnalysis.Testing
             }
         }
 
+        [DiagnosticAnalyzer(LanguageNames.CSharp)]
+        private class HighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer : AbstractHighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer
+        {
+        }
+
+        [DiagnosticAnalyzer(LanguageNames.CSharp)]
+        private class HighlightBraceSpanWithEndMarkedUnnecessaryNotJsonArrayAnalyzer : AbstractHighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer
+        {
+            protected override string UnnecessaryLocationsValue => "Text";
+        }
+
+        [DiagnosticAnalyzer(LanguageNames.CSharp)]
+        private class HighlightBraceSpanWithEndMarkedUnnecessaryIndexOutOfRangeAnalyzer : AbstractHighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer
+        {
+            protected override string UnnecessaryLocationsValue => "[1]";
+        }
+
+        [DiagnosticAnalyzer(LanguageNames.CSharp)]
+        private class HighlightBraceSpanWithEndMarkedUnnecessaryButDescriptorNotAnalyzer : AbstractHighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer
+        {
+            public HighlightBraceSpanWithEndMarkedUnnecessaryButDescriptorNotAnalyzer()
+                : base(customTags: new string[0])
+            {
+            }
+        }
+
+        private abstract class AbstractHighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer : AbstractHighlightBracesAnalyzer
+        {
+            public AbstractHighlightBraceSpanWithEndMarkedUnnecessaryAnalyzer(string[]? customTags = null)
+                : base(customTags: customTags ?? new[] { WellKnownDiagnosticTags.Unnecessary })
+            {
+            }
+
+            protected virtual string UnnecessaryLocationsValue => "[0]";
+
+            protected override Diagnostic CreateDiagnostic(SyntaxToken token)
+            {
+                var endLocation = token.Parent switch
+                {
+                    CSharp.Syntax.ClassDeclarationSyntax classDeclaration => classDeclaration.CloseBraceToken.GetLocation(),
+                    _ => throw new NotSupportedException(),
+                };
+
+                var additionalLocations = new[] { endLocation };
+                var properties = ImmutableDictionary.Create<string, string?>().Add(WellKnownDiagnosticTags.Unnecessary, UnnecessaryLocationsValue);
+                return CodeAnalysis.Diagnostic.Create(Descriptor, token.GetLocation(), additionalLocations, properties);
+            }
+        }
+
         [DiagnosticAnalyzer(LanguageNames.CSharp)]
         private class ReportCompilationDiagnosticAnalyzer : DiagnosticAnalyzer
         {

tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Testing.Utilities/TestAnalyzers/AbstractHighlightBracesAnalyzer.cs

@@ -9,8 +9,8 @@ namespace Microsoft.CodeAnalysis.Testing.TestAnalyzers
 {
     public abstract class AbstractHighlightBracesAnalyzer : AbstractHighlightTokensAnalyzer
     {
-        protected AbstractHighlightBracesAnalyzer(string id = "Brace")
-            : base(id, (int)CSharpSyntaxKind.OpenBraceToken, (int)VisualBasicSyntaxKind.OpenBraceToken)
+        protected AbstractHighlightBracesAnalyzer(string id = "Brace", string[]? customTags = null)
+            : base(id, customTags ?? new string[0], (int)CSharpSyntaxKind.OpenBraceToken, (int)VisualBasicSyntaxKind.OpenBraceToken)
         {
         }
     }

tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Testing.Utilities/TestAnalyzers/AbstractHighlightTokensAnalyzer.cs

@@ -10,8 +10,13 @@ namespace Microsoft.CodeAnalysis.Testing.TestAnalyzers
     public abstract class AbstractHighlightTokensAnalyzer : DiagnosticAnalyzer
     {
         protected AbstractHighlightTokensAnalyzer(string id, params int[] tokenKinds)
+            : this(id, customTags: new string[0], tokenKinds)
         {
-            Descriptor = new DiagnosticDescriptor(id, "title", "message", "category", DiagnosticSeverity.Warning, isEnabledByDefault: true);
+        }
+
+        protected AbstractHighlightTokensAnalyzer(string id, string[] customTags, params int[] tokenKinds)
+        {
+            Descriptor = new DiagnosticDescriptor(id, "title", "message", "category", DiagnosticSeverity.Warning, isEnabledByDefault: true, customTags: customTags);
             Tokens = ImmutableHashSet.CreateRange(tokenKinds);
         }