diff --git a/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest`1.cs b/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest`1.cs index 9834266f..6ab44778 100644 --- a/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest`1.cs +++ b/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 NonLocalDiagnostics = new ConditionalWeakTable(); + private static readonly Regex EncodedIndicesSyntax = new Regex(@"^\s*\[\s*((?[0-9]+)\s*(,\s*(?[0-9]+)\s*)*)?\]\s*$", RegexOptions.ExplicitCapture | RegexOptions.Compiled); /// /// 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().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 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().Select(capture => int.Parse(capture.Value)).ToArray(); + } + } + + for (var i = 0; i < diagnostic.AdditionalLocations.Count; i++) + { + var additionalLocation = diagnostic.AdditionalLocations[i]; + AppendLocation(additionalLocation, isUnnecessary: unnecessaryIndices.Contains(i)); } } - var arguments = diagnostics[i].Arguments(); + 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})"); } } diff --git a/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/DiagnosticLocationOptions.cs b/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/DiagnosticLocationOptions.cs index fb337a62..7886eb57 100644 --- a/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/DiagnosticLocationOptions.cs +++ b/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/DiagnosticLocationOptions.cs @@ -37,5 +37,10 @@ namespace Microsoft.CodeAnalysis.Testing /// /// InterpretAsMarkupKey = 2, + + /// + /// The diagnostic location is marked as unnecessary code. + /// + UnnecessaryCode = 4, } } diff --git a/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/PublicAPI.Unshipped.txt b/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/PublicAPI.Unshipped.txt index ae5efba2..668589bf 100644 --- a/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/PublicAPI.Unshipped.txt +++ b/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 diff --git a/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing.UnitTests/DiagnosticLocationTests.cs b/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing.UnitTests/DiagnosticLocationTests.cs index 726d806d..f09204a7 100644 --- a/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing.UnitTests/DiagnosticLocationTests.cs +++ b/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 + { + TestCode = @"class TestClass {|#0:{|} {|#1:}|}", + ExpectedDiagnostics = + { + Diagnostic().WithLocation(0).WithLocation(1, DiagnosticLocationOptions.UnnecessaryCode), + }, + }.RunAsync(); + } + + [Fact] + public async Task TestAdditionalUnnecessaryLocationsIgnored() + { + await new CSharpAnalyzerTest + { + TestCode = @"class TestClass {|#0:{|} {|#1:}|}", + ExpectedDiagnostics = + { + Diagnostic().WithLocation(0).WithOptions(DiagnosticOptions.IgnoreAdditionalLocations), + }, + }.RunAsync(); + } + + [Fact] + public async Task TestAdditionalUnnecessaryLocationRequiresExpectedMarkedUnnecessary() + { + var exception = await Assert.ThrowsAsync(async () => + { + await new CSharpAnalyzerTest + { + TestCode = @"class TestClass {|#0:{|} {|#1:}|}", + ExpectedDiagnostics = + { + Diagnostic().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(async () => + { + await new CSharpAnalyzerTest + { + TestCode = @"class TestClass {|#0:{|} {|#1:}|}", + ExpectedDiagnostics = + { + Diagnostic().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(async () => + { + await new CSharpAnalyzerTest + { + TestCode = @"class TestClass {|#0:{|} {|#1:}|}", + ExpectedDiagnostics = + { + Diagnostic().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(async () => + { + await new CSharpAnalyzerTest + { + TestCode = @"class TestClass {|#0:{|} {|#1:}|}", + ExpectedDiagnostics = + { + Diagnostic().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().Add(WellKnownDiagnosticTags.Unnecessary, UnnecessaryLocationsValue); + return CodeAnalysis.Diagnostic.Create(Descriptor, token.GetLocation(), additionalLocations, properties); + } + } + [DiagnosticAnalyzer(LanguageNames.CSharp)] private class ReportCompilationDiagnosticAnalyzer : DiagnosticAnalyzer { diff --git a/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Testing.Utilities/TestAnalyzers/AbstractHighlightBracesAnalyzer.cs b/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Testing.Utilities/TestAnalyzers/AbstractHighlightBracesAnalyzer.cs index e2aa204e..8dd36f77 100644 --- a/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Testing.Utilities/TestAnalyzers/AbstractHighlightBracesAnalyzer.cs +++ b/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) { } } diff --git a/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Testing.Utilities/TestAnalyzers/AbstractHighlightTokensAnalyzer.cs b/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Testing.Utilities/TestAnalyzers/AbstractHighlightTokensAnalyzer.cs index e89a4627..398c9649 100644 --- a/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Testing.Utilities/TestAnalyzers/AbstractHighlightTokensAnalyzer.cs +++ b/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); }